review comments addressed

Signed-off-by: Philippe Vlérick <[email protected]>
containers · Oct 11, 2023 · 3ae8eda · 3ae8eda
1 parent 2807bce
commit 3ae8eda
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 67 deletions.
diff --git a/oci/layout/fixtures/manifest/index.json b/oci/layout/fixtures/manifest/index.json
@@ -1 +1,17 @@
-{"schemaVersion":2,"manifests":[{"mediaType":"application/vnd.oci.image.manifest.v1+json","digest":"sha256:84afb6189c4d69f2d040c5f1dc4e0a16fed9b539ce9cfb4ac2526ae4e0576cc0","size":496,"annotations":{"org.opencontainers.image.ref.name":"v0.1.1"},"platform":{"architecture":"amd64","os":"linux"}}]}
+{
+    "schemaVersion": 2,
+    "manifests": [
+        {
+            "mediaType": "application/vnd.oci.image.manifest.v1+json",
+            "digest": "sha256:84afb6189c4d69f2d040c5f1dc4e0a16fed9b539ce9cfb4ac2526ae4e0576cc0",
+            "size": 496,
+            "annotations": {
+                "org.opencontainers.image.ref.name": "v0.1.1"
+            },
+            "platform": {
+                "architecture": "amd64",
+                "os": "linux"
+            }
+        }
+    ]
+}
diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go
@@ -75,7 +75,7 @@ func (ref ociReference) getBlobsUsedInImageIndex(descriptor *imgspecv1.Descripto
 	}
 
 	blobsUsedInImageRefIndex := make(map[digest.Digest]int)
-	err = ref.getBlobsUsedInIndex(blobsUsedInImageRefIndex, index, sharedBlobsDir)
+	err = ref.addBlobsUsedInIndex(blobsUsedInImageRefIndex, index, sharedBlobsDir)
 	if err != nil {
 		return nil, err
 	}
@@ -84,8 +84,8 @@ func (ref ociReference) getBlobsUsedInImageIndex(descriptor *imgspecv1.Descripto
 	return blobsUsedInImageRefIndex, nil
 }
 
