Repository: ansible/ansible-builder

Issue:
Account for PEP668 in `pip` invocations during build
Newer versions of `pip` implement [PEP668](https://peps.python.org/pep-0668/) support, which will soon be enabled default (TL;DR, it will fail to install packages to system-managed Python environments, on which ansible-builder relies heavily).

Switching to virtual environments in EEs is not really an option for the short-medium term anyway; we need to continue to assume the ability to install any and all necessary packages in the system Python environment. 

Luckily it should be an easy fix; we just need to ensure we're setting `PIP_BREAK_SYSTEM_PACKAGES=1` in the environment for on all of builder's `pip` invocations; there shouldn't be any consequence to setting this for older pip versions, and it'll keep new ones functioning.

Discussion:
nitzmahone: This looks good, but was just playing with it while testing some other core PEP668 work Matt C and I were doing and tripped over a nasty problem that's only apparent when actually running in a PEP668-marked environment: it doesn't look like `ensurepip` can respect the envvar, and will thus refuse to work at all.

It doesn't look like any RHELish OSs are marking things yet (only recent Debian/Ubuntu), but it's easy to fake it up for testing using the following as your base image:

``` Dockerfile
FROM fedora:39
RUN touch /usr/lib64/python3.12/EXTERNALLY-MANAGED
```

Build that locally, then use the resultant image as your base in a vanilla EE with the default Python settings, and it'll :bomb: hard on `ensurepip`. It [explicitly blocks](https://github.com/python/cpython/blob/v3.12.0/Lib/ensurepip/__init__.py#L111-L120) inheritance of all `PIP_` envvars when pip is launched to bootstrap itself, and there's no other apparent way to pass that through.

So either we'll need to [look up](https://peps.python.org/pep-0668/#marking-an-interpreter-as-using-an-external-package-manager), zap (and possibly re-create?) the marker file, not use `ensurepip`, or ... something else.
Shrews: Well that's a bunch of poo...  For reference, this builder EE is enough show the error:

```
---
version: 3

images:
  base_image:
    name: docker.io/library/fedora:39

additional_build_steps:
  prepend_base:
    - RUN touch /usr/lib64/python3.12/EXTERNALLY-MANAGED
```
sivel: I have no idea if this is intended behavior, but you can also bypass `EXTERNALLY-MANAGED` by just supplying `--root`, with either `pip` or `ensurepip`.  If you set `--root /` that works just like we do now.

```console
[root@8f8a1523eade /]# touch /usr/lib64/python3.12/EXTERNALLY-MANAGED
[root@8f8a1523eade /]# python3 -m ensurepip
error: externally-managed-environment

× This environment is externally managed
╰─> The Python environment under /usr is managed externally, and may not be
    manipulated by the user. Please use specific tooling from the distributor of
    the Python installation to interact with this environment instead.


note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
hint: See PEP 668 for the detailed specification.
[SNIP]
[root@8f8a1523eade /]# python3 -m ensurepip --root /
Looking in links: /tmp/tmpn6uk_n0x
Processing /tmp/tmpn6uk_n0x/pip-23.2.1-py3-none-any.whl
Installing collected packages: pip
Successfully installed pip-23.2.1
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
```
sivel: Ok, using `--root` is expected to bypass the check:

https://github.com/pypa/pip/blob/9685f64fe8c78e1e39cd9b32e5615f42e0a01f1c/src/pip/_internal/commands/install.py#L270-L274

```
        # Check whether the environment we're installing into is externally
        # managed, as specified in PEP 668. Specifying --root, --target, or
        # --prefix disables the check, since there's no reliable way to locate
        # the EXTERNALLY-MANAGED file for those cases. An exception is also
        # made specifically for "--dry-run --report" for convenience.
```


nitzmahone: I'm going to verify one more thing to be sure, but after messing around with `ensurepip --root /`, I think it's basically working for us by accident. I didn't really like adding `ensurepip` in the first place, but it's kind of a necessary evil for common vanilla base images that wouldn't require users to always manually add a `prepend_base` and figure out how to get pip in there.

Much as I hate to say it, I think we're going to want to hide the `ensurepip` stuff behind a reusable script indirection where we can do a little more work to make sure we've got a pip available that can do what we want. The risk is that that script indirection becomes another place where everyone wants to hang "magic" to silently make pip work for $random_base_image_with_unique_busted_pip, but I think it's preferable to the game of distributed whac-a-mole I can see this one becoming down the road :laughing: .

Shrews: Per internal discussion, going to move the ensurepip call to a new target script to encapsulate future changes that might be needed for non-RHEL distros, and add a flag to disable the call.

Patch:
diff --git a/docs/definition.rst b/docs/definition.rst
index 1e8e95c..0fc14c3 100644
--- a/docs/definition.rst
+++ b/docs/definition.rst
@@ -365,6 +365,13 @@ builder runtime functionality. Valid keys for this section are:
       of Ansible and Ansible Runner is performed on the final image. Set this
       value to ``True`` to not perform this check. The default is ``False``.
 
+    ``skip_pip_install``
+      This boolean value controls whether or not we attempt to install pip into
+      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
       write permission to ``/etc/passwd`` in the final container image. The default entrypoint
diff --git a/src/ansible_builder/_target_scripts/assemble b/src/ansible_builder/_target_scripts/assemble
index d82e5c4..c04c5f1 100755
--- a/src/ansible_builder/_target_scripts/assemble
+++ b/src/ansible_builder/_target_scripts/assemble
@@ -31,8 +31,6 @@ PKGMGR_PRESERVE_CACHE="${PKGMGR_PRESERVE_CACHE:-}"
 PYCMD="${PYCMD:=/usr/bin/python3}"
 PIPCMD="${PIPCMD:=$PYCMD -m pip}"
 
-$PYCMD -m ensurepip
-
 if [ -z $PKGMGR ]; then
     # Expect dnf to be installed, however if we find microdnf default to it.
     PKGMGR=/usr/bin/dnf
diff --git a/src/ansible_builder/_target_scripts/install-from-bindep b/src/ansible_builder/_target_scripts/install-from-bindep
index c580707..cee2068 100755
--- a/src/ansible_builder/_target_scripts/install-from-bindep
+++ b/src/ansible_builder/_target_scripts/install-from-bindep
@@ -25,8 +25,6 @@ PYCMD="${PYCMD:=/usr/bin/python3}"
 PIPCMD="${PIPCMD:=$PYCMD -m pip}"
 PIP_OPTS="${PIP_OPTS-}"
 
-$PYCMD -m ensurepip
-
 if [ -z $PKGMGR ]; then
     # Expect dnf to be installed, however if we find microdnf default to it.
     PKGMGR=/usr/bin/dnf
diff --git a/src/ansible_builder/_target_scripts/pip_install b/src/ansible_builder/_target_scripts/pip_install
new file mode 100755
index 0000000..46fcdde
--- /dev/null
+++ b/src/ansible_builder/_target_scripts/pip_install
@@ -0,0 +1,56 @@
+#!/bin/bash
+# Copyright (c) 2024 Red Hat, Inc.
+#
+# Licensed 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.
+
+#####################################################################
+# Script to encapsulate pip installation.
+#
+# Usage: pip_install <PYCMD>
+#
+# Options:
+#     PYCMD - The path to the python executable to use.
+#####################################################################
+
+set -x
+
+PYCMD=$1
+
+if [ -z "$PYCMD" ]
+then
+    echo "Usage: pip_install <PYCMD>"
+    exit 1
+fi
+
+if [ ! -x "$PYCMD" ]
+then
+    echo "$PYCMD is not an executable"
+    exit 1
+fi
+
+# This is going to be our default functionality for now. This will likely
+# need to change if we add support for non-RHEL distros.
+$PYCMD -m ensurepip --root /
+
+if [ $? -ne 0 ]
+then
+    cat<<EOF
+**********************************************************************
+ERROR - pip installation failed for Python $PYCMD
+**********************************************************************
+EOF
+    exit 1
+fi
+
+exit 0
diff --git a/src/ansible_builder/containerfile.py b/src/ansible_builder/containerfile.py
index 426f40a..d9092bf 100644
--- a/src/ansible_builder/containerfile.py
+++ b/src/ansible_builder/containerfile.py
@@ -76,9 +76,11 @@ class Containerfile:
             "# Base build stage",
             "FROM $EE_BASE_IMAGE as base",
             "USER root",
+            "ENV PIP_BREAK_SYSTEM_PACKAGES=1",
         ])
 
         self._insert_global_args()
