Repository: osbuild/osbuild
Pull request #1512 (2023-12-19 19:01:29)

osbuild: allow to export a tree without preserving the ownership (less tests)
This commit allows to exclude preserving ownership from an object
export. This is required to fix the issue that on macOS the an
podman based workflow cannot export objects with preserving
ownerships.

Originally this was a no_preserve: Optional[List[str]] = None)
to be super flexible in what we pass to cp but then I felt like
YAGNI - if we need more we can trivially change this (internal)
API again :)

A version of https://github.com/osbuild/osbuild/pull/1511/ without the crazy^Wcontroversial parts.


Patch:
diff --git a/osbuild.spec b/osbuild.spec
index ccc5b619..1a61328b 100644
--- a/osbuild.spec
+++ b/osbuild.spec
@@ -1,7 +1,7 @@
 %global         forgeurl https://github.com/osbuild/osbuild
 %global         selinuxtype targeted
 
-Version:        102
+Version:        103
 
 %forgemeta
 
diff --git a/osbuild/__init__.py b/osbuild/__init__.py
index 59b1d5b2..7dae94df 100644
--- a/osbuild/__init__.py
+++ b/osbuild/__init__.py
@@ -10,7 +10,7 @@ independent of osbuild but used across the osbuild codebase.
 
 from .pipeline import Manifest, Pipeline, Stage
 
-__version__ = "102"
+__version__ = "103"
 
 __all__ = [
     "Manifest",
diff --git a/osbuild/main_cli.py b/osbuild/main_cli.py
index 29a23286..81459470 100644
--- a/osbuild/main_cli.py
+++ b/osbuild/main_cli.py
@@ -52,8 +52,10 @@ def export(name_or_id, output_directory, store, manifest):
     obj = store.get(pipeline.id)
     dest = os.path.join(output_directory, name_or_id)
 
+    skip_preserve_owner = \
+        os.getenv("OSBUILD_EXPORT_FORCE_NO_PRESERVE_OWNER") == "1"
     os.makedirs(dest, exist_ok=True)
-    obj.export(dest)
+    obj.export(dest, skip_preserve_owner=skip_preserve_owner)
 
 
 def parse_arguments(sys_argv):
diff --git a/osbuild/objectstore.py b/osbuild/objectstore.py
index 922d5eec..4536e4a1 100644
--- a/osbuild/objectstore.py
+++ b/osbuild/objectstore.py
@@ -244,18 +244,20 @@ class Object:
         if self.mode != want:
             raise ValueError(f"Wrong object mode: {self.mode}, want {want}")
 
-    def export(self, to_directory: PathLike):
+    def export(self, to_directory: PathLike, skip_preserve_owner=False):
         """Copy object into an external directory"""
-        subprocess.run(
-            [
-                "cp",
-                "--reflink=auto",
-                "-a",
-                os.fspath(self.tree) + "/.",
-                os.fspath(to_directory),
-            ],
-            check=True,
-        )
+        cp_cmd = [
+            "cp",
+            "--reflink=auto",
+            "-a",
+        ]
+        if skip_preserve_owner:
+            cp_cmd += ["--no-preserve=ownership"]
+        cp_cmd += [
+            os.fspath(self.tree) + "/.",
+            os.fspath(to_directory),
+        ]
+        subprocess.run(cp_cmd, check=True)
 
     def __fspath__(self):
         return self.tree
diff --git a/setup.py b/setup.py
index fd81d20c..41c57286 100644
--- a/setup.py
+++ b/setup.py
@@ -2,7 +2,7 @@ import setuptools
 
 setuptools.setup(
     name="osbuild",
-    version="102",
+    version="103",
     description="A build system for OS images",
     packages=["osbuild", "osbuild.formats", "osbuild.util"],
     license='Apache-2.0',


Test patch:
diff --git a/test/mod/test_objectstore.py b/test/mod/test_objectstore.py
index e7aa348a..94711601 100644
--- a/test/mod/test_objectstore.py
+++ b/test/mod/test_objectstore.py
@@ -320,3 +320,32 @@ def test_store_server(tmp_path):
 
         with pytest.raises(RuntimeError):
             _ = client.read_tree_at("42", tmp_path, "/nonexistent")
+
+
+@pytest.mark.skipif(os.getuid() != 0, reason="needs root")
+def test_object_export_preserves_ownership_by_default(tmp_path, object_store):
+    tree = object_store.new("a")
+    p = Path(tree, "A")
+    p.touch()
+    os.chown(os.fspath(p), 1000, 1000)
+    tree.export(tmp_path)
+
+    expected_exported_path = Path(tmp_path, "A")
+    assert expected_exported_path.exists()
+    assert expected_exported_path.stat().st_uid == 1000
+    assert expected_exported_path.stat().st_gid == 1000
+
+
+@pytest.mark.skipif(os.getuid() != 0, reason="needs root")
+def test_object_export_preserves_override(tmp_path, object_store):
+    tree = object_store.new("a")
+    p = Path(tree, "A")
+    p.touch()
+    os.chown(os.fspath(p), 1000, 1000)
+    tree.export(tmp_path, skip_preserve_owner=True)
+
+    expected_exported_path = Path(tmp_path, "A")
+    assert expected_exported_path.exists()
+    assert expected_exported_path.stat().st_uid == 0
+    assert expected_exported_path.stat().st_gid == 0
+