Accept manifest lists with linux/amd64 platform

Author: Oleg Bulatov

pkg/image/importer/importer.go

@@ -1,6 +1,7 @@
 package importer
 
 import (
+	"errors"
 	"fmt"
 	"net/url"
 	"strings"
@@ -10,6 +11,7 @@ import (
 
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/digest"
+	"github.com/docker/distribution/manifest/manifestlist"
 	"github.com/docker/distribution/manifest/schema1"
 	"github.com/docker/distribution/manifest/schema2"
 	"github.com/docker/distribution/reference"
@@ -316,26 +318,21 @@ func applyErrorToRepository(repository *importRepository, err error) {
 	}
 }
 
-func formatRepositoryError(repository *importRepository, refName string, refID string, defErr error) (err error) {
-	err = defErr
+func formatRepositoryError(ref imageapi.DockerImageReference, err error) error {
 	switch {
 	case isDockerError(err, v2.ErrorCodeManifestUnknown):
-		ref := repository.Ref
-		ref.Tag, ref.ID = refName, refID
 		err = kapierrors.NewNotFound(imageapi.Resource("dockerimage"), ref.Exact())
 	case isDockerError(err, errcode.ErrorCodeUnauthorized):
-		err = kapierrors.NewUnauthorized(fmt.Sprintf("you may not have access to the Docker image %q", repository.Ref.Exact()))
+		err = kapierrors.NewUnauthorized(fmt.Sprintf("you may not have access to the Docker image %q", ref.Exact()))
 	case strings.HasSuffix(err.Error(), "no basic auth credentials"):
-		err = kapierrors.NewUnauthorized(fmt.Sprintf("you may not have access to the Docker image %q", repository.Ref.Exact()))
+		err = kapierrors.NewUnauthorized(fmt.Sprintf("you may not have access to the Docker image %q", ref.Exact()))
 	}
-	return
+	return err
 }
 
 // calculateImageSize gets and updates size of each image layer. If manifest v2 is converted to v1,
 // then it loses information about layers size. We have to get this information from server again.
-func (isi *ImageStreamImporter) calculateImageSize(ctx gocontext.Context, repo distribution.Repository, image *imageapi.Image) error {
-	bs := repo.Blobs(ctx)
-
+func (isi *ImageStreamImporter) calculateImageSize(ctx gocontext.Context, bs distribution.BlobStore, image *imageapi.Image) error {
 	blobSet := sets.NewString()
 	size := int64(0)
 	for i := range image.DockerImageLayers {
@@ -372,6 +369,64 @@ func (isi *ImageStreamImporter) calculateImageSize(ctx gocontext.Context, repo d
 	return nil
 }
 
+// Defaults for converting manifest lists to schema1/schema2 manifests.
+// See https://github.com/docker/distribution/blob/06fa77aa11a3913096efcb9b5bd25db8ef55a939/registry/handlers/manifests.go#L25
+const (
+	defaultArch = "amd64"
+	defaultOS   = "linux"
+)
+
+var errUnsupportedManifestList = errors.New("importer: unsupported manifest list")
+
+func (isi *ImageStreamImporter) importManifest(ctx gocontext.Context, manifest distribution.Manifest, ref imageapi.DockerImageReference, d digest.Digest, s distribution.ManifestService, b distribution.BlobStore) (image *imageapi.Image, err error) {
+	if manifestList, ok := manifest.(*manifestlist.DeserializedManifestList); ok {
+		var manifestDigest digest.Digest
+		for _, manifestDescriptor := range manifestList.Manifests {
+			if manifestDescriptor.Platform.Architecture == defaultArch && manifestDescriptor.Platform.OS == defaultOS {
+				manifestDigest = manifestDescriptor.Digest
+				break
+			}
+		}
+		if manifestDigest == "" {
+			return nil, errUnsupportedManifestList
+		}
+
+		manifest, err = s.Get(ctx, manifestDigest)
+		if err != nil {
+			glog.V(5).Infof("unable to get %s/%s manifest by digest %q for image %s: %#v", defaultOS, defaultArch, d, ref.Exact(), err)
+			return nil, formatRepositoryError(ref, err)
+		}
+	}
+
+	if signedManifest, isSchema1 := manifest.(*schema1.SignedManifest); isSchema1 {
+		image, err = schema1ToImage(signedManifest, d)
+	} else if deserializedManifest, isSchema2 := manifest.(*schema2.DeserializedManifest); isSchema2 {
+		imageConfig, getImportConfigErr := b.Get(ctx, deserializedManifest.Config.Digest)
+		if getImportConfigErr != nil {
+			glog.V(5).Infof("unable to get image config by digest %q for image %s: %#v", d, ref.Exact(), getImportConfigErr)
+			return image, formatRepositoryError(ref, getImportConfigErr)
+		}
+		image, err = schema2ToImage(deserializedManifest, imageConfig, d)
+	} else {
+		err = fmt.Errorf("unsupported image manifest type: %T", manifest)
+		glog.V(5).Info(err)
+	}
+	if err != nil {
+		return
+	}
+
+	if err := imageapi.ImageWithMetadata(image); err != nil {
+		return image, err
+	}
+
+	if image.DockerImageMetadata.Size == 0 {
+		if err := isi.calculateImageSize(ctx, b, image); err != nil {
+			return image, err
+		}
+	}
+	return
+}
+
 // importRepositoryFromDocker loads the tags and images requested in the passed importRepository, obeying the
 // optional rate limiter.  Errors are set onto the individual tags and digest objects.
 func (isi *ImageStreamImporter) importRepositoryFromDocker(ctx gocontext.Context, retriever RepositoryRetriever, repository *importRepository, limiter flowcontrol.RateLimiter) {
@@ -418,9 +473,10 @@ func (isi *ImageStreamImporter) importRepositoryFromDocker(ctx gocontext.Context
 	// get a blob context
 	b := repo.Blobs(ctx)
 
-	// if repository import is requested (MaximumTags), attempt to load the tags, sort them, and request the first N
+	// if repository import is requested (MaximumTags), attempt to load the tags, sort them, and request at most N tags
+	var tags []string
 	if count := repository.MaximumTags; count > 0 || count == -1 {
-		tags, err := repo.Tags(ctx).All(ctx)
+		tags, err = repo.Tags(ctx).All(ctx)
 		if err != nil {
 			glog.V(5).Infof("unable to access tags for repository %#v: %#v", repository, err)
 			switch {
@@ -441,16 +497,6 @@ func (isi *ImageStreamImporter) importRepositoryFromDocker(ctx gocontext.Context
 		tags = set.List()
 		// include only the top N tags in the result, put the rest in AdditionalTags
 		imageapi.PrioritizeTags(tags)
-		for _, s := range tags {
-			if count <= 0 && repository.MaximumTags != -1 {
-				repository.AdditionalTags = append(repository.AdditionalTags, s)
-				continue
-			}
-			count--
-			repository.Tags = append(repository.Tags, importTag{
-				Name: s,
-			})
-		}
 	}
 
 	// load digests
@@ -459,116 +505,71 @@ func (isi *ImageStreamImporter) importRepositoryFromDocker(ctx gocontext.Context
 		if importDigest.Err != nil || importDigest.Image != nil {
 			continue
 		}
+
 		d, err := digest.ParseDigest(importDigest.Name)
 		if err != nil {
 			importDigest.Err = err
 			continue
 		}
+
+		ref := repository.Ref
+		ref.Tag = ""
+		ref.ID = string(d)
+
 		limiter.Accept()
+
 		manifest, err := s.Get(ctx, d)
 		if err != nil {
-			glog.V(5).Infof("unable to access digest %q for repository %#v: %#v", d, repository, err)
-			importDigest.Err = formatRepositoryError(repository, "", importDigest.Name, err)
+			glog.V(5).Infof("unable to get manifest by digest %q for image %s: %#v", d, ref.Exact(), err)
+			importDigest.Err = formatRepositoryError(ref, err)
 			continue
 		}
 
-		if signedManifest, isSchema1 := manifest.(*schema1.SignedManifest); isSchema1 {
-			importDigest.Image, err = schema1ToImage(signedManifest, d)
-		} else if deserializedManifest, isSchema2 := manifest.(*schema2.DeserializedManifest); isSchema2 {
-			imageConfig, getImportConfigErr := b.Get(ctx, deserializedManifest.Config.Digest)
-			if getImportConfigErr != nil {
-				glog.V(5).Infof("unable to access the image config using digest %q for repository %#v: %#v", d, repository, getImportConfigErr)
-				if isDockerError(getImportConfigErr, v2.ErrorCodeManifestUnknown) {
-					ref := repository.Ref
-					ref.ID = deserializedManifest.Config.Digest.String()
-					importDigest.Err = kapierrors.NewNotFound(imageapi.Resource("dockerimage"), ref.Exact())
-				} else {
-					importDigest.Err = formatRepositoryError(repository, "", importDigest.Name, getImportConfigErr)
-				}
-				continue
-			}
+		importDigest.Image, importDigest.Err = isi.importManifest(ctx, manifest, ref, d, s, b)
+	}
 
-			importDigest.Image, err = schema2ToImage(deserializedManifest, imageConfig, d)
-		} else {
-			// TODO: Current this error means the imported received the manifest list
-			// which we don't support yet.
-			err = fmt.Errorf("unsupported image manifest schema: %T", manifest)
-			glog.V(5).Infof("unsupported manifest type: %T", manifest)
-		}
+	doImportTag := func(importTag *importTag) bool {
+		ref := repository.Ref
+		ref.Tag = importTag.Name
+		ref.ID = ""
+
+		limiter.Accept()
 
+		manifest, err := s.Get(ctx, "", distribution.WithTag(importTag.Name))
 		if err != nil {
-			importDigest.Err = err
-			continue
+			glog.V(5).Infof("unable to get manifest by tag %q for image %s: %#v", importTag.Name, ref.Exact(), err)
+			importTag.Err = formatRepositoryError(ref, err)
+			return true
 		}
 
-		if err := imageapi.ImageWithMetadata(importDigest.Image); err != nil {
-			importDigest.Err = err
-			continue
-		}
-		if importDigest.Image.DockerImageMetadata.Size == 0 {
-			if err := isi.calculateImageSize(ctx, repo, importDigest.Image); err != nil {
-				importDigest.Err = err
-				continue
-			}
-		}
+		importTag.Image, importTag.Err = isi.importManifest(ctx, manifest, ref, "", s, b)
+		return importTag.Err != errUnsupportedManifestList
 	}
 
 	for i := range repository.Tags {
 		importTag := &repository.Tags[i]
 		if importTag.Err != nil || importTag.Image != nil {
 			continue
 		}
-		limiter.Accept()
-
-		manifest, err := s.Get(ctx, "", distribution.WithTag(importTag.Name))
-		if err != nil {
-			glog.V(5).Infof("unable to get manifest by tag %q for repository %#v: %#v", importTag.Name, repository, err)
-			// try to resolve the tag and fetch manifest by digest instead
-			desc, getTagErr := repo.Tags(ctx).Get(ctx, importTag.Name)
-			if getTagErr != nil {
-				glog.V(5).Infof("unable to get tag %q for repository %#v: %#v", importTag.Name, repository, getTagErr)
-				importTag.Err = formatRepositoryError(repository, importTag.Name, "", err)
-				continue
-			}
-			m, getManifestErr := s.Get(ctx, desc.Digest)
-			if getManifestErr != nil {
-				glog.V(5).Infof("unable to access digest %q for tag %q for repository %#v: %#v", desc.Digest, importTag.Name, repository, getManifestErr)
-				importTag.Err = formatRepositoryError(repository, importTag.Name, "", err)
-				continue
-			}
-			manifest = m
-		}
 
-		if signedManifest, isSchema1 := manifest.(*schema1.SignedManifest); isSchema1 {
-			importTag.Image, err = schema1ToImage(signedManifest, "")
-		} else if deserializedManifest, isSchema2 := manifest.(*schema2.DeserializedManifest); isSchema2 {
-			imageConfig, getImportConfigErr := b.Get(ctx, deserializedManifest.Config.Digest)
-			if getImportConfigErr != nil {
-				glog.V(5).Infof("unable to access image config using digest %q for tag %q for repository %#v: %#v", deserializedManifest.Config.Digest, importTag.Name, repository, getImportConfigErr)
-				importTag.Err = formatRepositoryError(repository, importTag.Name, "", getImportConfigErr)
-				continue
-			}
-			importTag.Image, err = schema2ToImage(deserializedManifest, imageConfig, "")
-		} else {
-			// TODO: Current this error means the imported received the manifest list
-			// which we don't support yet.
-			err = fmt.Errorf("unsupported image manifest schema: %T", manifest)
-			glog.V(5).Infof("unsupported manifest type: %T", manifest)
-		}
+		doImportTag(importTag)
+	}
 
-		if err != nil {
-			importTag.Err = err
+	imported := 0
+	for _, tagName := range tags {
+		if repository.MaximumTags != -1 && imported >= repository.MaximumTags {
+			repository.AdditionalTags = append(repository.AdditionalTags, tagName)
 			continue
 		}
-		if err := imageapi.ImageWithMetadata(importTag.Image); err != nil {
-			importTag.Err = err
-			continue
+
+		it := importTag{
+			Name: tagName,
 		}
-		if importTag.Image.DockerImageMetadata.Size == 0 {
-			if err := isi.calculateImageSize(ctx, repo, importTag.Image); err != nil {
-				importTag.Err = err
-				continue
-			}
+		if doImportTag(&it) {
+			imported++
+			repository.Tags = append(repository.Tags, it)
+		} else {
+			repository.AdditionalTags = append(repository.AdditionalTags, tagName)
 		}
 	}
 }

test/integration/imageimporter_test.go

@@ -32,8 +32,6 @@ import (
 )
 
 func TestImageStreamImport(t *testing.T) {
-	t.Skip("This test was disabled until https://github.com/openshift/origin/issues/16323 is fixed!")
-
 	masterConfig, clusterAdminKubeConfig, err := testserver.StartTestMaster()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -799,8 +797,6 @@ func TestImageStreamImportScheduled(t *testing.T) {
 }
 
 func TestImageStreamImportDockerHub(t *testing.T) {
-	t.Skip("This test was disabled until https://github.com/openshift/origin/issues/16323 is fixed!")
-
 	rt, _ := restclient.TransportFor(&restclient.Config{})
 	importCtx := importer.NewContext(rt, nil).WithCredentials(importer.NoCredentials)