Skip to content

Conversation

@haydentherapper
Copy link
Contributor

This allows for deployers to serve read traffic without a separate
CDN/proxy. This is discouraged for GCP, since it's more costly to serve
read traffic in terms of egress costs, but can be used for other
backends and for local testing.

Rebased on #269.

Summary

Release Note

Documentation

@haydentherapper haydentherapper requested review from a team as code owners April 30, 2025 15:06
@codecov
Copy link

codecov bot commented Apr 30, 2025

Codecov Report

Attention: Patch coverage is 57.95455% with 37 lines in your changes missing coverage. Please review.

Project coverage is 39.73%. Comparing base (ec16bc7) to head (dcdfa09).

Files with missing lines Patch % Lines
pkg/tessera/tessera.go 0.00% 18 Missing ⚠️
pkg/server/service_read.go 85.96% 7 Missing and 1 partial ⚠️
cmd/rekor-server/app/serve.go 0.00% 7 Missing ⚠️
pkg/server/http.go 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
+ Coverage   39.11%   39.73%   +0.62%     
==========================================
  Files          39       40       +1     
  Lines        2751     2831      +80     
==========================================
+ Hits         1076     1125      +49     
- Misses       1571     1600      +29     
- Partials      104      106       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This allows for deployers to serve read traffic without a separate
CDN/proxy. This is discouraged for GCP, since it's more costly to serve
read traffic in terms of egress costs, but can be used for other
backends and for local testing.

Signed-off-by: Hayden B <[email protected]>
Copy link
Contributor

@cmurphy cmurphy left a comment

Choose a reason for hiding this comment

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

This is discouraged for GCP

I think we would never want to use this for GCP. Instead of adding a --serve-read-paths flag, could this just be directly tied to the backend in use?

rekorServer := server.NewServer(tesseraStorage, readOnly, algorithmRegistry)
var rekorServer server.RekorServer
if viper.GetBool("serve-read-paths") {
rekorServer = server.NewReadServer(tesseraStorage, readOnly, algorithmRegistry)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to fix - This breaks with --read-only because tesseraStorage is nil but is still needed on the read path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

transparency-dev/tessera#625 to see if Tessera can add a function that would give us a LogReader without an appender

@haydentherapper
Copy link
Contributor Author

I think we would never want to use this for GCP. Instead of adding a --serve-read-paths flag, could this just be directly tied to the backend in use?

For POSIX, it depends. I'd imagine a common deployment would be to serve the read path via nginx or some other load balancer that serves the /tiles directory. If a deployer wouldn't want to stand up an additional load balancer and doesn't expect significant read traffic, just the server should be able to handle both read and write traffic (pending load tests to verify this).

Technically, if someone didn't want to deal with managing a load balancer or public buckets, they could serve read traffic for GCP from the service.

I wouldn't be opposed to only allowing a subset of supported backends to serve read traffic though.

@haydentherapper haydentherapper marked this pull request as draft May 6, 2025 17:32
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.

2 participants