From fe2b95684d5942c5c7bcc084ac50f73d3d933ba8 Mon Sep 17 00:00:00 2001
From: David Shrewsbury <dshrewsb@redhat.com>
Date: Tue, 30 Jan 2024 16:03:31 -0500
Subject: [PATCH] Fix issue 646 bug

---
 docs/definition.rst                  |  1 +
 src/ansible_builder/containerfile.py |  6 ++--
 test/unit/test_containerfile.py      | 49 ++++++++++++++++++++++++----
 3 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/docs/definition.rst b/docs/definition.rst
index 2ccfdbe6..611b467d 100644
--- a/docs/definition.rst
+++ b/docs/definition.rst
@@ -357,6 +357,7 @@ builder runtime functionality. Valid keys for this section are:
       the base image. Pip is necessary for Python requirement installation, among
       other things. You may choose to disable this step and handle installing
       pip manually if the current method of pip installation does not work for you.
+      The default is ``False``.
 
     ``relax_passwd_permissions``
       This boolean value controls whether the ``root`` group (GID 0) is explicitly granted
diff --git a/src/ansible_builder/containerfile.py b/src/ansible_builder/containerfile.py
index 86dcc2b0..3aadf728 100644
--- a/src/ansible_builder/containerfile.py
+++ b/src/ansible_builder/containerfile.py
@@ -127,7 +127,7 @@ def prepare(self) -> None:
         # Second stage (aka, builder): assemble (pip installs, bindep run)
         ######################################################################
 
-        if self.definition.builder_image:
+        if self.definition.builder_image or self.definition.version == 1:
             # Note: A builder image can be specified only in V1 or V2 schema.
             image = "$EE_BUILDER_IMAGE"
         else:
@@ -147,7 +147,9 @@ def prepare(self) -> None:
         if image == "base":
             self.steps.append("RUN $PYCMD -m pip install --no-cache-dir bindep pyyaml requirements-parser")
         else:
-            # For < v3 with a custom builder image, we always make sure pip is available.
+            # 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("RUN /output/scripts/pip_install $PYCMD")
 
         self._insert_custom_steps('prepend_builder')
diff --git a/test/unit/test_containerfile.py b/test/unit/test_containerfile.py
index 2ea1e8a5..dd1eb478 100644
--- a/test/unit/test_containerfile.py
+++ b/test/unit/test_containerfile.py
@@ -209,9 +209,9 @@ def test__handle_additional_build_files(build_dir_and_ee_yml):
     assert (config_dir / 'ansible.cfg').exists()
 
 
-def test_pep668_env_var_v1(build_dir_and_ee_yml):
+def test_pep668_v1(build_dir_and_ee_yml):
     """
-    Test that we add the pip env var to handle PEP668.
+    Test PEP668 handling with v1 format.
     """
     ee_data = """
     version: 1
@@ -220,11 +220,12 @@ def test_pep668_env_var_v1(build_dir_and_ee_yml):
     c = make_containerfile(tmpdir, ee_path, run_validate=True)
     c.prepare()
     assert "ENV PIP_BREAK_SYSTEM_PACKAGES=1" in c.steps
+    assert "RUN /output/scripts/pip_install $PYCMD" in c.steps
 
 
-def test_pep668_env_var_v2(build_dir_and_ee_yml):
+def test_pep668_v2(build_dir_and_ee_yml):
     """
-    Test that we add the pip env var to handle PEP668.
+    Test PEP668 handling with v2 format.
     """
     ee_data = """
     version: 2
@@ -238,11 +239,12 @@ def test_pep668_env_var_v2(build_dir_and_ee_yml):
     c = make_containerfile(tmpdir, ee_path, run_validate=True)
     c.prepare()
     assert "ENV PIP_BREAK_SYSTEM_PACKAGES=1" in c.steps
+    assert "RUN /output/scripts/pip_install $PYCMD" in c.steps
 
 
-def test_pep668_env_var_v3(build_dir_and_ee_yml):
+def test_pep668_v3(build_dir_and_ee_yml):
     """
-    Test that we add the pip env var to handle PEP668.
+    Test PEP668 handling with v3 format.
     """
     ee_data = """
     version: 3
@@ -254,3 +256,38 @@ def test_pep668_env_var_v3(build_dir_and_ee_yml):
     c = make_containerfile(tmpdir, ee_path, run_validate=True)
     c.prepare()
     assert "ENV PIP_BREAK_SYSTEM_PACKAGES=1" in c.steps
+    assert "RUN /output/scripts/pip_install $PYCMD" in c.steps
+
+
+def test_pep668_v3_skip_pip_install(build_dir_and_ee_yml):
+    """
+    Test PEP668 handling with v3 format and skipping pip installation.
+    """
+    ee_data = """
+    version: 3
+    images:
+      base_image:
+        name: quay.io/user/mycustombaseimage:latest
+    options:
+      skip_pip_install: True
+    """
+    tmpdir, ee_path = build_dir_and_ee_yml(ee_data)
+    c = make_containerfile(tmpdir, ee_path, run_validate=True)
+    c.prepare()
+    assert "ENV PIP_BREAK_SYSTEM_PACKAGES=1" in c.steps
+    assert "RUN /output/scripts/pip_install $PYCMD" not in c.steps
+
+
+def test_v1_builder_image(build_dir_and_ee_yml):
+    """
+    Test for issue 646 (https://github.com/ansible/ansible-builder/issues/646).
+    """
+    ee_data = """
+    version: 1
+    """
+    tmpdir, ee_path = build_dir_and_ee_yml(ee_data)
+    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 "RUN /output/scripts/pip_install $PYCMD" in c.steps