This is an automated email from the ASF dual-hosted git repository. ephraimanierobi pushed a commit to branch v2-8-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 26d5e3f4e72c349b9f041fbb11f2e4791e6da1db Author: Jarek Potiuk <[email protected]> AuthorDate: Tue Nov 21 10:30:50 2023 +0100 Improve ownership fixing for Breeze (#35759) When we run airflow package preparation in docker containers and asset compilation, we use docker containers - similarly as breeze CI image, those constainers use root user in container. This means that the files created by those containers are owned by root on Linux - which means that we should change ownership of these files after they were generated. Used the opportunity to rewrtie "fix_ownership" part to Python and remove bash scripts used for it so far. --- .github/actions/build-ci-images/action.yml | 4 -- .github/actions/build-prod-images/action.yml | 4 -- .github/actions/post_tests_failure/action.yml | 3 - .github/actions/post_tests_success/action.yml | 3 - .github/workflows/build-images.yml | 3 - .github/workflows/ci.yml | 53 -------------- .github/workflows/release_dockerhub_image.yml | 3 - Dockerfile.ci | 2 - ...-root-ownership-after-exiting-docker-command.md | 60 ++++++++++++++++ .../airflow_breeze/commands/developer_commands.py | 7 ++ .../commands/release_management_commands.py | 21 ++++++ .../airflow_breeze/commands/testing_commands.py | 5 ++ .../airflow_breeze/utils/docker_command_utils.py | 17 +++-- dev/breeze/src/airflow_breeze/utils/parallel.py | 6 ++ dev/breeze/src/airflow_breeze/utils/run_utils.py | 2 +- scripts/docker/entrypoint_ci.sh | 2 - scripts/in_container/_in_container_utils.sh | 40 ----------- scripts/in_container/run_fix_ownership.py | 84 ++++++++++++++++++++++ scripts/in_container/run_fix_ownership.sh | 21 ------ .../in_container/run_prepare_airflow_packages.py | 12 ++-- 20 files changed, 201 insertions(+), 151 deletions(-) diff --git a/.github/actions/build-ci-images/action.yml b/.github/actions/build-ci-images/action.yml index d8c0216bb4..f950c53b4e 100644 --- a/.github/actions/build-ci-images/action.yml +++ b/.github/actions/build-ci-images/action.yml @@ -45,7 +45,3 @@ runs: name: source-constraints path: ./files/constraints-*/constraints-*.txt retention-days: 7 - - name: "Fix ownership" - shell: bash - run: breeze ci fix-ownership - if: always() diff --git a/.github/actions/build-prod-images/action.yml b/.github/actions/build-prod-images/action.yml index 7572153169..5fdbb795c4 100644 --- a/.github/actions/build-prod-images/action.yml +++ b/.github/actions/build-prod-images/action.yml @@ -72,7 +72,3 @@ runs: env: COMMIT_SHA: ${{ github.sha }} if: ${{ inputs.build-provider-packages != 'true' }} - - name: "Fix ownership" - shell: bash - run: breeze ci fix-ownership - if: always() diff --git a/.github/actions/post_tests_failure/action.yml b/.github/actions/post_tests_failure/action.yml index 96e43bbe0e..5b51db97ed 100644 --- a/.github/actions/post_tests_failure/action.yml +++ b/.github/actions/post_tests_failure/action.yml @@ -39,6 +39,3 @@ runs: name: container-logs-${{env.JOB_ID}} path: "./files/other_logs*" retention-days: 7 - - name: "Fix ownership" - shell: bash - run: breeze ci fix-ownership diff --git a/.github/actions/post_tests_success/action.yml b/.github/actions/post_tests_success/action.yml index 1325c5dde3..ac1e4fb8d2 100644 --- a/.github/actions/post_tests_success/action.yml +++ b/.github/actions/post_tests_success/action.yml @@ -40,6 +40,3 @@ runs: name: coverage-${{env.JOB_ID}} flags: python-${{env.PYTHON_MAJOR_MINOR_VERSION}},${{env.BACKEND}}-${{env.BACKEND_VERSION}} directory: "./files/coverage-reposts/" - - name: "Fix ownership" - shell: bash - run: breeze ci fix-ownership diff --git a/.github/workflows/build-images.yml b/.github/workflows/build-images.yml index 82498ebde0..b29d49de17 100644 --- a/.github/workflows/build-images.yml +++ b/.github/workflows/build-images.yml @@ -362,6 +362,3 @@ jobs: - name: "Stop ARM instance" run: ./scripts/ci/images/ci_stop_arm_instance.sh if: always() - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da4dbfde53..45e0a9fca0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -260,11 +260,6 @@ jobs: if: > matrix.platform == 'linux/amd64' && needs.build-info.outputs.canary-run == 'true' && needs.build-info.outputs.default-branch == 'main' - - name: "Fix ownership" - run: breeze ci fix-ownership - if: > - always() && needs.build-info.outputs.canary-run == 'true' - && needs.build-info.outputs.default-branch == 'main' # Check that after earlier cache push, breeze command will build quickly check-that-image-builds-quickly: timeout-minutes: 5 @@ -291,9 +286,6 @@ jobs: uses: ./.github/actions/breeze - name: "Check that image builds quickly" run: breeze shell --max-time 120 - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() build-ci-images: timeout-minutes: 80 @@ -385,9 +377,6 @@ jobs: name: constraints path: ./files/constraints-*/constraints-*.txt retention-days: 7 - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() build-prod-images: timeout-minutes: 80 @@ -603,9 +592,6 @@ jobs: env: PYTHON_VERSIONS: ${{ needs.build-info.outputs.python-versions-list-as-string }} DEBUG_RESOURCES: ${{needs.build-info.outputs.debug-resources}} - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() static-checks: timeout-minutes: 45 @@ -645,9 +631,6 @@ jobs: SKIP_GROUP_OUTPUT: "true" DEFAULT_BRANCH: ${{ needs.build-info.outputs.default-branch }} RUFF_FORMAT: "github" - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() # Those checks are run if no image needs to be built for checks. This is for simple changes that # Do not touch any of the python code or any of the important files that might require building @@ -701,9 +684,6 @@ jobs: SKIP_IMAGE_PRE_COMMITS: "true" SKIP: ${{ needs.build-info.outputs.skip-pre-commits }} COLUMNS: "250" - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() build-docs: timeout-minutes: 60 @@ -768,9 +748,6 @@ jobs: - name: "Upload documentation to AWS S3" if: needs.build-info.outputs.canary-run == 'true' && needs.build-info.outputs.default-branch == 'main' run: aws s3 sync --delete ./files/documentation s3://apache-airflow-docs - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() spellcheck-docs: timeout-minutes: 60 @@ -802,9 +779,6 @@ jobs: - name: "Spellcheck docs" run: > breeze build-docs ${{ needs.build-info.outputs.docs-list-as-string }} --spellcheck-only - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() prepare-test-provider-packages-wheel: timeout-minutes: 80 @@ -857,9 +831,6 @@ jobs: if: needs.build-info.outputs.affected-providers-list-as-string != '' env: SKIP_CONSTRAINTS: "${{ needs.build-info.outputs.upgrade-to-newer-dependencies }}" - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() provider-airflow-compatibility-check: timeout-minutes: 80 @@ -925,9 +896,6 @@ jobs: if: > needs.build-info.outputs.affected-providers-list-as-string != '' && needs.build-info.outputs.affected-providers-list-as-string != 'openlineage' - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() prepare-install-provider-packages-sdist: timeout-minutes: 80 @@ -974,9 +942,6 @@ jobs: breeze release-management install-provider-packages --package-format sdist --use-airflow-version sdist --run-in-parallel if: needs.build-info.outputs.affected-providers-list-as-string != '' - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() test-airflow-release-commands: @@ -1012,9 +977,6 @@ jobs: - name: "Check Airflow release process command" run: | breeze release-management start-release --release-candidate 2.4.3rc1 --previous-release 2.4.2 -a y - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() tests-helm: timeout-minutes: 80 @@ -1717,9 +1679,6 @@ jobs: env: PYTHON_VERSIONS: ${{ needs.build-info.outputs.python-versions-list-as-string }} DEBUG_RESOURCES: ${{needs.build-info.outputs.debug-resources}} - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() test-docker-compose-quick-start: @@ -1746,9 +1705,6 @@ jobs: pull-image-type: 'PROD' - name: "Test docker-compose quick start" run: breeze testing docker-compose-tests - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() tests-kubernetes: timeout-minutes: 240 @@ -1816,9 +1772,6 @@ jobs: - name: "Delete clusters just in case they are left" run: breeze k8s delete-cluster --all if: always() - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() update-constraints: permissions: @@ -1974,9 +1927,6 @@ jobs: - name: "Stop ARM instance" run: ./scripts/ci/images/ci_stop_arm_instance.sh if: always() && matrix.platform == 'linux/arm64' - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() # This is only a check if ARM images are successfully building when committer runs PR from # Apache repository. This is needed in case you want to fix failing cache job in "canary" run @@ -2033,6 +1983,3 @@ jobs: - name: "Stop ARM instance" run: ./scripts/ci/images/ci_stop_arm_instance.sh if: always() - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() diff --git a/.github/workflows/release_dockerhub_image.yml b/.github/workflows/release_dockerhub_image.yml index 02d7d6fcc0..6889539387 100644 --- a/.github/workflows/release_dockerhub_image.yml +++ b/.github/workflows/release_dockerhub_image.yml @@ -150,6 +150,3 @@ jobs: - name: "Docker logout" run: docker logout if: always() - - name: "Fix ownership" - run: breeze ci fix-ownership - if: always() diff --git a/Dockerfile.ci b/Dockerfile.ci index 45a1166070..81a9d424a0 100644 --- a/Dockerfile.ci +++ b/Dockerfile.ci @@ -998,8 +998,6 @@ if [[ ${SKIP_ENVIRONMENT_INITIALIZATION=} != "true" ]]; then touch /usr/lib/google-cloud-sdk/bin/gcloud ln -s -f /usr/bin/gcloud /usr/lib/google-cloud-sdk/bin/gcloud - in_container_fix_ownership - if [[ ${SKIP_SSH_SETUP="false"} == "false" ]]; then # Set up ssh keys echo 'yes' | ssh-keygen -t rsa -C [email protected] -m PEM -P '' -f ~/.ssh/id_rsa \ diff --git a/dev/breeze/doc/adr/0014-fix-root-ownership-after-exiting-docker-command.md b/dev/breeze/doc/adr/0014-fix-root-ownership-after-exiting-docker-command.md new file mode 100644 index 0000000000..5ca0c4681a --- /dev/null +++ b/dev/breeze/doc/adr/0014-fix-root-ownership-after-exiting-docker-command.md @@ -0,0 +1,60 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + --> + +<!-- START doctoc generated TOC please keep comment here to allow auto update --> +<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> +**Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* + +- [14. Fix root ownership after exiting docker command](#14-fix-root-ownership-after-exiting-docker-command) + - [Status](#status) + - [Context](#context) + - [Decision](#decision) + - [Consequences](#consequences) + +<!-- END doctoc generated TOC please keep comment here to allow auto update --> + +# 14. Fix root ownership after exiting docker command + +Date: 2023-11-20 + +## Status + +Accepted + +Builds on [6. Using root user and fixing ownership for-ci-container](0006-using-root-user-and-fixing-ownership-for-ci-container.md) + +## Context + +As discussed in [6. Using root user and fixing ownership for-ci-container](0006-using-root-user-and-fixing-ownership-for-ci-container.md) +we run Breeze CI container as root user. We used to use TRAP to fix the ownership of files created in +the container by root user. However, this is not fool-proof and using TRAP has its caveats (for example +it can be overridden by another TRAP command). Also TRAP might be invalidated by signal handling +or abrupt killing the docker container. Running the cleanup command after docker command completed +seems to do be much more robust solution. + +## Decision + +Instead of running the command as TRAP when exiting we simply attempt to clean the ownership whenever +we exit the container using `breeze`, `breeze shell`, `tests` or `breeze start-airflow` commands that run +commands in the CI image. + +## Consequences + +Users running Breeze on Linux will have less problems with root owned files and we can also remove +dedicated `ci fix-ownership` command in CI. diff --git a/dev/breeze/src/airflow_breeze/commands/developer_commands.py b/dev/breeze/src/airflow_breeze/commands/developer_commands.py index 7f4b38d27b..a8c257a2df 100644 --- a/dev/breeze/src/airflow_breeze/commands/developer_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/developer_commands.py @@ -84,6 +84,7 @@ from airflow_breeze.utils.console import get_console from airflow_breeze.utils.custom_param_types import BetterChoice from airflow_breeze.utils.docker_command_utils import ( check_docker_resources, + fix_ownership_using_docker, get_env_variables_for_docker_commands, get_extra_docker_flags, perform_environment_checks, @@ -259,6 +260,7 @@ def shell( run_db_tests_only=run_db_tests_only, skip_db_tests=skip_db_tests, ) + fix_ownership_using_docker() sys.exit(result.returncode) @@ -382,6 +384,7 @@ def start_airflow( standalone_dag_processor=standalone_dag_processor, database_isolation=database_isolation, ) + fix_ownership_using_docker() sys.exit(result.returncode) @@ -426,6 +429,7 @@ def build_docs( Build documents. """ perform_environment_checks() + fix_ownership_using_docker() cleanup_python_generated_files() params = BuildCiParams( github_repository=github_repository, python=DEFAULT_PYTHON_MAJOR_MINOR_VERSION, builder=builder @@ -460,6 +464,7 @@ def build_docs( *doc_builder.args_doc_builder, ] process = run_command(cmd, text=True, env=env, check=False) + fix_ownership_using_docker() if process.returncode == 0: get_console().print( "[info]Start the webserver in breeze and view the built docs at http://localhost:28080/docs/[/]"; @@ -639,6 +644,7 @@ def static_checks( text=True, env=env, ) + fix_ownership_using_docker() if static_checks_result.returncode != 0: if os.environ.get("CI"): get_console().print("\n[error]This error means that you have to fix the issues listed above:[/]") @@ -756,6 +762,7 @@ def enter_shell(**kwargs) -> RunCommandResult: """ perform_environment_checks() + fix_ownership_using_docker() cleanup_python_generated_files() if read_from_cache_file("suppress_asciiart") is None: get_console().print(ASCIIART, style=ASCIIART_STYLE) diff --git a/dev/breeze/src/airflow_breeze/commands/release_management_commands.py b/dev/breeze/src/airflow_breeze/commands/release_management_commands.py index c7a29b5aa2..732475d5df 100644 --- a/dev/breeze/src/airflow_breeze/commands/release_management_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/release_management_commands.py @@ -96,6 +96,7 @@ from airflow_breeze.utils.console import MessageType, Output, get_console from airflow_breeze.utils.custom_param_types import BetterChoice from airflow_breeze.utils.docker_command_utils import ( check_remote_ghcr_io_commands, + fix_ownership_using_docker, get_env_variables_for_docker_commands, get_extra_docker_flags, perform_environment_checks, @@ -220,6 +221,7 @@ NODE_BUILD_IMAGE_TAG = "node:21.2.0-bookworm-slim" def _compile_assets_in_docker(): clean_www_assets() + get_console().print("[info]Compiling assets in docker container\n") result = run_command( [ "docker", @@ -242,8 +244,14 @@ def _compile_assets_in_docker(): get_console().print("[error]Error compiling assets[/]") get_console().print(result.stdout) get_console().print(result.stderr) + fix_ownership_using_docker() sys.exit(result.returncode) + get_console().print("[success]compiled assets in docker container\n") + get_console().print("[info]Fixing ownership of compiled assets\n") + fix_ownership_using_docker() + get_console().print("[success]Fixing ownership of compiled assets\n") + @release_management.command( name="prepare-airflow-package", @@ -267,6 +275,7 @@ def prepare_airflow_packages( use_container_for_assets_compilation: bool, ): perform_environment_checks() + fix_ownership_using_docker() cleanup_python_generated_files() get_console().print("[info]Compiling assets\n") from sys import platform @@ -303,6 +312,9 @@ def prepare_airflow_packages( check=True, ) get_console().print("[success]Successfully prepared Airflow package!\n\n") + get_console().print("\n[info]Cleaning ownership of generated files\n") + fix_ownership_using_docker() + get_console().print("\n[success]Cleaned ownership of generated files\n") def provider_action_summary(description: str, message_type: MessageType, packages: list[str]): @@ -371,6 +383,8 @@ def prepare_provider_documentation( update_release_notes, ) + perform_environment_checks() + fix_ownership_using_docker() cleanup_python_generated_files() if not provider_packages: provider_packages = get_available_packages() @@ -516,6 +530,7 @@ def prepare_provider_packages( provider_packages: tuple[str, ...], ): perform_environment_checks() + fix_ownership_using_docker() cleanup_python_generated_files() packages_list = get_packages_list_to_act_on(package_list_file, provider_packages) if not skip_tag_check: @@ -601,6 +616,7 @@ def run_generate_constraints( output=output, output_outside_the_group=True, ) + fix_ownership_using_docker() return ( generate_constraints_result.returncode, f"Constraints {shell_params.airflow_constraints_mode}:{shell_params.python}", @@ -687,6 +703,7 @@ def generate_constraints( ): perform_environment_checks() check_remote_ghcr_io_commands() + fix_ownership_using_docker() cleanup_python_generated_files() if debug and run_in_parallel: get_console().print("\n[error]Cannot run --debug and --run-in-parallel at the same time[/]\n") @@ -854,6 +871,7 @@ def install_provider_packages( include_success_outputs: bool, ): perform_environment_checks() + fix_ownership_using_docker() cleanup_python_generated_files() shell_params = ShellParams( mount_sources=MOUNT_SELECTED, @@ -944,6 +962,7 @@ def install_provider_packages( debug=debug, output_outside_the_group=True, ) + fix_ownership_using_docker() sys.exit(result_command.returncode) @@ -977,6 +996,7 @@ def verify_provider_packages( get_console().print("Forcing use_packages_from_dist as installing selected_providers is set") use_packages_from_dist = True perform_environment_checks() + fix_ownership_using_docker() cleanup_python_generated_files() shell_params = ShellParams( mount_sources=MOUNT_SELECTED, @@ -1000,6 +1020,7 @@ def verify_provider_packages( debug=debug, output_outside_the_group=True, ) + fix_ownership_using_docker() sys.exit(result_command.returncode) diff --git a/dev/breeze/src/airflow_breeze/commands/testing_commands.py b/dev/breeze/src/airflow_breeze/commands/testing_commands.py index 66ffb593c3..fdd712087b 100644 --- a/dev/breeze/src/airflow_breeze/commands/testing_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/testing_commands.py @@ -70,6 +70,7 @@ from airflow_breeze.utils.common_options import ( from airflow_breeze.utils.console import Output, get_console from airflow_breeze.utils.custom_param_types import BetterChoice from airflow_breeze.utils.docker_command_utils import ( + fix_ownership_using_docker, get_env_variables_for_docker_commands, perform_environment_checks, remove_docker_networks, @@ -592,6 +593,7 @@ def _run_test_command( parallel_test_types_list=test_list, ) rebuild_or_pull_ci_image_if_needed(command_params=exec_shell_params) + fix_ownership_using_docker() cleanup_python_generated_files() perform_environment_checks() if run_in_parallel: @@ -695,6 +697,7 @@ def integration_tests( github_repository=github_repository, enable_coverage=enable_coverage, ) + fix_ownership_using_docker() cleanup_python_generated_files() perform_environment_checks() returncode, _ = _run_test( @@ -752,6 +755,7 @@ def helm_tests( if helm_test_package != "all": env_variables["HELM_TEST_PACKAGE"] = helm_test_package perform_environment_checks() + fix_ownership_using_docker() cleanup_python_generated_files() pytest_args = generate_args_for_pytest( test_type="Helm", @@ -769,4 +773,5 @@ def helm_tests( ) cmd = ["docker", "compose", "run", "--service-ports", "--rm", "airflow", *pytest_args, *extra_pytest_args] result = run_command(cmd, env=env_variables, check=False, output_outside_the_group=True) + fix_ownership_using_docker() sys.exit(result.returncode) diff --git a/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py b/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py index e936773162..36c976b2fe 100644 --- a/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py +++ b/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py @@ -41,8 +41,10 @@ except ImportError: from airflow_breeze.branch_defaults import AIRFLOW_BRANCH from airflow_breeze.global_constants import ( ALLOWED_CELERY_BROKERS, + ALLOWED_DEBIAN_VERSIONS, ALLOWED_PACKAGE_FORMATS, APACHE_AIRFLOW_GITHUB_REPOSITORY, + DEFAULT_PYTHON_MAJOR_MINOR_VERSION, FLOWER_HOST_PORT, MIN_DOCKER_COMPOSE_VERSION, MIN_DOCKER_VERSION, @@ -763,8 +765,15 @@ LABEL description="test warmup image" ) +OWNERSHIP_CLEANUP_DOCKER_TAG = ( + f"python:{DEFAULT_PYTHON_MAJOR_MINOR_VERSION}-slim-{ALLOWED_DEBIAN_VERSIONS[0]}" +) + + def fix_ownership_using_docker(): - perform_environment_checks() + if get_host_os() != "linux": + # no need to even attempt fixing ownership on MacOS/Windows + return shell_params = find_available_ci_image( github_repository=APACHE_AIRFLOW_GITHUB_REPOSITORY, ) @@ -775,10 +784,8 @@ def fix_ownership_using_docker(): "run", "-t", *extra_docker_flags, - "--pull", - "never", - shell_params.airflow_image_name_with_tag, - "/opt/airflow/scripts/in_container/run_fix_ownership.sh", + OWNERSHIP_CLEANUP_DOCKER_TAG, + "/opt/airflow/scripts/in_container/run_fix_ownership.py", ] run_command(cmd, text=True, env=env, check=False) diff --git a/dev/breeze/src/airflow_breeze/utils/parallel.py b/dev/breeze/src/airflow_breeze/utils/parallel.py index ad7e197d1c..eca700a18a 100644 --- a/dev/breeze/src/airflow_breeze/utils/parallel.py +++ b/dev/breeze/src/airflow_breeze/utils/parallel.py @@ -443,9 +443,15 @@ def check_async_run_results( try: if errors: get_console().print("\n[error]There were errors when running some tasks. Quitting.[/]\n") + from airflow_breeze.utils.docker_command_utils import fix_ownership_using_docker + + fix_ownership_using_docker() sys.exit(1) else: get_console().print(f"\n[success]{success}[/]\n") + from airflow_breeze.utils.docker_command_utils import fix_ownership_using_docker + + fix_ownership_using_docker() finally: if not skip_cleanup: for output in outputs: diff --git a/dev/breeze/src/airflow_breeze/utils/run_utils.py b/dev/breeze/src/airflow_breeze/utils/run_utils.py index f9415394b6..2553e75ce0 100644 --- a/dev/breeze/src/airflow_breeze/utils/run_utils.py +++ b/dev/breeze/src/airflow_breeze/utils/run_utils.py @@ -457,7 +457,7 @@ def clean_www_assets(): WWW_ASSET_HASH_FILE.unlink(missing_ok=True) shutil.rmtree(WWW_NODE_MODULES_DIR, ignore_errors=True) shutil.rmtree(WWW_STATIC_DIST_DIR, ignore_errors=True) - get_console().print("[info]Cleaned www assets[/]") + get_console().print("[success]Cleaned www assets[/]") def run_compile_www_assets( diff --git a/scripts/docker/entrypoint_ci.sh b/scripts/docker/entrypoint_ci.sh index 1de27480d1..d16134b482 100755 --- a/scripts/docker/entrypoint_ci.sh +++ b/scripts/docker/entrypoint_ci.sh @@ -296,8 +296,6 @@ if [[ ${SKIP_ENVIRONMENT_INITIALIZATION=} != "true" ]]; then touch /usr/lib/google-cloud-sdk/bin/gcloud ln -s -f /usr/bin/gcloud /usr/lib/google-cloud-sdk/bin/gcloud - in_container_fix_ownership - if [[ ${SKIP_SSH_SETUP="false"} == "false" ]]; then # Set up ssh keys echo 'yes' | ssh-keygen -t rsa -C [email protected] -m PEM -P '' -f ~/.ssh/id_rsa \ diff --git a/scripts/in_container/_in_container_utils.sh b/scripts/in_container/_in_container_utils.sh index b3606973c4..a3e09e0f1e 100644 --- a/scripts/in_container/_in_container_utils.sh +++ b/scripts/in_container/_in_container_utils.sh @@ -56,46 +56,6 @@ function in_container_script_start() { fi } -# -# Fixes ownership of files generated in container - if they are owned by root, they will be owned by -# The host user. Only needed if the host is Linux - on Mac, ownership of files is automatically -# changed to the Host user via osxfs filesystem -# -function in_container_fix_ownership() { - if [[ ${HOST_OS:=} == "linux" ]]; then - if [[ ${DOCKER_IS_ROOTLESS=} == "true" ]]; then - echo "${COLOR_YELLOW}Skip fixing ownership of generated files: Docker is rootless${COLOR_RESET}" - return - fi - DIRECTORIES_TO_FIX=( - "/dist" - "/files" - "${AIRFLOW_SOURCES}/logs" - "${AIRFLOW_SOURCES}/docs" - "${AIRFLOW_SOURCES}/dags" - "${AIRFLOW_SOURCES}/airflow/" - "${AIRFLOW_SOURCES}/constraints/" - "${AIRFLOW_SOURCES}/images/" - "${AIRFLOW_SOURCES}/.mypy_cache/" - "${AIRFLOW_SOURCES}/dev/" - ) - count_matching=$(find "${DIRECTORIES_TO_FIX[@]}" -mindepth 1 -user root -printf . 2>/dev/null | wc -m || true) - if [[ ${count_matching=} != "0" && ${count_matching=} != "" ]]; then - echo - echo "${COLOR_BLUE}Fixing ownership of ${count_matching} root owned files on ${HOST_OS}${COLOR_RESET}" - echo - find "${DIRECTORIES_TO_FIX[@]}" -mindepth 1 -user root -print0 2> /dev/null | - xargs --null chown "${HOST_USER_ID}:${HOST_GROUP_ID}" --no-dereference || true >/dev/null 2>&1 - echo "${COLOR_BLUE}Fixed ownership of generated files${COLOR_RESET}." - echo - fi - else - echo - echo "${COLOR_YELLOW}Skip fixing ownership of generated files as Host OS is ${HOST_OS}${COLOR_RESET}" - echo - fi -} - function in_container_go_to_airflow_sources() { pushd "${AIRFLOW_SOURCES}" >/dev/null 2>&1 || exit 1 } diff --git a/scripts/in_container/run_fix_ownership.py b/scripts/in_container/run_fix_ownership.py new file mode 100755 index 0000000000..68fc3a9904 --- /dev/null +++ b/scripts/in_container/run_fix_ownership.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import os +import pwd +import sys +from pathlib import Path + +HOST_OS = os.environ.get("HOST_OS", "") +DOCKER_IS_ROOTLESS = os.environ.get("DOCKER_IS_ROOTLESS", "false") == "true" + + +def change_ownership_of_files(path: Path) -> None: + host_user_id = os.environ.get("HOST_USER_ID", "") + host_group_id = os.environ.get("HOST_GROUP_ID", "") + if host_user_id == "" or host_group_id == "": + print( + f"ERROR: HOST_USER_ID or HOST_GROUP_ID environment variables " + f"are not set: {host_user_id}:{host_group_id}" + ) + sys.exit(1) + if not host_user_id.isnumeric() or not host_group_id.isnumeric(): + print( + f"ERROR: HOST_USER_ID or HOST_GROUP_ID environment variables " + f"should be numeric: {host_user_id}:{host_group_id}" + ) + sys.exit(1) + count_files = 0 + root_uid = pwd.getpwnam("root").pw_uid + print("Attempting to see if there are files that need to be changed to host user ownership") + for file in path.rglob("*"): + try: + if file.is_symlink() and file.lstat().st_uid == root_uid: + # Change ownership of symlink itself (by default stat/chown follow the symlinks) + os.chown(file, int(host_user_id), int(host_group_id), follow_symlinks=False) + count_files += 1 + if os.environ.get("VERBOSE_COMMANDS", "false") == "true": + print(f"Changed ownership of symlink {file}") + if file.stat().st_uid == root_uid: + # And here change ownership of the file (or if it is a symlink - the file it points to) + os.chown(file, int(host_user_id), int(host_group_id)) + count_files += 1 + if os.environ.get("VERBOSE_COMMANDS", "false") == "true": + print(f"Changed ownership of {file.resolve()}") + except FileNotFoundError: + # This is OK - file might have been deleted in the meantime or linked in Host from + # another place + if os.environ.get("VERBOSE_COMMANDS", "false") == "true": + print(f"Could not change ownership of {file}") + if count_files: + print(f"Changed ownership of {count_files} files back to {host_user_id}:{host_group_id}.") + else: + print("No files needed to change ownership") + + +if __name__ == "__main__": + if HOST_OS == "": + print("ERROR: HOST_OS environment variable is not set") + sys.exit(1) + if HOST_OS == "Linux": + print("Since host OS is not Linux, we don't need to fix ownership.") + sys.exit(0) + if DOCKER_IS_ROOTLESS: + print("Since Docker is in rootless mode , we don't need to fix ownership even on Linux.") + sys.exit(0) + + change_ownership_of_files(Path("/opt/airflow")) diff --git a/scripts/in_container/run_fix_ownership.sh b/scripts/in_container/run_fix_ownership.sh deleted file mode 100755 index d9e98ff168..0000000000 --- a/scripts/in_container/run_fix_ownership.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/usr/bin/env bash -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# shellcheck source=scripts/in_container/_in_container_utils.sh -. "$( dirname "${BASH_SOURCE[0]}" )/_in_container_utils.sh" - -in_container_fix_ownership diff --git a/scripts/in_container/run_prepare_airflow_packages.py b/scripts/in_container/run_prepare_airflow_packages.py index c1d2fb8aa5..4b319a722c 100755 --- a/scripts/in_container/run_prepare_airflow_packages.py +++ b/scripts/in_container/run_prepare_airflow_packages.py @@ -44,8 +44,6 @@ def process_summary(success_message: str, error_message: str, completed_process: AIRFLOW_SOURCES_ROOT = Path(__file__).parents[2].resolve() WWW_DIRECTORY = AIRFLOW_SOURCES_ROOT / "airflow" / "www" -rich.print("[green]Installed packages\n") - rich.print("[bright_blue]Cleaning build directories\n") for egg_info_file in AIRFLOW_SOURCES_ROOT.glob("*egg-info*"): @@ -53,7 +51,7 @@ for egg_info_file in AIRFLOW_SOURCES_ROOT.glob("*egg-info*"): rmtree(AIRFLOW_SOURCES_ROOT / "build", ignore_errors=True) -rich.print("[bright_blue]Cleaned build directories\n") +rich.print("[green]Cleaned build directories\n\n") version_suffix = os.environ.get("VERSION_SUFFIX_FOR_PYPI", "") package_format = os.environ.get("PACKAGE_FORMAT", "wheel") @@ -76,7 +74,7 @@ subprocess.run( check=True, ) -rich.print(f"[bright_blue]Marked {AIRFLOW_SOURCES_ROOT} as safe directory for git commands.\n") +rich.print(f"[green]Marked {AIRFLOW_SOURCES_ROOT} as safe directory for git commands.\n") rich.print("[bright_blue]Checking airflow version\n") @@ -84,7 +82,7 @@ airflow_version = subprocess.check_output( [sys.executable, "setup.py", "--version"], text=True, cwd=AIRFLOW_SOURCES_ROOT ).strip() -rich.print(f"[bright_blue]Airflow version: {airflow_version}\n") +rich.print(f"[green]Airflow version: {airflow_version}\n") RELEASED_VERSION_MATCHER = re.compile(r"^\d+\.\d+\.\d+$") @@ -92,7 +90,7 @@ command = [sys.executable, "setup.py"] if version_suffix: if RELEASED_VERSION_MATCHER.match(airflow_version): - rich.print(f"[bright_blue]Adding {version_suffix} suffix to the {airflow_version}") + rich.print(f"[warning]Adding {version_suffix} suffix to the {airflow_version}") command.extend(["egg_info", "--tag-build", version_suffix]) elif not airflow_version.endswith(version_suffix): rich.print(f"[red]Version {airflow_version} does not end with {version_suffix}. Using !") @@ -112,7 +110,7 @@ process_summary("Airflow packages built successfully", "Error building Airflow p if os.environ.get("GITHUB_ACTIONS", "") != "": print("::endgroup::") -rich.print("[bright_blue]Packages built successfully:\n") +rich.print("[green]Packages built successfully:\n") for file in (AIRFLOW_SOURCES_ROOT / "dist").glob("apache*"): rich.print(file.name) rich.print()
