copy: split selection of images to be copied

Following PR splits `copyMultipleImages` into a new function called `prepareEditInstances` which returns a list of `ListEdit`. `prepareEditInstances` does that in these two steps * How to copy via ListOperation * What to copy by the content of the returned list. See newly added unit test to see how it is used. Signed-off-by: Aditya R <[email protected]>
containers · Jun 4, 2023 · cd47ec0 · cd47ec0
1 parent f5f7ac8
commit cd47ec0
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 25 deletions.
diff --git a/copy/multiple.go b/copy/multiple.go
@@ -12,11 +12,35 @@ import (
 	internalManifest "github.com/containers/image/v5/internal/manifest"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/signature"
+	digest "github.com/opencontainers/go-digest"
 	imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/exp/slices"
 )
 
+// prepareEditList prepares a list of instances which needs to edited without setting their content,
+// reason is to be able to test `what and how to add/edit` in the manifest list
+func prepareEditList(instanceDigests []digest.Digest, options *Options) ([]internalManifest.ListEdit, int) {
+	imagesToCopy := len(instanceDigests)
+	if options.ImageListSelection == CopySpecificImages {
+		imagesToCopy = len(options.Instances)
+	}
+	instanceEdits := []internalManifest.ListEdit{}
+	for i, instanceDigest := range instanceDigests {
+		if options.ImageListSelection == CopySpecificImages &&
+			!slices.Contains(options.Instances, instanceDigest) {
+			logrus.Debugf("Skipping instance %s (%d/%d)", instanceDigest, i+1, len(instanceDigests))
+			continue
+		}
+		// Record the result of a possible conversion here.
+		instanceEdits = append(instanceEdits, internalManifest.ListEdit{
+			ListOperation:   internalManifest.ListOpUpdate,
+			UpdateOldDigest: instanceDigest,
+		})
+	}
+	return instanceEdits, imagesToCopy
+}
+
 // copyMultipleImages copies some or all of an image list's instances, using
 // policyContext to validate source image admissibility.
 func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signature.PolicyContext, options *Options, unparsedToplevel *image.UnparsedImage) (copiedManifest []byte, retErr error) {
@@ -88,34 +112,26 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
 
 	// Copy each image, or just the ones we want to copy, in turn.
 	instanceDigests := updatedList.Instances()
-	imagesToCopy := len(instanceDigests)
-	if options.ImageListSelection == CopySpecificImages {
-		imagesToCopy = len(options.Instances)
-	}
+	instanceEdits, imagesToCopy := prepareEditList(instanceDigests, options)
 	c.Printf("Copying %d of %d images in list\n", imagesToCopy, len(instanceDigests))
-	instanceEdits := []internalManifest.ListEdit{}
 	instancesCopied := 0
-	for i, instanceDigest := range instanceDigests {
-		if options.ImageListSelection == CopySpecificImages &&
-			!slices.Contains(options.Instances, instanceDigest) {
-			logrus.Debugf("Skipping instance %s (%d/%d)", instanceDigest, i+1, len(instanceDigests))
-			continue
-		}
-		logrus.Debugf("Copying instance %s (%d/%d)", instanceDigest, i+1, len(instanceDigests))
-		c.Printf("Copying image %s (%d/%d)\n", instanceDigest, instancesCopied+1, imagesToCopy)
-		unparsedInstance := image.UnparsedInstance(c.rawSource, &instanceDigest)
-		updatedManifest, updatedManifestType, updatedManifestDigest, err := c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedInstance, &instanceDigest)
-		if err != nil {
-			return nil, fmt.Errorf("copying image %d/%d from manifest list: %w", instancesCopied+1, imagesToCopy, err)
+	for i, instance := range instanceEdits {
+		// Update instances to be edited by their `ListOperation` and
+		// populate necessary fields.
+		if instance.ListOperation == internalManifest.ListOpUpdate {
+			logrus.Debugf("Copying instance %s (%d/%d)", instance.UpdateOldDigest, i+1, len(instanceDigests))
+			c.Printf("Copying image %s (%d/%d)\n", instance.UpdateOldDigest, instancesCopied+1, imagesToCopy)
+			unparsedInstance := image.UnparsedInstance(c.rawSource, &instance.UpdateOldDigest)
+			updatedManifest, updatedManifestType, updatedManifestDigest, err := c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedInstance, &instance.UpdateOldDigest)
+			if err != nil {
+				return nil, fmt.Errorf("copying image %d/%d from manifest list: %w", instancesCopied+1, imagesToCopy, err)
+			}
+			instancesCopied++
+			// Record the result of a possible conversion here.
+			instance.UpdateDigest = updatedManifestDigest
+			instance.UpdateSize = int64(len(updatedManifest))
+			instance.UpdateMediaType = updatedManifestType
 		}
-		instancesCopied++
-		// Record the result of a possible conversion here.
-		instanceEdits = append(instanceEdits, internalManifest.ListEdit{
-			ListOperation:   internalManifest.ListOpUpdate,
-			UpdateOldDigest: instanceDigest,
-			UpdateDigest:    updatedManifestDigest,
-			UpdateSize:      int64(len(updatedManifest)),
-			UpdateMediaType: updatedManifestType})
 	}
 
 	// Now reset the digest/size/types of the manifests in the list to account for any conversions that we made.

diff --git a/copy/multiple_test.go b/copy/multiple_test.go
@@ -0,0 +1,36 @@
+package copy
+
+import (
+	"testing"
+
+	digest "github.com/opencontainers/go-digest"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPrepareEditList(t *testing.T) {
+	// Test CopyAllImages
+	options := Options{}
+	instances := []digest.Digest{
+		digest.Digest("sha256:5b0bcabd1ed22e9fb1310cf6c2dec7cdef19f0ad69efa1f392e94a4333501270"),
+		digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+		digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"),
+	}
+
+	instancesToEdit, imagesToCopy := prepareEditList(instances, &options)
+	assert.Equal(t, imagesToCopy, len(instances))
+	assert.Equal(t, instancesToEdit[0].UpdateOldDigest, instances[0])
+	assert.Equal(t, instancesToEdit[1].UpdateOldDigest, instances[1])
+	assert.Equal(t, instancesToEdit[2].UpdateOldDigest, instances[2])
+
+	// Test with CopySpecific Images
+	copyOnly := []digest.Digest{
+		digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+	}
+	options = Options{
+		Instances:          copyOnly,
+		ImageListSelection: CopySpecificImages,
+	}
+	instancesToEdit, imagesToCopy = prepareEditList(instances, &options)
+	assert.Equal(t, imagesToCopy, len(copyOnly))
+	assert.Equal(t, instancesToEdit[0].UpdateOldDigest, copyOnly[0])
+}