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 6, 2023 · bd6a6ae · bd6a6ae
1 parent abe5133
commit bd6a6ae
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 26 deletions.
diff --git a/copy/multiple.go b/copy/multiple.go
@@ -12,11 +12,41 @@ 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
+}
+
+// prepareInstanceCopies prepares a list of instances which needs to copied to the manifest list.
+func prepareInstanceCopies(instanceDigests []digest.Digest, options *Options) []instanceCopy {
+	res := []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
+		}
+		res = append(res, instanceCopy{
+			op:           instanceCopyCopy,
+			sourceDigest: instanceDigest,
+		})
+	}
+	return res
+}
+
 // 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 +118,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{}
-	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)
+	instanceCopyList := prepareInstanceCopies(instanceDigests, options)
+	c.Printf("Copying %d of %d images in list\n", len(instanceCopyList), len(instanceDigests))
+	for i, instance := range instanceCopyList {
+		// 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)
+			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", i+1, len(instanceCopyList), err)
+			}
+			// 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})
+		default:
+			return nil, fmt.Errorf("copying image: invalid copy operation %d", instance.op)
 		}
-		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
+	sourceInstances := []digest.Digest{
+		digest.Digest("sha256:5b0bcabd1ed22e9fb1310cf6c2dec7cdef19f0ad69efa1f392e94a4333501270"),
+		digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+		digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"),
+	}
+
+	instancesToCopy := prepareInstanceCopies(sourceInstances, &Options{})
+	compare := []instanceCopy{}
+	for _, instance := range sourceInstances {
+		compare = append(compare, instanceCopy{op: instanceCopyCopy,
+			sourceDigest: instance})
+	}
+	assert.Equal(t, instancesToCopy, compare)
+
+	// Test with CopySpecific Images
+	copyOnly := []digest.Digest{
+		digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+	}
+	instancesToCopy = prepareInstanceCopies(sourceInstances, &Options{
+		Instances:          copyOnly,
+		ImageListSelection: CopySpecificImages})
+	assert.Equal(t, instancesToCopy, []instanceCopy{{
+		op:           instanceCopyCopy,
+		sourceDigest: copyOnly[0]}})
+}