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.

Sign up for GitHub

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/single: accept custom *Options and wrap arguments in copySingleImageOptions #2050

Merged
merged 3 commits into from
Jul 21, 2023
Merged

copy/single: accept custom *Options and wrap arguments in copySingleImageOptions #2050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7983ec2
copy/single: wrap arguments in copySingleImageOptions
flouthoc Jul 21, 2023
481ed6a
copy/single: add custom compressionFormat, compressionLevel fields
flouthoc Jul 21, 2023
adbfd0d
copy/single: honor c.options.DestCtx for regular copy
flouthoc Jul 21, 2023
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
5 changes: 2 additions & 3 deletions copy/copy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,

if !multiImage {
// The simple case: just copy a single image.
single, err := c.copySingleImage(ctx, c.unparsedToplevel, nil, false)
single, err := c.copySingleImage(ctx, c.unparsedToplevel, nil, copySingleImageOptions{requireCompressionFormatMatch: false})
if err != nil {
return nil, err
}
Expand All @@ -273,8 +273,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, false)
single, err := c.copySingleImage(ctx, unparsedInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: false})
if err != nil {
return nil, fmt.Errorf("copying system image from manifest list: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion copy/multiple.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,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, false)
updated, err := c.copySingleImage(ctx, unparsedInstance, &instanceCopyList[i].sourceDigest, copySingleImageOptions{requireCompressionFormatMatch: false})
if err != nil {
return nil, fmt.Errorf("copying image %d/%d from manifest list: %w", i+1, len(instanceCopyList), err)
}
Expand Down
17 changes: 13 additions & 4 deletions copy/single.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ type imageCopier struct {
requireCompressionFormatMatch bool
}

type copySingleImageOptions struct {
requireCompressionFormatMatch bool
compressionFormat *compressiontypes.Algorithm // Compression algorithm to use, if the user explicitly requested one, or nil.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
compressionFormat *compressiontypes.Algorithm // Compression algorithm to use, if the user explicitly requested one, or nil.
compressionFormat *compressiontypes.Algorithm // Compression algorithm to use, if the caller explicitly requested one, or nil.

for future reference

compressionLevel *int
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
compressionLevel *int
compressionLevel *int // only used if compressionFormat is set

}

// copySingleImageResult carries data produced by copySingleImage
type copySingleImageResult struct {
manifest []byte
Expand All @@ -50,7 +56,7 @@ type copySingleImageResult struct {

// copySingleImage copies a single (non-manifest-list) image unparsedImage, using c.policyContext to validate
// source image admissibility.
func (c *copier) copySingleImage(ctx context.Context, unparsedImage *image.UnparsedImage, targetInstance *digest.Digest, requireCompressionFormatMatch bool) (copySingleImageResult, error) {
func (c *copier) copySingleImage(ctx context.Context, unparsedImage *image.UnparsedImage, targetInstance *digest.Digest, opts copySingleImageOptions) (copySingleImageResult, error) {
// The caller is handling manifest lists; this could happen only if a manifest list contains a manifest list.
// Make sure we fail cleanly in such cases.
multiImage, err := isMultiImage(ctx, unparsedImage)
Expand Down Expand Up @@ -131,9 +137,12 @@ func (c *copier) copySingleImage(ctx context.Context, unparsedImage *image.Unpar
src: src,
// diffIDsAreNeeded is computed later
cannotModifyManifestReason: cannotModifyManifestReason,
requireCompressionFormatMatch: requireCompressionFormatMatch,
requireCompressionFormatMatch: opts.requireCompressionFormatMatch,
}
if c.options.DestinationCtx != nil {
if opts.compressionFormat != nil {
ic.compressionFormat = opts.compressionFormat
ic.compressionLevel = opts.compressionLevel
} else if c.options.DestinationCtx != nil {
// Note that compressionFormat and compressionLevel can be nil.
ic.compressionFormat = c.options.DestinationCtx.CompressionFormat
ic.compressionLevel = c.options.DestinationCtx.CompressionLevel
Expand Down Expand Up @@ -180,7 +189,7 @@ func (c *copier) copySingleImage(ctx context.Context, unparsedImage *image.Unpar
shouldUpdateSigs := len(sigs) > 0 || len(c.signers) != 0 // TODO: Consider allowing signatures updates only and skipping the image's layers/manifest copy if possible
noPendingManifestUpdates := ic.noPendingManifestUpdates()

logrus.Debugf("Checking if we can skip copying: has signatures=%t, OCI encryption=%t, no manifest updates=%t, compression match required for resuing blobs=%t", shouldUpdateSigs, destRequiresOciEncryption, noPendingManifestUpdates, requireCompressionFormatMatch)
logrus.Debugf("Checking if we can skip copying: has signatures=%t, OCI encryption=%t, no manifest updates=%t, compression match required for resuing blobs=%t", shouldUpdateSigs, destRequiresOciEncryption, noPendingManifestUpdates, opts.requireCompressionFormatMatch)
if !shouldUpdateSigs && !destRequiresOciEncryption && noPendingManifestUpdates && !ic.requireCompressionFormatMatch {
matchedResult, err := ic.compareImageDestinationManifestEqual(ctx, targetInstance)
if err != nil {
Expand Down