Repository: osbuild/osbuild
Pull request #1478 (2023-11-30 10:41:55)

osbuild-mpp: conditional losetup
`mpp-define-images` can create an image file, using `losetup` to deal with non-standard sector sizes requires root. Not all users run `osbuild-mpp` as root.

While I am not a fan of "suddenly sudo" based on the input manifest this does alleviate builds breaking for manifests with default sector sizes when non-root.

achilleas-k: LGTM as a temp fix

Patch:
diff --git a/stages/org.osbuild.machine-id b/stages/org.osbuild.machine-id
new file mode 100755
index 00000000..3e04cce5
--- /dev/null
+++ b/stages/org.osbuild.machine-id
@@ -0,0 +1,55 @@
+#!/usr/bin/python3
+"""
+Deal with /etc/machine-id
+
+Explicitly define the state to /etc/machine-id. The possible values for
+first-boot are:
+
+- yes: This sets the machine-id to "uninitialized" and this will trigger
+       ContidionFirstBoot in systemd
+- no: This creates an empty machine-id. It will trigger the generation
+      of a new machine-id but *not* the ConditionFirstBoot
+- preserve: Leave the existing machine-id in place. Not having a machine-id
+            with that set is an error.
+"""
+
+import pathlib
+import sys
+
+import osbuild.api
+
+SCHEMA = """
+"additionalProperties": false,
+"required": ["first-boot"],
+"properties": {
+  "first-boot": {
+    "enum": ["yes", "no", "preserve"],
+    "description": "Set the first boot behavior of the /etc/machine-id file in the tree"
+  }
+}
+"""
+
+
+def main(tree, options):
+    mode = options["first-boot"]
+
+    machine_id_file = pathlib.Path(tree) / "etc/machine-id"
+    if mode == "yes":
+        # available since systemd v247, systemd PR#16939
+        machine_id_file.write_bytes(b"uninitialized\n")
+    elif mode == "no":
+        with machine_id_file.open("wb") as fp:
+            fp.truncate(0)
+    elif mode == "preserve":
+        if not machine_id_file.is_file():
+            print(f"{tree}/etc/machine-id cannot be preserved, it does not exist")
+            return 1
+    else:
+        raise ValueError(f"unexpected machine-id mode '{mode}'")
+    return 0
+
+
+if __name__ == '__main__':
+    args = osbuild.api.arguments()
+    r = main(args["tree"], args["options"])
+    sys.exit(r)
diff --git a/tools/osbuild-mpp b/tools/osbuild-mpp
index 3214e647..ec66c6df 100755
--- a/tools/osbuild-mpp
+++ b/tools/osbuild-mpp
@@ -968,6 +968,8 @@ class PartitionTable:
 
 
 class Image:
+    DEFAULT_SECTOR_SIZE = 512
+
     def __init__(self, size, layout):
         self.size = size
         self.layout = layout
@@ -976,19 +978,27 @@ class Image:
     def from_dict(cls, js):
         size = js["size"]
         data = js["table"]
-        sector_size = js.get('sector_size', 512)
+        sector_size = js.get('sector_size', Image.DEFAULT_SECTOR_SIZE)
 
         with tempfile.TemporaryDirectory() as tmp:
             image = os.path.join(tmp, "disk.img")
             subprocess.run(["truncate", "--size", size, image], check=True)
-            cp = subprocess.run(["losetup", "--find", "--show", f"--sector-size={sector_size}", image],
-                                stdout=subprocess.PIPE, check=True)
-            loopimage = cp.stdout.rstrip()
-            try:
-                table = PartitionTable.from_dict(data)
-                table.write_to(loopimage)
-            finally:
-                subprocess.run(["losetup", "-d", loopimage])
+
+            table = PartitionTable.from_dict(data)
+
+            # Running losetup requires to be root but losetup is only necessary if the sector size
+            # is different from the default.
+            if sector_size == Image.DEFAULT_SECTOR_SIZE:
+                table.write_to(image)
+            else:
+                cp = subprocess.run(["losetup", "--find", "--show", f"--sector-size={sector_size}", image],
+                                    stdout=subprocess.PIPE, check=True)
+                loopimage = cp.stdout.rstrip()
+
+                try:
+                    table.write_to(loopimage)
+                finally:
+                    subprocess.run(["losetup", "-d", loopimage])
 
         return cls(size, table)
 


