distribution: fix passing platform struct to puller

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Upstream-commit: 337ba71fc1124603302e28d94e2f08674e31a756
Component: engine

components/engine/distribution/config.go

@@ -60,9 +60,8 @@ type ImagePullConfig struct {
 	// Schema2Types is the valid schema2 configuration types allowed
 	// by the pull operation.
 	Schema2Types []string
-	// OS is the requested operating system of the image being pulled to ensure it can be validated
-	// when the host OS supports multiple image operating systems.
-	OS string
+	// Platform is the requested platform of the image being pulled
+	Platform *specs.Platform
 }
 
 // ImagePushConfig stores push configuration.
@@ -171,7 +170,7 @@ func (s *imageConfigStore) PlatformFromConfig(c []byte) (*specs.Platform, error)
 	if !system.IsOSSupported(os) {
 		return nil, system.ErrNotSupportedOperatingSystem
 	}
-	return &specs.Platform{OS: os, OSVersion: unmarshalledConfig.OSVersion}, nil
+	return &specs.Platform{OS: os, Architecture: unmarshalledConfig.Architecture, OSVersion: unmarshalledConfig.OSVersion}, nil
 }
 
 type storeLayerProvider struct {

components/engine/distribution/pull.go

@@ -11,6 +11,7 @@ import (
 	refstore "github.com/docker/docker/reference"
 	"github.com/docker/docker/registry"
 	"github.com/opencontainers/go-digest"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -20,7 +21,7 @@ type Puller interface {
 	// Pull tries to pull the image referenced by `tag`
 	// Pull returns an error if any, as well as a boolean that determines whether to retry Pull on the next configured endpoint.
 	//
-	Pull(ctx context.Context, ref reference.Named, os string) error
+	Pull(ctx context.Context, ref reference.Named, platform *specs.Platform) error
 }
 
 // newPuller returns a Puller interface that will pull from either a v1 or v2
@@ -114,7 +115,7 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
 			continue
 		}
 
-		if err := puller.Pull(ctx, ref, imagePullConfig.OS); err != nil {
+		if err := puller.Pull(ctx, ref, imagePullConfig.Platform); err != nil {
 			// Was this pull cancelled? If so, don't try to fall
 			// back.
 			fallback := false

components/engine/distribution/pull_v1.go

@@ -25,6 +25,7 @@ import (
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/registry"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 )
 
@@ -36,7 +37,7 @@ type v1Puller struct {
 	session     *registry.Session
 }
 
-func (p *v1Puller) Pull(ctx context.Context, ref reference.Named, os string) error {
+func (p *v1Puller) Pull(ctx context.Context, ref reference.Named, _ *specs.Platform) error {
 	if _, isCanonical := ref.(reference.Canonical); isCanonical {
 		// Allowing fallback, because HTTPS v1 is before HTTP v2
 		return fallbackError{err: ErrNoSupport{Err: errors.New("Cannot pull by digest with v1 registry")}}

components/engine/distribution/pull_v2.go

@@ -11,6 +11,7 @@ import (
 	"runtime"
 	"strings"
 
+	"github.com/containerd/containerd/platforms"
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/manifest/manifestlist"
 	"github.com/docker/distribution/manifest/schema1"
@@ -63,7 +64,7 @@ type v2Puller struct {
 	confirmedV2 bool
 }
 
-func (p *v2Puller) Pull(ctx context.Context, ref reference.Named, os string) (err error) {
+func (p *v2Puller) Pull(ctx context.Context, ref reference.Named, platform *specs.Platform) (err error) {
 	// TODO(tiborvass): was ReceiveTimeout
 	p.repo, p.confirmedV2, err = NewV2Repository(ctx, p.repoInfo, p.endpoint, p.config.MetaHeaders, p.config.AuthConfig, "pull")
 	if err != nil {
@@ -71,7 +72,7 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named, os string) (er
 		return err
 	}
 
-	if err = p.pullV2Repository(ctx, ref, os); err != nil {
+	if err = p.pullV2Repository(ctx, ref, platform); err != nil {
 		if _, ok := err.(fallbackError); ok {
 			return err
 		}
@@ -86,10 +87,10 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named, os string) (er
 	return err
 }
 
-func (p *v2Puller) pullV2Repository(ctx context.Context, ref reference.Named, os string) (err error) {
+func (p *v2Puller) pullV2Repository(ctx context.Context, ref reference.Named, platform *specs.Platform) (err error) {
 	var layersDownloaded bool
 	if !reference.IsNameOnly(ref) {
-		layersDownloaded, err = p.pullV2Tag(ctx, ref, os)
+		layersDownloaded, err = p.pullV2Tag(ctx, ref, platform)
 		if err != nil {
 			return err
 		}
@@ -111,7 +112,7 @@ func (p *v2Puller) pullV2Repository(ctx context.Context, ref reference.Named, os
 			if err != nil {
 				return err
 			}
-			pulledNew, err := p.pullV2Tag(ctx, tagRef, os)
+			pulledNew, err := p.pullV2Tag(ctx, tagRef, platform)
 			if err != nil {
 				// Since this is the pull-all-tags case, don't
 				// allow an error pulling a particular tag to
@@ -327,7 +328,7 @@ func (ld *v2LayerDescriptor) Registered(diffID layer.DiffID) {
 	ld.V2MetadataService.Add(diffID, metadata.V2Metadata{Digest: ld.digest, SourceRepository: ld.repoInfo.Name.Name()})
 }
 
-func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named, os string) (tagUpdated bool, err error) {
+func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named, platform *specs.Platform) (tagUpdated bool, err error) {
 	manSvc, err := p.repo.Manifests(ctx)
 	if err != nil {
 		return false, err
@@ -391,17 +392,17 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named, os string
 		if p.config.RequireSchema2 {
 			return false, fmt.Errorf("invalid manifest: not schema2")
 		}
-		id, manifestDigest, err = p.pullSchema1(ctx, ref, v, os)
+		id, manifestDigest, err = p.pullSchema1(ctx, ref, v, platform)
 		if err != nil {
 			return false, err
 		}
 	case *schema2.DeserializedManifest:
-		id, manifestDigest, err = p.pullSchema2(ctx, ref, v, os)
+		id, manifestDigest, err = p.pullSchema2(ctx, ref, v, platform)
 		if err != nil {
 			return false, err
 		}
 	case *manifestlist.DeserializedManifestList:
-		id, manifestDigest, err = p.pullManifestList(ctx, ref, v, os)
+		id, manifestDigest, err = p.pullManifestList(ctx, ref, v, platform)
 		if err != nil {
 			return false, err
 		}
@@ -437,7 +438,7 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named, os string
 	return true, nil
 }
 
-func (p *v2Puller) pullSchema1(ctx context.Context, ref reference.Reference, unverifiedManifest *schema1.SignedManifest, requestedOS string) (id digest.Digest, manifestDigest digest.Digest, err error) {
+func (p *v2Puller) pullSchema1(ctx context.Context, ref reference.Reference, unverifiedManifest *schema1.SignedManifest, platform *specs.Platform) (id digest.Digest, manifestDigest digest.Digest, err error) {
 	var verifiedManifest *schema1.Manifest
 	verifiedManifest, err = verifySchema1Manifest(unverifiedManifest, ref)
 	if err != nil {
@@ -513,7 +514,10 @@ func (p *v2Puller) pullSchema1(ctx context.Context, ref reference.Reference, unv
 	// we support the operating system, switch to that operating system.
 	// eg FROM supertest2014/nyan with no platform specifier, and docker build
 	// with no --platform= flag under LCOW.
-	if requestedOS == "" && system.IsOSSupported(configOS) {
+	requestedOS := ""
+	if platform != nil {
+		requestedOS = platform.OS
+	} else if system.IsOSSupported(configOS) {
 		requestedOS = configOS
 	}
 
@@ -544,7 +548,7 @@ func (p *v2Puller) pullSchema1(ctx context.Context, ref reference.Reference, unv
 	return imageID, manifestDigest, nil
 }
 
-func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *schema2.DeserializedManifest, requestedOS string) (id digest.Digest, manifestDigest digest.Digest, err error) {
+func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *schema2.DeserializedManifest, platform *specs.Platform) (id digest.Digest, manifestDigest digest.Digest, err error) {
 	manifestDigest, err = schema2ManifestDigest(ref, mfst)
 	if err != nil {
 		return "", "", err
@@ -600,6 +604,11 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
 		configPlatform   *specs.Platform // for LCOW when registering downloaded layers
 	)
 
+	layerStoreOS := runtime.GOOS
+	if platform != nil {
+		layerStoreOS = platform.OS
+	}
+
 	// https://github.com/docker/docker/issues/24766 - Err on the side of caution,
 	// explicitly blocking images intended for linux from the Windows daemon. On
 	// Windows, we do this before the attempt to download, effectively serialising
@@ -623,13 +632,14 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
 		if len(descriptors) != len(configRootFS.DiffIDs) {
 			return "", "", errRootFSMismatch
 		}
-
-		// Early bath if the requested OS doesn't match that of the configuration.
-		// This avoids doing the download, only to potentially fail later.
-		if !system.IsOSSupported(configPlatform.OS) {
-			return "", "", fmt.Errorf("cannot download image with operating system %q when requesting %q", configPlatform.OS, requestedOS)
+		if platform == nil {
+			// Early bath if the requested OS doesn't match that of the configuration.
+			// This avoids doing the download, only to potentially fail later.
+			if !system.IsOSSupported(configPlatform.OS) {
+				return "", "", fmt.Errorf("cannot download image with operating system %q when requesting %q", configPlatform.OS, layerStoreOS)
+			}
+			layerStoreOS = configPlatform.OS
 		}
-		requestedOS = configPlatform.OS
 
 		// Populate diff ids in descriptors to avoid downloading foreign layers
 		// which have been side loaded
@@ -638,10 +648,6 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
 		}
 	}
 
-	if requestedOS == "" {
-		requestedOS = runtime.GOOS
-	}
-
 	if p.config.DownloadManager != nil {
 		go func() {
 			var (
@@ -649,7 +655,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
 				rootFS image.RootFS
 			)
 			downloadRootFS := *image.NewRootFS()
-			rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, requestedOS, descriptors, p.config.ProgressOutput)
+			rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, layerStoreOS, descriptors, p.config.ProgressOutput)
 			if err != nil {
 				// Intentionally do not cancel the config download here
 				// as the error from config download (if there is one)
@@ -735,22 +741,22 @@ func receiveConfig(s ImageConfigStore, configChan <-chan []byte, errChan <-chan
 
 // pullManifestList handles "manifest lists" which point to various
 // platform-specific manifests.
-func (p *v2Puller) pullManifestList(ctx context.Context, ref reference.Named, mfstList *manifestlist.DeserializedManifestList, requestedOS string) (id digest.Digest, manifestListDigest digest.Digest, err error) {
+func (p *v2Puller) pullManifestList(ctx context.Context, ref reference.Named, mfstList *manifestlist.DeserializedManifestList, pp *specs.Platform) (id digest.Digest, manifestListDigest digest.Digest, err error) {
 	manifestListDigest, err = schema2ManifestDigest(ref, mfstList)
 	if err != nil {
 		return "", "", err
 	}
 
-	logOS := requestedOS // May be "" indicating any OS
-	if logOS == "" {
-		logOS = "*"
+	var platform specs.Platform
+	if pp != nil {
+		platform = *pp
 	}
-	logrus.Debugf("%s resolved to a manifestList object with %d entries; looking for a %s/%s match", ref, len(mfstList.Manifests), logOS, runtime.GOARCH)
+	logrus.Debugf("%s resolved to a manifestList object with %d entries; looking for a %s/%s match", ref, len(mfstList.Manifests), platforms.Format(platform), runtime.GOARCH)
 
-	manifestMatches := filterManifests(mfstList.Manifests, requestedOS)
+	manifestMatches := filterManifests(mfstList.Manifests, platform)
 
 	if len(manifestMatches) == 0 {
-		errMsg := fmt.Sprintf("no matching manifest for %s/%s in the manifest list entries", logOS, runtime.GOARCH)
+		errMsg := fmt.Sprintf("no matching manifest for %s in the manifest list entries", platforms.Format(platform))
 		logrus.Debugf(errMsg)
 		return "", "", errors.New(errMsg)
 	}
@@ -781,12 +787,14 @@ func (p *v2Puller) pullManifestList(ctx context.Context, ref reference.Named, mf
 
 	switch v := manifest.(type) {
 	case *schema1.SignedManifest:
-		id, _, err = p.pullSchema1(ctx, manifestRef, v, manifestMatches[0].Platform.OS)
+		platform := toOCIPlatform(manifestMatches[0].Platform)
+		id, _, err = p.pullSchema1(ctx, manifestRef, v, &platform)
 		if err != nil {
 			return "", "", err
 		}
 	case *schema2.DeserializedManifest:
-		id, _, err = p.pullSchema2(ctx, manifestRef, v, manifestMatches[0].Platform.OS)
+		platform := toOCIPlatform(manifestMatches[0].Platform)
+		id, _, err = p.pullSchema2(ctx, manifestRef, v, &platform)
 		if err != nil {
 			return "", "", err
 		}
@@ -956,3 +964,13 @@ func fixManifestLayers(m *schema1.Manifest) error {
 func createDownloadFile() (*os.File, error) {
 	return ioutil.TempFile("", "GetImageBlob")
 }
+
+func toOCIPlatform(p manifestlist.PlatformSpec) specs.Platform {
+	return specs.Platform{
+		OS:           p.OS,
+		Architecture: p.Architecture,
+		Variant:      p.Variant,
+		OSFeatures:   p.OSFeatures,
+		OSVersion:    p.OSVersion,
+	}
+}

components/engine/distribution/pull_v2_unix.go

@@ -4,10 +4,11 @@ package distribution // import "github.com/docker/docker/distribution"
 
 import (
 	"context"
-	"runtime"
 
+	"github.com/containerd/containerd/platforms"
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/manifest/manifestlist"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 )
 
@@ -16,15 +17,27 @@ func (ld *v2LayerDescriptor) open(ctx context.Context) (distribution.ReadSeekClo
 	return blobs.Open(ctx, ld.digest)
 }
 
-func filterManifests(manifests []manifestlist.ManifestDescriptor, _ string) []manifestlist.ManifestDescriptor {
+func filterManifests(manifests []manifestlist.ManifestDescriptor, p specs.Platform) []manifestlist.ManifestDescriptor {
+	p = withDefault(p)
 	var matches []manifestlist.ManifestDescriptor
-	for _, manifestDescriptor := range manifests {
-		if manifestDescriptor.Platform.Architecture == runtime.GOARCH && manifestDescriptor.Platform.OS == runtime.GOOS {
-			matches = append(matches, manifestDescriptor)
-
-			logrus.Debugf("found match for %s/%s with media type %s, digest %s", runtime.GOOS, runtime.GOARCH, manifestDescriptor.MediaType, manifestDescriptor.Digest.String())
+	for _, desc := range manifests {
+		if compareNormalized(toOCIPlatform(desc.Platform), p) {
+			matches = append(matches, desc)
+			logrus.Debugf("found match for %s with media type %s, digest %s", platforms.Format(p), desc.MediaType, desc.Digest.String())
 		}
 	}
+
+	// deprecated: backwards compatibility with older versions that didn't compare variant
+	if len(matches) == 0 && p.Architecture == "arm" {
+		p = normalize(p)
+		for _, desc := range manifests {
+			if desc.Platform.OS == p.OS && desc.Platform.Architecture == p.Architecture {
+				matches = append(matches, desc)
+				logrus.Debugf("found deprecated partial match for %s with media type %s, digest %s", platforms.Format(p), desc.MediaType, desc.Digest.String())
+			}
+		}
+	}
+
 	return matches
 }
 
@@ -32,3 +45,38 @@ func filterManifests(manifests []manifestlist.ManifestDescriptor, _ string) []ma
 func checkImageCompatibility(imageOS, imageOSVersion string) error {
 	return nil
 }
+
+func withDefault(p specs.Platform) specs.Platform {
+	def := platforms.DefaultSpec()
+	if p.OS == "" {
+		p.OS = def.OS
+	}
+	if p.Architecture == "" {
+		p.Architecture = def.Architecture
+		p.Variant = def.Variant
+	}
+	return p
+}
+
+func compareNormalized(p1, p2 specs.Platform) bool {
+	// remove after https://github.com/containerd/containerd/pull/2414
+	return p1.OS == p2.OS &&
+		p1.Architecture == p2.Architecture &&
+		p1.Variant == p2.Variant
+}
+
+func normalize(p specs.Platform) specs.Platform {
+	p = platforms.Normalize(p)
+	// remove after https://github.com/containerd/containerd/pull/2414
+	if p.Architecture == "arm" {
+		if p.Variant == "" {
+			p.Variant = "v7"
+		}
+	}
+	if p.Architecture == "arm64" {
+		if p.Variant == "" {
+			p.Variant = "v8"
+		}
+	}
+	return p
+}

components/engine/distribution/pull_v2_windows.go

@@ -16,6 +16,7 @@ import (
 	"github.com/docker/distribution/manifest/schema2"
 	"github.com/docker/distribution/registry/client/transport"
 	"github.com/docker/docker/pkg/system"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 )
 
@@ -62,7 +63,7 @@ func (ld *v2LayerDescriptor) open(ctx context.Context) (distribution.ReadSeekClo
 	return rsc, err
 }
 
-func filterManifests(manifests []manifestlist.ManifestDescriptor, requestedOS string) []manifestlist.ManifestDescriptor {
+func filterManifests(manifests []manifestlist.ManifestDescriptor, p specs.Platform) []manifestlist.ManifestDescriptor {
 	version := system.GetOSVersion()
 	osVersion := fmt.Sprintf("%d.%d.%d", version.MajorVersion, version.MinorVersion, version.Build)
 	logrus.Debugf("will prefer Windows entries with version %s", osVersion)
@@ -71,8 +72,8 @@ func filterManifests(manifests []manifestlist.ManifestDescriptor, requestedOS st
 	foundWindowsMatch := false
 	for _, manifestDescriptor := range manifests {
 		if (manifestDescriptor.Platform.Architecture == runtime.GOARCH) &&
-			((requestedOS != "" && manifestDescriptor.Platform.OS == requestedOS) || // Explicit user request for an OS we know we support
-				(requestedOS == "" && system.IsOSSupported(manifestDescriptor.Platform.OS))) { // No user requested OS, but one we can support
+			((p.OS != "" && manifestDescriptor.Platform.OS == p.OS) || // Explicit user request for an OS we know we support
+				(p.OS == "" && system.IsOSSupported(manifestDescriptor.Platform.OS))) { // No user requested OS, but one we can support
 			matches = append(matches, manifestDescriptor)
 			logrus.Debugf("found match %s/%s %s with media type %s, digest %s", manifestDescriptor.Platform.OS, runtime.GOARCH, manifestDescriptor.Platform.OSVersion, manifestDescriptor.MediaType, manifestDescriptor.Digest.String())
 			if strings.EqualFold("windows", manifestDescriptor.Platform.OS) {

components/engine/distribution/registry_unit_test.go

@@ -5,7 +5,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"runtime"
 	"strings"
 	"testing"
 
@@ -84,7 +83,7 @@ func testTokenPassThru(t *testing.T, ts *httptest.Server) {
 	logrus.Debug("About to pull")
 	// We expect it to fail, since we haven't mock'd the full registry exchange in our handler above
 	tag, _ := reference.WithTag(n, "tag_goes_here")
-	_ = p.pullV2Repository(ctx, tag, runtime.GOOS)
+	_ = p.pullV2Repository(ctx, tag, nil)
 }
 
 func TestTokenPassThru(t *testing.T) {