feat(dockerfile)!: remove venv, install pip packages into base Python (#59)

* feat(dockerfile)!: remove venv, install pip packages into base Python

Pip packages used to be installed into /opt/venv, which was created
with --system-site-packages. On base images that already shipped a
Python environment (pytorch/pytorch, python:3.11-slim, etc.) pip's
dependency resolver still re-downloaded packages like torch into the
venv, duplicating gigabytes of content. The --system-site-packages
flag only helps at import time, not at install time.

Install pip packages directly into whatever Python is on PATH. On
images with a pre-existing Python environment (conda, python images)
pip sees existing packages and skips them. On Debian/Ubuntu images
where pip would otherwise be blocked by PEP 668, the marker file at
/usr/lib/python*/EXTERNALLY-MANAGED is removed once before the first
pip step. PEP 668 is a host-system protection against apt/pip
conflicts; it does not apply inside containers.

BREAKING CHANGE: /opt/venv no longer exists in generated Dockerfiles.
ENV VIRTUAL_ENV and the /opt/venv/bin PATH prefix are gone. Scripts
that explicitly reference /opt/venv/bin/python or similar paths must
be updated to use /usr/bin/python3, /opt/conda/bin/python, or
whichever Python the chosen base image provides.

The runtime user no longer has write access to pip-installed
packages by default. Interactive 'pip install' inside a running
container now requires either 'pip install --user foo' (installs to
the user's home) or running the container with '--user 0' to
temporarily run as root.

* test(dockerfile): cover pip marker removal and no-venv behaviour

* docs: remove venv mention from features list

* fix(dockerfile): flatten create_user shell structure for shellcheck

Replace nested 'else; if' patterns with 'elif' and '||' short-circuits
to eliminate hadolint SC1075 errors on generated Dockerfiles. Also
suppress DL4006 on the user-creation RUN: the pipe's failure semantics
are intentional (getent failure means 'no such user', which is the
branch we want to take) and pipefail would be a no-op.

Resolves shellcheck-flagged errors in generated Dockerfiles across
all projects. No behavioural change.

Author: markhedleyjones

README.md

@@ -68,7 +68,7 @@ Build and run the production image:
 * **Automatic user handling** - host user identity in dev, dedicated user in prod, no manual setup
 * **GPU, display, and audio** - NVIDIA GPU passthrough, X11/Wayland forwarding, PulseAudio/PipeWire
 * **Custom commands** - define once, use in both dev and prod with port publishing and environment variables
-* **Multi-stage builds** - share steps between stages, automatic virtual environments for pip
+* **Multi-stage builds** - share steps between stages, pip packages install into the base image's Python (no venv duplication)
 * **Transparent execution** - run commands from anywhere in your repo with automatic path translation
 * **Data volumes** - shorthand for sibling folders (`outputs`, `cache`) that persist across runs without entering the image
 * **AWS credential forwarding** - mount host AWS config into the container

src/container_magic/generators/dockerfile.py

@@ -178,7 +178,7 @@ def process_stage_steps(
     registry: Dict = None,
     asset_map: Dict[str, str] = None,
     workspace_symlinks: List[tuple] = None,
-    venv_active: bool = False,
+    pip_prepared: bool = False,
     implicit_user: bool = False,
 ) -> tuple:
     """Process build steps for a stage.
@@ -190,7 +190,7 @@ def process_stage_steps(
     symlink overlay data for the Dockerfile template.
 
     Returns:
-        (ordered_steps, venv_active) tuple
+        (ordered_steps, pip_prepared) tuple
     """
     if registry is None:
         registry = load_registry()
@@ -202,7 +202,7 @@ def process_stage_steps(
     # Default build order if not specified
     if stage.steps is None:
         if ":" in stage.frm or "/" in stage.frm:
-            return [], venv_active
+            return [], pip_prepared
         else:
             steps: List[Union[str, Dict[str, Any]]] = []
             if stage_name == "production":
@@ -235,13 +235,13 @@ def _resolve_user_ref(name: str) -> str:
     ordered_steps = []
 
     for step in steps:
-        if _step_is_pip(step) and not venv_active:
+        if _step_is_pip(step) and not pip_prepared:
             is_root = current_user is None
-            venv_step = {"type": "venv_setup", "is_root": is_root}
+            prepare_step = {"type": "prepare_pip", "is_root": is_root}
             if not is_root:
-                venv_step["restore_user"] = current_user
-            ordered_steps.append(venv_step)
-            venv_active = True
+                prepare_step["restore_user"] = current_user
+            ordered_steps.append(prepare_step)
+            pip_prepared = True
 
         parsed = parse_step(step, registry)
 
@@ -307,7 +307,7 @@ def _resolve_user_ref(name: str) -> str:
         elif step_type == "passthrough":
             ordered_steps.append({"type": "custom", "command": parsed["command"]})
 
-    return ordered_steps, venv_active
+    return ordered_steps, pip_prepared
 
 
 def generate_dockerfile(
@@ -369,7 +369,7 @@ def generate_dockerfile(
     leaf_stages = set(stages.keys()) - inherited_stages
 
     stages_data = []
-    venv_state: Dict[str, bool] = {}
+    pip_prepared_state: Dict[str, bool] = {}
     user_created_state: Dict[str, bool] = {}
     for stage_name, stage_config in stages.items():
         base_image = stage_config.frm
@@ -392,13 +392,13 @@ def generate_dockerfile(
         user_creation_style = distro_ucs or detect_user_creation_style(resolved_image)
 
         if from_is_image:
-            inherited_venv = False
+            inherited_pip_prepared = False
             inherited_user_created = False
         else:
-            inherited_venv = venv_state.get(base_image, False)
+            inherited_pip_prepared = pip_prepared_state.get(base_image, False)
             inherited_user_created = user_created_state.get(base_image, False)
 
-        ordered_steps, venv_active = process_stage_steps(
+        ordered_steps, pip_prepared = process_stage_steps(
             stage_config,
             stage_name,
             project_dir,
@@ -408,10 +408,10 @@ def generate_dockerfile(
             registry,
             asset_map,
             workspace_symlinks,
-            venv_active=inherited_venv,
+            pip_prepared=inherited_pip_prepared,
             implicit_user=implicit_user,
         )
-        venv_state[stage_name] = venv_active
+        pip_prepared_state[stage_name] = pip_prepared
 
         # Inject implicit create_user for from-image stages
         has_explicit_create_in_steps = any(
@@ -425,21 +425,6 @@ def generate_dockerfile(
         else:
             user_created_state[stage_name] = inherited_user_created
 
-        # Chown venv to runtime user in leaf stages so it's writable in development
-        if has_user and stage_name in leaf_stages and venv_active:
-            # Determine if we're in a non-root context (inherited from parent)
-            last_become_user = None
-            for s in reversed(ordered_steps):
-                if s.get("type") == "become":
-                    last_become_user = s.get("name")
-                    break
-            inherited_context = _get_parent_user_context(stage_name, stages, user_name)
-            is_root = last_become_user is None and inherited_context is None
-            chown_step = {"type": "venv_chown", "is_root": is_root}
-            if not is_root:
-                chown_step["restore_user"] = last_become_user or inherited_context
-            ordered_steps.append(chown_step)
-
         # Inject implicit become at end of leaf stages only
         # Intermediate stages stay as root so child stages inherit root context
         if has_user and stage_name in leaf_stages:

src/container_magic/templates/Dockerfile.j2

@@ -34,53 +34,38 @@ ARG USER_GID={{ stage.user_gid }} \
 {% for step in stage.ordered_steps %}
 {% if step.type == "create_user" %}
 # Create user account (handles renaming if UID exists with different name)
+# hadolint ignore=DL4006
 {% if stage.user_creation_style == "alpine" %}
 RUN if [ "${USER_NAME}" != "root" ]; then \
-        # Check if the user already exists with exact match (name + uid + gid)
-        if getent passwd "${USER_UID}" | grep -q "^${USER_NAME}:"; then \
+        EXISTING_USER=$(getent passwd "${USER_UID}" | cut -d: -f1); \
+        if [ "${EXISTING_USER}" = "${USER_NAME}" ]; then \
             echo "User ${USER_NAME} with UID ${USER_UID} already exists, skipping creation"; \
+        elif [ -n "${EXISTING_USER}" ]; then \
+            echo "Renaming user ${EXISTING_USER} to ${USER_NAME}"; \
+            deluser "${EXISTING_USER}" && \
+            addgroup -g "${USER_GID}" "${USER_NAME}" 2>/dev/null; \
+            adduser -D -u "${USER_UID}" -G "${USER_NAME}" -h "${USER_HOME}" "${USER_NAME}"; \
         else \
-            # Check if UID exists with a different name
-            EXISTING_USER=$(getent passwd "${USER_UID}" | cut -d: -f1 || echo ""); \
-            if [ -n "${EXISTING_USER}" ] && [ "${EXISTING_USER}" != "${USER_NAME}" ]; then \
-                # Rename the existing user to our desired name
-                echo "Renaming user ${EXISTING_USER} to ${USER_NAME}"; \
-                deluser "${EXISTING_USER}"; \
-                addgroup -g "${USER_GID}" "${USER_NAME}" 2>/dev/null || true; \
-                adduser -D -u "${USER_UID}" -G "${USER_NAME}" -h "${USER_HOME}" "${USER_NAME}"; \
-            else \
-                # Create new user
-                if ! getent group "${USER_GID}" >/dev/null 2>&1; then \
-                    addgroup -g "${USER_GID}" "${USER_NAME}"; \
-                fi && \
-                adduser -D -u "${USER_UID}" -G "${USER_NAME}" -h "${USER_HOME}" "${USER_NAME}"; \
-            fi \
+            getent group "${USER_GID}" >/dev/null 2>&1 || \
+                addgroup -g "${USER_GID}" "${USER_NAME}"; \
+            adduser -D -u "${USER_UID}" -G "${USER_NAME}" -h "${USER_HOME}" "${USER_NAME}"; \
         fi && \
-        # Ensure home directory ownership (may have been created by WORKDIR)
         chown "${USER_UID}:${USER_GID}" "${USER_HOME}"; \
     fi
 {% else %}
 RUN if [ "${USER_NAME}" != "root" ]; then \
-        # Check if the user already exists with exact match (name + uid + gid)
-        if getent passwd "${USER_UID}" | grep -q "^${USER_NAME}:"; then \
+        EXISTING_USER=$(getent passwd "${USER_UID}" | cut -d: -f1); \
+        if [ "${EXISTING_USER}" = "${USER_NAME}" ]; then \
             echo "User ${USER_NAME} with UID ${USER_UID} already exists, skipping creation"; \
+        elif [ -n "${EXISTING_USER}" ]; then \
+            echo "Renaming user ${EXISTING_USER} to ${USER_NAME}"; \
+            usermod -l "${USER_NAME}" "${EXISTING_USER}" && \
+            usermod -d "${USER_HOME}" -m "${USER_NAME}" || true; \
         else \
-            # Check if UID exists with a different name
-            EXISTING_USER=$(getent passwd "${USER_UID}" | cut -d: -f1 || echo ""); \
-            if [ -n "${EXISTING_USER}" ] && [ "${EXISTING_USER}" != "${USER_NAME}" ]; then \
-                # Rename the existing user to our desired name
-                echo "Renaming user ${EXISTING_USER} to ${USER_NAME}"; \
-                usermod -l "${USER_NAME}" "${EXISTING_USER}" && \
-                usermod -d "${USER_HOME}" -m "${USER_NAME}" || true; \
-            else \
-                # Create new user
-                if ! getent group "${USER_GID}" >/dev/null 2>&1; then \
-                    groupadd --gid "${USER_GID}" "${USER_NAME}"; \
-                fi && \
-                useradd --uid "${USER_UID}" --gid "${USER_GID}" --home-dir "${USER_HOME}" --create-home "${USER_NAME}"; \
-            fi \
+            getent group "${USER_GID}" >/dev/null 2>&1 || \
+                groupadd --gid "${USER_GID}" "${USER_NAME}"; \
+            useradd --uid "${USER_UID}" --gid "${USER_GID}" --home-dir "${USER_HOME}" --create-home "${USER_NAME}"; \
         fi && \
-        # Ensure home directory ownership (may have been created by WORKDIR)
         chown "${USER_UID}:${USER_GID}" "${USER_HOME}"; \
     fi
 {% endif %}
@@ -122,21 +107,16 @@ ENV {% for key, value in step.vars.items() %}{% if not loop.first %}    {% endif
 {% endif %}{% endfor %}
 
 {% endif %}
-{% elif step.type == "venv_setup" %}
+{% elif step.type == "prepare_pip" %}
 {% if not step.is_root %}
 USER root
 {% endif %}
-RUN test -f /opt/venv/pyvenv.cfg || python3 -m venv --system-site-packages /opt/venv
-ENV VIRTUAL_ENV=/opt/venv
-ENV PATH="${VIRTUAL_ENV}/bin:${PATH}"
-{% if not step.is_root %}
-USER {{ step.restore_user }}
-{% endif %}
-{% elif step.type == "venv_chown" %}
-{% if not step.is_root %}
-USER root
-{% endif %}
-RUN chown -R "${USER_UID}:${USER_GID}" /opt/venv
+# Disable PEP 668 (externally-managed-environment): host-system protection
+# against apt/pip conflicts, not applicable inside containers where the whole
+# environment is disposable.
+RUN rm -f /usr/lib/python*/EXTERNALLY-MANAGED \
+          /usr/local/lib/python*/EXTERNALLY-MANAGED \
+          /opt/conda/lib/python*/EXTERNALLY-MANAGED
 {% if not step.is_root %}
 USER {{ step.restore_user }}
 {% endif %}

tests/unit/test_implicit_user.py

@@ -308,7 +308,7 @@ def test_child_distro_overrides_parent(self):
         assert "useradd" in other
 
 
-class TestImplicitUserWithVenv:
+class TestImplicitUserWithPip:
     def test_implicit_user_with_pip_step(self):
         """Pip step with implicit user creation should work correctly."""
         config = {
@@ -324,8 +324,8 @@ def test_implicit_user_with_pip_step(self):
         }
         content = _generate(config)
         assert "useradd" in content
-        assert "python3 -m venv" in content
+        assert "# Disable PEP 668" in content
         lines = content.splitlines()
         create_idx = next(i for i, ln in enumerate(lines) if "useradd" in ln)
-        venv_idx = next(i for i, ln in enumerate(lines) if "venv" in ln and "RUN" in ln)
-        assert create_idx < venv_idx
+        marker_idx = next(i for i, ln in enumerate(lines) if "# Disable PEP 668" in ln)
+        assert create_idx < marker_idx