Skip to content

Commit

Permalink
airbyte-ci: give explicit file ownership on manifest-only connector b…
Browse files Browse the repository at this point in the history
…uild (#48836)
  • Loading branch information
alafanechere authored Dec 9, 2024
1 parent 6d49acb commit b5fab48
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 21 deletions.
1 change: 1 addition & 0 deletions airbyte-ci/connectors/pipelines/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,7 @@ airbyte-ci connectors --language=low-code migrate-to-manifest-only

| Version | PR | Description |
| ------- | ---------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- |
| 4.44.1 | [#48836](https://github.com/airbytehq/airbyte/pull/48836) | Manifest-only connector build: give ownership of copied file to the current user. |
| 4.44.0 | [#48818](https://github.com/airbytehq/airbyte/pull/48818) | Use local CDK or CDK ref for manifest only connector build. |
| 4.43.1 | [#48824](https://github.com/airbytehq/airbyte/pull/48824) | Allow uploading CI reports to GCS with fewer permissions set. |
| 4.43.0 | [#36545](https://github.com/airbytehq/airbyte/pull/36545) | Switch to `airbyte` user when available in Python base image. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
from typing import TYPE_CHECKING

import docker # type: ignore
from base_images.bases import AirbyteConnectorBaseImage # type: ignore
from click import UsageError
from connector_ops.utils import Connector # type: ignore
from dagger import Container, ExecError, Platform, QueryError
from pipelines.airbyte_ci.connectors.context import ConnectorContext
from pipelines.helpers.utils import export_container_to_tarball
from pipelines.helpers.utils import export_container_to_tarball, sh_dash_c
from pipelines.models.steps import Step, StepResult, StepStatus

if TYPE_CHECKING:
Expand All @@ -30,6 +31,7 @@ class BuildConnectorImagesBase(Step, ABC):
"""

context: ConnectorContext
USER = AirbyteConnectorBaseImage.USER

@property
def title(self) -> str:
Expand Down Expand Up @@ -74,6 +76,20 @@ async def _build_connector(self, platform: Platform, *args: Any, **kwargs: Any)
"""
raise NotImplementedError("`BuildConnectorImagesBase`s must define a '_build_connector' attribute.")

async def _get_image_user(self, base_container: Container) -> str:
"""If the base image in use has a user named 'airbyte', we will use it as the user for the connector image.
Args:
base_container (Container): The base container to use to build the connector.
Returns:
str: The user to use for the connector image.
"""
users = (await base_container.with_exec(sh_dash_c(["cut -d: -f1 /etc/passwd | sort | uniq"])).stdout()).splitlines()
if self.USER in users:
return self.USER
return "root"


class LoadContainerToLocalDockerHost(Step):
context: ConnectorContext
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,12 @@ async def _build_from_base_image(self, platform: Platform) -> Container:
self.logger.info(f"Building connector from base image in metadata for {platform}")

# Mount manifest file
base_container = self._get_base_container(platform).with_file(
base_container = self._get_base_container(platform)
user = await self._get_image_user(base_container)
base_container = base_container.with_file(
f"source_declarative_manifest/{MANIFEST_FILE_PATH}",
(await self.context.get_connector_dir(include=[MANIFEST_FILE_PATH])).file(MANIFEST_FILE_PATH),
owner=user,
)

# Mount components file if it exists
Expand All @@ -56,6 +59,7 @@ async def _build_from_base_image(self, platform: Platform) -> Container:
base_container = base_container.with_file(
f"source_declarative_manifest/{COMPONENTS_FILE_PATH}",
(await self.context.get_connector_dir(include=[COMPONENTS_FILE_PATH])).file(COMPONENTS_FILE_PATH),
owner=user,
)
connector_container = build_customization.apply_airbyte_entrypoint(base_container, self.context.connector)
connector_container = await apply_python_development_overrides(self.context, connector_container)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@

from typing import Any

from base_images.bases import AirbyteConnectorBaseImage # type: ignore
from dagger import Container, Platform
from pipelines.airbyte_ci.connectors.build_image.steps import build_customization
from pipelines.airbyte_ci.connectors.build_image.steps.common import BuildConnectorImagesBase
from pipelines.airbyte_ci.connectors.context import ConnectorContext
from pipelines.dagger.actions.python.common import apply_python_development_overrides, with_python_connector_installed
from pipelines.helpers.utils import sh_dash_c
from pipelines.models.steps import StepResult


Expand All @@ -23,21 +21,6 @@ class BuildConnectorImages(BuildConnectorImagesBase):

context: ConnectorContext
PATH_TO_INTEGRATION_CODE = "/airbyte/integration_code"
USER = AirbyteConnectorBaseImage.USER

async def _get_image_user(self, base_container: Container) -> str:
"""If the base image in use has a user named 'airbyte', we will use it as the user for the connector image.
Args:
base_container (Container): The base container to use to build the connector.
Returns:
str: The user to use for the connector image.
"""
users = (await base_container.with_exec(sh_dash_c(["cut -d: -f1 /etc/passwd | sort | uniq"])).stdout()).splitlines()
if self.USER in users:
return self.USER
return "root"

async def _build_connector(self, platform: Platform, *args: Any) -> Container:
if (
Expand Down
2 changes: 1 addition & 1 deletion airbyte-ci/connectors/pipelines/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api"

[tool.poetry]
name = "pipelines"
version = "4.44.0"
version = "4.44.1"
description = "Packaged maintained by the connector operations team to perform CI for connectors' pipelines"
authors = ["Airbyte <[email protected]>"]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ async def test__run_using_base_image_with_components_file(
return_value=container_built_from_base,
)

mocker.patch.object(
manifest_only_connectors.BuildConnectorImages,
"_get_image_user",
return_value="airbyte",
)

mocker.patch.object(
build_customization,
"apply_airbyte_entrypoint",
Expand All @@ -107,7 +113,9 @@ async def test__run_using_base_image_with_components_file(

await step._build_connector(all_platforms[0], container_built_from_base)
if components_file_exists:
container_built_from_base.with_file.assert_any_call("source_declarative_manifest/components.py", mock_components_file)
container_built_from_base.with_file.assert_any_call(
"source_declarative_manifest/components.py", mock_components_file, owner="airbyte"
)
mock_connector_dir.file.assert_any_call("components.py")
else:
self._assert_file_not_handled(container_built_from_base, "source_declarative_manifest/components.py")
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ def test_get_selected_connectors_by_name_and_support_level_or_languages_leads_to
assert len(selected_connectors) == 1


@pytest.mark.flaky
def test_get_selected_connectors_with_modified():
first_modified_connector = pick_a_random_connector()
second_modified_connector = pick_a_random_connector(other_picked_connectors=[first_modified_connector])
Expand Down

0 comments on commit b5fab48

Please sign in to comment.