Add --output-bundle and fix --upload for new bundle #4499
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4499 +/- ##
==========================================
- Coverage 40.10% 36.66% -3.44%
==========================================
Files 155 220 +65
Lines 10044 12150 +2106
==========================================
+ Hits 4028 4455 +427
- Misses 5530 7007 +1477
- Partials 486 688 +202 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cmd/cosign/cli/options/sign.go
Outdated
| cmd.Flags().StringVar(&o.OutputBundle, "output-bundle", "", | ||
| "write the bundle to FILE") |
There was a problem hiding this comment.
Minor: in sign-blob and attest-blob we just call this --bundle. Arguably --output-bundle is more descriptive, but --bundle would be more consistent. I slightly lean towards consistency here, but open to other opinions.
Separately, taking inspiration from sign-blob maybe the description could be something like:
write everything required to verify the image to a FILE
cmd/cosign/cli/attest/attest.go
Outdated
|
|
||
| if c.SigningConfig != nil { | ||
| return signcommon.WriteNewBundleWithSigningConfig(ctx, c.KeyOpts, c.CertPath, c.CertChainPath, payload, digest, types.CosignSignPredicateType, "", c.SigningConfig, c.TrustedMaterial, ociremoteOpts...) | ||
| return signcommon.WriteNewBundleWithSigningConfig(ctx, c.KeyOpts, c.CertPath, c.CertChainPath, payload, digest, types.CosignSignPredicateType, "", !c.NoUpload, c.SigningConfig, c.TrustedMaterial, ociremoteOpts...) |
There was a problem hiding this comment.
Minor: this is starting to be a lot of arguments to the signcommon.Write* functions! I'm fairly new to Go, but I think the idiomatic thing would be at some point to move these into a signcommon.WriteBundleWithSigningConfigOptions or some such. I'm not convinced this has to change in this pull request though, because this is an internal method that's easy to refactor later if needed.
There was a problem hiding this comment.
I added a second commit to create a separate struct for these functions.
Without this change, --upload=false was not respected with the new bundle format. It also would not have made sense because there was no way to output the bundle locally. This change adds a flag --bundle so that the bundle can be created on disk without attaching it to the image, and also passes through the Upload parameter to bypass uploading it if desired. Signed-off-by: Colleen Murphy <[email protected]>
Use a common options struct for WriteBundle and WriteNewBundleWithSigningConfig to reduce the number of arguments in each function. Signed-off-by: Colleen Murphy <[email protected]>
cmurphy
force-pushed
the
output-bundle
branch
from
80dbd5c to
4450852
Compare
3 participants
Without this change, --upload=false was not respected with the new bundle format. It also would not have made sense because there was no way to output the bundle locally. This change adds a flag --output-bundle so that the bundle can be created on disk without attaching it to the image, and also passes through the Upload parameter to bypass uploading it if desired.
Fixes #4474
Summary
Release Note
Documentation