Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use root for python installs #49136

Merged
merged 14 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading