-
Notifications
You must be signed in to change notification settings - Fork 527
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
Add crane flatten #1104
Add crane flatten #1104
Conversation
This is adapted from google#735 but changes the UX a bit to match what we've come to expect from crane. Add pkg/crane.Upload as well, for uploading a layer.
Codecov Report
@@ Coverage Diff @@
## main #1104 +/- ##
==========================================
+ Coverage 75.41% 75.44% +0.02%
==========================================
Files 108 108
Lines 7685 7692 +7
==========================================
+ Hits 5796 5803 +7
Misses 1334 1334
Partials 555 555
Continue to review full report at Codecov.
|
Re: reproducibility... a stupid idea: If the original image ref is by digest, we could include only the digest in the It's a weak heuristic, but I think pretty okay. The only issue is if someone is flattening by digest and wants the original image ref, we'd exclude it. |
cmd/crane/cmd/flatten.go
Outdated
// If the new ref isn't provided, write over the original image. | ||
// If that ref was provided by digest (e.g., output from | ||
// another crane command), then strip that and push the | ||
// mutated image by digest instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #960 if you rebase by digest we'll push to :rebased
unless you tell us what to push to. I don't love it, but I especially don't love it if it's going to disagree with the UX of crane flatten <img-by-digest>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love that either, can do the same thing in #960 that we do here?
log.Fatalf("parsing %s: %v", newRef, err) | ||
} | ||
if _, ok := r.(name.Digest); ok { | ||
// If we're pushing by digest, we need to upload the layer first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because the stream.Layer's digest won't be known until it's uploaded, right? Does pushing to some fallback tag instead help here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not push to a fallback tag because that might overwrite an existing tag or pollute the registry.
Can/should we detect and unset base image annotations from the flattened image? |
I'd lean towards no. The base image is still the same, because in theory this is an equivalent but flattened image. If you wanted to rebuild based on the base changing, you could do |
// Stupid hack to support insecure flag. | ||
nameOpt := []name.Option{} | ||
if ok, err := cmd.Parent().PersistentFlags().GetBool("insecure"); err != nil { | ||
log.Fatalf("flag problems: %v", err) | ||
} else if ok { | ||
nameOpt = append(nameOpt, name.Insecure) | ||
} | ||
r, err := name.ParseReference(newRef, nameOpt...) | ||
if err != nil { | ||
log.Fatalf("parsing %s: %v", newRef, err) | ||
} | ||
|
||
desc, err := crane.Head(ref, *options...) | ||
if err != nil { | ||
log.Fatalf("checking %s: %v", ref, err) | ||
} | ||
if !cmd.Parent().PersistentFlags().Changed("platform") && desc.MediaType.IsIndex() { | ||
log.Fatalf("flattening an index is not yet supported") | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
brb stealing these 😄
This is adapted from #735 but changes the UX a bit to match what we've come to expect from crane.
Add pkg/crane.Upload as well, for uploading a layer.
Closes #1103
Note that we preserve the original history in
comment
and what was flattened in thecreated_by
comment. I'm not sure if we actually want to include the original image ref, or if we should just include the original image digest here to make this more reproducible.