From b5fab48ca430a5b1ef6b0aeacc7b320e870edb58 Mon Sep 17 00:00:00 2001 From: Augustin Date: Mon, 9 Dec 2024 11:44:04 +0100 Subject: [PATCH] airbyte-ci: give explicit file ownership on manifest-only connector build (#48836) --- airbyte-ci/connectors/pipelines/README.md | 1 + .../connectors/build_image/steps/common.py | 18 +++++++++++++++++- .../steps/manifest_only_connectors.py | 6 +++++- .../build_image/steps/python_connectors.py | 17 ----------------- airbyte-ci/connectors/pipelines/pyproject.toml | 2 +- .../test_manifest_only_connectors.py | 10 +++++++++- .../test_groups/test_connectors.py | 1 + 7 files changed, 34 insertions(+), 21 deletions(-) diff --git a/airbyte-ci/connectors/pipelines/README.md b/airbyte-ci/connectors/pipelines/README.md index 9e5e2253d172..eeec0c8ed4c4 100644 --- a/airbyte-ci/connectors/pipelines/README.md +++ b/airbyte-ci/connectors/pipelines/README.md @@ -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. | diff --git a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/common.py b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/common.py index e2d0db12b36e..78a97b26afba 100644 --- a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/common.py +++ b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/common.py @@ -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: @@ -30,6 +31,7 @@ class BuildConnectorImagesBase(Step, ABC): """ context: ConnectorContext + USER = AirbyteConnectorBaseImage.USER @property def title(self) -> str: @@ -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 diff --git a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/manifest_only_connectors.py b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/manifest_only_connectors.py index cae18f5e7bef..78d864276e17 100644 --- a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/manifest_only_connectors.py +++ b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/manifest_only_connectors.py @@ -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 @@ -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) diff --git a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/python_connectors.py b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/python_connectors.py index f61bfe656697..b49503382202 100644 --- a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/python_connectors.py +++ b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/python_connectors.py @@ -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 @@ -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 ( diff --git a/airbyte-ci/connectors/pipelines/pyproject.toml b/airbyte-ci/connectors/pipelines/pyproject.toml index c29ec466d704..c9ea2ab29fb9 100644 --- a/airbyte-ci/connectors/pipelines/pyproject.toml +++ b/airbyte-ci/connectors/pipelines/pyproject.toml @@ -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 "] diff --git a/airbyte-ci/connectors/pipelines/tests/test_build_image/test_manifest_only_connectors.py b/airbyte-ci/connectors/pipelines/tests/test_build_image/test_manifest_only_connectors.py index b29ce1d31162..09e4b75c2e2c 100644 --- a/airbyte-ci/connectors/pipelines/tests/test_build_image/test_manifest_only_connectors.py +++ b/airbyte-ci/connectors/pipelines/tests/test_build_image/test_manifest_only_connectors.py @@ -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", @@ -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") diff --git a/airbyte-ci/connectors/pipelines/tests/test_commands/test_groups/test_connectors.py b/airbyte-ci/connectors/pipelines/tests/test_commands/test_groups/test_connectors.py index a5db866f26b2..3b33ce91a425 100644 --- a/airbyte-ci/connectors/pipelines/tests/test_commands/test_groups/test_connectors.py +++ b/airbyte-ci/connectors/pipelines/tests/test_commands/test_groups/test_connectors.py @@ -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])