Skip to content

Commit

Permalink
Merge pull request #2068 from flouthoc/implement-forcecompressionformat
Browse files Browse the repository at this point in the history
copy: add support for `ForceCompressionFormat`
  • Loading branch information
mtrmac authored Aug 3, 2023
2 parents 2869e26 + f406c36 commit 9446ffa
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 6 deletions.
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) {
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

0 comments on commit 9446ffa

Please sign in to comment.