Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

copy: add support for ForceCompressionFormat #2068

Merged
merged 1 commit into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions copy/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Hiding this somewhere at the bottom, not near the top of the file, might help newcomers to the codebase who want to understand what this file is about.)

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.
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
Expand All @@ -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)
}
Expand Down
15 changes: 12 additions & 3 deletions copy/multiple.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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)
}
Expand Down
5 changes: 4 additions & 1 deletion copy/multiple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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.
Expand Down