From cd47ec0f05795065c600e48246df8574ccc474c7 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 `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 --- copy/multiple.go | 66 +++++++++++++++++++++++++++---------------- copy/multiple_test.go | 36 +++++++++++++++++++++++ 2 files changed, 77 insertions(+), 25 deletions(-) create mode 100644 copy/multiple_test.go diff --git a/copy/multiple.go b/copy/multiple.go index 524a716732..d737a1f08a 100644 --- 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 new file mode 100644 index 0000000000..8ae968b2db --- /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 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]) +}