Merge pull request #45644 from vvoland/c8d-load-unpack-attestation

c8d/load: Don't unpack pseudo images

daemon/containerd/handlers.go

@@ -9,16 +9,13 @@ import (
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
-// walkPresentChildren is a simple wrapper for containerdimages.Walk with
-// presentChildrenHandler wrapping a simple handler that only operates on
-// walked Descriptor and doesn't return any errror.
+// walkPresentChildren is a simple wrapper for containerdimages.Walk with presentChildrenHandler.
 // This is only a convenient helper to reduce boilerplate.
-func (i *ImageService) walkPresentChildren(ctx context.Context, target ocispec.Descriptor, f func(context.Context, ocispec.Descriptor)) error {
+func (i *ImageService) walkPresentChildren(ctx context.Context, target ocispec.Descriptor, f func(context.Context, ocispec.Descriptor) error) error {
 	store := i.client.ContentStore()
 	return containerdimages.Walk(ctx, presentChildrenHandler(store, containerdimages.HandlerFunc(
 		func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
-			f(ctx, desc)
-			return nil, nil
+			return nil, f(ctx, desc)
 		})), target)
 }
 

daemon/containerd/image_delete.go

@@ -124,10 +124,11 @@ func (i *ImageService) deleteAll(ctx context.Context, img images.Image, force, p
 
 	// Workaround for: https://github.com/moby/buildkit/issues/3797
 	possiblyDeletedConfigs := map[digest.Digest]struct{}{}
-	err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, d ocispec.Descriptor) {
+	err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, d ocispec.Descriptor) error {
 		if images.IsConfigType(d.MediaType) {
 			possiblyDeletedConfigs[d.Digest] = struct{}{}
 		}
+		return nil
 	})
 	if err != nil {
 		return nil, err

daemon/containerd/image_exporter.go

@@ -18,6 +18,7 @@ import (
 	"github.com/docker/docker/pkg/streamformatter"
 	"github.com/opencontainers/image-spec/specs-go"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
@@ -125,16 +126,9 @@ func (i *ImageService) LoadImage(ctx context.Context, inTar io.ReadCloser, outSt
 		return errdefs.System(err)
 	}
 
-	store := i.client.ContentStore()
 	progress := streamformatter.NewStdoutWriter(outStream)
 
 	for _, img := range imgs {
-		allPlatforms, err := containerdimages.Platforms(ctx, store, img.Target)
-		if err != nil {
-			logrus.WithError(err).WithField("image", img.Name).Debug("failed to get image platforms")
-			return errdefs.Unknown(err)
-		}
-
 		name := img.Name
 		loadedMsg := "Loaded image"
 
@@ -145,17 +139,25 @@ func (i *ImageService) LoadImage(ctx context.Context, inTar io.ReadCloser, outSt
 			name = reference.FamiliarName(reference.TagNameOnly(named))
 		}
 
-		for _, platform := range allPlatforms {
+		err = i.walkImageManifests(ctx, img, func(platformImg *ImageManifest) error {
 			logger := logrus.WithFields(logrus.Fields{
-				"platform": platform,
 				"image":    name,
+				"manifest": platformImg.Target().Digest,
 			})
-			platformImg := containerd.NewImageWithPlatform(i.client, img, cplatforms.OnlyStrict(platform))
+
+			if isPseudo, err := platformImg.IsPseudoImage(ctx); isPseudo || err != nil {
+				if err != nil {
+					logger.WithError(err).Warn("failed to read manifest")
+				} else {
+					logger.Debug("don't unpack non-image manifest")
+				}
+				return nil
+			}
 
 			unpacked, err := platformImg.IsUnpacked(ctx, i.snapshotter)
 			if err != nil {
-				logger.WithError(err).Debug("failed to check if image is unpacked")
-				continue
+				logger.WithError(err).Warn("failed to check if image is unpacked")
+				return nil
 			}
 
 			if !unpacked {
@@ -166,6 +168,10 @@ func (i *ImageService) LoadImage(ctx context.Context, inTar io.ReadCloser, outSt
 				}
 			}
 			logger.WithField("alreadyUnpacked", unpacked).WithError(err).Debug("unpack")
+			return nil
+		})
+		if err != nil {
+			return errors.Wrap(err, "failed to unpack loaded image")
 		}
 
 		fmt.Fprintf(progress, "%s: %s\n", loadedMsg, name)

daemon/containerd/image_list.go

@@ -6,17 +6,14 @@ import (
 	"strings"
 	"time"
 
-	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/content"
 	cerrdefs "github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/labels"
-	"github.com/containerd/containerd/platforms"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	timetypes "github.com/docker/docker/api/types/time"
-	"github.com/moby/buildkit/util/attestation"
 	"github.com/opencontainers/go-digest"
 	"github.com/opencontainers/image-spec/identity"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -87,72 +84,41 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 			continue
 		}
 
-		err := images.Walk(ctx, images.HandlerFunc(func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
-			if images.IsIndexType(desc.MediaType) {
-				return images.Children(ctx, contentStore, desc)
+		err := i.walkImageManifests(ctx, img, func(img *ImageManifest) error {
+			if isPseudo, err := img.IsPseudoImage(ctx); isPseudo || err != nil {
+				return err
 			}
 
-			if images.IsManifestType(desc.MediaType) {
-				// Ignore buildkit attestation manifests
-				// https://github.com/moby/buildkit/blob/v0.11.4/docs/attestations/attestation-storage.md
-				// This would have also been caught by the isImageManifest call below, but it requires
-				// an additional content read and deserialization of Manifest.
-				if _, has := desc.Annotations[attestation.DockerAnnotationReferenceType]; has {
-					return nil, nil
-				}
-
-				mfst, err := images.Manifest(ctx, contentStore, desc, platforms.All)
-				if err != nil {
-					if cerrdefs.IsNotFound(err) {
-						return nil, nil
-					}
-					return nil, err
-				}
-
-				if !isImageManifest(mfst) {
-					return nil, nil
-				}
-
-				platform, err := getManifestPlatform(ctx, contentStore, desc, mfst.Config)
-				if err != nil {
-					if cerrdefs.IsNotFound(err) {
-						return nil, nil
-					}
-					return nil, err
-				}
+			available, err := img.CheckContentAvailable(ctx)
+			if err != nil {
+				logrus.WithFields(logrus.Fields{
+					logrus.ErrorKey: err,
+					"manifest":      img.Target(),
+					"image":         img.Name(),
+				}).Warn("checking availability of platform specific manifest failed")
+				return nil
+			}
 
-				pm := platforms.OnlyStrict(platform)
-				available, _, _, missing, err := images.Check(ctx, contentStore, img.Target, pm)
-				if err != nil {
-					logrus.WithFields(logrus.Fields{
-						logrus.ErrorKey: err,
-						"platform":      platform,
-						"image":         img.Target,
-					}).Warn("checking availability of platform content failed")
-					return nil, nil
-				}
-				if !available || len(missing) > 0 {
-					return nil, nil
-				}
+			if !available {
+				return nil
+			}
 
-				c8dImage := containerd.NewImageWithPlatform(i.client, img, pm)
-				image, chainIDs, err := i.singlePlatformImage(ctx, contentStore, c8dImage)
-				if err != nil {
-					return nil, err
-				}
+			image, chainIDs, err := i.singlePlatformImage(ctx, contentStore, img)
+			if err != nil {
+				return err
+			}
 
-				summaries = append(summaries, image)
+			summaries = append(summaries, image)
 
-				if opts.SharedSize {
-					root = append(root, &chainIDs)
-					for _, id := range chainIDs {
-						layers[id] = layers[id] + 1
-					}
+			if opts.SharedSize {
+				root = append(root, &chainIDs)
+				for _, id := range chainIDs {
+					layers[id] = layers[id] + 1
 				}
 			}
 
-			return nil, nil
-		}), img.Target)
+			return nil
+		})
 
 		if err != nil {
 			return nil, err
@@ -173,7 +139,7 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 	return summaries, nil
 }
 
-func (i *ImageService) singlePlatformImage(ctx context.Context, contentStore content.Store, image containerd.Image) (*types.ImageSummary, []digest.Digest, error) {
+func (i *ImageService) singlePlatformImage(ctx context.Context, contentStore content.Store, image *ImageManifest) (*types.ImageSummary, []digest.Digest, error) {
 	diffIDs, err := image.RootFS(ctx)
 	if err != nil {
 		return nil, nil, err
@@ -508,36 +474,6 @@ func computeSharedSize(chainIDs []digest.Digest, layers map[digest.Digest]int, s
 	return sharedSize, nil
 }
 
-// getManifestPlatform returns a platform specified by the manifest descriptor
-// or reads it from its config.
-func getManifestPlatform(ctx context.Context, store content.Provider, manifestDesc, configDesc ocispec.Descriptor) (ocispec.Platform, error) {
-	var platform ocispec.Platform
-	if manifestDesc.Platform != nil {
-		platform = *manifestDesc.Platform
-	} else {
-		// Config is technically a v1.Image, but it has the same member as v1.Platform
-		// which makes the v1.Platform a subset of Image so we can unmarshal directly.
-		if err := readConfig(ctx, store, configDesc, &platform); err != nil {
-			return platform, err
-		}
-	}
-	return platforms.Normalize(platform), nil
-}
-
-// isImageManifest returns true if the manifest has no layers or any of its layers is a known image layer.
-// Some manifests use the image media type for compatibility, even if they are not a real image.
-func isImageManifest(mfst ocispec.Manifest) bool {
-	if len(mfst.Layers) == 0 {
-		return true
-	}
-	for _, l := range mfst.Layers {
-		if images.IsLayerType(l.MediaType) {
-			return true
-		}
-	}
-	return false
-}
-
 // readConfig reads content pointed by the descriptor and unmarshals it into a specified output.
 func readConfig(ctx context.Context, store content.Provider, desc ocispec.Descriptor, out interface{}) error {
 	data, err := content.ReadBlob(ctx, store, desc)

daemon/containerd/image_manifest.go

@@ -0,0 +1,151 @@
+package containerd
+
+import (
+	"context"
+	"encoding/json"
+
+	"github.com/containerd/containerd"
+	"github.com/containerd/containerd/content"
+	"github.com/containerd/containerd/images"
+	containerdimages "github.com/containerd/containerd/images"
+	cplatforms "github.com/containerd/containerd/platforms"
+	"github.com/docker/docker/errdefs"
+	"github.com/moby/buildkit/util/attestation"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/pkg/errors"
+)
+
+var (
+	errNotManifestOrIndex = errdefs.InvalidParameter(errors.New("descriptor is neither a manifest or index"))
+	errNotManifest        = errdefs.InvalidParameter(errors.New("descriptor isn't a manifest"))
+)
+
+// walkImageManifests calls the handler for each locally present manifest in
+// the image. The image implements the containerd.Image interface, but all
+// operations act on the specific manifest instead of the index.
+func (i *ImageService) walkImageManifests(ctx context.Context, img containerdimages.Image, handler func(img *ImageManifest) error) error {
+	desc := img.Target
+
+	handleManifest := func(ctx context.Context, d ocispec.Descriptor) error {
+		platformImg, err := i.NewImageManifest(ctx, img, d)
+		if err != nil {
+			if err == errNotManifest {
+				return nil
+			}
+			return err
+		}
+		return handler(platformImg)
+	}
+
+	if containerdimages.IsManifestType(desc.MediaType) {
+		return handleManifest(ctx, desc)
+	}
+
+	if containerdimages.IsIndexType(desc.MediaType) {
+		return i.walkPresentChildren(ctx, desc, handleManifest)
+	}
+
+	return errNotManifestOrIndex
+}
+
+type ImageManifest struct {
+	containerd.Image
+
+	// Parent of the manifest (index/manifest list)
+	RealTarget ocispec.Descriptor
+
+	manifest *ocispec.Manifest
+}
+
+func (i *ImageService) NewImageManifest(ctx context.Context, img containerdimages.Image, manifestDesc ocispec.Descriptor) (*ImageManifest, error) {
+	if !containerdimages.IsManifestType(manifestDesc.MediaType) {
+		return nil, errNotManifest
+	}
+
+	parent := img.Target
+	img.Target = manifestDesc
+
+	c8dImg := containerd.NewImageWithPlatform(i.client, img, cplatforms.All)
+	return &ImageManifest{
+		Image:      c8dImg,
+		RealTarget: parent,
+	}, nil
+}
+
+func (im *ImageManifest) Metadata() containerdimages.Image {
+	md := im.Image.Metadata()
+	md.Target = im.RealTarget
+	return md
+}
+
+// IsPseudoImage returns false if the manifest has no layers or any of its layers is a known image layer.
+// Some manifests use the image media type for compatibility, even if they are not a real image.
+func (im *ImageManifest) IsPseudoImage(ctx context.Context) (bool, error) {
+	desc := im.Target()
+
+	// Quick check for buildkit attestation manifests
+	// https://github.com/moby/buildkit/blob/v0.11.4/docs/attestations/attestation-storage.md
+	// This would have also been caught by the layer check below, but it requires
+	// an additional content read and deserialization of Manifest.
+	if _, has := desc.Annotations[attestation.DockerAnnotationReferenceType]; has {
+		return true, nil
+	}
+
+	mfst, err := im.Manifest(ctx)
+	if err != nil {
+		return true, err
+	}
+	if len(mfst.Layers) == 0 {
+		return false, nil
+	}
+	for _, l := range mfst.Layers {
+		if images.IsLayerType(l.MediaType) {
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
+func (im *ImageManifest) Manifest(ctx context.Context) (ocispec.Manifest, error) {
+	if im.manifest != nil {
+		return *im.manifest, nil
+	}
+
+	mfst, err := readManifest(ctx, im.ContentStore(), im.Target())
+	if err != nil {
+		return ocispec.Manifest{}, err
+	}
+
+	im.manifest = &mfst
+	return mfst, nil
+}
+
+func (im *ImageManifest) CheckContentAvailable(ctx context.Context) (bool, error) {
+	// The target is already a platform-specific manifest, so no need to match platform.
+	pm := cplatforms.All
+
+	available, _, _, missing, err := containerdimages.Check(ctx, im.ContentStore(), im.Target(), pm)
+	if err != nil {
+		return false, err
+	}
+
+	if !available || len(missing) > 0 {
+		return false, nil
+	}
+
+	return true, nil
+}
+
+func readManifest(ctx context.Context, store content.Provider, desc ocispec.Descriptor) (ocispec.Manifest, error) {
+	p, err := content.ReadBlob(ctx, store, desc)
+	if err != nil {
+		return ocispec.Manifest{}, err
+	}
+
+	var mfst ocispec.Manifest
+	if err := json.Unmarshal(p, &mfst); err != nil {
+		return ocispec.Manifest{}, err
+	}
+
+	return mfst, nil
+}

daemon/containerd/image_prune.go

@@ -125,11 +125,12 @@ func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFu
 
 		blobs := []ocispec.Descriptor{}
 
-		err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, desc ocispec.Descriptor) {
+		err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, desc ocispec.Descriptor) error {
 			blobs = append(blobs, desc)
 			if containerdimages.IsConfigType(desc.MediaType) {
 				possiblyDeletedConfigs[desc.Digest] = struct{}{}
 			}
+			return nil
 		})
 		if err != nil {
 			errs = multierror.Append(errs, err)
@@ -186,10 +187,11 @@ func (i *ImageService) unleaseSnapshotsFromDeletedConfigs(ctx context.Context, p
 
 	var errs error
 	for _, img := range all {
-		err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, desc ocispec.Descriptor) {
+		err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, desc ocispec.Descriptor) error {
 			if containerdimages.IsConfigType(desc.MediaType) {
 				delete(possiblyDeletedConfigs, desc.Digest)
 			}
+			return nil
 		})
 		if err != nil {
 			errs = multierror.Append(errs, err)