Simplify apivfs_cmd() and chroot_cmd()

We move the setpgid logic to run(), avoiding the need to pass a tools
argument to chroot_cmd() and apivfs_cmd().

We also try to remove as much logic from these functions as possible.
Since we can't really assume that any logic we execute during the
function will still hold true in the sandbox, so it's best to delay
any logic execution until we're already in the sandbox (using the
--ro-bind-try options of bubblewrap).

We also rework the /etc/resolv.conf handling to simply make sure that
/run/systemd/resolve exists in the chroot since if /etc/resolv.conf
points to /run it'll almost certainly be to
/run/systemd/resolv/stub-resolv.conf.

diff --git a/mkosi/__init__.py b/mkosi/__init__.py
index 66a138ea4a599f049ca05a2829a5d6954492d4d9..924dc9f4813ec1b150f12fc6d39dc83705d286c2 100644 (file)
--- a/mkosi/__init__.py
+++ b/mkosi/__init__.py
@@ -442,7 +442,6 @@ def run_prepare_scripts(context: Context, build: bool) -> None:
             chroot = chroot_cmd(
                 context.root,
                 resolve=True,
-                tools=context.config.tools(),
                 options=[
                     "--bind", "/work", "/work",
                     "--chdir", "/work/src",
@@ -517,7 +516,6 @@ def run_build_scripts(context: Context) -> None:
             chroot = chroot_cmd(
                 context.root,
                 resolve=context.config.with_network,
-                tools=context.config.tools(),
                 options=[
                     "--bind", "/work", "/work",
                     "--chdir", "/work/src",
@@ -587,7 +585,6 @@ def run_postinst_scripts(context: Context) -> None:
             chroot = chroot_cmd(
                 context.root,
                 resolve=context.config.with_network,
-                tools=context.config.tools(),
                 options=[
                     "--bind", "/work", "/work",
                     "--chdir", "/work/src",
@@ -648,7 +645,6 @@ def run_finalize_scripts(context: Context) -> None:
             chroot = chroot_cmd(
                 context.root,
                 resolve=context.config.with_network,
-                tools=context.config.tools(),
                 options=[
                     "--bind", "/work", "/work",
                     "--chdir", "/work/src",

diff --git a/mkosi/distributions/gentoo.py b/mkosi/distributions/gentoo.py
index 1dbff224f72da57bcad04dd7046d48cee65ffcf1..e6dfe95bc1b756110e10f789688ef26d1d2932c0 100644 (file)
--- a/mkosi/distributions/gentoo.py
+++ b/mkosi/distributions/gentoo.py
@@ -26,7 +26,7 @@ from mkosi.util import sort_packages
 
 def invoke_emerge(context: Context, packages: Sequence[str] = (), apivfs: bool = True) -> None:
     run(
-        apivfs_cmd(context.root, tools=context.config.tools()) + [
+        apivfs_cmd(context.root) + [
             # We can't mount the stage 3 /usr using `options`, because bwrap isn't available in the stage 3
             # tarball which is required by apivfs_cmd(), so we have to mount /usr from the tarball later
             # using another bwrap exec.
@@ -161,7 +161,6 @@ class Installer(DistributionInstaller):
 
         chroot = chroot_cmd(
             stage3,
-            tools=context.config.tools(),
             options=["--bind", context.cache_dir / "repos", "/var/db/repos"],
         )
 

diff --git a/mkosi/installer/__init__.py b/mkosi/installer/__init__.py
index 36afdab7344196e190e8fc871deda6b646c18988..6135e03e0b4b66f7ea8fd249cc3d0a818faea6ce 100644 (file)
--- a/mkosi/installer/__init__.py
+++ b/mkosi/installer/__init__.py
@@ -41,12 +41,12 @@ def clean_package_manager_metadata(context: Context) -> None:
 
 def package_manager_scripts(context: Context) -> dict[str, list[PathString]]:
     return {
-        "pacman": apivfs_cmd(context.root, tools=context.config.tools()) + pacman_cmd(context),
-        "zypper": apivfs_cmd(context.root, tools=context.config.tools()) + zypper_cmd(context),
-        "dnf"   : apivfs_cmd(context.root, tools=context.config.tools()) + dnf_cmd(context),
-        "rpm"   : apivfs_cmd(context.root, tools=context.config.tools()) + rpm_cmd(context),
+        "pacman": apivfs_cmd(context.root) + pacman_cmd(context),
+        "zypper": apivfs_cmd(context.root) + zypper_cmd(context),
+        "dnf"   : apivfs_cmd(context.root) + dnf_cmd(context),
+        "rpm"   : apivfs_cmd(context.root) + rpm_cmd(context),
     } | {
-        command: apivfs_cmd(context.root, tools=context.config.tools()) + apt_cmd(context, command) for command in (
+        command: apivfs_cmd(context.root) + apt_cmd(context, command) for command in (
             "apt",
             "apt-cache",
             "apt-cdrom",

diff --git a/mkosi/installer/apt.py b/mkosi/installer/apt.py
index 594bdb6b0b5b6401549a058d90bb6349f2c38c15..0ccecebf07085714917ce9b4ebc7b74885572c85 100644 (file)
--- a/mkosi/installer/apt.py
+++ b/mkosi/installer/apt.py
@@ -113,7 +113,7 @@ def invoke_apt(
                     *finalize_crypto_mounts(tools=context.config.tools()),
                     *mounts,
                 ],
-            ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else [])
+            ) + (apivfs_cmd(context.root) if apivfs else [])
         ),
         env=context.config.environment,
     )

diff --git a/mkosi/installer/dnf.py b/mkosi/installer/dnf.py
index c410c7141786d68a05df30b777029f1daa2feecc..8a7c283f6cdf0211ef26d97fe081c6275ea80278 100644 (file)
--- a/mkosi/installer/dnf.py
+++ b/mkosi/installer/dnf.py
@@ -139,7 +139,7 @@ def invoke_dnf(context: Context, command: str, packages: Iterable[str], apivfs:
                     context.cache_dir / "lib" / dnf_subdir(context),
                     *finalize_crypto_mounts(tools=context.config.tools()),
                 ],
-            ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else [])
+            ) + (apivfs_cmd(context.root) if apivfs else [])
         ),
         env=context.config.environment,
     )

diff --git a/mkosi/installer/pacman.py b/mkosi/installer/pacman.py
index 1c3cfd90570028d1c4f6b9363fe31afd8b2f4548..993aeaebff22911f25663b6f01739113af5ef1fc 100644 (file)
--- a/mkosi/installer/pacman.py
+++ b/mkosi/installer/pacman.py
@@ -99,7 +99,7 @@ def invoke_pacman(
                     "--bind", context.cache_dir / "cache/pacman/pkg", context.cache_dir / "cache/pacman/pkg",
                     *finalize_crypto_mounts(tools=context.config.tools()),
                 ],
-            ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else [])
+            ) + (apivfs_cmd(context.root) if apivfs else [])
         ),
         env=context.config.environment,
     )

diff --git a/mkosi/installer/zypper.py b/mkosi/installer/zypper.py
index c800038f9bf4dfd0a2ab0612a79125f255048012..91e0caadd3424c4af559534ff67c4b8cf6247bd6 100644 (file)
--- a/mkosi/installer/zypper.py
+++ b/mkosi/installer/zypper.py
@@ -87,7 +87,7 @@ def invoke_zypper(
                     "--bind", context.cache_dir / "cache/zypp", context.cache_dir / "cache/zypp",
                     *finalize_crypto_mounts(tools=context.config.tools()),
                 ],
-            ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else [])
+            ) + (apivfs_cmd(context.root) if apivfs else [])
         ),
         env=context.config.environment,
     )

