Repository: osbuild/osbuild
Pull request #1752 (2024-04-24 08:31:59)

stages: add support for destination {dir,oci-archive} to the skopeo stage
Skopeo support destination dir mainly to check if this works for signatures as expected
implemented oci-archive as it could also be helpful and should be simple

schuellerf: Draft for now, as there are no tests yet
schuellerf: If someone has hints how to mock-up a signed archive to test `--remove-signatures` I'd be happy to implement then 🤔
achilleas-k: If we're in a hurry to get this in, I'm good to approve and clean up the comments in a follow-up since they're all about code style mostly.
schuellerf: I should be able to fix those things in the afternoon 
schuellerf: If someone has hints how to mock-up a signed archive to test `--remove-signatures` I'd be happy to implement then 🤔
schuellerf: with the changed `make_container()` interface (returning a tag and no hash) the test case here just doesn't check the SHA value - but that might be enough testing here

Patch:
diff --git a/stages/org.osbuild.skopeo b/stages/org.osbuild.skopeo
index f50935d5..ec62c595 100755
--- a/stages/org.osbuild.skopeo
+++ b/stages/org.osbuild.skopeo
@@ -1,5 +1,6 @@
 #!/usr/bin/python3
 import os
+import pathlib
 import subprocess
 import sys
 
@@ -12,6 +13,7 @@ def main(inputs, output, options):
 
     destination = options["destination"]
     dest_type = destination["type"]
+    remove_signatures = destination.get("remove-signatures")
 
     for image in images.values():
         with containers.container_source(image) as (image_name, image_source):
@@ -19,13 +21,19 @@ def main(inputs, output, options):
                 storage_root = destination.get("storage-path", "/var/lib/containers/storage")
                 storage_driver = destination.get("storage-driver", "overlay")
                 dest = f"containers-storage:[{storage_driver}@{output}{storage_root}+/run/containers/storage]{image_name}"
-            elif dest_type == "oci":
+            elif dest_type in ("oci", "oci-archive", "dir"):
                 path = destination["path"]
-                dest = f"oci:{output}{path}"
+                dest = f"{dest_type}:{output}{path}"
+                pathlib.Path(f"{output}{path}").parent.mkdir(parents=True, exist_ok=True)
             else:
                 raise ValueError(f"Unknown destination type '{dest_type}'")
 
-            subprocess.run(["skopeo", "copy", image_source, dest], check=True)
+            cmd = ["skopeo", "copy"]
+            if remove_signatures:
+                cmd.append("--remove-signatures")
+            cmd.extend([image_source, dest])
+
+            subprocess.run(cmd, check=True)
 
     if dest_type == "containers-storage" and storage_driver == "overlay":
         # Each time the overlay backend runs on an xfs fs it creates this file:
diff --git a/stages/org.osbuild.skopeo.meta.json b/stages/org.osbuild.skopeo.meta.json
index 777ebe0a..95812a90 100644
--- a/stages/org.osbuild.skopeo.meta.json
+++ b/stages/org.osbuild.skopeo.meta.json
@@ -47,6 +47,44 @@
             "type": "string"
           }
         }
+      },
+      "destination-oci-archive": {
+        "type": "object",
+        "additionalProperties": false,
+        "required": [
+          "type",
+          "path"
+        ],
+        "properties": {
+          "type": {
+            "enum": [
+              "oci-archive"
+            ]
+          },
+          "path": {
+            "description": "Location of a tar archive compliant with 'Open Container Image Layout Specification'",
+            "type": "string"
+          }
+        }
+      },
+      "destination-dir": {
+        "type": "object",
+        "additionalProperties": false,
+        "required": [
+          "type",
+          "path"
+        ],
+        "properties": {
+          "type": {
+            "enum": [
+              "dir"
+            ]
+          },
+          "path": {
+            "description": "Location of a directory storing the manifest, layer tarballs and signatures as individual files. This is a non-standardized format, primarily useful for debugging or noninvasive container inspection.",
+            "type": "string"
+          }
+        }
       }
     },
     "inputs": {
@@ -80,8 +118,19 @@
             },
             {
               "$ref": "#/definitions/destination-oci"
+            },
+            {
+              "$ref": "#/definitions/destination-oci-archive"
+            },
+            {
+              "$ref": "#/definitions/destination-dir"
             }
           ]
+        },
+        "remove-signatures": {
+          "type": "boolean",
+          "default": false,
+          "description": "Do not copy signatures, if any, from source-image. Necessary when copying a signed image to a destination which does not support signatures."
         }
       }
     }


