From e35d841509b74edf012779dffced9350fc6ffb88 Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Tue, 19 Dec 2023 17:35:29 +0100
Subject: [PATCH] objectstore: add new `skip_preserve_owner` to
 `Object.export()`

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 :)
---
 osbuild/objectstore.py       | 24 +++++++++++++-----------
 test/mod/test_objectstore.py | 29 +++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 11 deletions(-)

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/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
+