Repository: osbuild/osbuild
Pull request #1904 (2024-10-16 11:43:31)

org.osbuild.mkdir: support creating dirs on mounts
This allows creating new directories on mounts:
```
- type: org.osbuild.mkdir
  options:
    paths:
      - path: mount:///boot/efi
  devices:
    disk: ...
  mounts:
    - name: boot
      target: /boot
      ...
```

nikita-dubrovskii: Required for https://github.com/osbuild/osbuild/issues/1877

Patch:
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/org.osbuild.mkdir b/stages/org.osbuild.mkdir
index f04549f6..d2d11a7a 100755
--- a/stages/org.osbuild.mkdir
+++ b/stages/org.osbuild.mkdir
@@ -3,23 +3,26 @@ import os
 import sys
 
 import osbuild.api
-from osbuild.util.path import in_tree
+from osbuild.util import parsing
 
 
-def main(tree, options):
+def main(args):
+    options = args["options"]
+
     for item in options["paths"]:
         path = item["path"]
         mode = item.get("mode", 0o777)
         parents = item.get("parents", False)
         exist_ok = item.get("exist_ok", False)
 
-        if not path.startswith("/"):
-            print("WARNING: relative path used, this is discouraged!")
-
-        target = os.path.join(tree, path.lstrip("/"))
-        if not in_tree(target, tree):
-            raise ValueError(f"path {path} not in tree")
+        if "://" not in path:
+            if not path.startswith("/"):
+                print("WARNING: relative path used, this is discouraged!")
+                path = f"tree:///{path}"
+            else:
+                path = f"tree://{path}"
 
+        target = parsing.parse_location(path, args)
         if parents:
             os.makedirs(target, mode=mode, exist_ok=exist_ok)
         else:
@@ -33,5 +36,4 @@ def main(tree, options):
 
 
 if __name__ == "__main__":
