-
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
factor out keyless verification certificate loading function #3762
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3762 +/- ##
==========================================
- Coverage 40.10% 37.08% -3.03%
==========================================
Files 155 200 +45
Lines 10044 12289 +2245
==========================================
+ Hits 4028 4557 +529
- Misses 5530 7182 +1652
- Partials 486 550 +64 ☔ View full report in Codecov by Sentry. |
04e6465
to
09f8ece
Compare
9857e15
to
14a166d
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! Can you rebase, that'll fix the failing CI
Signed-off-by: Dmitry S <[email protected]>
Signed-off-by: Dmitry S <[email protected]>
14a166d
to
32949de
Compare
Signed-off-by: Dmitry Savintsev <[email protected]>
done, CI 🟢 |
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
Summary
Factor out the code loading certificates for keyless verification (from a certificate chain, provided roots / intermediate or from Fulcio) into a helper function
loadCertsKeylessVerification
. This reduces the size of the long methodVerifyCommand.Exec
(from the current 252 to 206 lines), helps with unit testing, and will later allow the same logic and helper function to be shared and called fromverify_attestation.go
,verify_blob.go
etc. (related to #3759).The helper function has a corresponding unit test, the est coverage in the
cmd/cosign/cli/verify
is increased from 38.2% of statements in the trunk version to 42.6%.Release Note
NONE
Documentation
n/a - no updates needed