Conversation
|
Hold to merge until #4449 is in, then I'll rebase. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4458 +/- ##
==========================================
- Coverage 40.10% 36.47% -3.63%
==========================================
Files 155 220 +65
Lines 10044 12239 +2195
==========================================
+ Hits 4028 4464 +436
- Misses 5530 7082 +1552
- Partials 486 693 +207 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c281e31 to
936b111
Compare
|
Rebased and refactored common logic into a shared method. |
|
Going to update to override the signing config with a printed warning. I don't love doing this, but there's clearly a lot of usage still of tlog-upload and we didn't deprecate this in v2.6 |
936b111 to
e586ad9
Compare
|
@cmurphy @steiza This is ready to go. I initially had implemented this to err out like how we err out if any service URLs are provided along with a signing config. However, I think there's far more users specifying this flag while using the public instance than users who are providing URLs for private deployments. I chose to override the tlog service URLs in the signing config instead and log a deprecation message instead. |
This actually seems kind of unfriendly to me. It feels like a lot of overhead to have to edit the signing config that cosign downloaded for you, which will get overwritten if you edited it in-place. The use case is often that the user is just trying out a cosign command and doesn't want to add a few more kilobytes of junk to the public log, so having to make them jump through this hoop is going to be confusing. |
Transparency is a key part of Sigstore, and so putting a significant speed bump in place is reasonable. I don't think users are using this flag to avoid adding junk. We see this flag mentioned in blog posts online. It could be because of privacy concerns (users can use a key rather than certificate then), or because of private infrastructure, or I think because of Cosign v1 (when all the blog posts were written) defaulting to no Rekor so the posts mentioned the flag for parity. This is also a concern for verification - either users are defaulting to using the "insecure ignore tlog" flag, which should be strongly discouraged, or there's not a lot of verification so this hasn't come up frequently. We need a well-lit path for private infrastructure. What's great about using the signing config and trusted root is that we can remove these flags and instead signers can explicitly configure what infra they use and verifiers can configure what's trusted, and we don't have to build a different path. Regardless, what's implemented here is just a deprecation warning for now. |
ae8f5ab to
76b3a2e
Compare
Clients that don't want to use a transparency log should provide a signing config without tlog service instances rather than use this flag. This will also throw an error when a client uses this flag when a signing config will be used, since Cosign/sigstore-go ignores this flag and we don't want a user to unexpectedly upload to the public instance. Signed-off-by: Hayden <[email protected]>
The one difference between sign/attest and sign/attest-blob is whether a bundle output flag is present, so the error message has been adjusted. Signed-off-by: Hayden <[email protected]>
Signed-off-by: Hayden <[email protected]>
76b3a2e to
be0bd63
Compare
|
@steiza Following up from the Go meeting (finally, sorry!), I reverted the changes to not mutate the signing config and instead err out. The error message is now more detailed and provides a one-liner to grab the latest signing config and drop tlogs. |
|
a 13 day deprecation warning wasn't much of a buffer, guys Many of us use templating to support reusable CI/CD jobs and this is a breaking change now that it defaults to erroring out |
|
Hey, sorry for the breakage, but this was resolving an issue as the flag's value was being ignored. This flag should have been marked as deprecated when we released v3.0.0. |
* Deprecate tlog-upload flag Clients that don't want to use a transparency log should provide a signing config without tlog service instances rather than use this flag. This will also throw an error when a client uses this flag when a signing config will be used, since Cosign/sigstore-go ignores this flag and we don't want a user to unexpectedly upload to the public instance. Signed-off-by: Hayden <[email protected]> * Refactor TR and SC initialization into common method The one difference between sign/attest and sign/attest-blob is whether a bundle output flag is present, so the error message has been adjusted. Signed-off-by: Hayden <[email protected]> * Fix e2e test, better error message Signed-off-by: Hayden <[email protected]> --------- Signed-off-by: Hayden <[email protected]>
Clients that don't want to use a transparency log should provide a signing config without tlog service instances rather than use this flag. This provides a detailed error message for what clients should do.
Fixes #4503
Summary
Release Note
Documentation