-
Notifications
You must be signed in to change notification settings - Fork 83
copy: add option to strip sparse manifest lists #655
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
base: main
Are you sure you want to change the base?
Changes from all commits
1d0b67c
1f2d527
a4800f0
d195aa9
1f261d2
a893f86
b2468d9
c46fc44
7ebc22f
ff24aa0
aa35685
fb810d9
d6b69fd
61fe95f
a3791c7
639bc5a
d2c8d89
1094da3
3d709c0
5d05c0d
50bbedd
5b1910c
6d9fd31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,9 +77,28 @@ const ( | |
| // specific images from the source reference. | ||
| type ImageListSelection int | ||
|
|
||
| const ( | ||
| // KeepSparseManifestList is the default value which, when set in | ||
| // Options.SparseManifestListAction, indicates that the manifest is kept | ||
| // as is even though some images from the list may be missing. Some | ||
| // registries may not support this. | ||
| KeepSparseManifestList SparseManifestListAction = iota | ||
|
|
||
| // StripSparseManifestList will strip missing images from the manifest | ||
| // list. When images are stripped the digest will differ from the original. | ||
| StripSparseManifestList | ||
| ) | ||
|
|
||
| // SparseManifestListAction is one of KeepSparseManifestList or StripSparseManifestList | ||
| // to control the behavior when only a subset of images from a manifest list is copied | ||
| type SparseManifestListAction int | ||
|
|
||
| // Options allows supplying non-default configuration modifying the behavior of CopyImage. | ||
| type Options struct { | ||
| RemoveSignatures bool // Remove any pre-existing signatures. Signers and SignBy… will still add a new signature. | ||
| // RemoveListSignatures removes the manifest list signature while preserving per-instance signatures. | ||
| // If RemoveSignatures is also true, RemoveSignatures takes precedence. | ||
| RemoveListSignatures bool | ||
| // Signers to use to add signatures during the copy. | ||
| // Callers are still responsible for closing these Signer objects; they can be reused for multiple copy.Image operations in a row. | ||
| Signers []*signer.Signer | ||
|
|
@@ -102,6 +121,9 @@ type Options struct { | |
| ImageListSelection ImageListSelection // set to either CopySystemImage (the default), CopyAllImages, or CopySpecificImages to control which instances we copy when the source reference is a list; ignored if the source reference is not a list | ||
| Instances []digest.Digest // if ImageListSelection is CopySpecificImages, copy only these instances, instances matching the InstancePlatforms list, and the list itself | ||
| InstancePlatforms []InstancePlatformFilter // if ImageListSelection is CopySpecificImages, copy instances with matching OS/Architecture (all variants and compressions), it also copies the index/manifest_list instance. | ||
| // When only a subset of images of a list is copied, this action indicates if the manifest should be kept or stripped. | ||
| // See CopySpecificImages. | ||
| SparseManifestListAction SparseManifestListAction | ||
| // Give priority to pulling gzip images if multiple images are present when configured to OptionalBoolTrue, | ||
| // prefers the best compression if this is configured as OptionalBoolFalse. Choose automatically (and the choice may change over time) | ||
| // if this is set to OptionalBoolUndefined (which is the default behavior, and recommended for most callers). | ||
|
|
@@ -318,30 +340,15 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, | |
| return nil, fmt.Errorf("determining manifest MIME type for %s: %w", transports.ImageName(srcRef), err) | ||
| } | ||
|
|
||
| var singleInstance *image.UnparsedImage | ||
| var singleInstanceErrorWrapping string | ||
| if !multiImage { | ||
| 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: requireCompressionFormatMatch}) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| copiedManifest = single.manifest | ||
| singleInstance = c.unparsedToplevel | ||
| singleInstanceErrorWrapping = "" | ||
| } else if c.options.ImageListSelection == CopySystemImage { | ||
| 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. | ||
| // 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. | ||
| mfest, manifestType, err := c.unparsedToplevel.Manifest(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("reading manifest for %s: %w", transports.ImageName(srcRef), err) | ||
|
|
@@ -355,13 +362,30 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, | |
| return nil, fmt.Errorf("choosing an image from manifest list %s: %w", transports.ImageName(srcRef), err) | ||
| } | ||
| 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: requireCompressionFormatMatch}) | ||
| singleInstance = image.UnparsedInstance(rawSource, &instanceDigest) | ||
| singleInstanceErrorWrapping = "copying system image from manifest list" | ||
| } // else: a multi-instance copy | ||
|
|
||
| if singleInstance != nil { | ||
| if len(options.EnsureCompressionVariantsExist) > 0 { | ||
| return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image") | ||
| } | ||
| if c.options.RemoveListSignatures { | ||
| return nil, fmt.Errorf("RemoveListSignatures is not applicable when copying a single instance") | ||
| } | ||
| requireCompressionFormatMatch, err := shouldRequireCompressionFormatMatch(options) | ||
mtrmac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| single, err := c.copySingleImage(ctx, singleInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: requireCompressionFormatMatch}) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("copying system image from manifest list: %w", err) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hum, we probably want this context (at least Skopeo defaults to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed at commit 4a6817d.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m sorry, I by “this context” I meant “system image from manifest list”.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in commit df33edfba4. |
||
| if singleInstanceErrorWrapping != "" { | ||
| return nil, fmt.Errorf("%s: %w", singleInstanceErrorWrapping, err) | ||
| } | ||
| return nil, err | ||
| } | ||
| copiedManifest = single.manifest | ||
| } else { /* c.options.ImageListSelection == CopyAllImages or c.options.ImageListSelection == CopySpecificImages, */ | ||
| } else { | ||
| // If we were asked to copy multiple images and can't, that's an error. | ||
| if !supportsMultipleImages(c.dest) { | ||
| return nil, fmt.Errorf("copying multiple images: destination transport %q does not support copying multiple images as a group", destRef.Transport().Name()) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.