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

Remove requirement of PayloadHash for intoto 0.0.1 #1490

Merged
merged 2 commits into from
May 24, 2023

Conversation

haydentherapper
Copy link
Contributor

The payload hash was added later on, so old committed entries are missing the payload hash. This means that canonicalization will fail when searching for the entry if we require the payload hash.

Summary

Release Note

Documentation

The payload hash was added later on, so old committed entries are
missing the payload hash. This means that canonicalization will fail
when searching for the entry if we require the payload hash.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #1490 (bfbb87f) into main (9b37074) will increase coverage by 0.07%.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##             main    #1490      +/-   ##
==========================================
+ Coverage   66.47%   66.55%   +0.07%     
==========================================
  Files          80       80              
  Lines        8022     8020       -2     
==========================================
+ Hits         5333     5338       +5     
+ Misses       2038     2033       -5     
+ Partials      651      649       -2     
Flag Coverage Δ
e2etests 48.25% <9.09%> (+0.42%) ⬆️
unittests 46.89% <72.72%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/types/intoto/v0.0.1/entry.go 69.34% <72.72%> (+1.66%) ⬆️

... and 2 files with indirect coverage changes

@bobcallaway
Copy link
Member

Could you please verify this case is covered by the tests in harness and if not, add one explicitly for this? thx

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Copy link
Contributor Author

@bobcallaway added a test to demonstrate no failure without payload hash

@bobcallaway bobcallaway merged commit d508eba into sigstore:main May 24, 2023
14 checks passed
@github-actions github-actions bot added this to the v1.1.0 milestone May 24, 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.

None yet

2 participants