Skip to content

Commit

Permalink
fix: use root for python installs (airbytehq#49136)
Browse files Browse the repository at this point in the history
Co-authored-by: alafanechere <[email protected]>
  • Loading branch information
2 people authored and barduinor committed Dec 17, 2024
1 parent f5e62f7 commit 8698274
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 37 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 @@ -854,6 +854,7 @@ airbyte-ci connectors --language=low-code migrate-to-manifest-only

| Version | PR | Description |
| ------- | ---------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- |
| 4.46.2 | [#49136](https://github.com/airbytehq/airbyte/pull/49136) | Fix failed install of python components due to non-root permissions. |
| 4.46.1 | [#49146](https://github.com/airbytehq/airbyte/pull/49146) | Update `crane` image address as the one we were using has been deleted by the maintainer. |
| 4.46.0 | [#48790](https://github.com/airbytehq/airbyte/pull/48790) | Add unit tests step for manifest-only connectors |
| 4.45.3 | [#48927](https://github.com/airbytehq/airbyte/pull/48927) | Fix bug in determine_changelog_entry_comment |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from pipelines.airbyte_ci.connectors.test.steps.python_connectors import PytestStep
from pipelines.consts import LOCAL_BUILD_PLATFORM
from pipelines.helpers.execution.run_steps import STEP_TREE, StepToRun
from pipelines.models.steps import StepResult


def get_test_steps(context: ConnectorTestContext) -> STEP_TREE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from pipelines.dagger.actions import secrets
from pipelines.dagger.actions.python.poetry import with_poetry
from pipelines.helpers.execution.run_steps import STEP_TREE, StepToRun
from pipelines.helpers.utils import raise_if_not_user
from pipelines.models.steps import STEP_PARAMS, Step, StepResult

# Pin the PyAirbyte version to avoid updates from breaking CI
Expand Down Expand Up @@ -77,6 +78,7 @@ async def _run(self, connector_under_test: Container) -> StepResult:
test_environment = await self.install_testing_environment(
connector_under_test, test_config_file_name, test_config_file, self.extra_dependencies_names
)

pytest_command = self.get_pytest_command(test_config_file_name)

if self.bind_to_docker_host:
Expand Down Expand Up @@ -149,8 +151,9 @@ async def install_testing_environment(
Returns:
Container: The container with the test environment installed.
"""
secret_mounting_function = await secrets.mounted_connector_secrets(self.context, "secrets", self.secrets)
user = await BuildConnectorImages.get_image_user(built_connector_container)
secret_mounting_function = await secrets.mounted_connector_secrets(self.context, "secrets", self.secrets, owner=user)

container_with_test_deps = (
# Install the connector python package in /test_environment with the extra dependencies
await pipelines.dagger.actions.python.common.with_python_connector_installed(
Expand All @@ -163,15 +166,26 @@ async def install_testing_environment(
)
)
if self.common_test_dependencies:
container_with_test_deps = container_with_test_deps.with_exec(["pip", "install", f'{" ".join(self.common_test_dependencies)}'])
return (
container_with_test_deps = container_with_test_deps.with_user("root").with_exec(
["pip", "install", f'{" ".join(self.common_test_dependencies)}']
)

container_with_test_deps = (
container_with_test_deps
# Mount the test config file
.with_mounted_file(test_config_file_name, test_config_file)
.with_mounted_file(test_config_file_name, test_config_file, owner=user)
# Mount the secrets
.with_(secret_mounting_function).with_env_variable("PYTHONPATH", ".")
.with_(secret_mounting_function)
.with_env_variable("PYTHONPATH", ".")
# Make sure all files that were created or mounted under /airbyte are owned by the user
.with_user("root")
.with_exec(["chown", "-R", f"{user}:{user}", "/airbyte"])
.with_user(user)
)

await raise_if_not_user(container_with_test_deps, user)
return container_with_test_deps


class UnitTests(PytestStep):
"""A step to run the connector unit tests with Pytest."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
from pipelines import hacks
from pipelines.airbyte_ci.connectors.context import ConnectorContext, PipelineContext
from pipelines.consts import PATH_TO_LOCAL_CDK
from pipelines.dagger.containers.python import with_pip_cache, with_poetry_cache, with_python_base, with_testing_dependencies
from pipelines.helpers.utils import check_path_in_workdir, get_file_contents
from pipelines.dagger.containers.python import with_pip_cache, with_poetry_cache, with_python_base
from pipelines.helpers.utils import check_path_in_workdir, get_file_contents, raise_if_not_user


def with_python_package(
Expand All @@ -21,6 +21,7 @@ def with_python_package(
package_source_code_path: str,
exclude: Optional[List] = None,
include: Optional[List] = None,
owner: str | None = None,
) -> Container:
"""Load a python package source code to a python environment container.
Expand All @@ -30,13 +31,16 @@ def with_python_package(
package_source_code_path (str): The local path to the package source code.
additional_dependency_groups (Optional[List]): extra_requires dependency of setup.py to install. Defaults to None.
exclude (Optional[List]): A list of file or directory to exclude from the python package source code.
include (Optional[List]): A list of file or directory to include from the python package source code.
owner (str, optional): The owner of the mounted directory. Defaults to None.
Returns:
Container: A python environment container with the python package source code.
"""
package_source_code_directory: Directory = context.get_repo_dir(package_source_code_path, exclude=exclude, include=include)
work_dir_path = f"/{package_source_code_path}"
container = python_environment.with_mounted_directory(work_dir_path, package_source_code_directory).with_workdir(work_dir_path)
container = python_environment.with_mounted_directory(work_dir_path, package_source_code_directory, owner=owner).with_workdir(
work_dir_path
)
return container


Expand Down Expand Up @@ -109,7 +113,6 @@ async def find_local_python_dependencies(
package_source_code_path (str): The local path to the python package source code.
search_dependencies_in_setup_py (bool, optional): Whether to search for local dependencies in the setup.py file. Defaults to True.
search_dependencies_in_requirements_txt (bool, optional): Whether to search for local dependencies in the requirements.txt file. Defaults to True.
Returns:
List[str]: Paths to the local dependencies relative to the airbyte repo.
"""
Expand Down Expand Up @@ -205,7 +208,7 @@ async def with_installed_python_package(
Container: A python environment container with the python package installed.
"""

container = with_python_package(context, python_environment, package_source_code_path, exclude=exclude, include=include)
container = with_python_package(context, python_environment, package_source_code_path, exclude=exclude, include=include, owner=user)
local_dependencies = await find_local_python_dependencies(context, package_source_code_path)

for dependency_directory in local_dependencies:
Expand All @@ -215,8 +218,11 @@ async def with_installed_python_package(
has_requirements_txt = await check_path_in_workdir(container, "requirements.txt")
has_pyproject_toml = await check_path_in_workdir(container, "pyproject.toml")

container = container.with_user("root")
# All of these will require root access to install dependencies
# Dependencies are installed at the system level, if the user is not root it is not allowed to install system level dependencies
if has_pyproject_toml:
container = with_poetry_cache(container, context.dagger_client)
container = with_poetry_cache(container, context.dagger_client, owner=user)
container = _install_python_dependencies_from_poetry(container, additional_dependency_groups, install_root_package)
elif has_setup_py:
container = with_pip_cache(container, context.dagger_client)
Expand All @@ -225,21 +231,9 @@ async def with_installed_python_package(
container = with_pip_cache(container, context.dagger_client)
container = _install_python_dependencies_from_requirements_txt(container)

return container


def with_python_connector_source(context: ConnectorContext) -> Container:
"""Load an airbyte connector source code in a testing environment.
container = container.with_user(user)

Args:
context (ConnectorContext): The current test context, providing the repository directory from which the connector sources will be pulled.
Returns:
Container: A python environment container (with the connector source code).
"""
connector_source_path = str(context.connector.code_directory)
testing_environment: Container = with_testing_dependencies(context)

return with_python_package(context, testing_environment, connector_source_path)
return container


def apply_python_development_overrides(context: ConnectorContext, connector_container: Container, current_user: str) -> Container:
Expand Down Expand Up @@ -324,7 +318,7 @@ async def with_python_connector_installed(
)

container = await apply_python_development_overrides(context, container, user)

await raise_if_not_user(container, user)
return container


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,15 @@ async def upload(context: ConnectorContext, gcp_gsm_env_variable_name: str = "GC


async def mounted_connector_secrets(
context: ConnectorContext, secret_directory_path: str, connector_secrets: List[Secret]
context: ConnectorContext, secret_directory_path: str, connector_secrets: List[Secret], owner: str | None = None
) -> Callable[[Container], Container]:
"""Returns an argument for a dagger container's with_ method which mounts all connector secrets in it.
Args:
context (ConnectorContext): The context providing a connector object and its secrets.
secret_directory_path (str): Container directory where the secrets will be mounted, as files.
connector_secrets (List[secrets]): List of secrets to mount to the connector container.
owner (str, optional): The owner of the mounted secrets. Defaults to None.
Returns:
fn (Callable[[Container], Container]): A function to pass as argument to the connector container's with_ method.
"""
Expand All @@ -73,7 +74,7 @@ def with_secrets_mounted_as_dagger_secrets(container: Container) -> Container:
for secret in connector_secrets:
if secret.file_name:
container = container.with_mounted_secret(
f"{secret_directory_path}/{secret.file_name}", secret.as_dagger_secret(context.dagger_client)
f"{secret_directory_path}/{secret.file_name}", secret.as_dagger_secret(context.dagger_client), owner=owner
)
return container

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,25 +67,25 @@ def with_testing_dependencies(context: PipelineContext) -> Container:
)


def with_pip_cache(container: Container, dagger_client: Client) -> Container:
def with_pip_cache(container: Container, dagger_client: Client, owner: str | None = None) -> Container:
"""Mounts the pip cache in the container.
Args:
container (Container): A container with python installed
owner (str, optional): The owner of the cache. Defaults to None.
Returns:
Container: A container with the pip cache mounted.
"""
pip_cache_volume = dagger_client.cache_volume(PIP_CACHE_VOLUME_NAME)
return container.with_mounted_cache(PIP_CACHE_PATH, pip_cache_volume, sharing=CacheSharingMode.SHARED)
return container.with_mounted_cache(PIP_CACHE_PATH, pip_cache_volume, sharing=CacheSharingMode.SHARED, owner=owner)


def with_poetry_cache(container: Container, dagger_client: Client) -> Container:
def with_poetry_cache(container: Container, dagger_client: Client, owner: str | None = None) -> Container:
"""Mounts the poetry cache in the container.
Args:
container (Container): A container with python installed
owner (str, optional): The owner of the cache. Defaults to None.
Returns:
Container: A container with the poetry cache mounted.
"""
poetry_cache_volume = dagger_client.cache_volume(POETRY_CACHE_VOLUME_NAME)
return container.with_mounted_cache(POETRY_CACHE_PATH, poetry_cache_volume, sharing=CacheSharingMode.SHARED)
return container.with_mounted_cache(POETRY_CACHE_PATH, poetry_cache_volume, sharing=CacheSharingMode.SHARED, owner=owner)
10 changes: 10 additions & 0 deletions airbyte-ci/connectors/pipelines/pipelines/helpers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -374,3 +374,13 @@ def dagger_directory_as_zip_file(dagger_client: Client, directory: Directory, di
.with_exec(["zip", "-r", "/zipped.zip", f"/{directory_name}"])
.file("/zipped.zip")
)


async def raise_if_not_user(container: Container, user: str) -> None:
"""Raise an error if the container is not running as the specified user.
Args:
container (Container): The container to check.
user (str): The user to check.
"""
assert (await container.with_exec(["whoami"]).stdout()).strip() == user, f"Container is not running as {user}."
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.46.1"
version = "4.46.2"
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 @@ -72,6 +72,7 @@ async def test_with_python_connector_installed_from_setup(context_with_setup, py
# Uninstall and reinstall the latest cdk version
cdk_install_latest_output = (
await container.with_env_variable("CACHEBUSTER", datetime.datetime.now().isoformat())
.with_user("root")
.with_exec(["pip", "uninstall", "-y", f"airbyte-cdk=={latest_cdk_version}"])
.with_exec(["pip", "install", f"airbyte-cdk=={latest_cdk_version}"])
.stdout()
Expand Down

0 comments on commit 8698274

Please sign in to comment.