From f406c3689332f8995f3a29137b5b88fedbe45d7d Mon Sep 17 00:00:00 2001 From: Aditya R Date: Wed, 2 Aug 2023 07:11:45 +0530 Subject: [PATCH] copy: add support for ForceCompressionFormat ForceCompressionFormat allows end users to force selected compression format (set in DestinationCtx.CompressionFormat) which ensures that while copying blobs of other compression algorithms are not reused. Following flag is a frontend wrapper for: /~https://github.com/containers/image/pull/2023 Will help in: * /~https://github.com/containers/buildah/issues/4613 * /~https://github.com/containers/podman/issues/18660 Signed-off-by: Aditya R --- copy/copy.go | 24 ++++++++++++++++++++++-- copy/multiple.go | 15 ++++++++++++--- copy/multiple_test.go | 5 ++++- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index ac0e6f2fa2..11b2dbfd52 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -133,6 +133,10 @@ type Options struct { // Invalid when copying a non-multi-architecture image. That will probably // change in the future. EnsureCompressionVariantsExist []OptionCompressionVariant + // ForceCompressionFormat ensures that the compression algorithm set in + // DestinationCtx.CompressionFormat is used exclusively, and blobs of other + // compression algorithms are not reused. + ForceCompressionFormat bool } // OptionCompressionVariant allows to supply information about @@ -163,6 +167,14 @@ type copier struct { signersToClose []*signer.Signer // Signers that should be closed when this copier is destroyed. } +// Internal function to validate `requireCompressionFormatMatch` for copySingleImageOptions +func shouldRequireCompressionFormatMatch(options *Options) (bool, error) { + if options.ForceCompressionFormat && (options.DestinationCtx == nil || options.DestinationCtx.CompressionFormat == nil) { + return false, fmt.Errorf("cannot use ForceCompressionFormat with undefined default compression format") + } + return options.ForceCompressionFormat, nil +} + // Image copies image from srcRef to destRef, using policyContext to validate // source image admissibility. It returns the manifest which was written to // the new copy of the image. @@ -269,8 +281,12 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, if len(options.EnsureCompressionVariantsExist) > 0 { return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image") } + requireCompressionFormatMatch, err := shouldRequireCompressionFormatMatch(options) + if err != nil { + return nil, err + } // The simple case: just copy a single image. - single, err := c.copySingleImage(ctx, c.unparsedToplevel, nil, copySingleImageOptions{requireCompressionFormatMatch: false}) + single, err := c.copySingleImage(ctx, c.unparsedToplevel, nil, copySingleImageOptions{requireCompressionFormatMatch: requireCompressionFormatMatch}) if err != nil { return nil, err } @@ -279,6 +295,10 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, if len(options.EnsureCompressionVariantsExist) > 0 { return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image") } + requireCompressionFormatMatch, err := shouldRequireCompressionFormatMatch(options) + if err != nil { + return nil, err + } // This is a manifest list, and we weren't asked to copy multiple images. Choose a single image that // matches the current system to copy, and copy it. mfest, manifestType, err := c.unparsedToplevel.Manifest(ctx) @@ -295,7 +315,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, } logrus.Debugf("Source is a manifest list; copying (only) instance %s for current system", instanceDigest) unparsedInstance := image.UnparsedInstance(rawSource, &instanceDigest) - single, err := c.copySingleImage(ctx, unparsedInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: false}) + single, err := c.copySingleImage(ctx, unparsedInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: requireCompressionFormatMatch}) if err != nil { return nil, fmt.Errorf("copying system image from manifest list: %w", err) } diff --git a/copy/multiple.go b/copy/multiple.go index 34f2129d69..30f6da2511 100644 --- a/copy/multiple.go +++ b/copy/multiple.go @@ -32,6 +32,10 @@ type instanceCopy struct { op instanceCopyKind sourceDigest digest.Digest + // Fields which can be used by callers when operation + // is `instanceCopyCopy` + copyForceCompressionFormat bool + // Fields which can be used by callers when operation // is `instanceCopyClone` cloneCompressionVariant OptionCompressionVariant @@ -122,9 +126,14 @@ func prepareInstanceCopies(list internalManifest.List, instanceDigests []digest. if err != nil { return res, fmt.Errorf("getting details for instance %s: %w", instanceDigest, err) } + forceCompressionFormat, err := shouldRequireCompressionFormatMatch(options) + if err != nil { + return nil, err + } res = append(res, instanceCopy{ - op: instanceCopyCopy, - sourceDigest: instanceDigest, + op: instanceCopyCopy, + sourceDigest: instanceDigest, + copyForceCompressionFormat: forceCompressionFormat, }) platform := platformV1ToPlatformComparable(instanceDetails.ReadOnly.Platform) compressionList := compressionsByPlatform[platform] @@ -230,7 +239,7 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, 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) - updated, err := c.copySingleImage(ctx, unparsedInstance, &instanceCopyList[i].sourceDigest, copySingleImageOptions{requireCompressionFormatMatch: false}) + updated, err := c.copySingleImage(ctx, unparsedInstance, &instanceCopyList[i].sourceDigest, copySingleImageOptions{requireCompressionFormatMatch: instance.copyForceCompressionFormat}) if err != nil { return nil, fmt.Errorf("copying image %d/%d from manifest list: %w", i+1, len(instanceCopyList), err) } diff --git a/copy/multiple_test.go b/copy/multiple_test.go index 223dd274dc..f753c247ea 100644 --- a/copy/multiple_test.go +++ b/copy/multiple_test.go @@ -32,7 +32,7 @@ func TestPrepareCopyInstancesforInstanceCopyCopy(t *testing.T) { for _, instance := range sourceInstances { compare = append(compare, instanceCopy{op: instanceCopyCopy, - sourceDigest: instance}) + sourceDigest: instance, copyForceCompressionFormat: false}) } assert.Equal(t, instancesToCopy, compare) @@ -42,6 +42,9 @@ func TestPrepareCopyInstancesforInstanceCopyCopy(t *testing.T) { compare = []instanceCopy{{op: instanceCopyCopy, sourceDigest: sourceInstances[1]}} assert.Equal(t, instancesToCopy, compare) + + _, err = prepareInstanceCopies(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, ForceCompressionFormat: true}) + require.EqualError(t, err, "cannot use ForceCompressionFormat with undefined default compression format") } // Test `instanceCopyClone` cases.