copy: add option to strip sparse manifest lists

Add the ability to strip non-copied instances from manifest lists when
using CopySpecificImages. This implements the functionality originally
proposed in containers/image#1707.

When copying a subset of images from a multi-architecture manifest list
using CopySpecificImages, the current behavior always produces a sparse
manifest list - a manifest list that references platforms that weren't
actually copied. While this maintains the original digest, many
registries reject sparse manifests with "blob unknown to registry"
errors.

This commit adds a new SparseManifestListAction option that gives users
control over this behavior:

- KeepSparseManifestList (default): Preserves the manifest list as-is,
  even when some instances aren't copied. Some registries may not
  support sparse manifest lists.

- StripSparseManifestList (new): Removes instances that are not being copied
  from the destination manifest list. This changes the digest but
  ensures compatibility with all registries.

Based on original work by @bertbaron and @mtrmac in
containers/image#1707, adapted for the container-libs monorepo
structure.

Relates to #227

Signed-off-by: Alex Guidi <aguidi@redhat.com>

Author: aguidirh

image/copy/copy.go

@@ -77,9 +77,28 @@ const (
 // specific images from the source reference.
 type ImageListSelection int
 
+const (
+	// KeepSparseManifestList is the default value which, when set in
+	// Options.SparseManifestListAction, indicates that the manifest is kept
+	// as is even though some images from the list may be missing. Some
+	// registries may not support this.
+	KeepSparseManifestList SparseManifestListAction = iota
+
+	// StripSparseManifestList will strip missing images from the manifest
+	// list. When images are stripped the digest will differ from the original.
+	StripSparseManifestList
+)
+
+// SparseManifestListAction is one of KeepSparseManifestList or StripSparseManifestList
+// to control the behavior when only a subset of images from a manifest list is copied
+type SparseManifestListAction int
+
 // Options allows supplying non-default configuration modifying the behavior of CopyImage.
 type Options struct {
 	RemoveSignatures bool // Remove any pre-existing signatures. Signers and SignBy… will still add a new signature.
+	// RemoveListSignatures removes the manifest list signature while preserving per-instance signatures.
+	// If RemoveSignatures is also true, RemoveSignatures takes precedence.
+	RemoveListSignatures bool
 	// Signers to use to add signatures during the copy.
 	// Callers are still responsible for closing these Signer objects; they can be reused for multiple copy.Image operations in a row.
 	Signers                          []*signer.Signer
@@ -102,6 +121,9 @@ type Options struct {
 	ImageListSelection    ImageListSelection       // set to either CopySystemImage (the default), CopyAllImages, or CopySpecificImages to control which instances we copy when the source reference is a list; ignored if the source reference is not a list
 	Instances             []digest.Digest          // if ImageListSelection is CopySpecificImages, copy only these instances, instances matching the InstancePlatforms list, and the list itself
 	InstancePlatforms     []InstancePlatformFilter // if ImageListSelection is CopySpecificImages, copy instances with matching OS/Architecture (all variants and compressions), it also copies the index/manifest_list instance.
+	// When only a subset of images of a list is copied, this action indicates if the manifest should be kept or stripped.
+	// See CopySpecificImages.
+	SparseManifestListAction SparseManifestListAction
 	// Give priority to pulling gzip images if multiple images are present when configured to OptionalBoolTrue,
 	// prefers the best compression if this is configured as OptionalBoolFalse. Choose automatically (and the choice may change over time)
 	// if this is set to OptionalBoolUndefined (which is the default behavior, and recommended for most callers).
@@ -318,30 +340,15 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
 		return nil, fmt.Errorf("determining manifest MIME type for %s: %w", transports.ImageName(srcRef), err)
 	}
 
+	var singleInstance *image.UnparsedImage
+	var singleInstanceErrorWrapping string
 	if !multiImage {
-		if len(options.EnsureCompressionVariantsExist) > 0 {
-			return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image")
-		}
-		requireCompressionFormatMatch, err := shouldRequireCompressionFormatMatch(options)
-		if err != nil {
-			return nil, err
-		}
 		// The simple case: just copy a single image.
-		single, err := c.copySingleImage(ctx, c.unparsedToplevel, nil, copySingleImageOptions{requireCompressionFormatMatch: requireCompressionFormatMatch})
-		if err != nil {
-			return nil, err
-		}
-		copiedManifest = single.manifest
+		singleInstance = c.unparsedToplevel
+		singleInstanceErrorWrapping = ""
 	} else if c.options.ImageListSelection == CopySystemImage {
-		if len(options.EnsureCompressionVariantsExist) > 0 {
-			return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image")
-		}
-		requireCompressionFormatMatch, err := shouldRequireCompressionFormatMatch(options)
-		if err != nil {
-			return nil, err
-		}
-		// This is a manifest list, and we weren't asked to copy multiple images.  Choose a single image that
-		// matches the current system to copy, and copy it.
+		// This is a manifest list, and we weren't asked to copy multiple images.
+		// Choose a single image that matches the current system to copy.
 		mfest, manifestType, err := c.unparsedToplevel.Manifest(ctx)
 		if err != nil {
 			return nil, fmt.Errorf("reading manifest for %s: %w", transports.ImageName(srcRef), err)
@@ -355,13 +362,30 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
 			return nil, fmt.Errorf("choosing an image from manifest list %s: %w", transports.ImageName(srcRef), err)
 		}
 		logrus.Debugf("Source is a manifest list; copying (only) instance %s for current system", instanceDigest)
-		unparsedInstance := image.UnparsedInstance(rawSource, &instanceDigest)
-		single, err := c.copySingleImage(ctx, unparsedInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: requireCompressionFormatMatch})
+		singleInstance = image.UnparsedInstance(rawSource, &instanceDigest)
+		singleInstanceErrorWrapping = "copying system image from manifest list"
+	} // else: a multi-instance copy
+
+	if singleInstance != nil {
+		if len(options.EnsureCompressionVariantsExist) > 0 {
+			return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image")
+		}
+		if c.options.RemoveListSignatures {
+			return nil, fmt.Errorf("RemoveListSignatures is not applicable when copying a single instance")
+		}
+		requireCompressionFormatMatch, err := shouldRequireCompressionFormatMatch(options)
+		if err != nil {
+			return nil, err
+		}
+		single, err := c.copySingleImage(ctx, singleInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: requireCompressionFormatMatch})
 		if err != nil {
-			return nil, fmt.Errorf("copying system image from manifest list: %w", err)
+			if singleInstanceErrorWrapping != "" {
+				return nil, fmt.Errorf("%s: %w", singleInstanceErrorWrapping, err)
+			}
+			return nil, err
 		}
 		copiedManifest = single.manifest
-	} else { /* c.options.ImageListSelection == CopyAllImages or c.options.ImageListSelection == CopySpecificImages, */
+	} else {
 		// If we were asked to copy multiple images and can't, that's an error.
 		if !supportsMultipleImages(c.dest) {
 			return nil, fmt.Errorf("copying multiple images: destination transport %q does not support copying multiple images as a group", destRef.Transport().Name())

image/copy/multiple.go

@@ -21,27 +21,32 @@ import (
 	"go.podman.io/image/v5/pkg/compression"
 )
 
-type instanceCopyKind int
+type instanceOpKind int
 
 const (
-	instanceCopyCopy instanceCopyKind = iota
-	instanceCopyClone
+	instanceOpCopy instanceOpKind = iota
+	instanceOpClone
+	instanceOpDelete
 )
 
-type instanceCopy struct {
-	op           instanceCopyKind
+type instanceOp struct {
+	op           instanceOpKind
 	sourceDigest digest.Digest
 
 	// Fields which can be used by callers when operation
-	// is `instanceCopyCopy`
+	// is `instanceOpCopy`
 	copyForceCompressionFormat bool
 
 	// Fields which can be used by callers when operation
-	// is `instanceCopyClone`
+	// is `instanceOpClone`
 	cloneArtifactType       string
 	cloneCompressionVariant OptionCompressionVariant
 	clonePlatform           *imgspecv1.Platform
 	cloneAnnotations        map[string]string
+
+	// Fields which can be used by callers when operation
+	// is `instanceOpDelete`
+	deleteIndex int
 }
 
 // internal type only to make imgspecv1.Platform comparable
@@ -99,72 +104,95 @@ func validateCompressionVariantExists(input []OptionCompressionVariant) error {
 	return nil
 }
 
-// prepareInstanceCopies prepares a list of instances which needs to copied to the manifest list.
-func prepareInstanceCopies(list internalManifest.List, instanceDigests []digest.Digest, options *Options) ([]instanceCopy, error) {
-	res := []instanceCopy{}
+// prepareInstanceOps prepares a list of operations to perform on instances (copy, clone, or delete).
+// It returns a unified list of all operations and the count of copy/clone operations (excluding deletes).
+func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Digest, options *Options, cannotModifyManifestListReason string) ([]instanceOp, int, error) {
+	res := []instanceOp{}
+	deleteOps := []instanceOp{}
 	if options.ImageListSelection == CopySpecificImages && len(options.EnsureCompressionVariantsExist) > 0 {
 		// List can already contain compressed instance for a compression selected in `EnsureCompressionVariantsExist`
 		// It's unclear what it means when `CopySpecificImages` includes an instance in options.Instances,
 		// EnsureCompressionVariantsExist asks for an instance with some compression,
 		// an instance with that compression already exists, but is not included in options.Instances.
 		// We might define the semantics and implement this in the future.
-		return res, fmt.Errorf("EnsureCompressionVariantsExist is not implemented for CopySpecificImages")
+		return res, -1, fmt.Errorf("EnsureCompressionVariantsExist is not implemented for CopySpecificImages")
 	}
 	err := validateCompressionVariantExists(options.EnsureCompressionVariantsExist)
 	if err != nil {
-		return res, err
+		return res, -1, err
 	}
 	compressionsByPlatform, err := platformCompressionMap(list, instanceDigests)
 	if err != nil {
-		return nil, err
+		return nil, -1, err
 	}
 
 	// Determine which specific images to copy (combining digest-based and platform-based selection)
 	var specificImages *set.Set[digest.Digest]
 	if options.ImageListSelection == CopySpecificImages {
 		specificImages, err = determineSpecificImages(options, list)
 		if err != nil {
-			return nil, err
+			return nil, -1, err
 		}
 	}
 
 	for i, instanceDigest := range instanceDigests {
 		if options.ImageListSelection == CopySpecificImages &&
 			!specificImages.Contains(instanceDigest) {
-			logrus.Debugf("Skipping instance %s (%d/%d)", instanceDigest, i+1, len(instanceDigests))
+			if options.SparseManifestListAction == StripSparseManifestList {
+				if cannotModifyManifestListReason != "" {
+					return nil, -1, fmt.Errorf("we should delete instance %s from manifest list, but we cannot: %s", instanceDigest, cannotModifyManifestListReason)
+				}
+				logrus.Debugf("deleting instance %s from destination’s manifest (%d/%d)", instanceDigest, i+1, len(instanceDigests))
+				deleteOps = append(deleteOps, instanceOp{
+					op:          instanceOpDelete,
+					deleteIndex: i,
+				})
+			} else {
+				logrus.Debugf("skipping instance %s (%d/%d)", instanceDigest, i+1, len(instanceDigests))
+			}
 			continue
 		}
 		instanceDetails, err := list.Instance(instanceDigest)
 		if err != nil {
-			return res, fmt.Errorf("getting details for instance %s: %w", instanceDigest, err)
+			return res, -1, fmt.Errorf("getting details for instance %s: %w", instanceDigest, err)
 		}
 		forceCompressionFormat, err := shouldRequireCompressionFormatMatch(options)
 		if err != nil {
-			return nil, err
+			return nil, -1, err
 		}
-		res = append(res, instanceCopy{
-			op:                         instanceCopyCopy,
+		res = append(res, instanceOp{
+			op:                         instanceOpCopy,
 			sourceDigest:               instanceDigest,
 			copyForceCompressionFormat: forceCompressionFormat,
 		})
 		platform := platformV1ToPlatformComparable(instanceDetails.ReadOnly.Platform)
 		compressionList := compressionsByPlatform[platform]
 		for _, compressionVariant := range options.EnsureCompressionVariantsExist {
 			if !compressionList.Contains(compressionVariant.Algorithm.Name()) {
-				res = append(res, instanceCopy{
-					op:                      instanceCopyClone,
+				res = append(res, instanceOp{
+					op:                      instanceOpClone,
 					sourceDigest:            instanceDigest,
 					cloneArtifactType:       instanceDetails.ReadOnly.ArtifactType,
 					cloneCompressionVariant: compressionVariant,
 					clonePlatform:           instanceDetails.ReadOnly.Platform,
 					cloneAnnotations:        maps.Clone(instanceDetails.ReadOnly.Annotations),
 				})
-				// add current compression to the list so that we don’t create duplicate clones
 				compressionList.Add(compressionVariant.Algorithm.Name())
 			}
 		}
 	}
-	return res, nil
+
+	// Add delete operations in reverse order (highest to lowest index) to avoid shifting
+	slices.Reverse(deleteOps)
+	copyLen := len(res) // Count copy/clone operations before appending deletes
+
+	if copyLen == 0 && len(deleteOps) == len(instanceDigests) {
+		return nil, -1, fmt.Errorf("requested operation filtered out all platforms and would create an empty image")
+	}
+
+	res = append(res, deleteOps...)
+
+	return res, copyLen, nil
 }
 
 // determineSpecificImages returns a set of images to copy based on the
@@ -222,7 +250,7 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte,
 
 	sigs, err := c.sourceSignatures(ctx, c.unparsedToplevel,
 		"Getting image list signatures",
-		"Checking if image list destination supports signatures")
+		"Checking if image list destination supports signatures", true)
 	if err != nil {
 		return nil, err
 	}
@@ -248,7 +276,7 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte,
 	// Compare, and perhaps keep in sync with, the version in copySingleImage.
 	cannotModifyManifestListReason := ""
 	if len(sigs) > 0 {
-		cannotModifyManifestListReason = "Would invalidate signatures"
+		cannotModifyManifestListReason = "Would invalidate signatures; consider removing them from the multi-platform list"
 	}
 	if destIsDigestedReference {
 		cannotModifyManifestListReason = "Destination specifies a digest"
@@ -272,29 +300,31 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte,
 	}
 	if selectedListType != originalList.MIMEType() {
 		if cannotModifyManifestListReason != "" {
-			return nil, fmt.Errorf("Manifest list must be converted to type %q to be written to destination, but we cannot modify it: %q", selectedListType, cannotModifyManifestListReason)
+			return nil, fmt.Errorf("Manifest list must be converted to type %q to be written to destination, but we cannot modify it: %s", selectedListType, cannotModifyManifestListReason)
 		}
 	}
 
 	// Copy each image, or just the ones we want to copy, in turn.
 	instanceDigests := updatedList.Instances()
 	instanceEdits := []internalManifest.ListEdit{}
-	instanceCopyList, err := prepareInstanceCopies(updatedList, instanceDigests, c.options)
+	instanceOpList, copyLen, err := prepareInstanceOps(updatedList, instanceDigests, c.options, cannotModifyManifestListReason)
 	if err != nil {
 		return nil, fmt.Errorf("preparing instances for copy: %w", err)
 	}
-	c.Printf("Copying %d images generated from %d images in list\n", len(instanceCopyList), len(instanceDigests))
-	for i, instance := range instanceCopyList {
+	c.Printf("Copying %d images generated from %d images in list\n", copyLen, len(instanceDigests))
+	copyCount := 0 // Track copy/clone operations separately from delete operations
+	for i, instance := range instanceOpList {
 		// Update instances to be edited by their `ListOperation` and
 		// populate necessary fields.
 		switch instance.op {
-		case instanceCopyCopy:
-			logrus.Debugf("Copying instance %s (%d/%d)", instance.sourceDigest, i+1, len(instanceCopyList))
-			c.Printf("Copying image %s (%d/%d)\n", instance.sourceDigest, i+1, len(instanceCopyList))
-			unparsedInstance := image.UnparsedInstance(c.rawSource, &instanceCopyList[i].sourceDigest)
-			updated, err := c.copySingleImage(ctx, unparsedInstance, &instanceCopyList[i].sourceDigest, copySingleImageOptions{requireCompressionFormatMatch: instance.copyForceCompressionFormat})
+		case instanceOpCopy:
+			copyCount++
+			logrus.Debugf("Copying instance %s (%d/%d)", instance.sourceDigest, copyCount, copyLen)
+			c.Printf("Copying image %s (%d/%d)\n", instance.sourceDigest, copyCount, copyLen)
+			unparsedInstance := image.UnparsedInstance(c.rawSource, &instanceOpList[i].sourceDigest)
+			updated, err := c.copySingleImage(ctx, unparsedInstance, &instanceOpList[i].sourceDigest, copySingleImageOptions{requireCompressionFormatMatch: instance.copyForceCompressionFormat})
 			if err != nil {
-				return nil, fmt.Errorf("copying image %d/%d from manifest list: %w", i+1, len(instanceCopyList), err)
+				return nil, fmt.Errorf("copying image %d/%d from manifest list: %w", copyCount, copyLen, err)
 			}
 			// Record the result of a possible conversion here.
 			instanceEdits = append(instanceEdits, internalManifest.ListEdit{
@@ -305,17 +335,18 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte,
 				UpdateCompressionAlgorithms: updated.compressionAlgorithms,
 				UpdateMediaType:             updated.manifestMIMEType,
 			})
-		case instanceCopyClone:
-			logrus.Debugf("Replicating instance %s (%d/%d)", instance.sourceDigest, i+1, len(instanceCopyList))
-			c.Printf("Replicating image %s (%d/%d)\n", instance.sourceDigest, i+1, len(instanceCopyList))
-			unparsedInstance := image.UnparsedInstance(c.rawSource, &instanceCopyList[i].sourceDigest)
-			updated, err := c.copySingleImage(ctx, unparsedInstance, &instanceCopyList[i].sourceDigest, copySingleImageOptions{
+		case instanceOpClone:
+			copyCount++
+			logrus.Debugf("Replicating instance %s (%d/%d)", instance.sourceDigest, copyCount, copyLen)
+			c.Printf("Replicating image %s (%d/%d)\n", instance.sourceDigest, copyCount, copyLen)
+			unparsedInstance := image.UnparsedInstance(c.rawSource, &instanceOpList[i].sourceDigest)
+			updated, err := c.copySingleImage(ctx, unparsedInstance, &instanceOpList[i].sourceDigest, copySingleImageOptions{
 				requireCompressionFormatMatch: true,
 				compressionFormat:             &instance.cloneCompressionVariant.Algorithm,
 				compressionLevel:              instance.cloneCompressionVariant.Level,
 			})
 			if err != nil {
-				return nil, fmt.Errorf("replicating image %d/%d from manifest list: %w", i+1, len(instanceCopyList), err)
+				return nil, fmt.Errorf("replicating image %d/%d from manifest list: %w", copyCount, copyLen, err)
 			}
 			// Record the result of a possible conversion here.
 			instanceEdits = append(instanceEdits, internalManifest.ListEdit{
@@ -328,12 +359,17 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte,
 				AddAnnotations:           instance.cloneAnnotations,
 				AddCompressionAlgorithms: updated.compressionAlgorithms,
 			})
+		case instanceOpDelete:
+			instanceEdits = append(instanceEdits, internalManifest.ListEdit{
+				ListOperation: internalManifest.ListOpDelete,
+				DeleteIndex:   instance.deleteIndex,
+			})
 		default:
 			return nil, fmt.Errorf("copying image: invalid copy operation %d", instance.op)
 		}
 	}
 
-	// Now reset the digest/size/types of the manifests in the list to account for any conversions that we made.
+	// Now reset the digest/size/types of the manifests in the list and remove deleted instances.
 	if err = updatedList.EditInstances(instanceEdits, cannotModifyManifestListReason != ""); err != nil {
 		return nil, fmt.Errorf("updating manifest list: %w", err)
 	}