Test patch:
diff --git a/stages/test/test_machine-id.py b/stages/test/test_machine-id.py
new file mode 100644
index 00000000..efe98a75
--- /dev/null
+++ b/stages/test/test_machine-id.py
@@ -0,0 +1,82 @@
+#!/usr/bin/python3
+
+import os
+import pathlib
+import unittest.mock
+
+import pytest
+
+import osbuild.meta
+from osbuild.testutil.imports import import_module_from_path
+
+
+def stage(stage_name):
+    test_dir = pathlib.Path(__file__).parent
+    stage_path = pathlib.Path(test_dir) / f"../org.osbuild.{stage_name}"
+    return import_module_from_path("stage", os.fspath(stage_path))
+
+
+@pytest.fixture(name="machine_id_path")
+def machine_id_path_fixture(tmp_path):
+    machine_id_path = tmp_path / "etc/machine-id"
+    machine_id_path.parent.mkdir()
+    return machine_id_path
+
+
+@pytest.mark.parametrize("already_has_etc_machine_id", [True, False])
+def test_machine_id_first_boot_yes(tmp_path, machine_id_path, already_has_etc_machine_id):
+    if already_has_etc_machine_id:
+        machine_id_path.touch()
+
+    stage("machine-id").main(tmp_path, {"first-boot": "yes"})
+    assert machine_id_path.read_bytes() == b"uninitialized\n"
+
+
+@pytest.mark.parametrize("already_has_etc_machine_id", [True, False])
+def test_machine_id_first_boot_no(tmp_path, machine_id_path, already_has_etc_machine_id):
+    if already_has_etc_machine_id:
+        machine_id_path.write_bytes(b"\x01\x02\x03")
+
+    stage("machine-id").main(tmp_path, {"first-boot": "no"})
+    assert machine_id_path.stat().st_size == 0
+
+
+@pytest.mark.parametrize("already_has_etc_machine_id", [True, False])
+@unittest.mock.patch("builtins.print")
+def test_machine_id_first_boot_preserve(mock_print, tmp_path, machine_id_path, already_has_etc_machine_id):
+    if already_has_etc_machine_id:
+        machine_id_path.write_bytes(b"\x01\x02\x03")
+
+    ret = stage("machine-id").main(tmp_path, {"first-boot": "preserve"})
+    if already_has_etc_machine_id:
+        assert os.stat(machine_id_path).st_size == 3
+    else:
+        assert ret == 1
+        mock_print.assert_called_with(f"{tmp_path}/etc/machine-id cannot be preserved, it does not exist")
+
+
+@pytest.mark.parametrize("test_data,expected_err", [
+    ({"first-boot": "invalid-option"}, "'invalid-option' is not one of "),
+])
+def test_machine_id_schema_validation(test_data, expected_err):
+    name = "org.osbuild.machine-id"
+    root = pathlib.Path(__file__).parents[2]
+    mod_info = osbuild.meta.ModuleInfo.load(root, "Stage", name)
+    schema = osbuild.meta.Schema(mod_info.get_schema(), name)
+
+    test_input = {
+        "name": "org.osbuild.machine-id",
+        "options": {},
+    }
+    test_input["options"].update(test_data)
+    res = schema.validate(test_input)
+
+    assert res.valid is False
+    assert len(res.errors) == 1
+    err_msgs = [e.as_dict()["message"] for e in res.errors]
+    assert expected_err in err_msgs[0]
+
+
+def test_machine_id_first_boot_unknown(tmp_path):
+    with pytest.raises(ValueError, match=r"unexpected machine-id mode 'invalid'"):
+        stage("machine-id").main(tmp_path, {"first-boot": "invalid"})