Merge pull request #2475 from mtrmac/oci-preserve

Preserve more-recently-added fields when copying/updating OCI indices
containers · Jul 2, 2024 · 9f7f648 · 9f7f648
2 parents 8d792a4 + 17bf7de
commit 9f7f648
Show file tree

Hide file tree

Showing 11 changed files with 214 additions and 16 deletions.
diff --git a/internal/manifest/docker_schema2_list_test.go b/internal/manifest/docker_schema2_list_test.go
@@ -107,3 +107,15 @@ func TestSchema2ListFromManifest(t *testing.T) {
 	// Extra fields are rejected
 	testValidManifestWithExtraFieldsIsRejected(t, parser, validManifest, []string{"config", "fsLayers", "history", "layers"})
 }
+
+func TestSchema2ListCloneInternal(t *testing.T) {
+	// This fixture should be kept updated to have all known fields set to non-empty values
+	blob, err := os.ReadFile(filepath.Join("testdata", "v2list.everything.json"))
+	require.NoError(t, err)
+	m, err := Schema2ListFromManifest(blob)
+	require.NoError(t, err)
+	clone_ := m.CloneInternal()
+	clone, ok := clone_.(*Schema2List)
+	require.True(t, ok)
+	assert.Equal(t, m.Schema2ListPublic, clone.Schema2ListPublic)
+}
diff --git a/internal/manifest/oci_index.go b/internal/manifest/oci_index.go
@@ -1,6 +1,7 @@
 package manifest
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"maps"
@@ -296,29 +297,51 @@ func OCI1IndexPublicFromComponents(components []imgspecv1.Descriptor, annotation
 		},
 	}
 	for i, component := range components {
-		var platform *imgspecv1.Platform
-		if component.Platform != nil {
-			platformCopy := ociPlatformClone(*component.Platform)
-			platform = &platformCopy
-		}
-		m := imgspecv1.Descriptor{
-			MediaType:    component.MediaType,
-			ArtifactType: component.ArtifactType,
-			Size:         component.Size,
-			Digest:       component.Digest,
-			URLs:         slices.Clone(component.URLs),
-			Annotations:  maps.Clone(component.Annotations),
-			Platform:     platform,
-		}
-		index.Manifests[i] = m
+		index.Manifests[i] = oci1DescriptorClone(component)
 	}
 	return &index
 }
 
+func oci1DescriptorClone(d imgspecv1.Descriptor) imgspecv1.Descriptor {
+	var platform *imgspecv1.Platform
+	if d.Platform != nil {
+		platformCopy := ociPlatformClone(*d.Platform)
+		platform = &platformCopy
+	}
+	return imgspecv1.Descriptor{
+		MediaType:    d.MediaType,
+		Digest:       d.Digest,
+		Size:         d.Size,
+		URLs:         slices.Clone(d.URLs),
+		Annotations:  maps.Clone(d.Annotations),
+		Data:         bytes.Clone(d.Data),
+		Platform:     platform,
+		ArtifactType: d.ArtifactType,
+	}
+}
+
 // OCI1IndexPublicClone creates a deep copy of the passed-in index.
 // This is publicly visible as c/image/manifest.OCI1IndexClone.
 func OCI1IndexPublicClone(index *OCI1IndexPublic) *OCI1IndexPublic {
-	return OCI1IndexPublicFromComponents(index.Manifests, index.Annotations)
+	var subject *imgspecv1.Descriptor
+	if index.Subject != nil {
+		s := oci1DescriptorClone(*index.Subject)
+		subject = &s
+	}
+	manifests := make([]imgspecv1.Descriptor, len(index.Manifests))
+	for i, m := range index.Manifests {
+		manifests[i] = oci1DescriptorClone(m)
+	}
+	return &OCI1IndexPublic{
+		Index: imgspecv1.Index{
+			Versioned:    index.Versioned,
+			MediaType:    index.MediaType,
+			ArtifactType: index.ArtifactType,
+			Manifests:    manifests,
+			Subject:      subject,
+			Annotations:  maps.Clone(index.Annotations),
+		},
+	}
 }
 
 // ToOCI1Index returns the index encoded as an OCI1 index.

