Skip to content
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

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

dylrich
Copy link
Contributor

@dylrich dylrich commented Nov 17, 2023

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 for verify and verify-blob commands. This change also affects the verify-attestation and verify-blob-attestation commands but I couldn't find e2e tests for these, so I left them untested.

Release Note

  • Added the --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!

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (f57aa2c) 30.26% compared to head (5983def) 30.22%.

Files Patch % Lines
cmd/cosign/cli/verify.go 0.00% 16 Missing ⚠️
cmd/cosign/cli/options/verify.go 0.00% 3 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@haydentherapper haydentherapper left a 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 -

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)
}

@dylrich
Copy link
Contributor Author

dylrich commented Nov 17, 2023

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.

Copy link
Contributor

@haydentherapper haydentherapper left a 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.

cmd/cosign/cli/options/verify.go Outdated Show resolved Hide resolved
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>
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@haydentherapper haydentherapper merged commit ef0877d into sigstore:main Nov 17, 2023
28 checks passed
@github-actions github-actions bot added this to the v2.3.0 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improvements to verification for private repositories without log upload
3 participants