Repository: ansible/ansible-builder

Issue:
Determine default upstream/downstream USER/WORKDIR/homedir behavior
Currently, builder 3.0 is defaulting to `USER root`, and no `WORKDIR` (which ends up being `/`) - this causes various issues when the container is run as an arbitrary non-existent non-root user under some container runtimes. The ansible-runner container base images (and some downstream derivatives) used various `ENTRYPOINT` script magic to paper over these issues, but at the cost of hardcoded/broken homedirs and a number of other issues. The need for these tweaks has significantly decreased now that runner no longer requires knowledge of the container user's homedir.

I'm guessing we're going to at least need to set `WORKDIR` to something like `/ansible-ee` or whatever that's `root/root` and `rwx` for the `root` group to accommodate container runtimes that want to run it as arbitrary bogus UIDs. Permissions on `/` set at build-time don't appear to be preserved in the final container image, at least under podman, so with the default `WORKDIR` of `/`, runtime-injected users that use `WORKDIR` as `HOME` are non-functional.

Need to get an actual documented set of requirements on this one to be sure we can cover all the bases.

Discussion:

Patch:
diff --git a/MANIFEST.in b/MANIFEST.in
deleted file mode 100644
index 9c9c8c8..0000000
--- a/MANIFEST.in
+++ /dev/null
@@ -1,1 +0,0 @@
-include ansible_builder/_target_scripts/*
diff --git a/ansible_builder/_target_scripts/entrypoint b/ansible_builder/_target_scripts/entrypoint
new file mode 100755
index 0000000..145381a
--- /dev/null
+++ b/ansible_builder/_target_scripts/entrypoint
@@ -0,0 +1,152 @@
+#!/usr/bin/env bash
+
+# Copyright: (c) 2023, Ansible Project
+# Apache License, Version 2.0 (see LICENSE.md or https://www.apache.org/licenses/LICENSE-2.0)
+
+# This entrypoint script papers over a number of problems that manifest under different container runtimes when
+# using ephemeral UIDs, then chain-execs to the requested init system and/or command. It is an implementation
+# detail for the convenience of Ansible execution environments built by ansible-builder.
+#
+# If we're running as a legit user that has an entry in /etc/passwd and a valid and writeable homedir, we're all good.
+#
+# If the current uid is not in /etc/passwd, we'll attempt to add it, but /etc/passwd is often not writable by GID 0.
+# `ansible-builder` defaults to making /etc/passwd writable by GID0 by default for maximum compatibility, but this is
+# not guaranteed. Some runtimes/wrappers (eg podman, cri-o) already create an /etc/passwd entry on the fly as-needed,
+# but they may set the homedir to something inaccessible (eg, `/`, WORKDIR).
+#
+# There are numerous cases where a missing or incorrect homedir in /etc/passwd are fatal. It breaks
+# `async` in ansible-core, things like `echo ~someuid`, and numerous other software packages that assume a valid POSIX
+# user configuration.
+#
+# If the homedir listed in /etc/passwd is not writeable by the current user (supposed to be primary GID0), we'll try
+# to make it writeable (except `/`), or select another writeable home directory from `$HOME`, `/runner`, or `/tmp` and
+# update $HOME (and /etc/passwd if possible) accordingly for the current process chain.
+#
+# This script is generally silent by default, but some likely-fatal cases will issue a brief warning to stderr. The
+# envvars described below can be set before container init to cause faster failures and/or get tracing output.
+
+# options:
+# EP_BASH_DEBUG=1 (enable set -x)
+# EP_DEBUG_TRACE=1 (enable debug trace to stderr)
+# EP_ON_ERROR=ignore/warn/fail (default ignore)
+
+set -eu
+
+if (( "${EP_BASH_DEBUG:=0}" == 1 )); then
+  set -x
+fi
+
+: "${EP_DEBUG_TRACE:=0}"
+: "${EP_ON_ERROR:=warn}"
+: "${HOME:=}"
+CUR_UID=$(id -u)
+CUR_USERNAME=$(id -u -n 2> /dev/null || true)  # whoami-free way to get current username, falls back to current uid
+
+DEFAULT_HOME="/runner"
+DEFAULT_SHELL="/bin/bash"
+
+if (( "$EP_DEBUG_TRACE" == 1 )); then
+  function log_debug() { echo "EP_DEBUG: $1" 1>&2; }
+else
+  function log_debug() { :; }
+fi
+
+log_debug "entrypoint.sh started"
+
+case "$EP_ON_ERROR" in
+  "fail")
+    function maybe_fail() { echo "EP_FAIL: $1" 1>&2; exit 1; }
+    ;;
+  "warn")
+    function maybe_fail() { echo "EP_WARN: $1" 1>&2; }
+    ;;
+  *)
+    function maybe_fail() { log_debug "EP_FAIL (ignored): $1"; }
+    ;;
+esac
+
+function is_dir_writable() {
+  [ -d "$1" ] && [ -w "$1" ] && [ -x "$1" ]
+}
+
+function ensure_current_uid_in_passwd() {
+  log_debug "is current uid ${CUR_UID} in /etc/passwd?"
+  
+  if ! getent passwd "${CUR_USERNAME}" &> /dev/null ; then
+    if [ -w "/etc/passwd" ]; then
+      log_debug "appending missing uid ${CUR_UID} into /etc/passwd"
+      # use the default homedir; we may have to rewrite it to another value later if it's inaccessible
+      echo "${CUR_UID}:x:${CUR_UID}:0:container user ${CUR_UID}:${DEFAULT_HOME}:${DEFAULT_SHELL}" >> /etc/passwd
+    else
+      maybe_fail "uid ${CUR_UID} is missing from /etc/passwd, which is not writable; this error is likely fatal"
+    fi
+  else
+    log_debug "current uid is already in /etc/passwd"
+  fi  
+}
+
+function ensure_writeable_homedir() {
+  if (is_dir_writable "${CANDIDATE_HOME}") ; then
+    log_debug "candidate homedir ${CANDIDATE_HOME} is valid and writeable"
+  else
+    if [ "${CANDIDATE_HOME}" == "/" ]; then
+      log_debug "skipping attempt to fix permissions on / as homedir"
+      return 1
+    fi
+
+    log_debug "candidate homedir ${CANDIDATE_HOME} is missing or not writeable; attempt to fix"
+    if ! (mkdir -p "${CANDIDATE_HOME}" >& /dev/null && chmod -R ug+rwx "${CANDIDATE_HOME}" >& /dev/null) ; then
+      log_debug "candidate homedir ${CANDIDATE_HOME} cannot be made writeable"
+      return 1
+    else
+      log_debug "candidate homedir ${CANDIDATE_HOME} was successfully made writeable"
+    fi
+  fi
+
+  # this might work; export it even if we end up not being able to update /etc/passwd
+  # this ensures the envvar matches current reality for this session; future sessions should set automatically if /etc/passwd is accurate
+  export HOME=${CANDIDATE_HOME}
+
+  if [ "${CANDIDATE_HOME}" == "${PASSWD_HOME}" ] ; then
+    log_debug "candidate homedir ${CANDIDATE_HOME} matches /etc/passwd"
+    return 0
+  fi
+
+  if ! [ -w /etc/passwd ]; then
+    log_debug "candidate homedir ${CANDIDATE_HOME} is valid for ${CUR_USERNAME}, but /etc/passwd is not writable to update it"
+    return 1
+  fi
+
+  log_debug "resetting homedir for user ${CUR_USERNAME} to ${CANDIDATE_HOME} in /etc/passwd"
+
+  # sed -i wants to create a tempfile next to the original, which won't work with /etc permissions in many cases,
+  # so just do it in memory and overwrite the existing file if we succeeded
+  NEWPW=$(sed -r "s;(^${CUR_USERNAME}:(.*:){4})(.*:);\1${CANDIDATE_HOME}:;g" /etc/passwd)
+  echo "${NEWPW}" > /etc/passwd
+}
+
+ensure_current_uid_in_passwd
+
+log_debug "current value of HOME is ${HOME}"
+
+PASSWD_HOME=$(getent passwd "${CUR_USERNAME}" | cut -d: -f6)
+log_debug "user ${CUR_USERNAME} homedir from /etc/passwd is ${PASSWD_HOME}"
+
+CANDIDATE_HOMES=("${PASSWD_HOME}" "${HOME}" "${DEFAULT_HOME}" "/tmp")
+
+# we'll set this in the loop as soon as we find a writeable dir
+unset HOME
+
+for CANDIDATE_HOME in "${CANDIDATE_HOMES[@]}"; do
+  if ensure_writeable_homedir ; then
+    break
+  fi
+done
+
+if ! [ -v HOME ] ; then
+  maybe_fail "a valid homedir could not be set for ${CUR_USERNAME}; this is likely fatal"
+fi
+
+# chain exec whatever we were asked to run (ideally an init system) to keep any envvar state we've set
+log_debug "chain exec-ing requested command $*"
+exec "${@}"
diff --git a/ansible_builder/containerfile.py b/ansible_builder/containerfile.py
index d934527..9ecce97 100644
--- a/ansible_builder/containerfile.py
+++ b/ansible_builder/containerfile.py
@@ -156,6 +156,12 @@ class Containerfile:
         self._prepare_galaxy_copy_steps()
         self._prepare_system_runtime_deps_steps()
 
+        if self.definition.version >= 3 and self.definition.options['relax_passwd_permissions']:
+            self._relax_etc_passwd_permissions()
+
+        if self.definition.version >= 3 and (final_workdir := self.definition.options['workdir']):
+            self._prepare_final_workdir(final_workdir)
+
         # install init package if specified
         # FUTURE: could move this into the pre-install wheel phase
         if init_pip_pkg := self.definition.container_init.get('package_pip'):
@@ -238,10 +244,10 @@ class Containerfile:
 
         # HACK: this sucks
         scriptres = importlib.resources.files('ansible_builder._target_scripts')
-        for script in ('assemble', 'get-extras-packages', 'install-from-bindep', 'introspect.py', 'check_galaxy', 'check_ansible'):
+        for script in ('assemble', 'get-extras-packages', 'install-from-bindep', 'introspect.py', 'check_galaxy', 'check_ansible', 'entrypoint'):
             with importlib.resources.as_file(scriptres / script) as script_path:
                 # FIXME: just use builtin copy?
-                copy_file(str(script_path), os.path.join(scripts_dir, script))
+                copy_file(str(script_path), scripts_dir)
 
         # later steps depend on base image containing these scripts
         context_dir = Path(self.build_outputs_dir).stem
@@ -307,6 +313,21 @@ class Containerfile:
                     lines = section_steps
                 self.steps.extend(lines)
 
+    def _relax_etc_passwd_permissions(self):
+        self.steps.append(
+            "RUN chmod ug+rw /etc/passwd"
+        )
+
+    def _prepare_final_workdir(self, workdir: str):
+        workdir = workdir.strip()
+        if not workdir:
+            return
+
+        self.steps.extend([
+            f"RUN mkdir -p {workdir} && chgrp 0 {workdir} && chmod -R ug+rwx {workdir}",
+            f"WORKDIR {workdir}"
+        ])
+
     def _prepare_label_steps(self):
         self.steps.extend([
             "LABEL ansible-execution-environment=true",
diff --git a/ansible_builder/ee_schema.py b/ansible_builder/ee_schema.py
index 8500d5f..8b1d601 100644
--- a/ansible_builder/ee_schema.py
+++ b/ansible_builder/ee_schema.py
@@ -318,10 +318,18 @@ schema_v3 = {
             "type": "object",
             "additionalProperties": False,
             "properties": {
+                "relax_passwd_permissions": {
+                    "description": "allows GID0 write access to /etc/passwd; currently necessary for many uses",
+                    "type": "boolean",
+                },
                 "skip_ansible_check": {
                     "description": "Disables the check for Ansible/Runner in final image",
                     "type": "boolean",
                 },
+                "workdir": {
+                    "description": "Default working directory, also often the homedir for ephemeral UIDs",
+                    "type": ["string", "null"],
+                },
                 "package_manager_path": {
                     "description": "Path to the system package manager to use",
                     "type": "string",
@@ -406,9 +414,11 @@ def _handle_options_defaults(ee_def: dict):
     options = ee_def.setdefault('options', {})
 
     options.setdefault('skip_ansible_check', False)
+    options.setdefault('relax_passwd_permissions', True)
+    options.setdefault('workdir', '/runner')
     options.setdefault('package_manager_path', '/usr/bin/dnf')
     options.setdefault('container_init', {
         'package_pip': 'dumb-init==1.2.5',
-        'entrypoint': '["dumb-init"]',
+        'entrypoint': '["/output/scripts/entrypoint", "dumb-init"]',
         'cmd': '["bash"]',
     })
diff --git a/ansible_builder/utils.py b/ansible_builder/utils.py
index 4414f3b..9954da3 100644
--- a/ansible_builder/utils.py
+++ b/ansible_builder/utils.py
@@ -169,11 +169,7 @@ def copy_file(source: str, dest: str) -> bool:
     if os.path.abspath(source) == os.path.abspath(dest):
         logger.info("File %s was placed in build context by user, leaving unmodified.", dest)
         return False
-    if Path(source).is_dir():
-        raise Exception(f"Source {source} can not be a directory. Please use copy_directory instead.")
-    if Path(dest).is_dir():
-        raise Exception(f"Destination {dest} can not be a directory. Please use copy_directory instead.")
-    if not os.path.exists(dest):
+    elif not os.path.exists(dest):
         logger.debug("File %s will be created.", dest)
         should_copy = True
     elif not filecmp.cmp(source, dest, shallow=False):
diff --git a/docs/definition.rst b/docs/definition.rst
index a272047..701b4f2 100644
--- a/docs/definition.rst
+++ b/docs/definition.rst
@@ -284,16 +284,29 @@ This section is a dictionary that contains keywords/options that can affect
 builder runtime functionality. Valid keys for this section are:
 
     ``container_init``
-      A dict with keys that allow for customization of the container
-      ``ENTRYPOINT`` and ``CMD`` instructions. Valid keys are:
-
-        * ``cmd`` - Literal value for the ``CMD`` instruction file directive.
-          The default value is ``["bash"]``.
-        * ``entrypoint`` - Literal value for the ``ENTRYPOINT`` instruction file directive.
-          The default value is ``["dumb-init"]``.
-        * ``package_pip`` - Package to install via pip for entrypoint support. This
-          package will be installed in the final build image. The default value is
-          ``dumb-init==1.2.5``.
+      A dictionary with keys that allow for customization of the container ``ENTRYPOINT`` and
+      ``CMD`` directives (and related behaviors). Customizing these behaviors is an advanced
+      task, and may result in subtle, difficult-to-debug failures. As the provided defaults for
+      this section control a number of intertwined behaviors, overriding any value will skip all
+      remaining defaults in this dictionary.
+      Valid keys are:
+
+      ``cmd``
+        Literal value for the ``CMD`` Containerfile directive. The default value is ``["bash"]``.
+
+      ``entrypoint``
+        Literal value for the ``ENTRYPOINT`` Containerfile directive. The
+        default entrypoint behavior handles signal propagation to subprocesses, as well as attempting to
+        ensure at runtime that the container user has a proper environment with a valid writeable
+        home directory, represented in ``/etc/passwd``, with the ``HOME`` envvar set to match. The default
+        entrypoint script may emit warnings to ``stderr`` in cases where it is unable to suitably adjust the
+        user runtime environment. This behavior can be ignored or elevated to a fatal error; consult the
+        source for the ``entrypoint`` target script for more details. The default value is
+        ``["/output/scripts/entrypoint", "dumb-init"]``.
+
+      ``package_pip``
+        Package to install via pip for entrypoint support. This package will be installed in the final build image.
+        The default value is ``dumb-init==1.2.5``.
 
     ``package_manager_path``
       A string with the path to the package manager (dnf or microdnf) to use.
@@ -306,6 +319,23 @@ 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``.
 
+    ``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
+      script may attempt to update ``/etc/passwd`` under some container runtimes with dynamically
+      created users to ensure a fully-functional POSIX user environment and home directory. Disabling
+      this capability can cause failures of software features that require users to be listed in
+      ``/etc/passwd`` with a valid and writeable home directory (eg, ``async`` in ansible-core, and the
+      ``~username`` shell expansion). The default is ``True``.
+
+    ``workdir``
+      Default current working directory for new processes started under the final container
+      image. Some container runtimes also use this value as ``HOME`` for dynamically-created
+      users in the ``root`` (GID 0) group. When this value is specified, the directory will be
+      created (if it doesn't already exist), set to ``root`` group ownership, and ``rwx`` group
+      permissions recursively applied to it. The default value is ``/runner``.
+
+
 Example ``options`` section:
 
 .. code:: yaml
@@ -316,7 +346,9 @@ Example ``options`` section:
             entrypoint: '["dumb-init"]'
             cmd: '["csh"]'
         package_manager_path: /usr/bin/microdnf
-        skip_ansible_check: True
+        relax_password_permissions: false
+        skip_ansible_check: true
+        workdir: /myworkdir
 
 version
 *******


Test patch:
diff --git a/test/integration/test_create.py b/test/integration/test_create.py
index c7d2291..ad57fc4 100644
--- a/test/integration/test_create.py
+++ b/test/integration/test_create.py
@@ -244,7 +244,10 @@ def test_v3_complete(cli, data_dir, tmp_path):
 
     # verify that the default init is being installed and that ENTRYPOINT is set
     assert "RUN $PYCMD -m pip install --no-cache-dir 'dumb-init==" in text
-    assert 'ENTRYPOINT ["dumb-init"]' in text
+    assert 'WORKDIR /runner' in text
+    assert 'RUN chmod ug+rw /etc/passwd' in text
+    assert 'RUN mkdir -p /runner' in text
+    assert 'ENTRYPOINT ["/output/scripts/entrypoint", "dumb-init"]' in text
 
     # check additional_build_files
     myconfigs_path = tmp_path / constants.user_content_subfolder / "myconfigs"
@@ -329,3 +332,65 @@ def test_v3_custom_container_init(cli, build_dir_and_ee_yml):
     assert "pip install --no-cache-dir 'custominit==1.2.3'" in text
     assert 'ENTRYPOINT ["custominit"]' in text
     assert 'CMD ["customcmd"]' in text
+
+
+def test_v3_no_relax_passwd_perms(cli, build_dir_and_ee_yml):
+    """
+    Test that disabling 'options.relax_passwd_permissions' works.
+    """
+    ee = """
+    version: 3
+    options:
+        relax_passwd_permissions: false
+    """
+
+    tmpdir, eeyml = build_dir_and_ee_yml(ee)
+    cli(f'ansible-builder create -c {tmpdir} -f {eeyml} --output-filename Containerfile')
+
+    containerfile = tmpdir / "Containerfile"
+    assert containerfile.exists()
+    text = containerfile.read_text()
+
+    assert "/etc/passwd" not in text
+
+
+def test_v3_custom_workdir(cli, build_dir_and_ee_yml):
+    """
+    Test that a custom 'options.workdir' creates the dir and sets it
+    """
+    ee = """
+    version: 3
+    options:
+        workdir: /yourmom
+    """
+
+    tmpdir, eeyml = build_dir_and_ee_yml(ee)
+    cli(f'ansible-builder create -c {tmpdir} -f {eeyml} --output-filename Containerfile')
+
+    containerfile = tmpdir / "Containerfile"
+    assert containerfile.exists()
+    text = containerfile.read_text()
+
+    assert "WORKDIR /yourmom" in text
+    assert "mkdir -p /yourmom && chgrp 0 /yourmom && chmod -R ug+rwx /yourmom" in text
+
+
+def test_v3_no_workdir(cli, build_dir_and_ee_yml):
+    """
+    Test that empty 'options.workdir' skips the setting and creation of the default.
+    """
+    ee = """
+    version: 3
+    options:
+        workdir:
+    """
+
+    tmpdir, eeyml = build_dir_and_ee_yml(ee)
+    cli(f'ansible-builder create -c {tmpdir} -f {eeyml} --output-filename Containerfile')
+
+    containerfile = tmpdir / "Containerfile"
+    assert containerfile.exists()
+    text = containerfile.read_text()
+
+    assert "WORKDIR" not in text
+    assert "mkdir -p /runner" not in text
diff --git a/test/unit/test_utils.py b/test/unit/test_utils.py
index fd9c01b..8caef19 100644
--- a/test/unit/test_utils.py
+++ b/test/unit/test_utils.py
@@ -55,21 +55,6 @@ def test_copy_touched_file(dest_file, source_file):
     assert not copy_file(source_file, dest_file)
 
 
-def test_copy_file_with_destination_directory(dest_file, source_file):
-    # Change source file to trigger copy_file
-    source_file.write_text('foo\nbar\nzoo')
-
-    with pytest.raises(Exception) as err:
-        copy_file(source_file, '/tmp')
-    assert "can not be a directory" in str(err.value.args[0])
-
-    with pytest.raises(Exception) as err:
-        copy_file('/tmp', dest_file)
-    assert "can not be a directory" in str(err.value.args[0])
-
-    assert copy_file(source_file, dest_file)
-
-
 @pytest.mark.run_command
 def test_failed_command(mocker):
     mocker.patch('ansible_builder.utils.subprocess.Popen.wait', return_value=1)