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 ability to enable/disable specific rekor API endpoints #1080

Merged
merged 3 commits into from
Oct 4, 2022

Conversation

bobcallaway
Copy link
Member

Signed-off-by: Bob Callaway bcallaway@google.com

Signed-off-by: Bob Callaway <bcallaway@google.com>
@bobcallaway bobcallaway requested a review from a team as a code owner September 27, 2022 17:33
@dlorenc
Copy link
Member

dlorenc commented Sep 27, 2022

This seems fine, just curious what the context is.

Signed-off-by: Bob Callaway <bcallaway@google.com>
@bobcallaway
Copy link
Member Author

This seems fine, just curious what the context is.

If we wanted isolation of specific read-only VS write-only paths for scaling, it might be useful to be able to narrow the API surface down.

This also removes a couple incorrect middleware handlers and caching rules that no longer apply.

@codecov-commenter
Copy link

codecov-commenter commented Sep 27, 2022

Codecov Report

Merging #1080 (994a989) into main (6904cff) will decrease coverage by 0.44%.
The diff coverage is 9.25%.

@@            Coverage Diff             @@
##             main    #1080      +/-   ##
==========================================
- Coverage   64.10%   63.65%   -0.45%     
==========================================
  Files          82       82              
  Lines        7482     7534      +52     
==========================================
  Hits         4796     4796              
- Misses       2065     2116      +51     
- Partials      621      622       +1     
Flag Coverage Δ
e2etests 48.51% <9.25%> (-0.30%) ⬇️
unittests 40.71% <0.00%> (-0.33%) ⬇️

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

Impacted Files Coverage Δ
pkg/api/public_key.go 37.50% <0.00%> (-29.17%) ⬇️
pkg/api/tlog.go 39.02% <0.00%> (-5.02%) ⬇️
pkg/api/entries.go 65.16% <3.44%> (-4.38%) ⬇️
cmd/rekor-server/app/root.go 56.66% <100.00%> (+1.49%) ⬆️
pkg/api/api.go 64.28% <100.00%> (ø)
pkg/types/alpine/v0.0.1/entry.go 72.35% <0.00%> (-1.22%) ⬇️

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

@lukehinds
Copy link
Member

code looks good, don't forget to update https://github.com/sigstore/sigstore-website/tree/docs/content/en/rekor too.

@dlorenc
Copy link
Member

dlorenc commented Sep 28, 2022

If we wanted isolation of specific read-only VS write-only paths for scaling, it might be useful to be able to narrow the API surface down.

Not sure I follow here... Rekor itself is stateless, if we had a bunch only doing reads I'm not sure how that would help with the database scaling. Would we point them at read only replicas of mysql/trillian or something?

@var-sdk
Copy link

var-sdk commented Sep 28, 2022

If we wanted isolation of specific read-only VS write-only paths for scaling, it might be useful to be able to narrow the API surface down.

Not sure I follow here... Rekor itself is stateless, if we had a bunch only doing reads I'm not sure how that would help with the database scaling. Would we point them at read only replicas of mysql/trillian or something?

That's one possibility. We could set up read replicas in other regions so the read path continues to operate even if our write instance goes down. The other goodness here might be splitting read & write paths within a cluster, so a query of death bug in write wouldn't completely take down read.

Signed-off-by: Bob Callaway <bobcallaway@users.noreply.github.com>
Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Thanks, look good! Thanks for adding in the validation/panic in case someone mis-types the endpoint.

@bobcallaway bobcallaway merged commit 3e2cdcf into sigstore:main Oct 4, 2022
@github-actions github-actions bot added this to the v1.0.0 milestone Oct 4, 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.

6 participants