diff --git a/mkosi/run.py b/mkosi/run.py
index 54885d3f301294e23cc1023101fb91c234e26986..5fd7ef12f77a5d219639b656b32de36ea7bd15f8 100644 (file)
--- a/mkosi/run.py
+++ b/mkosi/run.py
@@ -289,6 +289,12 @@ def run(
         if preexec_fn:
             preexec_fn()
 
+    if (
+        sandbox and
+        subprocess.run(sandbox + ["sh", "-c", "command -v setpgid"], stdout=subprocess.DEVNULL).returncode == 0
+    ):
+        cmdline = ["setpgid", "--foreground", "--"] + cmdline
+
     try:
         # subprocess.run() will use SIGKILL to kill processes when an exception is raised.
         # We'd prefer it to use SIGTERM instead but since this we can't configure which signal
@@ -373,6 +379,13 @@ def spawn(
         if preexec_fn:
             preexec_fn()
 
+    if (
+        foreground and
+        sandbox and
+        subprocess.run(sandbox + ["sh", "-c", "command -v setpgid"], stdout=subprocess.DEVNULL).returncode == 0
+    ):
+        cmdline = ["setpgid", "--foreground", "--"] + cmdline
+
     try:
         with subprocess.Popen(
             sandbox + cmdline,

diff --git a/mkosi/sandbox.py b/mkosi/sandbox.py
index 1e730fdae863bf324fe96268c509ae0e02ef6403..e574fd4b904d05584e6ceb714e91203d73c1b282 100644 (file)
--- a/mkosi/sandbox.py
+++ b/mkosi/sandbox.py
@@ -7,7 +7,6 @@ from collections.abc import Sequence
 from pathlib import Path
 from typing import Optional
 
-from mkosi.run import find_binary
 from mkosi.types import PathString
 from mkosi.util import INVOKING_USER, flatten, one_zero
 
@@ -33,18 +32,12 @@ def finalize_passwd_mounts(root: Path) -> list[PathString]:
     """
     If passwd or a related file exists in the apivfs directory, bind mount it over the host files while we
     run the command, to make sure that the command we run uses user/group information from the apivfs
-    directory instead of from the host. If the file doesn't exist yet, mount over /dev/null instead.
+    directory instead of from the host.
     """
     options: list[PathString] = []
 
     for f in ("passwd", "group", "shadow", "gshadow"):
-        if not (Path("/etc") / f).exists():
-            continue
-        p = root / "etc" / f
-        if p.exists():
-            options += ["--bind", p, f"/etc/{f}"]
-        else:
-            options += ["--bind", "/dev/null", f"/etc/{f}"]
+        options += ["--ro-bind-try", root / "etc" / f, f"/etc/{f}"]
 
     return options
 
@@ -161,14 +154,11 @@ def sandbox_cmd(
 
     cmdline += ["sh", "-c", f"{shm} && exec $0 \"$@\""]
 
-    if setpgid := find_binary("setpgid", root=tools):
-        cmdline += [setpgid, "--foreground", "--"]
-
     return cmdline
 
 
-def apivfs_cmd(root: Path, *, tools: Path = Path("/")) -> list[PathString]:
-    cmdline: list[PathString] = [
+def apivfs_cmd(root: Path) -> list[PathString]:
+    return [
         "bwrap",
         "--dev-bind", "/", "/",
         "--tmpfs", root / "run",
@@ -178,38 +168,30 @@ def apivfs_cmd(root: Path, *, tools: Path = Path("/")) -> list[PathString]:
         "--dev", root / "dev",
         # APIVFS generally means chrooting is going to happen so unset TMPDIR just to be safe.
         "--unsetenv", "TMPDIR",
-    ]
-
-    if (root / "etc/machine-id").exists():
         # Make sure /etc/machine-id is not overwritten by any package manager post install scripts.
-        cmdline += ["--ro-bind", root / "etc/machine-id", root / "etc/machine-id"]
-
-    cmdline += finalize_passwd_mounts(root)
-
-    if setpgid := find_binary("setpgid", root=tools):
-        cmdline += [setpgid, "--foreground", "--"]
-
-    chmod = f"chmod 1777 {root / 'tmp'} {root / 'var/tmp'} {root / 'dev/shm'}"
-    # Make sure anything running in the root directory thinks it's in a container. $container can't always be
-    # accessed so we write /run/host/container-manager as well which is always accessible.
-    container = f"mkdir {root}/run/host && echo mkosi >{root}/run/host/container-manager"
-
-    cmdline += ["sh", "-c", f"{chmod} && {container} && exec $0 \"$@\""]
-
-    return cmdline
+        "--ro-bind-try", root / "etc/machine-id", root / "etc/machine-id",
+        *finalize_passwd_mounts(root),
+        "sh", "-c",
+        f"chmod 1777 {root / 'tmp'} {root / 'var/tmp'} {root / 'dev/shm'} && "
+        f"chmod 755 {root / 'run'} && "
+        # Make sure anything running in the root directory thinks it's in a container. $container can't always be
+        # accessed so we write /run/host/container-manager as well which is always accessible.
+        f"mkdir -m 755 {root}/run/host && echo mkosi >{root}/run/host/container-manager && "
+        "exec $0 \"$@\"",
+    ]
 
 
-def chroot_cmd(
-    root: Path,
-    *,
-    resolve: bool = False,
-    tools: Path = Path("/"),
-    options: Sequence[PathString] = (),
-) -> list[PathString]:
+def chroot_cmd(root: Path, *, resolve: bool = False, options: Sequence[PathString] = ()) -> list[PathString]:
     cmdline: list[PathString] = [
         "sh", "-c",
+        f"trap 'rm -rf {root / 'work'}' EXIT && "
+        # /etc/resolv.conf can be a dangling symlink to /run/systemd/resolve/stub-resolv.conf. Bubblewrap tries to call
+        # mkdir() on each component of the path which means it will try to call
+        # mkdir(/run/systemd/resolve/stub-resolv.conf) which will fail unless /run/systemd/resolve exists already so
+        # we make sure that it already exists.
+        f"mkdir -p -m 755 {root / 'work'} {root / 'run/systemd'} {root / 'run/systemd/resolve'} && "
         # No exec here because we need to clean up the /work directory afterwards.
-        f"trap 'rm -rf {root / 'work'}' EXIT && mkdir -p {root / 'work'} && chown 777 {root / 'work'} && $0 \"$@\"",
+        f"$0 \"$@\"",
         "bwrap",
         "--dev-bind", root, "/",
         "--setenv", "container", "mkosi",
@@ -218,20 +200,8 @@ def chroot_cmd(
     ]
 
     if resolve:
-        p = Path("etc/resolv.conf")
-        if (root / p).is_symlink():
-            # For each component in the target path, bubblewrap will try to create it if it doesn't exist
-            # yet. If a component in the path is a dangling symlink, bubblewrap will end up calling
-            # mkdir(symlink) which obviously fails if multiple components of the dangling symlink path don't
-            # exist yet. As a workaround, we resolve the symlink ourselves so that bubblewrap will correctly
-            # create all missing components in the target path.
-            p = p.parent / (root / p).readlink()
-
-        cmdline += ["--ro-bind", "/etc/resolv.conf", Path("/") / p]
-
-    cmdline += [*options]
+        cmdline += ["--ro-bind-try", "/etc/resolv.conf", "/etc/resolv.conf"]
 
-    if setpgid := find_binary("setpgid", root=root):
-        cmdline += [setpgid, "--foreground", "--"]
+    cmdline += options
 
-    return apivfs_cmd(root, tools=tools) + cmdline
+    return apivfs_cmd(root) + cmdline