-
Notifications
You must be signed in to change notification settings - Fork 547
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 --private-infrastructure flag #3369
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3369 +/- ##
==========================================
- Coverage 30.26% 30.22% -0.05%
==========================================
Files 155 155
Lines 9941 9956 +15
==========================================
Hits 3009 3009
- Misses 6482 6497 +15
Partials 450 450 ☔ View full report in Codecov by Sentry. |
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.
Thanks for adding this! Can we use pflag aliases described in https://pkg.go.dev/github.com/spf13/pflag#section-readme:~:text=myFlagSet.SetNormalizeFunc(wordSepNormalizeFunc)-,Example%20%232%3A,-You%20want%20to? We've done that for certificate flag names -
cosign/cmd/cosign/cli/commands.go
Lines 37 to 53 in f57aa2c
func normalizeCertificateFlags(_ *pflag.FlagSet, name string) pflag.NormalizedName { | |
switch name { | |
case "cert": | |
name = "certificate" | |
case "cert-email": | |
name = "certificate-email" | |
case "cert-chain": | |
name = "certificate-chain" | |
case "cert-oidc-issuer": | |
name = "certificate-oidc-issuer" | |
case "output-cert": | |
name = "output-certificate" | |
case "cert-identity": | |
name = "certificate-identity" | |
} | |
return pflag.NormalizedName(name) | |
} |
I had tried looking at that originally but couldn't figure out how to differentiate between the two later on for the purpose of hiding the warning in the new flag but not the other. If you think it's OK to just remove the warning altogether I can refactor? Or maybe I am just missing something in the pflag docs that would support what I was trying to do. |
cb513f5
to
723dac3
Compare
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.
Ah, no, you’re correct, I don’t think there’d be a way to differentiate. Happy with this then.
723dac3
to
e9ef1c9
Compare
This commit adds the --private-infrastructure command line flag for verification commands. This flag is an alias for --insecure-ignore-tlog with the exception that it also silences the warning message the later flag prints. This flag is intended for users who do not rely on a public transparency log and have their own private infrastructure dedicated to verification. Signed-off-by: dylrich <dylan.richardson@mongodb.com>
e9ef1c9
to
5983def
Compare
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.
Thanks!
Partially fixes #2736
Summary
This commit adds the
--private-infrastructure
command line flag for verification commands. This flag is an alias for--insecure-ignore-tlog
with the exception that it also silences the warning message the later flag prints. This flag is intended for users who do not rely on a public transparency log and have their own private infrastructure dedicated to verification.I decided to make the
--private-infrastructure
flag automatically set the corresponding--insecure-ignore-tlog
option in order to preserve the warning that is printed only when the later flag is used, but I am open to other approaches if desired!I added a couple of calls using this flag instead of the
--insecure-ignore-tlog
flag to the e2e tests forverify
andverify-blob
commands. This change also affects theverify-attestation
andverify-blob-attestation
commands but I couldn't find e2e tests for these, so I left them untested.Release Note
--private-infrastructure
flag for verification commands as an alias for--insecure-ignore-tlog
Documentation
I will open a PR in the Sigstore docs repo once someone has had a chance to give the initial OK on this change!