-
Notifications
You must be signed in to change notification settings - Fork 460
feat(api): return BYOC cluster domain from volume endpoints #3490
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
Changes from all commits
bf93ab7
177ef51
2d63f7d
02f41af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package clusters | ||
|
|
||
| import ( | ||
| "github.com/google/uuid" | ||
|
|
||
| "github.com/e2b-dev/infra/packages/shared/pkg/smap" | ||
| ) | ||
|
|
||
| // NewTestPool builds a Pool pre-populated with the given clusters, for use in | ||
| // tests that need cluster lookups (e.g. GetClusterById) without spinning up a | ||
| // full synchronization loop. | ||
| func NewTestPool(clusters ...*Cluster) *Pool { | ||
| m := smap.New[*Cluster]() | ||
| for _, c := range clusters { | ||
| m.Insert(c.ID.String(), c) | ||
| } | ||
|
|
||
| return &Pool{clusters: m} | ||
| } | ||
|
|
||
| // NewTestCluster builds a minimal Cluster carrying just an ID and sandbox | ||
| // domain, for tests. Other collaborators (instances, synchronization, | ||
| // resources) are left nil. | ||
| func NewTestCluster(id uuid.UUID, sandboxDomain *string) *Cluster { | ||
| return &Cluster{ | ||
| ID: id, | ||
| SandboxDomain: sandboxDomain, | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2058,6 +2058,13 @@ components: | |
| token: | ||
| type: string | ||
| description: Auth token to use for interacting with volume content | ||
| domain: | ||
| type: string | ||
| description: | | ||
| Domain to use as the destination for volume content requests, | ||
| replacing the default `api.<E2B_DOMAIN>`. Only returned when the | ||
| team is connected to a custom (BYOC) cluster; absent otherwise, in | ||
| which case the default domain is used. | ||
|
Comment on lines
+2064
to
+2067
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This contract redirects SDK volume-content traffic from the configured control-plane API host to the BYOC cluster domain, but AGENTS.md reference: AGENTS.md:L7-L9 Useful? React with 👍 / 👎. |
||
| required: | ||
| - volumeID | ||
| - name | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a team is assigned to a cluster whose
sandbox_proxy_domainis null—which remains valid underAdminClusterCreateRequestinspec/openapi-dashboard.yml:422-424—this returns(nil, nil), soomitemptyremovesdomainand the SDK falls back to the default control-plane host. Volume-content requests are then routed to the wrong cluster; return a configuration error here or make the domain mandatory for BYOC clusters.AGENTS.md reference: AGENTS.md:L22-L26
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is acceptable, it's deep into undefined behavior. If there's no client proxy domain, the whole cluster doesn't work, so this is the least of that cluster's problem.