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

Fix timestamp verification, add verify-blob tests #2527

Merged
merged 2 commits into from
Dec 10, 2022

Conversation

haydentherapper
Copy link
Contributor

While adding unit tests for verify-blob with timestamping, I found that we try to verify with the current time if no bundle is provided, even if we have a timestamp. I cleaned up this function to check the timestamp time and bundle time if either are provided, and if neither is provided, check against the current time.

I also added a mock client to clean up tests. I'll move this mock client over to the timestamp-authority repo.

Signed-off-by: Hayden Blauzvern [email protected]

Summary

Release Note

Documentation

@codecov-commenter
Copy link

codecov-commenter commented Dec 8, 2022

Codecov Report

Merging #2527 (f823b01) into main (09f023f) will decrease coverage by 0.03%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main    #2527      +/-   ##
==========================================
- Coverage   30.14%   30.11%   -0.04%     
==========================================
  Files         139      139              
  Lines        8595     8597       +2     
==========================================
- Hits         2591     2589       -2     
- Misses       5629     5632       +3     
- Partials      375      376       +1     
Impacted Files Coverage Δ
pkg/cosign/verify.go 40.99% <33.33%> (-0.33%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

While adding unit tests for verify-blob with timestamping, I found that
we try to verify with the current time if no bundle is provided, even if
we have a timestamp. I cleaned up this function to check the timestamp
time and bundle time if either are provided, and if neither is provided,
check against the current time.

I also added a mock client to clean up tests. I'll move this mock client
over to the timestamp-authority repo.

Signed-off-by: Hayden Blauzvern <[email protected]>
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

lgtm, a small nit

cmd/cosign/cli/verify/verify_blob_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@hectorj2f hectorj2f left a comment

Choose a reason for hiding this comment

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

Lgtm

@hectorj2f hectorj2f merged commit b5c8486 into sigstore:main Dec 10, 2022
@github-actions github-actions bot added this to the v1.14.0 milestone Dec 10, 2022
@haydentherapper haydentherapper deleted the tsa-tests branch December 10, 2022 17:25
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.

5 participants