From a3e32f382386409d688b5cf4e8126569292c89b0 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 11 Sep 2024 09:50:41 +0200 Subject: [PATCH] util: drop absolute path from `Chroot.run()` calls We currently use the absolute path of these binaries in the helper. This has some advantages but given that we control the inputs for PATH in general it seems unnecessary. We are also slightly inconsistent about this in the codebase but favor the non absolute path version. A quick count: ``` $ git grep '"chroot"'|wc -l 13 $ git grep '"/usr/sbin/chroot"'|grep -v test_|wc -l 8 ``` for `mount` and `umount` it seems this is the only place that uses the absolute path. It's not an important change but it has the nice property that it allows us to use e.g. `testutil.mock_command()` in our tests and it would be nice to be consistent. --- osbuild/util/chroot.py | 10 +++++----- stages/test/test_dracut.py | 2 +- test/mod/test_util_chroot.py | 16 ++++++++-------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/osbuild/util/chroot.py b/osbuild/util/chroot.py index 3686b7d1..da14bf44 100644 --- a/osbuild/util/chroot.py +++ b/osbuild/util/chroot.py @@ -21,15 +21,15 @@ class Chroot: print(f"Making missing chroot directory: {d}") os.makedirs(self.root + d) - subprocess.run(["/usr/bin/mount", "-t", "proc", "-o", "nosuid,noexec,nodev", + subprocess.run(["mount", "-t", "proc", "-o", "nosuid,noexec,nodev", "proc", f"{self.root}/proc"], check=True) - subprocess.run(["/usr/bin/mount", "-t", "devtmpfs", "-o", "mode=0755,noexec,nosuid,strictatime", + subprocess.run(["mount", "-t", "devtmpfs", "-o", "mode=0755,noexec,nosuid,strictatime", "devtmpfs", f"{self.root}/dev"], check=True) - subprocess.run(["/usr/bin/mount", "-t", "sysfs", "-o", "nosuid,noexec,nodev", + subprocess.run(["mount", "-t", "sysfs", "-o", "nosuid,noexec,nodev", "sysfs", f"{self.root}/sys"], check=True) @@ -38,12 +38,12 @@ class Chroot: def __exit__(self, exc_type, exc_value, tracebk): failed_umounts = [] for d in ["/proc", "/dev", "/sys"]: - if subprocess.run(["/usr/bin/umount", "--lazy", self.root + d], check=False).returncode != 0: + if subprocess.run(["umount", "--lazy", self.root + d], check=False).returncode != 0: failed_umounts.append(d) if failed_umounts: print(f"Error unmounting paths from chroot: {failed_umounts}") def run(self, cmd, **kwargs): - cmd = ["/usr/sbin/chroot", self.root] + cmd + cmd = ["chroot", self.root] + cmd # pylint: disable=subprocess-run-check return subprocess.run(cmd, **kwargs) # noqa: PLW1510 diff --git a/stages/test/test_dracut.py b/stages/test/test_dracut.py index c8c1d7ed..99530589 100644 --- a/stages/test/test_dracut.py +++ b/stages/test/test_dracut.py @@ -27,5 +27,5 @@ def test_dracut_with_initoverlayfs(mocked_run, tmp_path, stage_module, with_init args, kwargs = mocked_run.call_args_list[3] # chroot is the 4th call assert kwargs.get("check") is True run_argv = args[0] - assert run_argv[0] == "/usr/sbin/chroot" + assert run_argv[0] == "chroot" assert run_argv[2] == expected_argv2 diff --git a/test/mod/test_util_chroot.py b/test/mod/test_util_chroot.py index 061f9388..46d23e75 100644 --- a/test/mod/test_util_chroot.py +++ b/test/mod/test_util_chroot.py @@ -27,17 +27,17 @@ def test_chroot_context(mocked_run, tmp_path): chroot.run(["/bin/false"], check=False) assert mocked_run.call_args_list == [ - call(["/usr/bin/mount", "-t", "proc", "-o", "nosuid,noexec,nodev", + call(["mount", "-t", "proc", "-o", "nosuid,noexec,nodev", "proc", os.fspath(tmp_path / "proc")], check=True), - call(["/usr/bin/mount", "-t", "devtmpfs", "-o", "mode=0755,noexec,nosuid,strictatime", + call(["mount", "-t", "devtmpfs", "-o", "mode=0755,noexec,nosuid,strictatime", "devtmpfs", os.fspath(tmp_path / "dev")], check=True), - call(["/usr/bin/mount", "-t", "sysfs", "-o", "nosuid,noexec,nodev", + call(["mount", "-t", "sysfs", "-o", "nosuid,noexec,nodev", "sysfs", os.fspath(tmp_path / "sys")], check=True), - call(["/usr/sbin/chroot", os.fspath(tmp_path), "/bin/true"], check=True), - call(["/usr/sbin/chroot", os.fspath(tmp_path), "/bin/false"], check=False), + call(["chroot", os.fspath(tmp_path), "/bin/true"], check=True), + call(["chroot", os.fspath(tmp_path), "/bin/false"], check=False), - call(["/usr/bin/umount", "--lazy", os.fspath(tmp_path / "proc")], check=False), - call(["/usr/bin/umount", "--lazy", os.fspath(tmp_path / "dev")], check=False), - call(["/usr/bin/umount", "--lazy", os.fspath(tmp_path / "sys")], check=False), + call(["umount", "--lazy", os.fspath(tmp_path / "proc")], check=False), + call(["umount", "--lazy", os.fspath(tmp_path / "dev")], check=False), + call(["umount", "--lazy", os.fspath(tmp_path / "sys")], check=False), ]