Allow shallow-copy of a bundle #758
Conversation
|
|
Signed-off-by: Gabe Rosenhouse <gabriel.rosenhouse@broadcom.com>
c30af67 to
4cac523
Compare
joaopapereira
left a comment
There was a problem hiding this comment.
Thanks for the PR. I added 2 comments, do you think you can address them?
pkg/imgpkg/cmd/copy.go
Outdated
| } | ||
|
|
||
| o.ImageFlags.SetCopy(cmd) | ||
| cmd.Flags().BoolVar(&o.ImageIsBundleCheck, "image-is-bundle-check", true, "Error when image is a bundle (disable shallow-copying bundles via -i)") |
There was a problem hiding this comment.
Not sure I like the phrasing here, I know that we used the same verbiage in other places but it feels hard to understand. What do you think about ignore-bundle-check and for the helper text maybe When using -i imgpkg will not check if the image is a bundle. This will allow shallow-copying the bundle image.?
There was a problem hiding this comment.
Yes, that is clearer. Updated.
pkg/imgpkg/v1/copy.go
Outdated
| return nil, nil, err | ||
| } | ||
| if ok { | ||
| if ok && !opts.AllowShallowCopyBundle { |
There was a problem hiding this comment.
Given that v1 serves as an API I think we should have a test to ensure the API is working as expected and not only rely on the e2e tests.
There was a problem hiding this comment.
Yes. I've added cases in pkg/imgpkg/v1/copy_test.go within the ToTar test , to cover the success case and the missing-flag case. Let me know if I should add similar cases to ToRepo or elsewhere.
Signed-off-by: Gabe Rosenhouse <gabriel.rosenhouse@broadcom.com>
Signed-off-by: Gabe Rosenhouse <gabriel.rosenhouse@broadcom.com>
joaopapereira
left a comment
There was a problem hiding this comment.
LGTM.
The only thing that I am not a fan of is for the API to return errors that only make sense for the CLI, but I guess that is a bigger issues that we have to address in other places as well.
|
Thanks! |
This addresses #757 by adding an new
--ignore-bundle-checkflag toimgpkg copy. That enables shallow-copying a bundle, by treating it like an opaque image.e.g.
It also works with
--to-tar(destination) and--tar(source) and with and without--cosign-signaturesalthough I didn't add tests specifically for those bits.Update: originally this flag was
--image-is-bundle-check(default true) to match the analogous flag onpull. After review, it is now named--ignore-bundle-check(default false) to make it more clear.