diff --git a/internal/manifest/oci_index_test.go b/internal/manifest/oci_index_test.go
@@ -294,3 +294,15 @@ func TestOCI1IndexChooseInstanceByCompression(t *testing.T) {
 		}
 	}
 }
+
+func TestOCI1IndexCloneInternal(t *testing.T) {
+	// This fixture should be kept updated to have all known fields set to non-empty values
+	blob, err := os.ReadFile(filepath.Join("testdata", "oci1.index.everything.json"))
+	require.NoError(t, err)
+	m, err := OCI1IndexFromManifest(blob)
+	require.NoError(t, err)
+	clone_ := m.CloneInternal()
+	clone, ok := clone_.(*OCI1Index)
+	require.True(t, ok)
+	assert.Equal(t, m.OCI1IndexPublic.Index, clone.OCI1IndexPublic.Index)
+}
diff --git a/internal/manifest/testdata/oci1.index.everything.json b/internal/manifest/testdata/oci1.index.everything.json
@@ -0,0 +1,40 @@
+{
+    "schemaVersion": 2,
+    "mediaType": "application/vnd.oci.image.index.v1+json",
+    "artifactType": "application/our-top-artifact",
+    "manifests": [
+        {
+            "mediaType": "application/vnd.oci.image.manifest.v1+json",
+            "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+            "size": 1,
+            "urls": ["https://some.example/manifest"],
+            "annotations": {"manifest": "1"},
+            "data": "YQ==",
+            "platform": {
+                "architecture": "A",
+                "os": "B",
+                "os.version": "C",
+                "os.features": ["D"],
+                "variant": "E"
+            },
+            "artifactType": "application/our-sub-artifact"
+        }
+    ],
+    "subject": {
+        "mediaType": "application/vnd.oci.image.manifest.v1+json",
+        "digest": "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+        "size": 2,
+        "urls": ["https://some.example/subject"],
+        "annotations": {"subject": "2"},
+        "data": "YmM=",
+        "platform": {
+            "architecture": "a",
+            "os": "b",
+            "os.version": "c",
+            "os.features": ["d"],
+            "variant": "e"
+        },
+        "artifactType": "application/our-subject"
+    },
+    "annotations": {"top": "0"}
+}
diff --git a/internal/manifest/testdata/v2list.everything.json b/internal/manifest/testdata/v2list.everything.json
@@ -0,0 +1,20 @@
+{
+    "schemaVersion": 2,
+    "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
+    "manifests": [
+        {
+            "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+            "size": 1,
+            "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+            "urls": ["https://some.example/manifest"],
+            "platform": {
+                "architecture": "a",
+                "os": "b",
+                "os.version": "c",
+                "os.features": ["d"],
+                "variant": "e",
+                "features": ["f"]
+            }
+        }
+    ]
+}
diff --git a/manifest/docker_schema1_test.go b/manifest/docker_schema1_test.go
@@ -78,6 +78,13 @@ func TestSchema1FromManifest(t *testing.T) {
 	testValidManifestWithExtraFieldsIsRejected(t, parser, validManifest, []string{"config", "layers", "manifests"})
 }
 
