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 unnecessary lookup of non-existent attestations from storage layer #909

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

bobcallaway
Copy link
Member

Currently only two Rekor pluggable types support the storage of attestations (intoto, cose); the previous code to fetch attestations was type-agnostic, but due to the fix #878, the server was doing unnecessary lookups for all types, regardless of whether they store attestation content or not.

This makes the attestation storage an explicit interface, which we can test casting for and avoid a round trip to the storage layer for types that don't support storing attestations.

Signed-off-by: Bob Callaway [email protected]

Currently only two Rekor pluggable types support the storage of
attestations (intoto, cose); the previous code to fetch
attestations was type-agnostic, but due to the fix sigstore#878 the
server was doing unnecessary lookups for all types, regardless
of whether they store attestation content or not.

This makes the attestation storage an explict interface, which
we can test casting for and avoid a roundtrip to the storage
layer for types that don't support storing attestations.

Signed-off-by: Bob Callaway <[email protected]>
@bobcallaway bobcallaway requested a review from a team as a code owner July 6, 2022 19:49
@codecov-commenter
Copy link

Codecov Report

Merging #909 (1839394) into main (3184a52) will increase coverage by 0.28%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #909      +/-   ##
==========================================
+ Coverage   47.84%   48.12%   +0.28%     
==========================================
  Files          62       62              
  Lines        5459     5427      -32     
==========================================
  Hits         2612     2612              
+ Misses       2559     2527      -32     
  Partials      288      288              
Impacted Files Coverage Δ
pkg/types/alpine/v0.0.1/entry.go 56.72% <ø> (+0.93%) ⬆️
pkg/types/entries.go 6.25% <ø> (ø)
pkg/types/hashedrekord/v0.0.1/entry.go 53.84% <ø> (+1.34%) ⬆️
pkg/types/helm/v0.0.1/entry.go 48.91% <ø> (+0.83%) ⬆️
pkg/types/jar/v0.0.1/entry.go 39.40% <ø> (+0.76%) ⬆️
pkg/types/rekord/v0.0.1/entry.go 48.82% <ø> (+0.64%) ⬆️
pkg/types/rfc3161/v0.0.1/entry.go 39.69% <ø> (+1.17%) ⬆️
pkg/types/rpm/v0.0.1/entry.go 56.25% <ø> (+0.86%) ⬆️
pkg/types/tuf/v0.0.1/entry.go 42.38% <ø> (+0.68%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3184a52...1839394. Read the comment docs.

@dlorenc
Copy link
Member

dlorenc commented Jul 6, 2022

Oooh nice!

@dlorenc dlorenc merged commit 751b19d into sigstore:main Jul 6, 2022
@github-actions github-actions bot added this to the v1.0.0 milestone Jul 6, 2022
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.

4 participants