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

Revisit endpoint logic #39

Closed
shikhar opened this issue Nov 4, 2024 · 1 comment · Fixed by #40
Closed

Revisit endpoint logic #39

shikhar opened this issue Nov 4, 2024 · 1 comment · Fixed by #40
Assignees

Comments

@shikhar
Copy link
Member

shikhar commented Nov 4, 2024

Goals

  • We want to be able to target a cell URI that can handle all requests (also basin-level requests, with the S2-Basin header) – this captures local testing and our sandbox/staging environments, as well as potentially in future PrivateLink where a direct cell-level endpoint will be required also in prod.

  • In general, we want to allow talking to a basin without knowing the cell ID and rely on DNS for that mapping. We also want to be able to test this in our sandbox & staging environments – so we need to avoid setting the S2-Basin header when including basin in the URI, to make sure we are exercising all the right bits.

Proposal

AccountService requests: look for S2_CELL_ENDPOINT

  • if present, use that
  • otherwise, use {cloud}.s2.dev

(Basin|Stream)Service requests: look for S2_BASIN_ZONE

  • if present, use {basin}.{S2_BASIN_ZONE} without a S2-Basin header
  • otherwise, look for S2_CELL_ENDPOINT
    • if present, use that with S2-Basin header
    • otherwise, use {basin}.b.{cloud}.s2.dev (effectively, S2_BASIN_ZONE="b.{cloud}.s2.dev")

These env vars should not contain the https:// scheme, that should be assumed as plaintext is not supported. They will correspond to the 'authority' portion of a URI.

@shikhar
Copy link
Member Author

shikhar commented Nov 4, 2024

Instead of environment variables, I think better to have optional args, and let app do the plumbing (I think this is the current pattern).

vrongmeal added a commit that referenced this issue Nov 4, 2024
Fixes: #39

Signed-off-by: Vaibhav Rabber <[email protected]>
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 a pull request may close this issue.

2 participants