+        self._create_folder_copy_files()
         self._insert_custom_steps('prepend_base')
 
         if not self.definition.builder_image:
@@ -86,13 +88,13 @@ class Containerfile:
                 step = 'RUN $PKGMGR install $PYPKG -y ; if [ -z $PKGMGR_PRESERVE_CACHE ]; then $PKGMGR clean all; fi'
                 self.steps.append(step)
 
-            # We should always make sure pip is available for later stages.
-            self.steps.append('RUN $PYCMD -m ensurepip')
+            # pip needs to be available for later stages.
+            if self.definition.version >= 3 and not self.definition.options['skip_pip_install']:
+                self.steps.append('RUN /output/scripts/pip_install $PYCMD')
 
             if self.definition.ansible_ref_install_list:
                 self.steps.append('RUN $PYCMD -m pip install --no-cache-dir $ANSIBLE_INSTALL_REFS')
 
-        self._create_folder_copy_files()
         self._insert_custom_steps('append_base')
 
         ######################################################################
@@ -124,7 +126,7 @@ class Containerfile:
         # 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:
@@ -135,6 +137,7 @@ class Containerfile:
             "",
             "# Builder build stage",
             f"FROM {image} as builder",
+            "ENV PIP_BREAK_SYSTEM_PACKAGES=1",
             "WORKDIR /build",
         ])
 
