Repository: containers/image
Pull request #1922 (2023-04-14 17:51:38)

Don't completely silently ignore non-OCI manifests in OCI layouts
Instead, report that we did find a match and that it is not supported.

Partially fixes https://github.com/containers/skopeo/issues/1975 .

Also, significantly improve unit tests of that code.


Patch:
diff --git a/oci/layout/fixtures/name_lookups/index.json b/oci/layout/fixtures/name_lookups/index.json
new file mode 100644
index 00000000..b79d1089
--- /dev/null
+++ b/oci/layout/fixtures/name_lookups/index.json
@@ -0,0 +1,38 @@
+{
+    "schemaVersion": 2,
+    "mediaType": "application/vnd.oci.image.index.v1+json",
+    "manifests": [
+        {
+            "mediaType": "application/vnd.oci.image.manifest.v1+json",
+            "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+            "size": 1,
+            "annotations": {
+                "org.opencontainers.image.ref.name": "a"
+            }
+        },
+        {
+            "mediaType": "application/vnd.oci.image.manifest.v1+json",
+            "digest": "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+            "size": 2,
+            "annotations": {
+                "org.opencontainers.image.ref.name": "b"
+            }
+        },
+        {
+            "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+            "digest": "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
+            "size": 3,
+            "annotations": {
+                "org.opencontainers.image.ref.name": "invalid-mime"
+            }
+        },
+        {
+            "mediaType": "x-completely-unknown",
+            "digest": "sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd",
+            "size": 4,
+            "annotations": {
+                "org.opencontainers.image.ref.name": "invalid-mime"
+            }
+        }
+    ]
+}
diff --git a/oci/layout/fixtures/two_images_manifest/index.json b/oci/layout/fixtures/two_images_manifest/index.json
index 62514440..0dd68afc 100644
--- a/oci/layout/fixtures/two_images_manifest/index.json
+++ b/oci/layout/fixtures/two_images_manifest/index.json
@@ -1,9 +1,9 @@
 {
   "schemaVersion": 2,
-  "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
+  "mediaType": "application/vnd.oci.image.index.v1+json",
   "manifests": [
     {
-      "mediaType": "application/vnd.docker.image.manifest.v2+json",
+      "mediaType": "application/vnd.oci.image.manifest.v1+json",
       "size": 7143,
       "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f",
       "platform": {
@@ -12,7 +12,7 @@
       }
     },
     {
-      "mediaType": "application/vnd.docker.image.manifest.v2+json",
+      "mediaType": "application/vnd.oci.image.manifest.v1+json",
       "size": 7682,
       "digest": "sha256:5b0bcabd1ed22e9fb1310cf6c2dec7cdef19f0ad69efa1f392e94a4333501270",
       "platform": {
diff --git a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go
index 4a4ab9b2..6586b844 100644
--- a/oci/layout/oci_transport.go
+++ b/oci/layout/oci_transport.go
@@ -188,14 +188,18 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) {
 		return index.Manifests[0], nil
 	} else {
 		// if image specified, look through all manifests for a match
+		var unsupportedMIMETypes []string
 		for _, md := range index.Manifests {
-			if md.MediaType != imgspecv1.MediaTypeImageManifest && md.MediaType != imgspecv1.MediaTypeImageIndex {
-				continue
-			}
 			if refName, ok := md.Annotations[imgspecv1.AnnotationRefName]; ok && refName == ref.image {
-				return md, nil
+				if md.MediaType == imgspecv1.MediaTypeImageManifest || md.MediaType == imgspecv1.MediaTypeImageIndex {
+					return md, nil
+				}
+				unsupportedMIMETypes = append(unsupportedMIMETypes, md.MediaType)
 			}
 		}
+		if len(unsupportedMIMETypes) != 0 {
+			return imgspecv1.Descriptor{}, fmt.Errorf("reference %q matches unsupported manifest MIME types %q", ref.image, unsupportedMIMETypes)
+		}
 	}
 	return imgspecv1.Descriptor{}, ImageNotFoundError{ref}
 }


Test patch:
diff --git a/oci/layout/oci_transport_test.go b/oci/layout/oci_transport_test.go
index d4fd0216..594e6dc3 100644
--- a/oci/layout/oci_transport_test.go
+++ b/oci/layout/oci_transport_test.go
@@ -8,20 +8,94 @@ import (
 
 	_ "github.com/containers/image/v5/internal/testing/explicitfilepath-tmpdir"
 	"github.com/containers/image/v5/types"
+	imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-// TestGetManifestDescriptor is testing a regression issue where a nil error was being wrapped,
-// this causes the returned error to be nil as well and the user wasn't getting a proper error output.
-//
-// More info: https://github.com/containers/skopeo/issues/496
 func TestGetManifestDescriptor(t *testing.T) {
-	imageRef, err := NewReference("fixtures/two_images_manifest", "")
-	require.NoError(t, err)
+	emptyDir := t.TempDir()
+
+	for _, c := range []struct {
+		dir, image string
+		expected   *imgspecv1.Descriptor // nil if a failure ie expected. errorIs / errorAs allows more specific checks.
+		errorIs    error
+		errorAs    any
+	}{
+		{ // Index is missing
+			dir:      emptyDir,
+			image:    "",
+			expected: nil,
+		},
+		{ // A valid reference to the only manifest
+			dir:   "fixtures/manifest",
+			image: "",
+			expected: &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",
+				},
+			},
+		},
+		{ // An ambiguous reference to a multi-manifest directory
+			dir:      "fixtures/two_images_manifest",
+			image:    "",
+			expected: nil,
+			errorIs:  ErrMoreThanOneImage,
+		},
+		{ // A valid reference in a multi-manifest directory
+			dir:   "fixtures/name_lookups",
+			image: "a",
+			expected: &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"},
+			},
+		},
+		{ // A valid reference in a multi-manifest directory
+			dir:   "fixtures/name_lookups",
+			image: "b",
+			expected: &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"},
+			},
+		},
+		{ // No entry found
+			dir:      "fixtures/name_lookups",
+			image:    "this-does-not-exist",
+			expected: nil,
+			errorAs:  &ImageNotFoundError{},
+		},
+		{ // Entries with invalid MIME types found
+			dir:      "fixtures/name_lookups",
+			image:    "invalid-mime",
+			expected: nil,
+		},
+	} {
+		ref, err := NewReference(c.dir, c.image)
+		require.NoError(t, err)
 
-	_, err = imageRef.(ociReference).getManifestDescriptor()
-	assert.EqualError(t, err, ErrMoreThanOneImage.Error())
+		res, err := ref.(ociReference).getManifestDescriptor()
+		if c.expected != nil {
+			require.NoError(t, err)
+			assert.Equal(t, *c.expected, res)
+		} else {
+			require.Error(t, err)
+			if c.errorIs != nil {
+				assert.ErrorIs(t, err, c.errorIs)
+			}
+			if c.errorAs != nil {
+				assert.ErrorAs(t, err, &c.errorAs)
+			}
+		}
+	}
 }
 
 func TestTransportName(t *testing.T) {