From 1d0b67c46336803f79ad57c15c5b60843a23776f Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Wed, 11 Feb 2026 11:20:15 +0100 Subject: [PATCH 01/23] 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 existing behavior - StripSparseManifestList (new): removes non-copied instances The implementation includes: 1. New SparseManifestListAction enum in image/copy 2. New ListOpDelete operation in image/internal/manifest with support for both OCI1Index and Schema2List formats 3. Index-based deletion (not digest-based) to handle platform variants that share the same digest 4. Stripping logic in copyMultipleImages that activates when StripSparseManifestList is set 5. Comprehensive test coverage for deletion operations 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 --- image/copy/copy.go | 71 ++++--- image/copy/multiple.go | 131 ++++++++---- image/copy/multiple_test.go | 191 +++++++++++++++--- image/copy/sign.go | 4 +- image/copy/single.go | 2 +- .../internal/manifest/docker_schema2_list.go | 5 + .../manifest/docker_schema2_list_test.go | 22 ++ image/internal/manifest/list.go | 4 + image/internal/manifest/oci_index.go | 5 + image/internal/manifest/oci_index_test.go | 22 ++ 10 files changed, 359 insertions(+), 98 deletions(-) diff --git a/image/copy/copy.go b/image/copy/copy.go index 5d70482519..6970ffb5ab 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -77,9 +77,29 @@ 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. + // This option is orthogonal to other copy options; if set, it removes list signatures when applicable. + // 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 +122,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 +341,14 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, return nil, fmt.Errorf("determining manifest MIME type for %s: %w", transports.ImageName(srcRef), err) } + // Determine if we're copying a single image or multiple images + var singleInstance *image.UnparsedImage 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 } 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) @@ -350,18 +357,30 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, if err != nil { return nil, fmt.Errorf("parsing primary manifest as list for %s: %w", transports.ImageName(srcRef), err) } - instanceDigest, err := manifestList.ChooseInstanceByCompression(c.options.SourceCtx, c.options.PreferGzipInstances) // try to pick one that matches c.options.SourceCtx + instanceDigest, err := manifestList.ChooseInstanceByCompression(c.options.SourceCtx, c.options.PreferGzipInstances) if err != nil { 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) + } // else: a multi-instance copy + + if singleInstance != nil { + // Single-image copy path + 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 + } + 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) + return nil, err } copiedManifest = single.manifest - } else { /* c.options.ImageListSelection == CopyAllImages or c.options.ImageListSelection == CopySpecificImages, */ + } else { + // Multi-image copy path // 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()) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index 4ee57e5f58..d62da87631 100644 --- a/image/copy/multiple.go +++ b/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,24 +104,26 @@ 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) ([]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, 0, fmt.Errorf("EnsureCompressionVariantsExist is not implemented for CopySpecificImages") } err := validateCompressionVariantExists(options.EnsureCompressionVariantsExist) if err != nil { - return res, err + return res, 0, err } compressionsByPlatform, err := platformCompressionMap(list, instanceDigests) if err != nil { - return nil, err + return nil, 0, err } // Determine which specific images to copy (combining digest-based and platform-based selection) @@ -124,26 +131,35 @@ func prepareInstanceCopies(list internalManifest.List, instanceDigests []digest. if options.ImageListSelection == CopySpecificImages { specificImages, err = determineSpecificImages(options, list) if err != nil { - return nil, err + return nil, 0, 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)) + // Create delete operation if we're stripping sparse manifests + if options.SparseManifestListAction == StripSparseManifestList { + 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, 0, fmt.Errorf("getting details for instance %s: %w", instanceDigest, err) } forceCompressionFormat, err := shouldRequireCompressionFormatMatch(options) if err != nil { - return nil, err + return nil, 0, err } - res = append(res, instanceCopy{ - op: instanceCopyCopy, + res = append(res, instanceOp{ + op: instanceOpCopy, sourceDigest: instanceDigest, copyForceCompressionFormat: forceCompressionFormat, }) @@ -151,20 +167,26 @@ func prepareInstanceCopies(list internalManifest.List, instanceDigests []digest. 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 + // 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 + res = append(res, deleteOps...) + + return res, copyLen, nil } // determineSpecificImages returns a set of images to copy based on the @@ -222,7 +244,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 +270,12 @@ 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" + // If RemoveListSignatures is set, we allow modification of the manifest list + // by stripping only the list-level signature while preserving per-instance signatures. + // This is handled below by setting sigs to empty (similar to RemoveSignatures). + if !c.options.RemoveListSignatures { + cannotModifyManifestListReason = "Would invalidate signatures" + } } if destIsDigestedReference { cannotModifyManifestListReason = "Destination specifies a digest" @@ -257,6 +284,14 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, cannotModifyManifestListReason = "Instructed to preserve digests" } + // Validate that when using StripSparseManifestList with signed images, the user explicitly + // acknowledges signature handling via either RemoveListSignatures or RemoveSignatures. + // This is consistent with requiring explicit acknowledgment for other manifest edits. + if c.options.SparseManifestListAction == StripSparseManifestList && len(sigs) > 0 && + !c.options.RemoveSignatures && !c.options.RemoveListSignatures { + return nil, fmt.Errorf("stripping unselected instances from a signed manifest list would invalidate signatures; remove all signatures, or remove only the list signature while preserving per-instance signatures") + } + // Determine if we'll need to convert the manifest list to a different format. forceListMIMEType := c.options.ForceManifestMIMEType switch forceListMIMEType { @@ -272,6 +307,9 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, } if selectedListType != originalList.MIMEType() { if cannotModifyManifestListReason != "" { + if cannotModifyManifestListReason == "Would invalidate signatures" { + return nil, fmt.Errorf("Manifest list must be converted to type %q to be written to destination, but we cannot modify it: %q; consider removing only the list signature to allow modification while preserving per-instance signatures", selectedListType, 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) } } @@ -279,22 +317,24 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, // 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) 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 +345,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 +369,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) } @@ -368,6 +414,9 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, // If we can't just use the original value, but we have to change it, flag an error. if !bytes.Equal(attemptedManifestList, originalManifestList) { if cannotModifyManifestListReason != "" { + if cannotModifyManifestListReason == "Would invalidate signatures" { + return nil, fmt.Errorf("Manifest list was edited, but we cannot modify it: %q; consider removing only the list signature to allow modification while preserving per-instance signatures", cannotModifyManifestListReason) + } return nil, fmt.Errorf("Manifest list was edited, but we cannot modify it: %q", cannotModifyManifestListReason) } logrus.Debugf("Manifest list has been updated") diff --git a/image/copy/multiple_test.go b/image/copy/multiple_test.go index 7486757ade..c57c412096 100644 --- a/image/copy/multiple_test.go +++ b/image/copy/multiple_test.go @@ -1,6 +1,7 @@ package copy import ( + "context" "os" "path/filepath" "slices" @@ -9,13 +10,20 @@ import ( digest "github.com/opencontainers/go-digest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.podman.io/image/v5/directory" internalManifest "go.podman.io/image/v5/internal/manifest" "go.podman.io/image/v5/pkg/compression" ) -// Test `instanceCopyCopy` cases. -func TestPrepareCopyInstancesforInstanceCopyCopy(t *testing.T) { - validManifest, err := os.ReadFile(filepath.Join("..", "internal", "manifest", "testdata", "oci1.index.zstd-selection.json")) +const ( + // Test manifest file paths + ociManifestFile = "../internal/manifest/testdata/ociv1.manifest.json" + ociIndexZstdFile = "../internal/manifest/testdata/oci1.index.zstd-selection.json" +) + +// Test `instanceOpCopy` cases. +func TestPrepareInstanceOpsForInstanceCopy(t *testing.T) { + validManifest, err := os.ReadFile(ociIndexZstdFile) require.NoError(t, err) list, err := internalManifest.ListFromBlob(validManifest, internalManifest.GuessMIMEType(validManifest)) require.NoError(t, err) @@ -27,39 +35,63 @@ func TestPrepareCopyInstancesforInstanceCopyCopy(t *testing.T) { digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), } - instancesToCopy, err := prepareInstanceCopies(list, sourceInstances, &Options{}) + instancesToCopy, _, err := prepareInstanceOps(list, sourceInstances, &Options{}) require.NoError(t, err) - compare := []instanceCopy{} + compare := []instanceOp{} for _, instance := range sourceInstances { - compare = append(compare, instanceCopy{ - op: instanceCopyCopy, + compare = append(compare, instanceOp{ + op: instanceOpCopy, sourceDigest: instance, copyForceCompressionFormat: false, }) } assert.Equal(t, instancesToCopy, compare) // Test CopySpecificImages where selected instance is sourceInstances[1] - instancesToCopy, err = prepareInstanceCopies(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages}) + instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages}) require.NoError(t, err) - compare = []instanceCopy{{ - op: instanceCopyCopy, + compare = []instanceOp{{ + op: instanceOpCopy, sourceDigest: sourceInstances[1], }} assert.Equal(t, instancesToCopy, compare) - _, err = prepareInstanceCopies(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, ForceCompressionFormat: true}) + // Test CopySpecificImages with StripSparseManifestList where selected instance is sourceInstances[1] + instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{ + Instances: []digest.Digest{sourceInstances[1]}, + ImageListSelection: CopySpecificImages, + SparseManifestListAction: StripSparseManifestList, + }) + require.NoError(t, err) + // Should have 1 copy operation followed by 2 delete operations (for indices 0 and 2) + expected := []instanceOp{ + { + op: instanceOpCopy, + sourceDigest: sourceInstances[1], + }, + { + op: instanceOpDelete, + deleteIndex: 2, // Delete from highest to lowest + }, + { + op: instanceOpDelete, + deleteIndex: 0, + }, + } + assert.Equal(t, expected, instancesToCopy) + + _, _, err = prepareInstanceOps(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, ForceCompressionFormat: true}) require.EqualError(t, err, "cannot use ForceCompressionFormat with undefined default compression format") } -// Test `instanceCopyClone` cases. -func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) { - validManifest, err := os.ReadFile(filepath.Join("..", "internal", "manifest", "testdata", "oci1.index.zstd-selection.json")) +// Test `instanceOpClone` cases. +func TestPrepareInstanceOpsForInstanceClone(t *testing.T) { + validManifest, err := os.ReadFile(ociIndexZstdFile) require.NoError(t, err) list, err := internalManifest.ListFromBlob(validManifest, internalManifest.GuessMIMEType(validManifest)) require.NoError(t, err) - // Prepare option for `instanceCopyClone` case. + // Prepare option for `instanceOpClone` case. ensureCompressionVariantsExist := []OptionCompressionVariant{{Algorithm: compression.Zstd}} sourceInstances := []digest.Digest{ @@ -69,7 +101,7 @@ func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) { } // CopySpecificImage must fail with error - _, err = prepareInstanceCopies(list, sourceInstances, &Options{ + _, _, err = prepareInstanceOps(list, sourceInstances, &Options{ EnsureCompressionVariantsExist: ensureCompressionVariantsExist, Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, @@ -77,7 +109,7 @@ func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) { require.EqualError(t, err, "EnsureCompressionVariantsExist is not implemented for CopySpecificImages") // Test copying all images with replication - instancesToCopy, err := prepareInstanceCopies(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}) + instancesToCopy, _, err := prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}) require.NoError(t, err) // Following test ensures @@ -91,12 +123,12 @@ func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) { expectedResponse := []simplerInstanceCopy{} for _, instance := range sourceInstances { expectedResponse = append(expectedResponse, simplerInstanceCopy{ - op: instanceCopyCopy, + op: instanceOpCopy, sourceDigest: instance, }) // If its `arm64` and sourceDigest[2] , expect a clone to happen if instance == sourceInstances[2] { - expectedResponse = append(expectedResponse, simplerInstanceCopy{op: instanceCopyClone, sourceDigest: instance, cloneCompressionVariant: "zstd", clonePlatform: "arm64-linux-"}) + expectedResponse = append(expectedResponse, simplerInstanceCopy{op: instanceOpClone, sourceDigest: instance, cloneCompressionVariant: "zstd", clonePlatform: "arm64-linux-"}) } } actualResponse := convertInstanceCopyToSimplerInstanceCopy(instancesToCopy) @@ -105,17 +137,17 @@ func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) { // Test option with multiple copy request for same compression format. // The above expectation should stay the same, if ensureCompressionVariantsExist requests zstd twice. ensureCompressionVariantsExist = []OptionCompressionVariant{{Algorithm: compression.Zstd}, {Algorithm: compression.Zstd}} - instancesToCopy, err = prepareInstanceCopies(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}) + instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}) require.NoError(t, err) expectedResponse = []simplerInstanceCopy{} for _, instance := range sourceInstances { expectedResponse = append(expectedResponse, simplerInstanceCopy{ - op: instanceCopyCopy, + op: instanceOpCopy, sourceDigest: instance, }) // If its `arm64` and sourceDigest[2] , expect a clone to happen if instance == sourceInstances[2] { - expectedResponse = append(expectedResponse, simplerInstanceCopy{op: instanceCopyClone, sourceDigest: instance, cloneCompressionVariant: "zstd", clonePlatform: "arm64-linux-"}) + expectedResponse = append(expectedResponse, simplerInstanceCopy{op: instanceOpClone, sourceDigest: instance, cloneCompressionVariant: "zstd", clonePlatform: "arm64-linux-"}) } } actualResponse = convertInstanceCopyToSimplerInstanceCopy(instancesToCopy) @@ -127,32 +159,32 @@ func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) { digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), } - instancesToCopy, err = prepareInstanceCopies(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}) + instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}) require.NoError(t, err) // two copies but clone should happen only once numberOfCopyClone := 0 for _, instance := range instancesToCopy { - if instance.op == instanceCopyClone { + if instance.op == instanceOpClone { numberOfCopyClone++ } } assert.Equal(t, 1, numberOfCopyClone) } -// simpler version of `instanceCopy` for testing where fields are string +// simpler version of `instanceOp` for testing where fields are string // instead of pointer type simplerInstanceCopy struct { - op instanceCopyKind + op instanceOpKind sourceDigest digest.Digest // Fields which can be used by callers when operation - // is `instanceCopyClone` + // is `instanceOpClone` cloneCompressionVariant string clonePlatform string cloneAnnotations map[string]string } -func convertInstanceCopyToSimplerInstanceCopy(copies []instanceCopy) []simplerInstanceCopy { +func convertInstanceCopyToSimplerInstanceCopy(copies []instanceOp) []simplerInstanceCopy { res := []simplerInstanceCopy{} for _, instance := range copies { platform := "" @@ -322,3 +354,106 @@ func TestDetermineSpecificImages(t *testing.T) { }) } } + +// TestStripSparseManifestListRequiresSignatureHandling tests that when using +// StripSparseManifestList with a signed manifest list, the user must explicitly +// choose how to handle signatures via RemoveSignatures or RemoveListSignatures. +func TestStripSparseManifestListRequiresSignatureHandling(t *testing.T) { + // Load a manifest list + manifest, err := os.ReadFile(ociIndexZstdFile) + require.NoError(t, err) + + tests := []struct { + name string + options *Options + addSignature bool + expectedError string + }{ + { + name: "Valid: StripSparseManifestList with signed manifest + RemoveSignatures", + options: &Options{ + ImageListSelection: CopySpecificImages, + Instances: []digest.Digest{digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")}, + SparseManifestListAction: StripSparseManifestList, + RemoveSignatures: true, + }, + addSignature: true, + expectedError: "", + }, + { + name: "Valid: StripSparseManifestList with signed manifest + RemoveListSignatures", + options: &Options{ + ImageListSelection: CopySpecificImages, + Instances: []digest.Digest{digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")}, + SparseManifestListAction: StripSparseManifestList, + RemoveListSignatures: true, + }, + addSignature: true, + expectedError: "", + }, + { + name: "Invalid: StripSparseManifestList with signed manifest without signature handling", + options: &Options{ + ImageListSelection: CopySpecificImages, + Instances: []digest.Digest{digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")}, + SparseManifestListAction: StripSparseManifestList, + }, + addSignature: true, + expectedError: "stripping unselected instances from a signed manifest list would invalidate signatures; remove all signatures, or remove only the list signature while preserving per-instance signatures", + }, + { + name: "Valid: StripSparseManifestList with unsigned manifest (no signature handling needed)", + options: &Options{ + ImageListSelection: CopySpecificImages, + Instances: []digest.Digest{digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")}, + SparseManifestListAction: StripSparseManifestList, + }, + addSignature: false, + expectedError: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Set up source directory with the manifest + srcDir := t.TempDir() + srcManifestPath := filepath.Join(srcDir, "manifest.json") + require.NoError(t, os.WriteFile(srcManifestPath, manifest, 0644)) + + // Add a signature file if requested + if tt.addSignature { + // For directory transport, signatures are stored as "signature-1", "signature-2", etc. + // Copy an existing signature file from testdata + existingSignature, err := os.ReadFile(filepath.Join("..", "internal", "signature", "testdata", "simple.signature")) + require.NoError(t, err) + signaturePath := filepath.Join(srcDir, "signature-1") + require.NoError(t, os.WriteFile(signaturePath, existingSignature, 0644)) + } + + // Set up destination directory + destDir := t.TempDir() + + // Create source and destination references + srcRef, err := directory.NewReference(srcDir) + require.NoError(t, err) + destRef, err := directory.NewReference(destDir) + require.NoError(t, err) + + // Call the real copy.Image() function + // Note: nil PolicyContext is invalid, but we expect validation to fail before PolicyContext is used + _, err = Image(context.Background(), nil, destRef, srcRef, tt.options) + + // Verify the error matches expectations + if tt.expectedError != "" { + require.Error(t, err, "Expected validation error from copy.Image()") + assert.ErrorContains(t, err, tt.expectedError) + } else { + // Note: The copy may fail for other reasons (missing blobs, etc.) + // but should not fail with the signature handling error + if err != nil { + assert.NotContains(t, err.Error(), "invalidate signatures") + } + } + }) + } +} diff --git a/image/copy/sign.go b/image/copy/sign.go index 70295538d0..fab53252f7 100644 --- a/image/copy/sign.go +++ b/image/copy/sign.go @@ -54,10 +54,10 @@ func (c *copier) setupSigners() error { // and verifies that they can be used (to avoid copying a large image when we // can tell in advance that it would ultimately fail) func (c *copier) sourceSignatures(ctx context.Context, unparsed private.UnparsedImage, - gettingSignaturesMessage, checkingDestMessage string, + gettingSignaturesMessage, checkingDestMessage string, isManifestList bool, ) ([]internalsig.Signature, error) { var sigs []internalsig.Signature - if c.options.RemoveSignatures { + if c.options.RemoveSignatures || (c.options.RemoveListSignatures && isManifestList) { sigs = []internalsig.Signature{} } else { c.Printf("%s\n", gettingSignaturesMessage) diff --git a/image/copy/single.go b/image/copy/single.go index 26a4d57e44..9f15c77e05 100644 --- a/image/copy/single.go +++ b/image/copy/single.go @@ -116,7 +116,7 @@ func (c *copier) copySingleImage(ctx context.Context, unparsedImage *image.Unpar sigs, err := c.sourceSignatures(ctx, src, "Getting image source signatures", - "Checking if image destination supports signatures") + "Checking if image destination supports signatures", false) if err != nil { return copySingleImageResult{}, err } diff --git a/image/internal/manifest/docker_schema2_list.go b/image/internal/manifest/docker_schema2_list.go index af4cda5bdb..5854a3d121 100644 --- a/image/internal/manifest/docker_schema2_list.go +++ b/image/internal/manifest/docker_schema2_list.go @@ -132,6 +132,11 @@ func (list *Schema2ListPublic) editInstances(editInstances []ListEdit, cannotMod }, schema2PlatformSpecFromOCIPlatform(*editInstance.AddPlatform), }) + case ListOpDelete: + if editInstance.DeleteIndex < 0 || editInstance.DeleteIndex >= len(list.Manifests) { + return fmt.Errorf("Schema2List.EditInstances: invalid delete index %d (list has %d instances)", editInstance.DeleteIndex, len(list.Manifests)) + } + list.Manifests = slices.Delete(list.Manifests, editInstance.DeleteIndex, editInstance.DeleteIndex+1) default: return fmt.Errorf("internal error: invalid operation: %d", editInstance.ListOperation) } diff --git a/image/internal/manifest/docker_schema2_list_test.go b/image/internal/manifest/docker_schema2_list_test.go index e347456658..564da23f42 100644 --- a/image/internal/manifest/docker_schema2_list_test.go +++ b/image/internal/manifest/docker_schema2_list_test.go @@ -90,6 +90,28 @@ func TestSchema2ListEditInstances(t *testing.T) { digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), ), list.Instances()) + + // Create a fresh list for delete tests + list, err = ListFromBlob(validManifest, GuessMIMEType(validManifest)) + require.NoError(t, err) + originalInstances := list.Instances() + require.GreaterOrEqual(t, len(originalInstances), 1) + + // Test successful deletion of first instance + err = list.EditInstances([]ListEdit{{ + ListOperation: ListOpDelete, + DeleteIndex: 0, + }}, false) + require.NoError(t, err) + assert.Equal(t, originalInstances[1:], list.Instances()) + + // Test error on invalid index + err = list.EditInstances([]ListEdit{{ + ListOperation: ListOpDelete, + DeleteIndex: 999, + }}, false) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid delete index") } func TestSchema2ListFromManifest(t *testing.T) { diff --git a/image/internal/manifest/list.go b/image/internal/manifest/list.go index a0d535891e..aa328ee1af 100644 --- a/image/internal/manifest/list.go +++ b/image/internal/manifest/list.go @@ -82,6 +82,7 @@ const ( listOpInvalid ListOp = iota ListOpAdd ListOpUpdate + ListOpDelete ) // ListEdit includes the fields which a List's EditInstances() method will modify. @@ -105,6 +106,9 @@ type ListEdit struct { AddPlatform *imgspecv1.Platform AddAnnotations map[string]string AddCompressionAlgorithms []compression.Algorithm + + // If Op = ListOpDelete. Callers should submit operations from highest index to lowest to avoid index shifting. + DeleteIndex int } // ListPublicFromBlob parses a list of manifests. diff --git a/image/internal/manifest/oci_index.go b/image/internal/manifest/oci_index.go index 4e4060255a..2e97906e76 100644 --- a/image/internal/manifest/oci_index.go +++ b/image/internal/manifest/oci_index.go @@ -176,6 +176,11 @@ func (index *OCI1IndexPublic) editInstances(editInstances []ListEdit, cannotModi Platform: editInstance.AddPlatform, Annotations: annotations, }) + case ListOpDelete: + if editInstance.DeleteIndex < 0 || editInstance.DeleteIndex >= len(index.Manifests) { + return fmt.Errorf("OCI1Index.EditInstances: invalid delete index %d (list has %d instances)", editInstance.DeleteIndex, len(index.Manifests)) + } + index.Manifests = slices.Delete(index.Manifests, editInstance.DeleteIndex, editInstance.DeleteIndex+1) default: return fmt.Errorf("internal error: invalid operation: %d", editInstance.ListOperation) } diff --git a/image/internal/manifest/oci_index_test.go b/image/internal/manifest/oci_index_test.go index 9045f1ed4c..359339cd87 100644 --- a/image/internal/manifest/oci_index_test.go +++ b/image/internal/manifest/oci_index_test.go @@ -228,6 +228,28 @@ func TestOCI1EditInstances(t *testing.T) { assert.Equal(t, originalInstance, instance) } } + + // Create a fresh list for delete tests + list, err = ListFromBlob(validManifest, GuessMIMEType(validManifest)) + require.NoError(t, err) + originalInstances := list.Instances() + require.GreaterOrEqual(t, len(originalInstances), 1) + + // Test successful deletion of first instance + err = list.EditInstances([]ListEdit{{ + ListOperation: ListOpDelete, + DeleteIndex: 0, + }}, false) + require.NoError(t, err) + assert.Equal(t, originalInstances[1:], list.Instances()) + + // Test error on invalid index + err = list.EditInstances([]ListEdit{{ + ListOperation: ListOpDelete, + DeleteIndex: 999, + }}, false) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid delete index") } func TestOCI1IndexChooseInstanceByCompression(t *testing.T) { From 1f2d527da4d54a28320749f21a960fb21f360466 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 15:42:41 +0100 Subject: [PATCH 02/23] fixup! copy: add option to strip sparse manifest lists Remove 'orthogonal' emphasis from RemoveListSignatures comment. API consumers should assume options don't interact unless documented. Signed-off-by: Alex Guidi --- image/copy/copy.go | 1 - 1 file changed, 1 deletion(-) diff --git a/image/copy/copy.go b/image/copy/copy.go index 6970ffb5ab..b38ff5f400 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -97,7 +97,6 @@ type SparseManifestListAction int 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. - // This option is orthogonal to other copy options; if set, it removes list signatures when applicable. // If RemoveSignatures is also true, RemoveSignatures takes precedence. RemoveListSignatures bool // Signers to use to add signatures during the copy. From a4800f082ce546a5726d74cb388dbb7130742186 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 15:45:47 +0100 Subject: [PATCH 03/23] fixup! copy: add option to strip sparse manifest lists Remove redundant comment about single vs multiple image copying. The code logic with singleInstance variable is self-explanatory. Signed-off-by: Alex Guidi --- image/copy/copy.go | 1 - 1 file changed, 1 deletion(-) diff --git a/image/copy/copy.go b/image/copy/copy.go index b38ff5f400..0476f15d62 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -340,7 +340,6 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, return nil, fmt.Errorf("determining manifest MIME type for %s: %w", transports.ImageName(srcRef), err) } - // Determine if we're copying a single image or multiple images var singleInstance *image.UnparsedImage if !multiImage { // The simple case: just copy a single image. From d195aa919e5290767477fccfce792365b8614abc Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 15:51:38 +0100 Subject: [PATCH 04/23] fixup! copy: add option to strip sparse manifest lists Restore inline comment for ChooseInstanceByCompression. Clarifies that SourceCtx is used to pick matching instance. Signed-off-by: Alex Guidi --- image/copy/copy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/image/copy/copy.go b/image/copy/copy.go index 0476f15d62..4d2b5413d3 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -355,7 +355,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, if err != nil { return nil, fmt.Errorf("parsing primary manifest as list for %s: %w", transports.ImageName(srcRef), err) } - instanceDigest, err := manifestList.ChooseInstanceByCompression(c.options.SourceCtx, c.options.PreferGzipInstances) + instanceDigest, err := manifestList.ChooseInstanceByCompression(c.options.SourceCtx, c.options.PreferGzipInstances) // try to pick one that matches c.options.SourceCtx if err != nil { return nil, fmt.Errorf("choosing an image from manifest list %s: %w", transports.ImageName(srcRef), err) } From 1f261d2ae73d812d2fa820f3105bad86a675b125 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 15:53:41 +0100 Subject: [PATCH 05/23] fixup! copy: add option to strip sparse manifest lists Remove redundant 'Single-image copy path' comment. The condition and variable name are self-documenting. Signed-off-by: Alex Guidi --- image/copy/copy.go | 1 - 1 file changed, 1 deletion(-) diff --git a/image/copy/copy.go b/image/copy/copy.go index 4d2b5413d3..4b2b915be1 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -364,7 +364,6 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, } // else: a multi-instance copy if singleInstance != nil { - // Single-image copy path if len(options.EnsureCompressionVariantsExist) > 0 { return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image") } From a893f86376124ca717c131d9847f2744eb57c583 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 15:55:10 +0100 Subject: [PATCH 06/23] fixup! copy: add option to strip sparse manifest lists Remove redundant 'Multi-image copy path' comment. The else block context makes this self-evident. Signed-off-by: Alex Guidi --- image/copy/copy.go | 1 - 1 file changed, 1 deletion(-) diff --git a/image/copy/copy.go b/image/copy/copy.go index 4b2b915be1..a2780fbb8e 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -377,7 +377,6 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, } copiedManifest = single.manifest } else { - // Multi-image copy path // 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()) From b2468d91c56ab2706268f5ecdab88d0f9740bf69 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 16:07:55 +0100 Subject: [PATCH 07/23] fixup! copy: add option to strip sparse manifest lists Add validation to reject RemoveListSignatures in single instance mode. RemoveListSignatures only applies when copying manifest lists. Signed-off-by: Alex Guidi --- image/copy/copy.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/image/copy/copy.go b/image/copy/copy.go index a2780fbb8e..8250793dd5 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -367,6 +367,9 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, 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 From c46fc442e469162441acbb2aa25b3c90a086647d Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 16:11:22 +0100 Subject: [PATCH 08/23] fixup! copy: add option to strip sparse manifest lists Return -1 instead of 0 on error paths in prepareInstanceOps. Makes it more obvious if the count is accidentally used despite an error. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index d62da87631..8506f01cce 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -115,15 +115,15 @@ func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Dig // 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, 0, 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, 0, err + return res, -1, err } compressionsByPlatform, err := platformCompressionMap(list, instanceDigests) if err != nil { - return nil, 0, err + return nil, -1, err } // Determine which specific images to copy (combining digest-based and platform-based selection) @@ -131,7 +131,7 @@ func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Dig if options.ImageListSelection == CopySpecificImages { specificImages, err = determineSpecificImages(options, list) if err != nil { - return nil, 0, err + return nil, -1, err } } @@ -152,11 +152,11 @@ func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Dig } instanceDetails, err := list.Instance(instanceDigest) if err != nil { - return res, 0, 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, 0, err + return nil, -1, err } res = append(res, instanceOp{ op: instanceOpCopy, From 7ebc22fc672c47e3db20e02673b1eb2b699ec2c1 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 16:13:29 +0100 Subject: [PATCH 09/23] fixup! copy: add option to strip sparse manifest lists Remove redundant comment about delete operation. The condition and debug text are self-explanatory. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 1 - 1 file changed, 1 deletion(-) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index 8506f01cce..22a054aa83 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -138,7 +138,6 @@ func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Dig for i, instanceDigest := range instanceDigests { if options.ImageListSelection == CopySpecificImages && !specificImages.Contains(instanceDigest) { - // Create delete operation if we're stripping sparse manifests if options.SparseManifestListAction == StripSparseManifestList { logrus.Debugf("deleting instance %s from destination’s manifest (%d/%d)", instanceDigest, i+1, len(instanceDigests)) deleteOps = append(deleteOps, instanceOp{ From ff24aa0ed4616b125231f5123b12069fa877843f Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 16:37:54 +0100 Subject: [PATCH 10/23] fixup! copy: add option to strip sparse manifest lists Refactor signature validation per mtrmac's suggestion. Move validation into prepareInstanceOps where delete operations occur. Simplify error handling by removing string matching in consumer code. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 22 +++++++--------------- image/copy/multiple_test.go | 18 +++++++++--------- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index 22a054aa83..75136026d8 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -106,7 +106,7 @@ func validateCompressionVariantExists(input []OptionCompressionVariant) error { // 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) ([]instanceOp, int, error) { +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 { @@ -139,6 +139,9 @@ func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Dig if options.ImageListSelection == CopySpecificImages && !specificImages.Contains(instanceDigest) { 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, @@ -273,7 +276,7 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, // by stripping only the list-level signature while preserving per-instance signatures. // This is handled below by setting sigs to empty (similar to RemoveSignatures). if !c.options.RemoveListSignatures { - cannotModifyManifestListReason = "Would invalidate signatures" + cannotModifyManifestListReason = "Would invalidate signatures; consider removing them from the multi-platform list" } } if destIsDigestedReference { @@ -283,14 +286,6 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, cannotModifyManifestListReason = "Instructed to preserve digests" } - // Validate that when using StripSparseManifestList with signed images, the user explicitly - // acknowledges signature handling via either RemoveListSignatures or RemoveSignatures. - // This is consistent with requiring explicit acknowledgment for other manifest edits. - if c.options.SparseManifestListAction == StripSparseManifestList && len(sigs) > 0 && - !c.options.RemoveSignatures && !c.options.RemoveListSignatures { - return nil, fmt.Errorf("stripping unselected instances from a signed manifest list would invalidate signatures; remove all signatures, or remove only the list signature while preserving per-instance signatures") - } - // Determine if we'll need to convert the manifest list to a different format. forceListMIMEType := c.options.ForceManifestMIMEType switch forceListMIMEType { @@ -306,17 +301,14 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, } if selectedListType != originalList.MIMEType() { if cannotModifyManifestListReason != "" { - if cannotModifyManifestListReason == "Would invalidate signatures" { - return nil, fmt.Errorf("Manifest list must be converted to type %q to be written to destination, but we cannot modify it: %q; consider removing only the list signature to allow modification while preserving per-instance signatures", selectedListType, 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{} - instanceOpList, copyLen, err := prepareInstanceOps(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) } diff --git a/image/copy/multiple_test.go b/image/copy/multiple_test.go index c57c412096..5e0b8af281 100644 --- a/image/copy/multiple_test.go +++ b/image/copy/multiple_test.go @@ -35,7 +35,7 @@ func TestPrepareInstanceOpsForInstanceCopy(t *testing.T) { digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), } - instancesToCopy, _, err := prepareInstanceOps(list, sourceInstances, &Options{}) + instancesToCopy, _, err := prepareInstanceOps(list, sourceInstances, &Options{}, "") require.NoError(t, err) compare := []instanceOp{} @@ -48,7 +48,7 @@ func TestPrepareInstanceOpsForInstanceCopy(t *testing.T) { assert.Equal(t, instancesToCopy, compare) // Test CopySpecificImages where selected instance is sourceInstances[1] - instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages}) + instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages}, "") require.NoError(t, err) compare = []instanceOp{{ op: instanceOpCopy, @@ -61,7 +61,7 @@ func TestPrepareInstanceOpsForInstanceCopy(t *testing.T) { Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, SparseManifestListAction: StripSparseManifestList, - }) + }, "") require.NoError(t, err) // Should have 1 copy operation followed by 2 delete operations (for indices 0 and 2) expected := []instanceOp{ @@ -80,7 +80,7 @@ func TestPrepareInstanceOpsForInstanceCopy(t *testing.T) { } assert.Equal(t, expected, instancesToCopy) - _, _, err = prepareInstanceOps(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, ForceCompressionFormat: true}) + _, _, err = prepareInstanceOps(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, ForceCompressionFormat: true}, "") require.EqualError(t, err, "cannot use ForceCompressionFormat with undefined default compression format") } @@ -105,11 +105,11 @@ func TestPrepareInstanceOpsForInstanceClone(t *testing.T) { EnsureCompressionVariantsExist: ensureCompressionVariantsExist, Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, - }) + }, "") require.EqualError(t, err, "EnsureCompressionVariantsExist is not implemented for CopySpecificImages") // Test copying all images with replication - instancesToCopy, _, err := prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}) + instancesToCopy, _, err := prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}, "") require.NoError(t, err) // Following test ensures @@ -137,7 +137,7 @@ func TestPrepareInstanceOpsForInstanceClone(t *testing.T) { // Test option with multiple copy request for same compression format. // The above expectation should stay the same, if ensureCompressionVariantsExist requests zstd twice. ensureCompressionVariantsExist = []OptionCompressionVariant{{Algorithm: compression.Zstd}, {Algorithm: compression.Zstd}} - instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}) + instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}, "") require.NoError(t, err) expectedResponse = []simplerInstanceCopy{} for _, instance := range sourceInstances { @@ -159,7 +159,7 @@ func TestPrepareInstanceOpsForInstanceClone(t *testing.T) { digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), } - instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}) + instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}, "") require.NoError(t, err) // two copies but clone should happen only once numberOfCopyClone := 0 @@ -399,7 +399,7 @@ func TestStripSparseManifestListRequiresSignatureHandling(t *testing.T) { SparseManifestListAction: StripSparseManifestList, }, addSignature: true, - expectedError: "stripping unselected instances from a signed manifest list would invalidate signatures; remove all signatures, or remove only the list signature while preserving per-instance signatures", + expectedError: "we should delete instance", }, { name: "Valid: StripSparseManifestList with unsigned manifest (no signature handling needed)", From aa356852353e04ebb119b3276b14534df6391991 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 16:46:21 +0100 Subject: [PATCH 11/23] fixup! copy: add option to strip sparse manifest lists Add error context for single image copy. Helps users understand when copying single instance from manifest list. Signed-off-by: Alex Guidi --- image/copy/copy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/image/copy/copy.go b/image/copy/copy.go index 8250793dd5..6458d18020 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -376,7 +376,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, } single, err := c.copySingleImage(ctx, singleInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: requireCompressionFormatMatch}) if err != nil { - return nil, err + return nil, fmt.Errorf("copying single image: %w", err) } copiedManifest = single.manifest } else { From fb810d9bc18bf122faea28a3d388c784ab6d57cf Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Thu, 5 Mar 2026 16:50:01 +0100 Subject: [PATCH 12/23] fixup! copy: add option to strip sparse manifest lists Remove unused ociManifestFile constant from tests. Signed-off-by: Alex Guidi --- image/copy/multiple_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/image/copy/multiple_test.go b/image/copy/multiple_test.go index 5e0b8af281..8c7978d360 100644 --- a/image/copy/multiple_test.go +++ b/image/copy/multiple_test.go @@ -16,8 +16,6 @@ import ( ) const ( - // Test manifest file paths - ociManifestFile = "../internal/manifest/testdata/ociv1.manifest.json" ociIndexZstdFile = "../internal/manifest/testdata/oci1.index.zstd-selection.json" ) From d6b69fd83a854811387522c41be942ea02f8ba4b Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Fri, 6 Mar 2026 12:55:00 +0100 Subject: [PATCH 13/23] fixup! copy: add option to strip sparse manifest lists Fix gofumpt formatting by using explicit octal prefix (0o644 instead of 0644) for file permissions in os.WriteFile calls Signed-off-by: Alex Guidi --- image/copy/multiple_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/image/copy/multiple_test.go b/image/copy/multiple_test.go index 8c7978d360..078fcfa3bc 100644 --- a/image/copy/multiple_test.go +++ b/image/copy/multiple_test.go @@ -416,7 +416,7 @@ func TestStripSparseManifestListRequiresSignatureHandling(t *testing.T) { // Set up source directory with the manifest srcDir := t.TempDir() srcManifestPath := filepath.Join(srcDir, "manifest.json") - require.NoError(t, os.WriteFile(srcManifestPath, manifest, 0644)) + require.NoError(t, os.WriteFile(srcManifestPath, manifest, 0o644)) // Add a signature file if requested if tt.addSignature { @@ -425,7 +425,7 @@ func TestStripSparseManifestListRequiresSignatureHandling(t *testing.T) { existingSignature, err := os.ReadFile(filepath.Join("..", "internal", "signature", "testdata", "simple.signature")) require.NoError(t, err) signaturePath := filepath.Join(srcDir, "signature-1") - require.NoError(t, os.WriteFile(signaturePath, existingSignature, 0644)) + require.NoError(t, os.WriteFile(signaturePath, existingSignature, 0o644)) } // Set up destination directory From 61fe95f074b2ba33a5d7bf2ef2c25e569436b0cf Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Tue, 10 Mar 2026 16:30:08 +0100 Subject: [PATCH 14/23] fixup! copy: remove outdated comment referencing removed code The comment stated 'This is handled below by setting sigs to empty' but the code that set sigs to empty has been refactored away. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 1 - 1 file changed, 1 deletion(-) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index 75136026d8..eceefb3aae 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -274,7 +274,6 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, if len(sigs) > 0 { // If RemoveListSignatures is set, we allow modification of the manifest list // by stripping only the list-level signature while preserving per-instance signatures. - // This is handled below by setting sigs to empty (similar to RemoveSignatures). if !c.options.RemoveListSignatures { cannotModifyManifestListReason = "Would invalidate signatures; consider removing them from the multi-platform list" } From a3791c7b1a0abd2a8c8624bff3383f00f1df87f2 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Tue, 10 Mar 2026 16:33:49 +0100 Subject: [PATCH 15/23] fixup! copy: remove dead code in signature validation The condition checking for exact string 'Would invalidate signatures' is unreachable because the actual value set is 'Would invalidate signatures; consider removing them from the multi-platform list'. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index eceefb3aae..caa739b4f2 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -404,9 +404,6 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, // If we can't just use the original value, but we have to change it, flag an error. if !bytes.Equal(attemptedManifestList, originalManifestList) { if cannotModifyManifestListReason != "" { - if cannotModifyManifestListReason == "Would invalidate signatures" { - return nil, fmt.Errorf("Manifest list was edited, but we cannot modify it: %q; consider removing only the list signature to allow modification while preserving per-instance signatures", cannotModifyManifestListReason) - } return nil, fmt.Errorf("Manifest list was edited, but we cannot modify it: %q", cannotModifyManifestListReason) } logrus.Debugf("Manifest list has been updated") From 639bc5aa15ee3f29de6830f236f1399ebd6c6a5e Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Tue, 10 Mar 2026 16:38:54 +0100 Subject: [PATCH 16/23] fixup! copy: validate against empty manifest list Fail if we would delete all instances from a manifest list without copying any, as this would result in an invalid empty manifest list. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index caa739b4f2..1ff91d2e59 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -186,6 +186,12 @@ func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Dig // 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 + + // Validate that we're not deleting all entries without copying any + if copyLen == 0 && len(deleteOps) == len(instanceDigests) { + return nil, -1, fmt.Errorf("cannot delete all instances from manifest list without copying any") + } + res = append(res, deleteOps...) return res, copyLen, nil From d2c8d89e8543fdf7c0afd0a942feb3170e14ee90 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Tue, 10 Mar 2026 16:45:19 +0100 Subject: [PATCH 17/23] fixup! copy: improve error context for system image from manifest list When copying a system-specific image selected from a manifest list, the error message now says 'copying system image from manifest list' to help users understand what operation failed. This is especially important since Skopeo defaults to CopySystemImage. Signed-off-by: Alex Guidi --- image/copy/copy.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/image/copy/copy.go b/image/copy/copy.go index 6458d18020..8291e6d7c7 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -376,6 +376,9 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, } single, err := c.copySingleImage(ctx, singleInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: requireCompressionFormatMatch}) if err != nil { + if multiImage && c.options.ImageListSelection == CopySystemImage { + return nil, fmt.Errorf("copying system image from manifest list: %w", err) + } return nil, fmt.Errorf("copying single image: %w", err) } copiedManifest = single.manifest From 1094da3aff22f309a35df0873ba93247991b311d Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Fri, 13 Mar 2026 11:53:37 +0100 Subject: [PATCH 18/23] fixup! copy: simplify signature validation in manifest list modification Remove nested condition checking RemoveListSignatures. The outer check len(sigs) > 0 is sufficient since sourceSignatures filters out signatures when RemoveListSignatures is set. This simplification makes the logic clearer and ensures we fail loudly if something breaks in the future. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index 1ff91d2e59..1c4e2f4af3 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -278,11 +278,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 { - // If RemoveListSignatures is set, we allow modification of the manifest list - // by stripping only the list-level signature while preserving per-instance signatures. - if !c.options.RemoveListSignatures { - cannotModifyManifestListReason = "Would invalidate signatures; consider removing them from the multi-platform list" - } + cannotModifyManifestListReason = "Would invalidate signatures; consider removing them from the multi-platform list" } if destIsDigestedReference { cannotModifyManifestListReason = "Destination specifies a digest" From 3d709c01ab1c4433788692a8d93b14ad8ee7b905 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Fri, 13 Mar 2026 12:00:12 +0100 Subject: [PATCH 19/23] fixup! copy: remove redundant comment in compression list handling The comment explaining why we add to compressionList is unnecessary as the code is self-explanatory in context. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 1 - 1 file changed, 1 deletion(-) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index 1c4e2f4af3..8851e27a7b 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -177,7 +177,6 @@ func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Dig 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()) } } From 5d05c0de0dbf647828c29e999c5cd60538c8ec97 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Fri, 13 Mar 2026 12:05:36 +0100 Subject: [PATCH 20/23] fixup! copy: improve error message for empty manifest list validation Use a more user-friendly error message that explains the operation filtered out all platforms rather than the technical detail about deletion. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index 8851e27a7b..e7a57106ed 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -188,7 +188,7 @@ func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Dig // Validate that we're not deleting all entries without copying any if copyLen == 0 && len(deleteOps) == len(instanceDigests) { - return nil, -1, fmt.Errorf("cannot delete all instances from manifest list without copying any") + return nil, -1, fmt.Errorf("requested operation filtered out all platforms and would create an empty image") } res = append(res, deleteOps...) From 50bbedd815bd4ab20779d7831fd4404df3e4744a Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Fri, 13 Mar 2026 12:13:33 +0100 Subject: [PATCH 21/23] fixup! copy: improve error context for system image from manifest list Set error wrapping string when determining single instance to keep it in sync. Remove unhelpful 'copying single image' wrapper and return the underlying error directly for plain single image copies. Signed-off-by: Alex Guidi --- image/copy/copy.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/image/copy/copy.go b/image/copy/copy.go index 8291e6d7c7..577473718b 100644 --- a/image/copy/copy.go +++ b/image/copy/copy.go @@ -341,9 +341,11 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, } var singleInstance *image.UnparsedImage + var singleInstanceErrorWrapping string if !multiImage { // The simple case: just copy a single image. singleInstance = c.unparsedToplevel + singleInstanceErrorWrapping = "" } else if c.options.ImageListSelection == CopySystemImage { // 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. @@ -361,6 +363,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, } logrus.Debugf("Source is a manifest list; copying (only) instance %s for current system", instanceDigest) singleInstance = image.UnparsedInstance(rawSource, &instanceDigest) + singleInstanceErrorWrapping = "copying system image from manifest list" } // else: a multi-instance copy if singleInstance != nil { @@ -376,10 +379,10 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, } single, err := c.copySingleImage(ctx, singleInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: requireCompressionFormatMatch}) if err != nil { - if multiImage && c.options.ImageListSelection == CopySystemImage { - return nil, fmt.Errorf("copying system image from manifest list: %w", err) + if singleInstanceErrorWrapping != "" { + return nil, fmt.Errorf("%s: %w", singleInstanceErrorWrapping, err) } - return nil, fmt.Errorf("copying single image: %w", err) + return nil, err } copiedManifest = single.manifest } else { From 5b1910c151c3cab3a1cfc090c15ae3f216cce512 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Fri, 13 Mar 2026 12:47:59 +0100 Subject: [PATCH 22/23] fixup! copy: add assertions for copy count in prepareInstanceOps tests. Signed-off-by: Alex Guidi --- image/copy/multiple_test.go | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/image/copy/multiple_test.go b/image/copy/multiple_test.go index 078fcfa3bc..3856d2c939 100644 --- a/image/copy/multiple_test.go +++ b/image/copy/multiple_test.go @@ -33,8 +33,9 @@ func TestPrepareInstanceOpsForInstanceCopy(t *testing.T) { digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), } - instancesToCopy, _, err := prepareInstanceOps(list, sourceInstances, &Options{}, "") + instancesToCopy, copyCount, err := prepareInstanceOps(list, sourceInstances, &Options{}, "") require.NoError(t, err) + assert.Equal(t, len(sourceInstances), copyCount) compare := []instanceOp{} for _, instance := range sourceInstances { @@ -46,16 +47,17 @@ func TestPrepareInstanceOpsForInstanceCopy(t *testing.T) { assert.Equal(t, instancesToCopy, compare) // Test CopySpecificImages where selected instance is sourceInstances[1] - instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages}, "") + instancesToCopy, copyCount, err = prepareInstanceOps(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages}, "") require.NoError(t, err) compare = []instanceOp{{ op: instanceOpCopy, sourceDigest: sourceInstances[1], }} + assert.Equal(t, len(compare), copyCount) assert.Equal(t, instancesToCopy, compare) // Test CopySpecificImages with StripSparseManifestList where selected instance is sourceInstances[1] - instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{ + instancesToCopy, copyCount, err = prepareInstanceOps(list, sourceInstances, &Options{ Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, SparseManifestListAction: StripSparseManifestList, @@ -76,6 +78,19 @@ func TestPrepareInstanceOpsForInstanceCopy(t *testing.T) { deleteIndex: 0, }, } + // Count copy/clone operations and delete operations separately + expectedCopyCount := 0 + expectedDeleteCount := 0 + for _, op := range expected { + switch op.op { + case instanceOpCopy, instanceOpClone: + expectedCopyCount++ + case instanceOpDelete: + expectedDeleteCount++ + } + } + assert.Equal(t, expectedCopyCount, copyCount) + assert.Equal(t, expectedDeleteCount, len(instancesToCopy)-copyCount) assert.Equal(t, expected, instancesToCopy) _, _, err = prepareInstanceOps(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, ForceCompressionFormat: true}, "") @@ -107,7 +122,7 @@ func TestPrepareInstanceOpsForInstanceClone(t *testing.T) { require.EqualError(t, err, "EnsureCompressionVariantsExist is not implemented for CopySpecificImages") // Test copying all images with replication - instancesToCopy, _, err := prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}, "") + instancesToCopy, copyCount, err := prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}, "") require.NoError(t, err) // Following test ensures @@ -130,12 +145,13 @@ func TestPrepareInstanceOpsForInstanceClone(t *testing.T) { } } actualResponse := convertInstanceCopyToSimplerInstanceCopy(instancesToCopy) + assert.Equal(t, len(expectedResponse), copyCount) assert.Equal(t, expectedResponse, actualResponse) // Test option with multiple copy request for same compression format. // The above expectation should stay the same, if ensureCompressionVariantsExist requests zstd twice. ensureCompressionVariantsExist = []OptionCompressionVariant{{Algorithm: compression.Zstd}, {Algorithm: compression.Zstd}} - instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}, "") + instancesToCopy, copyCount, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}, "") require.NoError(t, err) expectedResponse = []simplerInstanceCopy{} for _, instance := range sourceInstances { @@ -149,6 +165,7 @@ func TestPrepareInstanceOpsForInstanceClone(t *testing.T) { } } actualResponse = convertInstanceCopyToSimplerInstanceCopy(instancesToCopy) + assert.Equal(t, len(expectedResponse), copyCount) assert.Equal(t, expectedResponse, actualResponse) // Add same instance twice but clone must appear only once. @@ -157,7 +174,7 @@ func TestPrepareInstanceOpsForInstanceClone(t *testing.T) { digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), } - instancesToCopy, _, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}, "") + instancesToCopy, copyCount, err = prepareInstanceOps(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist}, "") require.NoError(t, err) // two copies but clone should happen only once numberOfCopyClone := 0 @@ -167,6 +184,7 @@ func TestPrepareInstanceOpsForInstanceClone(t *testing.T) { } } assert.Equal(t, 1, numberOfCopyClone) + assert.Equal(t, len(sourceInstances)+numberOfCopyClone, copyCount) } // simpler version of `instanceOp` for testing where fields are string From 6d9fd3147331e33ad46db05a6ae98ae72ceab8c8 Mon Sep 17 00:00:00 2001 From: Alex Guidi Date: Fri, 13 Mar 2026 14:52:29 +0100 Subject: [PATCH 23/23] fixup! copy: remove redundant comment about validation The comment is redundant as the code and error message are self-explanatory. Signed-off-by: Alex Guidi --- image/copy/multiple.go | 1 - 1 file changed, 1 deletion(-) diff --git a/image/copy/multiple.go b/image/copy/multiple.go index e7a57106ed..e49cfd8c2a 100644 --- a/image/copy/multiple.go +++ b/image/copy/multiple.go @@ -186,7 +186,6 @@ func prepareInstanceOps(list internalManifest.List, instanceDigests []digest.Dig slices.Reverse(deleteOps) copyLen := len(res) // Count copy/clone operations before appending deletes - // Validate that we're not deleting all entries without copying any if copyLen == 0 && len(deleteOps) == len(instanceDigests) { return nil, -1, fmt.Errorf("requested operation filtered out all platforms and would create an empty image") }