-    args = osbuild.api.arguments()
-    sys.exit(main(args["tree"], args["options"]))
+    sys.exit(main(osbuild.api.arguments()))
diff --git a/stages/org.osbuild.mkdir.meta.json b/stages/org.osbuild.mkdir.meta.json
index 5534120a..6cebaaf5 100644
--- a/stages/org.osbuild.mkdir.meta.json
+++ b/stages/org.osbuild.mkdir.meta.json
@@ -1,5 +1,5 @@
 {
-  "summary": "Create directories within the tree.",
+  "summary": "Create directories within the tree or mount.",
   "description": [
     "Can create one or more directories, optionally also the",
     "intermediate directories. The stage can gracefully handle",
@@ -31,8 +31,23 @@
             ],
             "properties": {
               "path": {
-                "type": "string",
-                "pattern": "^\\/?(?!\\.\\.)((?!\\/\\.\\.\\/).)+$"
+                "anyOf": [
+                  {
+                    "type": "string",
+                    "description": "Target path, if a tree",
+                    "pattern": "^\\/?(?!\\.\\.)((?!\\/\\.\\.\\/).)+$"
+                  },
+                  {
+                    "type": "string",
+                    "description": "Target path, if a mount",
+                    "pattern": "^mount://.+"
+                  },
+                  {
+                    "type": "string",
+                    "description": "Target path, if a tree",
+                    "pattern": "^tree://.+"
+                  }
+                ]
               },
               "mode": {
                 "type": "number",


Test patch:
diff --git a/osbuild/testutil/__init__.py b/osbuild/testutil/__init__.py
index 0681ff9b..5e937494 100644
--- a/osbuild/testutil/__init__.py
+++ b/osbuild/testutil/__init__.py
@@ -107,7 +107,7 @@ def mock_command(cmd_name: str, script: str):
         # will break. easy enough to fix by using \0 as the separator but
         # then \n in args is kinda rare
         fake_cmd_content = textwrap.dedent(f"""\
-        #!/bin/sh -e
+        #!/bin/bash -e
 
         for arg in "$@"; do
            echo "$arg" >> {cmd_calllog_path}
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/stages/test/test_mkdir.py b/stages/test/test_mkdir.py
new file mode 100644
index 00000000..6285a380
--- /dev/null
+++ b/stages/test/test_mkdir.py
@@ -0,0 +1,89 @@
+#!/usr/bin/python3
+
+import contextlib
+import os
+import subprocess
+
+import pytest  # type: ignore
+
+from osbuild.testutil import has_executable
+
+STAGE_NAME = "org.osbuild.mkdir"
+
+
+def test_mkdir(tmp_path, stage_module):
+    tree = tmp_path / "tree"
+    tree.mkdir()
+
+    options = {
+        "paths": [
+            {"path": "/fake_dir"},
+            {"path": "fake_relative_dir"}
+        ]
+    }
+    args = {
+        "tree": f"{tree}",
+        "options": options
+    }
+    stage_module.main(args)
+    assert (tree / "fake_dir").exists()
+    assert (tree / "fake_relative_dir").exists()
+
+
+def test_mkdir_on_a_tree(tmp_path, stage_module):
+    tree = tmp_path / "tree"
+    tree.mkdir()
+
+    options = {
+        "paths": [
+            {
+                "path": "tree:///fake_parent/fake_dir",
+                "parents": 1
+            }
+        ]
+    }
+    args = {
+        "tree": f"{tree}",
+        "options": options
+    }
+    stage_module.main(args)
+    assert (tree / "fake_parent/fake_dir").exists()
+
+
+@pytest.mark.skipif(os.getuid() != 0, reason="needs root")
+@pytest.mark.skipif(not has_executable("mkfs.ext4"), reason="need mkfs.ext4")
+def test_mkdir_on_a_mount(tmp_path, stage_module):
+    tree = tmp_path / "tree"
+    tree.mkdir()
+
+    # Create fake EXT4 disk image
+    fake_disk_path = tmp_path / "fake.img"
+    with fake_disk_path.open("w") as fp:
+        fp.truncate(10 * 1024 * 1024)
+    subprocess.run(
+        ["mkfs.ext4", os.fspath(fake_disk_path)], check=True)
+
+    fake_disk_mnt = tmp_path / "mounts"
+    fake_disk_mnt.mkdir()
+
+    with contextlib.ExitStack() as cm:
+        subprocess.run(["mount", fake_disk_path, fake_disk_mnt], check=True)
+        cm.callback(subprocess.run, ["umount", fake_disk_mnt], check=True)
+
+        options = {
+            "paths": [
+                {
+                    "path": "mount:///fake_parent/fake_dir",
+                    "parents": 1
+                }
+            ]
+        }
+        args = {
+            "tree": f"{tree}",
+            "options": options,
+            "paths": {
+                "mounts": fake_disk_mnt,
+            }
+        }
+        stage_module.main(args)
+        assert (fake_disk_mnt / "fake_parent/fake_dir").exists()
diff --git a/test/mod/test_util_chroot.py b/test/mod/test_util_chroot.py
index 061f9388..4b6812c9 100644
--- a/test/mod/test_util_chroot.py
+++ b/test/mod/test_util_chroot.py
@@ -5,6 +5,7 @@
 import os
 from unittest.mock import call, patch
 
+from osbuild.testutil import mock_command
 from osbuild.util.chroot import Chroot
 
 
@@ -27,17 +28,29 @@ 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),
     ]
+
+
+def test_chroot_integration(tmp_path):
+    # drop the first two arguments ("chroot", "target-dir") from our fake
+    # chroot
+    fake_chroot = r'exec "${@:2}"'
+    with mock_command("mount", ""), mock_command("umount", ""), mock_command("chroot", fake_chroot):
+        with Chroot(os.fspath(tmp_path)) as chroot:
+            ret = chroot.run(["/bin/true"], check=True)
+            assert ret.returncode == 0
+            ret = chroot.run(["/bin/false"], check=False)
+            assert ret.returncode == 1