Test patch:
diff --git a/stages/test/test_skopeo.py b/stages/test/test_skopeo.py
index 2b591e72..8fac0ff4 100644
--- a/stages/test/test_skopeo.py
+++ b/stages/test/test_skopeo.py
@@ -1,8 +1,12 @@
 #!/usr/bin/python3
+import json
+import os
+import subprocess
 
 import pytest
 
 from osbuild import testutil
+from osbuild.testutil import has_executable, make_container
 
 STAGE_NAME = "org.osbuild.skopeo"
 
@@ -13,8 +17,15 @@ STAGE_NAME = "org.osbuild.skopeo"
     ({"destination": {}}, "is not valid under any of the given schemas"),
     ({"destination": {"type": "foo"}}, "is not valid under any of the given schemas"),
     ({"destination": {"type": "oci"}}, "is not valid under any of the given schemas"),
+    ({"destination": {"type": "dir"}}, "is not valid under any of the given schemas"),
+    ({"destination": {"type": "os-archive"}}, "is not valid under any of the given schemas"),
+    ({"destination": {"type": "dir", "path": "/foo"}, "remove-signatures": "YesPlease"},
+     "'YesPlease' is not of type 'boolean'"),
     # good
     ({"destination": {"type": "oci", "path": "/foo"}}, ""),
+    ({"destination": {"type": "oci-archive", "path": "/foo"}}, ""),
+    ({"destination": {"type": "dir", "path": "/foo"}}, ""),
+    ({"destination": {"type": "dir", "path": "/foo"}, "remove-signatures": True}, ""),
 
     # this one might not be expected but it's valid because we don't require any
     # *inputs* and it'll be a no-op in the stage
@@ -33,3 +44,84 @@ def test_schema_validation_skopeo(stage_schema, test_data, expected_err):
     else:
         assert res.valid is False
         testutil.assert_jsonschema_error_contains(res, expected_err, expected_num_errs=1)
+
+
+@pytest.mark.skipif(os.getuid() != 0, reason="needs root")
+@pytest.mark.skipif(not has_executable("podman"), reason="no podman executable")
+@pytest.mark.parametrize("dest_type,local_output_path", [
+    ("dir", "/tmp/test-output-skopeo-dir"),
+    ("oci-archive", "/tmp/test-output-skopeo.tar"),
+    ("oci", "/tmp/test-output-skopeo-dir"),
+])
+def test_skopeo_copy(tmp_path, stage_module, dest_type, local_output_path):
+    with make_container(tmp_path, {"file1": "file1 from final layer"}) as cont_tag:
+        # export for the container-deploy stage
+        fake_container_dst = tmp_path / "fake-container"
+        subprocess.check_call([
+            "podman", "save",
+            "--format=oci-archive",
+            f"--output={fake_container_dst}",
+            cont_tag,
+        ])
+
+    inputs = {
+        "images": {
+            # seems to be unused with fake_container_path?
+            "path": fake_container_dst,
+            "data": {
+                "archives": {
+                    fake_container_dst: {
+                        "format": "oci-archive",
+                        "name": cont_tag,
+                    },
+                },
+            },
+        },
+    }
+
+    output_dir = tmp_path / "output"
+    options = {
+        "destination": {
+            "type": dest_type,
+            "path": local_output_path
+        }
+    }
+
+    stage_module.main(inputs, output_dir, options)
+
+    assert output_dir.exists()
+
+    result = output_dir / local_output_path.lstrip("/")
+    assert result.exists()
+
+    if dest_type == "dir":
+        assert (result / "version").exists()
+
+        manifest_file = (result / "manifest.json")
+    elif dest_type == "oci-archive":
+        # TBD extract TAR and check content
+        assert result.exists()
+    elif dest_type == "oci":
+        assert (result / "oci-layout").exists()
+
+        index_file = (result / "index.json")
+        assert index_file.exists()
+
+        data = json.loads(index_file.read_bytes())
+
+        assert data.get("manifests") is not None, "'index.json' seems corrupt - no 'manifests' section found"
+        assert data["manifests"][0].get("digest") is not None, \
+            "'manifest.json' seems corrupt - no 'manifests[0].digest' section found"
+        data_digest = data["manifests"][0]["digest"].split(':')
+
+        manifest_file = result / "blobs" / data_digest[0] / data_digest[1]
+
+    if dest_type in ["dir", "oci"]:
+        assert manifest_file.exists()
+
+        data = json.loads(manifest_file.read_bytes())
+
+        assert data.get("config") is not None, "'manifest.json' seems corrupt - no 'config' section found"
+
+        assert data["config"].get("digest") is not None, \
+            "'manifest.json' seems corrupt - no 'config.digest' section found"