copy: split selection of images to be copied

Following PR splits `copyMultipleImages` into a new function called `prepareCopyInstances` which returns a list of `instanceCopy`. `prepareCopyInstances` does that in these two steps * How to copy via instanceCopyKind * 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 5, 2023 · 8ec2734 · 8ec2734
1 parent abe5133
commit 8ec2734
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 25 deletions.
diff --git a/copy/multiple.go b/copy/multiple.go
@@ -12,11 +12,42 @@ 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"
 )
 
+type instanceCopyKind int
+
+const (
+	InstanceCopyCopy instanceCopyKind = iota
+	InstanceCopyClone
+)
+
+type instanceCopy struct {
+	op           instanceCopyKind
+	sourceDigest digest.Digest
+}
+
+// prepareCopyInstances prepares a list of instances which needs to copied to the manifest list.
+func prepareCopyInstances(instanceDigests []digest.Digest, options *Options) []instanceCopy {
+	instanceCopyList := []instanceCopy{}
+	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.
+		instanceCopyList = append(instanceCopyList, instanceCopy{
+			op:           InstanceCopyCopy,
+			sourceDigest: instanceDigest,
+		})
+	}
+	return instanceCopyList
+}
+
 // 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 +119,31 @@ 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)
-	}
-	c.Printf("Copying %d of %d images in list\n", imagesToCopy, len(instanceDigests))
 	instanceEdits := []internalManifest.ListEdit{}
+	instanceCopyList := prepareCopyInstances(instanceDigests, options)
+	imagesToCopy := len(instanceEdits)
+	c.Printf("Copying %d of %d images in list\n", imagesToCopy, len(instanceDigests))
 	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 instanceCopyList {
+		// Update instances to be edited by their `ListOperation` and
+		// populate necessary fields.
+		if instance.op == InstanceCopyCopy {
+			logrus.Debugf("Copying instance %s (%d/%d)", instance.sourceDigest, i+1, len(instanceDigests))
+			c.Printf("Copying image %s (%d/%d)\n", instance.sourceDigest, instancesCopied+1, imagesToCopy)
+			unparsedInstance := image.UnparsedInstance(c.rawSource, &instanceCopyList[i].sourceDigest)
+			updatedManifest, updatedManifestType, updatedManifestDigest, err := c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedInstance, &instanceCopyList[i].sourceDigest)
+			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.
+			instanceEdits = append(instanceEdits, internalManifest.ListEdit{
+				ListOperation:   internalManifest.ListOpUpdate,
+				UpdateOldDigest: instance.sourceDigest,
+				UpdateDigest:    updatedManifestDigest,
+				UpdateSize:      int64(len(updatedManifest)),
+				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 TestPrepareCopyInstances(t *testing.T) {
+	// Test CopyAllImages
+	options := Options{}
+	instances := []digest.Digest{
+		digest.Digest("sha256:5b0bcabd1ed22e9fb1310cf6c2dec7cdef19f0ad69efa1f392e94a4333501270"),
+		digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+		digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"),
+	}
+
+	instancesToCopy := prepareCopyInstances(instances, &options)
+	assert.Equal(t, len(instancesToCopy), len(instances))
+	assert.Equal(t, instancesToCopy[0].sourceDigest, instances[0])
+	assert.Equal(t, instancesToCopy[1].sourceDigest, instances[1])
+	assert.Equal(t, instancesToCopy[2].sourceDigest, instances[2])
+
+	// Test with CopySpecific Images
+	copyOnly := []digest.Digest{
+		digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+	}
+	options = Options{
+		Instances:          copyOnly,
+		ImageListSelection: CopySpecificImages,
+	}
+	instancesToCopy = prepareCopyInstances(instances, &options)
+	assert.Equal(t, len(instancesToCopy), len(copyOnly))
+	assert.Equal(t, instancesToCopy[0].sourceDigest, copyOnly[0])
+}