-// Returns a map of digest with the usage count, so a blob that is referenced three times will have 3 in the map
-func (ref ociReference) getBlobsUsedInIndex(destination map[digest.Digest]int, index *imgspecv1.Index, sharedBlobsDir string) error {
+// Updates a map of digest with the usage count, so a blob that is referenced three times will have 3 in the map
+func (ref ociReference) addBlobsUsedInIndex(destination map[digest.Digest]int, index *imgspecv1.Index, sharedBlobsDir string) error {
 	for _, descriptor := range index.Manifests {
 		destination[descriptor.Digest]++
 		switch descriptor.MediaType {
@@ -106,7 +106,7 @@ func (ref ociReference) getBlobsUsedInIndex(destination map[digest.Digest]int, i
 			if err != nil {
 				return err
 			}
-			err = ref.getBlobsUsedInIndex(destination, index, sharedBlobsDir)
+			err = ref.addBlobsUsedInIndex(destination, index, sharedBlobsDir)
 			if err != nil {
 				return err
 			}
@@ -137,7 +137,7 @@ func (ref ociReference) getBlobsToDelete(blobsUsedByDescriptorToDelete map[diges
 		return nil, err
 	}
 	blobsUsedInRootIndex := make(map[digest.Digest]int)
-	err = ref.getBlobsUsedInIndex(blobsUsedInRootIndex, rootIndex, sharedBlobsDir)
+	err = ref.addBlobsUsedInIndex(blobsUsedInRootIndex, rootIndex, sharedBlobsDir)
 	if err != nil {
 		return nil, err
 	}

diff --git a/oci/layout/oci_transport_test.go b/oci/layout/oci_transport_test.go
@@ -17,99 +17,81 @@ func TestGetManifestDescriptor(t *testing.T) {
 	emptyDir := t.TempDir()
 
 	for _, c := range []struct {
-		dir, image string
-		expected   *struct {
-			descriptor *imgspecv1.Descriptor
-			index      int
-		} // nil if a failure ie expected. errorIs / errorAs allows more specific checks.
-		errorIs error
-		errorAs any
+		dir, image         string
+		expectedDescriptor *imgspecv1.Descriptor // nil if a failure ie expected. errorIs / errorAs allows more specific checks.
+		expectedIndex      int
+		errorIs            error
+		errorAs            any
 	}{
 		{ // Index is missing
-			dir:      emptyDir,
-			image:    "",
-			expected: nil,
+			dir:                emptyDir,
+			image:              "",
+			expectedDescriptor: nil,
 		},
 		{ // A valid reference to the only manifest
 			dir:   "fixtures/manifest",
 			image: "",
-			expected: &struct {
-				descriptor *imgspecv1.Descriptor
-				index      int
-			}{
-				descriptor: &imgspecv1.Descriptor{
-					MediaType:   "application/vnd.oci.image.manifest.v1+json",
-					Digest:      "sha256:84afb6189c4d69f2d040c5f1dc4e0a16fed9b539ce9cfb4ac2526ae4e0576cc0",
-					Size:        496,
-					Annotations: map[string]string{"org.opencontainers.image.ref.name": "v0.1.1"},
-					Platform: &imgspecv1.Platform{
-						Architecture: "amd64",
-						OS:           "linux",
-					},
+			expectedDescriptor: &imgspecv1.Descriptor{
+				MediaType:   "application/vnd.oci.image.manifest.v1+json",
+				Digest:      "sha256:84afb6189c4d69f2d040c5f1dc4e0a16fed9b539ce9cfb4ac2526ae4e0576cc0",
+				Size:        496,
+				Annotations: map[string]string{"org.opencontainers.image.ref.name": "v0.1.1"},
+				Platform: &imgspecv1.Platform{
+					Architecture: "amd64",
+					OS:           "linux",
 				},
-				index: 0,
 			},
+			expectedIndex: 0,
 		},
 		{ // An ambiguous reference to a multi-manifest directory
-			dir:      "fixtures/two_images_manifest",
-			image:    "",
-			expected: nil,
-			errorIs:  ErrMoreThanOneImage,
+			dir:                "fixtures/two_images_manifest",
+			image:              "",
+			expectedDescriptor: nil,
+			errorIs:            ErrMoreThanOneImage,
 		},
 		{ // A valid reference in a multi-manifest directory
 			dir:   "fixtures/name_lookups",
 			image: "a",
-			expected: &struct {
-				descriptor *imgspecv1.Descriptor
-				index      int
-			}{
-				descriptor: &imgspecv1.Descriptor{
-					MediaType:   "application/vnd.oci.image.manifest.v1+json",
-					Digest:      "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
-					Size:        1,
-					Annotations: map[string]string{"org.opencontainers.image.ref.name": "a"},
-				},
-				index: 0,
+			expectedDescriptor: &imgspecv1.Descriptor{
+				MediaType:   "application/vnd.oci.image.manifest.v1+json",
+				Digest:      "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+				Size:        1,
+				Annotations: map[string]string{"org.opencontainers.image.ref.name": "a"},
 			},
+			expectedIndex: 0,
 		},
 		{ // A valid reference in a multi-manifest directory
 			dir:   "fixtures/name_lookups",
 			image: "b",
-			expected: &struct {
-				descriptor *imgspecv1.Descriptor
-				index      int
-			}{
-				descriptor: &imgspecv1.Descriptor{
-					MediaType:   "application/vnd.oci.image.manifest.v1+json",
-					Digest:      "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
-					Size:        2,
-					Annotations: map[string]string{"org.opencontainers.image.ref.name": "b"},
-				},
-				index: 0,
+			expectedDescriptor: &imgspecv1.Descriptor{
+				MediaType:   "application/vnd.oci.image.manifest.v1+json",
+				Digest:      "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+				Size:        2,
+				Annotations: map[string]string{"org.opencontainers.image.ref.name": "b"},
 			},
+			expectedIndex: 1,
 		},
 		{ // No entry found
-			dir:      "fixtures/name_lookups",
-			image:    "this-does-not-exist",
-			expected: nil,
-			errorAs:  &ImageNotFoundError{},
+			dir:                "fixtures/name_lookups",
+			image:              "this-does-not-exist",
+			expectedDescriptor: nil,
+			errorAs:            &ImageNotFoundError{},
 		},
 		{ // Entries with invalid MIME types found
-			dir:      "fixtures/name_lookups",
-			image:    "invalid-mime",
-			expected: nil,
+			dir:                "fixtures/name_lookups",
+			image:              "invalid-mime",
+			expectedDescriptor: nil,
 		},
 	} {
 		ref, err := NewReference(c.dir, c.image)
 		require.NoError(t, err)
 
 		res, i, err := ref.(ociReference).getManifestDescriptor()
-		if c.expected != nil {
+		if c.expectedDescriptor != nil {
 			require.NoError(t, err)
-			assert.Equal(t, 0, c.expected.index)
-			assert.Equal(t, *c.expected.descriptor, res)
+			assert.Equal(t, c.expectedIndex, i)
+			assert.Equal(t, *c.expectedDescriptor, res)
 		} else {
-			assert.Equal(t, -1, i)
 			require.Error(t, err)
 			if c.errorIs != nil {
 				assert.ErrorIs(t, err, c.errorIs)