From 3d9c61ee04a6259cb069a7e31b740d34ab6c1ed3 Mon Sep 17 00:00:00 2001 From: Kevin Shurtz Date: Wed, 30 Oct 2024 21:24:26 -0500 Subject: [PATCH 1/3] fix: include chmod flag during file copy This fixes an issue for Podman on Windows, which copies over executable files without the executable bit on, which causes subsequent Containerfile steps to fail. By explicitly marking copied files as executable, this issue is remedied, and Ansible Builder is made usable on more platforms. --- src/ansible_builder/containerfile.py | 6 +++--- test/unit/test_containerfile.py | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ansible_builder/containerfile.py b/src/ansible_builder/containerfile.py index 55befca2..4c910905 100644 --- a/src/ansible_builder/containerfile.py +++ b/src/ansible_builder/containerfile.py @@ -149,7 +149,7 @@ def prepare(self) -> None: else: # For an EE schema earlier than v3 with a custom builder image, we always make sure pip is available. context_dir = Path(self.build_outputs_dir).stem - self.steps.append(f'COPY {context_dir}/scripts/pip_install /output/scripts/pip_install') + self.steps.append(f'COPY --chmod=755 {context_dir}/scripts/pip_install /output/scripts/pip_install') self.steps.append("RUN /output/scripts/pip_install $PYCMD") self._insert_custom_steps('prepend_builder') @@ -313,11 +313,11 @@ def _create_folder_copy_files(self) -> None: # Later intermediate stages depend on base image containing these scripts. # Copy them to a location that we do not need in the final image. context_dir = Path(self.build_outputs_dir).stem - self.steps.append(f'COPY {context_dir}/scripts/ /output/scripts/') + self.steps.append(f'COPY --chmod=755 {context_dir}/scripts/ /output/scripts/') # The final image will have /output purged, but certain scripts we want # to retain in that image. - self.steps.append(f'COPY {context_dir}/scripts/entrypoint {constants.FINAL_IMAGE_BIN_PATH}/entrypoint') + self.steps.append(f'COPY --chmod=755 {context_dir}/scripts/entrypoint {constants.FINAL_IMAGE_BIN_PATH}/entrypoint') def _handle_additional_build_files(self) -> None: """ diff --git a/test/unit/test_containerfile.py b/test/unit/test_containerfile.py index c8036072..2d440e83 100644 --- a/test/unit/test_containerfile.py +++ b/test/unit/test_containerfile.py @@ -296,7 +296,7 @@ def test_v1_builder_image(build_dir_and_ee_yml): c = make_containerfile(tmpdir, ee_path, run_validate=True) c.prepare() assert "FROM $EE_BUILDER_IMAGE AS builder" in c.steps - assert "COPY _build/scripts/pip_install /output/scripts/pip_install" in c.steps + assert "COPY --chmod=755 _build/scripts/pip_install /output/scripts/pip_install" in c.steps assert "RUN /output/scripts/pip_install $PYCMD" in c.steps @@ -315,8 +315,9 @@ def test_v2_builder_image(build_dir_and_ee_yml): tmpdir, ee_path = build_dir_and_ee_yml(ee_data) c = make_containerfile(tmpdir, ee_path, run_validate=True) c.prepare() + print(c.steps) assert "FROM $EE_BUILDER_IMAGE AS builder" in c.steps - assert "COPY _build/scripts/pip_install /output/scripts/pip_install" in c.steps + assert "COPY --chmod=755 _build/scripts/pip_install /output/scripts/pip_install" in c.steps assert "RUN /output/scripts/pip_install $PYCMD" in c.steps @@ -334,7 +335,7 @@ def test_v2_builder_image_default(build_dir_and_ee_yml): c = make_containerfile(tmpdir, ee_path, run_validate=True) c.prepare() assert "FROM base AS builder" in c.steps - assert "COPY _build/scripts/pip_install /output/scripts/pip_install" not in c.steps + assert "COPY --chmod=755 _build/scripts/pip_install /output/scripts/pip_install" not in c.steps def test_prepare_introspect_assemble_steps(build_dir_and_ee_yml): From 278f9082315fa9ced713629f8721fbb40ac2007a Mon Sep 17 00:00:00 2001 From: Kevin Shurtz Date: Wed, 30 Oct 2024 22:02:44 -0500 Subject: [PATCH 2/3] fix: use posix path for entrypoint Since Containerfiles use a POSIX-like syntax, and I don't believe that Windows containers are supported base images, I think it's safe to say that Windows hosts should not insert backslash characters into entrypoint path for their Containerfiles. --- src/ansible_builder/ee_schema.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ansible_builder/ee_schema.py b/src/ansible_builder/ee_schema.py index dcb71439..dda31a42 100644 --- a/src/ansible_builder/ee_schema.py +++ b/src/ansible_builder/ee_schema.py @@ -1,4 +1,5 @@ import os +import posixpath from jsonschema import validate, SchemaError, ValidationError @@ -472,7 +473,7 @@ def _handle_options_defaults(ee_def: dict): """ options = ee_def.setdefault('options', {}) - entrypoint_path = os.path.join(constants.FINAL_IMAGE_BIN_PATH, "entrypoint") + entrypoint_path = posixpath.join(constants.FINAL_IMAGE_BIN_PATH, "entrypoint") options.setdefault('skip_ansible_check', False) options.setdefault('skip_pip_install', False) From fa672cc914f93100ced5f93b63a96f1fc385023b Mon Sep 17 00:00:00 2001 From: Kevin Shurtz Date: Wed, 30 Oct 2024 22:45:31 -0500 Subject: [PATCH 3/3] test: add unit test for posix path separator This adds a unit test to make sure that when Windows NT path separator characters are preferred by os.path, the entrypoint will still be defined with POSIX path separators. --- test/unit/test_containerfile.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/unit/test_containerfile.py b/test/unit/test_containerfile.py index 2d440e83..f30474eb 100644 --- a/test/unit/test_containerfile.py +++ b/test/unit/test_containerfile.py @@ -189,6 +189,26 @@ def test_v3_various(build_dir_and_ee_yml): assert 'CMD ["csh"]' in c.steps +def test__posix_separator_in_entrypoint_on_windows(build_dir_and_ee_yml, mocker): + """ + Test that the generated entrypoint uses the POSIX separator on Windows. + """ + ee_data = """ + version: 3 + images: + base_image: + name: quay.io/user/mycustombaseimage:latest + """ + import ntpath + mocker.patch("os.path", ntpath) + + tmpdir, ee_path = build_dir_and_ee_yml(ee_data) + c = make_containerfile(tmpdir, ee_path, run_validate=True) + c.prepare() + + assert 'ENTRYPOINT ["/opt/builder/bin/entrypoint", "dumb-init"]' in c.steps + + def test__handle_additional_build_files(build_dir_and_ee_yml): """ Test additional build file handling works as expected.