+func TestSchema1Clone(t *testing.T) {
+	// This fixture should be kept updated to have all known fields set to non-empty values
+	m := manifestSchema1FromFixture(t, "v2s1.everything.json")
+	clone := Schema1Clone(m)
+	assert.Equal(t, m, clone)
+}
+
 func TestSchema1Initialize(t *testing.T) {
 	// Test this indirectly via Schema1FromComponents; otherwise we would have to break the API and create an instance manually.
 

diff --git a/manifest/docker_schema2_test.go b/manifest/docker_schema2_test.go
@@ -64,6 +64,13 @@ func TestSchema2FromManifest(t *testing.T) {
 	testValidManifestWithExtraFieldsIsRejected(t, parser, validManifest, []string{"fsLayers", "history", "manifests"})
 }
 
+func TestSchema2Clone(t *testing.T) {
+	// This fixture should be kept updated to have all known fields set to non-empty values
+	m := manifestSchema2FromFixture(t, "v2s2.everything.json")
+	clone := Schema2Clone(m)
+	assert.Equal(t, m, clone)
+}
+
 func TestSchema2UpdateLayerInfos(t *testing.T) {
 	for _, c := range []struct {
 		name            string

diff --git a/manifest/fixtures/ociv1.everything.json b/manifest/fixtures/ociv1.everything.json
@@ -0,0 +1,40 @@
+{
+    "schemaVersion": 2,
+    "mediaType": "application/vnd.oci.image.manifest.v1+json",
+    "artifactType": "application/our-artifact",
+    "config": {
+        "mediaType": "application/our-artifact-config",
+        "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+        "size": 1,
+        "urls": ["https://some.example/config"],
+        "annotations": {"config": "1"},
+        "data": "YQ=="
+    },
+    "layers": [
+        {
+            "mediaType": "application/our-artifact-layer",
+            "digest": "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+            "size": 2,
+            "urls": ["https://some.example/layer"],
+            "annotations": {"layer": "2"},
+            "data": "YmM="
+        }
+    ],
+    "subject": {
+        "mediaType": "application/vnd.oci.image.manifest.v1+json",
+        "digest": "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
+        "size": 3,
+        "urls": ["https://some.example/subject"],
+        "annotations": {"subject": "3"},
+        "data": "ZGVm",
+        "platform": {
+            "architecture": "a",
+            "os": "b",
+            "os.version": "c",
+            "os.features": ["d"],
+            "variant": "e"
+        },
+        "artifactType": "application/our-subject"
+    },
+    "annotations": {"top": "0"}
+}
diff --git a/manifest/fixtures/v2s1.everything.json b/manifest/fixtures/v2s1.everything.json
@@ -0,0 +1,12 @@
+{
+    "name": "registry.example/foo",
+    "tag": "notlatest",
+    "architecture": "a",
+    "fsLayers": [
+        {"blobSum": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}
+    ],
+    "history": [
+        {"v1Compatibility": "{\"id\":\"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\",\"comment\":\"comment\",\"created\":\"1970-01-01T01:00:01+01:00\",\"container_config\":{\"Cmd\":[\"cmd\"]},\"author\":\"author\",\"throwaway\":true}"}
+    ],
+    "schemaVersion": 1
+}
diff --git a/manifest/fixtures/v2s2.everything.json b/manifest/fixtures/v2s2.everything.json
@@ -0,0 +1,18 @@
+{
+    "schemaVersion": 2,
+    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+    "config": {
+        "mediaType": "application/vnd.docker.container.image.v1+json",
+        "size": 1,
+        "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+        "urls": ["https://some.example/config"]
+    },
+    "layers": [
+        {
+            "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+            "size": 2,
+            "digest": "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+            "urls": ["https://some.example/layer"]
+        }
+    ]
+}
diff --git a/manifest/oci_test.go b/manifest/oci_test.go
@@ -69,6 +69,13 @@ func TestOCI1FromManifest(t *testing.T) {
 	testValidManifestWithExtraFieldsIsRejected(t, parser, validManifest, []string{"fsLayers", "history", "manifests"})
 }
 
+func TestOCI1Clone(t *testing.T) {
+	// This fixture should be kept updated to have all known fields set to non-empty values
+	m := manifestOCI1FromFixture(t, "ociv1.everything.json")
+	clone := OCI1Clone(m)
+	assert.Equal(t, m.Manifest, clone.Manifest)
+}
+
 func TestOCI1UpdateLayerInfos(t *testing.T) {
 	customCompression := compression.Algorithm{}