From bd6a6aedcae822c35c09b7115b7e017717bd8083 Mon Sep 17 00:00:00 2001 From: Aditya R Date: Sun, 4 Jun 2023 14:07:02 +0530 Subject: [PATCH] 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 --- copy/multiple.go | 79 +++++++++++++++++++++++++++++-------------- copy/multiple_test.go | 36 ++++++++++++++++++++ 2 files changed, 89 insertions(+), 26 deletions(-) create mode 100644 copy/multiple_test.go diff --git a/copy/multiple.go b/copy/multiple.go index 524a716732..41ea1b11b0 100644 --- 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 new file mode 100644 index 0000000000..99b74f2371 --- /dev/null +++ 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]}}) +}