@@ -142,6 +145,11 @@ class Containerfile:
 
         if image == "base":
             self.steps.append("RUN $PYCMD -m pip install --no-cache-dir bindep pyyaml requirements-parser")
+        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("RUN /output/scripts/pip_install $PYCMD")
 
         self._insert_custom_steps('prepend_builder')
         self._prepare_galaxy_copy_steps()
@@ -156,6 +164,7 @@ class Containerfile:
             "",
             "# Final build stage",
             "FROM base as final",
+            "ENV PIP_BREAK_SYSTEM_PACKAGES=1",
         ])
 
         self._insert_global_args()
@@ -279,7 +288,7 @@ class Containerfile:
         scriptres = importlib.resources.files('ansible_builder._target_scripts')
         script_files = (
             'assemble', 'install-from-bindep', 'introspect.py', 'check_galaxy',
-            'check_ansible', 'entrypoint'
+            'check_ansible', 'pip_install', 'entrypoint'
         )
         for script in script_files:
             with importlib.resources.as_file(scriptres / script) as script_path:
diff --git a/src/ansible_builder/ee_schema.py b/src/ansible_builder/ee_schema.py
index 61db11c..9cdda75 100644
--- a/src/ansible_builder/ee_schema.py
+++ b/src/ansible_builder/ee_schema.py
@@ -341,6 +341,10 @@ schema_v3 = {
                     "description": "Disables the check for Ansible/Runner in final image",
                     "type": "boolean",
                 },
+                "skip_pip_install": {
+                    "description": "Disables the installation of pip in the base image",
+                    "type": "boolean",
+                },
                 "workdir": {
                     "description": "Default working directory, also often the homedir for ephemeral UIDs",
                     "type": ["string", "null"],
@@ -444,6 +448,7 @@ def _handle_options_defaults(ee_def: dict):
     entrypoint_path = os.path.join(constants.FINAL_IMAGE_BIN_PATH, "entrypoint")
 
     options.setdefault('skip_ansible_check', False)
+    options.setdefault('skip_pip_install', False)
     options.setdefault('relax_passwd_permissions', True)
     options.setdefault('workdir', '/runner')
     options.setdefault('package_manager_path', '/usr/bin/dnf')


Test patch:
diff --git a/test/unit/test_containerfile.py b/test/unit/test_containerfile.py
index b78be4a..d988b4e 100644
--- a/test/unit/test_containerfile.py
+++ b/test/unit/test_containerfile.py
@@ -207,3 +207,125 @@ def test__handle_additional_build_files(build_dir_and_ee_yml):
     config_dir = tmpdir / '_build' / 'configs'
     assert config_dir.exists()
     assert (config_dir / 'ansible.cfg').exists()
+
+
+def test_pep668_v1(build_dir_and_ee_yml):
+    """
+    Test PEP668 handling with v1 format.
+    """
+    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 "ENV PIP_BREAK_SYSTEM_PACKAGES=1" in c.steps
+    assert "RUN /output/scripts/pip_install $PYCMD" in c.steps
+
+
+def test_pep668_v2(build_dir_and_ee_yml):
+    """
+    Test PEP668 handling with v2 format.
+    """
+    ee_data = """
+    version: 2
+    images:
+      base_image:
+        name: quay.io/user/mycustombaseimage:latest
+      builder_image:
+        name: quay.io/user/mycustombuilderimage:latest
+    """
+    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" in c.steps
+
+
+def test_pep668_v3(build_dir_and_ee_yml):
+    """
+    Test PEP668 handling with v3 format.
+    """
+    ee_data = """
+    version: 3
+    images:
+      base_image:
+        name: quay.io/user/mycustombaseimage:latest
+    """
+    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" 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).
+    Also test that pip_install is installed into the custom builder image.
+    """
+    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
+
+
+def test_v2_builder_image(build_dir_and_ee_yml):
+    """
+    Test that pip_install is installed into the custom v2 builder image.
+    """
+    ee_data = """
+    version: 2
+    images:
+      base_image:
+        name: quay.io/user/mycustombaseimage:latest
+      builder_image:
+        name: quay.io/user/mycustombuilderimage:latest
+    """
+    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
+
+
+def test_v2_builder_image_default(build_dir_and_ee_yml):
+    """
+    Test that pip_install is NOT installed into the builder image when not defined.
+    """
+    ee_data = """
+    version: 2
+    images:
+      base_image:
+        name: quay.io/user/mycustombaseimage:latest
+    """
+    tmpdir, ee_path = build_dir_and_ee_yml(ee_data)
+    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