Move cpu variant checks into platform matcher

Wrap platforms.Only and fallback to our ignore mismatches due to  empty
CPU variants. This just cleans things up and makes the logic re-usable
in other places.

Signed-off-by: Brian Goff <[email protected]>
(cherry picked from commit 50f39e7)
Signed-off-by: Brian Goff <[email protected]>

daemon/create.go

@@ -12,6 +12,7 @@ import (
 	containertypes "github.com/docker/docker/api/types/container"
 	networktypes "github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/container"
+	"github.com/docker/docker/daemon/images"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/idtools"
@@ -89,7 +90,7 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.Container
 			Variant:      img.Variant,
 		}
 
-		if !platforms.Only(p).Match(imgPlat) {
+		if !images.OnlyPlatformWithFallback(p).Match(imgPlat) {
 			warnings = append(warnings, fmt.Sprintf("The requested image's platform (%s) does not match the detected host platform (%s) and no specific platform was requested", platforms.Format(imgPlat), platforms.Format(p)))
 		}
 	}

daemon/images/image.go

@@ -55,6 +55,8 @@ func (i *ImageService) manifestMatchesPlatform(img *image.Image, platform specs.
 		return false
 	}
 
+	// Note we are comparing against manifest lists here, which we expect to always have a CPU variant set (where applicable).
+	// So there is no need for the fallback matcher here.
 	comparer := platforms.Only(platform)
 
 	var (
@@ -161,31 +163,24 @@ func (i *ImageService) GetImage(refOrID string, platform *specs.Platform) (retIm
 		p := *platform
 		// Note that `platforms.Only` will fuzzy match this for us
 		// For example: an armv6 image will run just fine an an armv7 CPU, without emulation or anything.
-		if !platforms.Only(p).Match(imgPlat) {
-			// Sometimes image variant is not populated due to legacy builders
-			// We still should support falling back here.
-			if imgPlat.OS == platform.OS && imgPlat.Architecture == platform.Architecture && imgPlat.Variant == "" {
-				logrus.WithField("image", refOrID).WithField("platform", platforms.Format(p)).Debug("Image platform cpu variant is not populated, but otherwise it matches what was requested")
-				return
-			}
-
-			// In some cases the image config can actually be wrong (e.g. classic `docker build` may not handle `--platform` correctly)
-			// So we'll look up the manifest list that coresponds to this imaage to check if at least the manifest list says it is the correct image.
-			if i.manifestMatchesPlatform(retImg, p) {
-				return
-			}
-
-			// This allows us to tell clients that we don't have the image they asked for
-			// Where this gets hairy is the image store does not currently support multi-arch images, e.g.:
-			//   An image `foo` may have a multi-arch manifest, but the image store only fetches the image for a specific platform
-			//   The image store does not store the manifest list and image tags are assigned to architecture specific images.
-			//   So we can have a `foo` image that is amd64 but the user requested armv7. If the user looks at the list of images.
-			//   This may be confusing.
-			//   The alternative to this is to return a errdefs.Conflict error with a helpful message, but clients will not be
-			//   able to automatically tell what causes the conflict.
-			retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform: wanted %s, actual: %s", refOrID, platforms.Format(p), platforms.Format(imgPlat)))
+		if OnlyPlatformWithFallback(p).Match(imgPlat) {
 			return
 		}
+		// In some cases the image config can actually be wrong (e.g. classic `docker build` may not handle `--platform` correctly)
+		// So we'll look up the manifest list that coresponds to this imaage to check if at least the manifest list says it is the correct image.
+		if i.manifestMatchesPlatform(retImg, p) {
+			return
+		}
+
+		// This allows us to tell clients that we don't have the image they asked for
+		// Where this gets hairy is the image store does not currently support multi-arch images, e.g.:
+		//   An image `foo` may have a multi-arch manifest, but the image store only fetches the image for a specific platform
+		//   The image store does not store the manifest list and image tags are assigned to architecture specific images.
+		//   So we can have a `foo` image that is amd64 but the user requested armv7. If the user looks at the list of images.
+		//   This may be confusing.
+		//   The alternative to this is to return a errdefs.Conflict error with a helpful message, but clients will not be
+		//   able to automatically tell what causes the conflict.
+		retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform: wanted %s, actual: %s", refOrID, platforms.Format(p), platforms.Format(imgPlat)))
 	}()
 	ref, err := reference.ParseAnyReference(refOrID)
 	if err != nil {
@@ -223,3 +218,34 @@ func (i *ImageService) GetImage(refOrID string, platform *specs.Platform) (retIm
 
 	return nil, ErrImageDoesNotExist{ref}
 }
+
+// OnlyPlatformWithFallback uses `platforms.Only` with a fallback to handle the case where the platform
+//  being matched does not have a CPU variant.
+//
+// The reason for this is that CPU variant is not even if the official image config spec as of this writing.
+// See: https://github.com/opencontainers/image-spec/pull/809
+// Since Docker tends to compare platforms from the image config, we need to handle this case.
+func OnlyPlatformWithFallback(p specs.Platform) platforms.Matcher {
+	return &onlyFallbackMatcher{only: platforms.Only(p), p: platforms.Normalize(p)}
+}
+
+type onlyFallbackMatcher struct {
+	only platforms.Matcher
+	p    specs.Platform
+}
+
+func (m *onlyFallbackMatcher) Match(other specs.Platform) bool {
+	if m.only.Match(other) {
+		// It matches, no reason to fallback
+		return true
+	}
+	if other.Variant != "" {
+		// If there is a variant then this fallback does not apply, and there is no match
+		return false
+	}
+	otherN := platforms.Normalize(other)
+	otherN.Variant = "" // normalization adds a default variant... which is the whole problem with `platforms.Only`
+
+	return m.p.OS == otherN.OS &&
+		m.p.Architecture == otherN.Architecture
+}

daemon/images/images_test.go

@@ -0,0 +1,33 @@
+package images
+
+import (
+	"testing"
+
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
+	"gotest.tools/v3/assert"
+)
+
+func TestOnlyPlatformWithFallback(t *testing.T) {
+	p := specs.Platform{
+		OS:           "linux",
+		Architecture: "arm",
+		Variant:      "v8",
+	}
+
+	// Check no variant
+	assert.Assert(t, OnlyPlatformWithFallback(p).Match(specs.Platform{
+		OS:           p.OS,
+		Architecture: p.Architecture,
+	}))
+	// check with variant
+	assert.Assert(t, OnlyPlatformWithFallback(p).Match(specs.Platform{
+		OS:           p.OS,
+		Architecture: p.Architecture,
+		Variant:      p.Variant,
+	}))
+	// Make sure non-matches are false.
+	assert.Assert(t, !OnlyPlatformWithFallback(p).Match(specs.Platform{
+		OS:           p.OS,
+		Architecture: "amd64",
+	}))
+}