From 553d4e0f20570de5f6b954fd441924167beebe6e Mon Sep 17 00:00:00 2001 From: Brad Ison Date: Thu, 6 Aug 2026 18:11:57 +0200 Subject: [PATCH] feat(agentnetwork): store the endpoint as domain + proxy address Replace the settings row's (cluster, subdomain) identity columns with (domain, proxy_address): domain is the endpoint hostname agents call, proxy_address the declared cluster address of the proxy serving it. The serving shape is the shape of the pin -- self-addressed (domain == proxy_address, a proxy dedicated to the account) or labeled (domain one label beneath a shared cluster's address) -- so no mode flag or config exists anywhere, and any mix of shapes coexists per account on one deployment. Bootstrap becomes an explicit POST /api/agent-network/settings taking exactly one of proxy_address (the server allocates an adjective-noun label beneath it) or endpoint (claimed verbatim, address-first). The identity fields leave the PUT schema entirely -- immutability by shape rather than by runtime rejection -- and provider create loses bootstrap_cluster and all settings side effects, which also retires two latent bugs: the dashboard's unsorted [0] free-domain pick making a permanent decision, and bootstrap failures swallowed at debug level inside a 200 provider create. The global unique index moves from the label to the full hostname -- the actual invariant. Labels may repeat across clusters again, and self-addressed rows (no label) cannot collide on an empty string. The reverse lookup becomes a point query on that index, deleting the clusterFromDomain suffix heuristic, and the synthesizer stamps ProxyCluster from the settings row as a field read. A pre-AutoMigrate migration backfills existing rows (domain = subdomain.cluster, proxy_address = cluster) and drops the legacy columns, failing loudly on rows with no identity to derive. Co-Authored-By: Claude Fable 5 --- .../handlers/budget_handler_test.go | 4 +- .../handlers/providers_handler.go | 7 +- .../agentnetwork/handlers/settings_handler.go | 47 ++- .../handlers/settings_handler_test.go | 174 +++++++---- .../internals/modules/agentnetwork/manager.go | 275 +++++++++++------- .../agentnetwork/provider_bootstrap_test.go | 134 --------- .../modules/agentnetwork/reconcile_test.go | 6 +- .../agentnetwork/settings_bootstrap_test.go | 225 ++++++++++++++ .../modules/agentnetwork/synthesizer.go | 64 ++-- .../modules/agentnetwork/synthesizer_test.go | 7 +- .../modules/agentnetwork/types/settings.go | 126 ++++++-- .../agentnetwork_budgetrule_realstack_test.go | 45 ++- .../server/agentnetwork_realstack_test.go | 5 +- .../migration/migration_agentnetwork.go | 97 ++++++ management/server/migration/migration_test.go | 82 ++++++ .../server/store/sql_store_agentnetwork.go | 54 +++- .../sql_store_agentnetwork_budgetrule_test.go | 6 +- management/server/store/store.go | 7 +- management/server/store/store_mock.go | 41 ++- shared/management/http/api/openapi.yml | 100 +++++-- shared/management/http/api/types.gen.go | 48 ++- 21 files changed, 1093 insertions(+), 461 deletions(-) delete mode 100644 management/internals/modules/agentnetwork/provider_bootstrap_test.go create mode 100644 management/internals/modules/agentnetwork/settings_bootstrap_test.go create mode 100644 management/server/migration/migration_agentnetwork.go diff --git a/management/internals/modules/agentnetwork/handlers/budget_handler_test.go b/management/internals/modules/agentnetwork/handlers/budget_handler_test.go index 4038761c5..3a7709461 100644 --- a/management/internals/modules/agentnetwork/handlers/budget_handler_test.go +++ b/management/internals/modules/agentnetwork/handlers/budget_handler_test.go @@ -102,8 +102,8 @@ func TestSettingsHandler_GetExposesCollectionToggles(t *testing.T) { require.NoError(t, f.store.SaveAgentNetworkSettings(context.Background(), &agentNetworkTypes.Settings{ AccountID: testAccountID, - Cluster: "eu.proxy.netbird.io", - Subdomain: "violet", + Domain: "violet.eu.proxy.netbird.io", + ProxyAddress: "eu.proxy.netbird.io", EnableLogCollection: true, EnablePromptCollection: true, RedactPii: false, diff --git a/management/internals/modules/agentnetwork/handlers/providers_handler.go b/management/internals/modules/agentnetwork/handlers/providers_handler.go index c05363101..0d8a44ca3 100644 --- a/management/internals/modules/agentnetwork/handlers/providers_handler.go +++ b/management/internals/modules/agentnetwork/handlers/providers_handler.go @@ -155,12 +155,7 @@ func (h *handler) createProvider(w http.ResponseWriter, r *http.Request) { provider := types.NewProvider(userAuth.AccountId) provider.FromAPIRequest(&req) - bootstrapCluster := "" - if req.BootstrapCluster != nil { - bootstrapCluster = *req.BootstrapCluster - } - - created, err := h.manager.CreateProvider(r.Context(), userAuth.UserId, provider, bootstrapCluster) + created, err := h.manager.CreateProvider(r.Context(), userAuth.UserId, provider) if err != nil { util.WriteError(r.Context(), err, w) return diff --git a/management/internals/modules/agentnetwork/handlers/settings_handler.go b/management/internals/modules/agentnetwork/handlers/settings_handler.go index 171750838..912bf7b36 100644 --- a/management/internals/modules/agentnetwork/handlers/settings_handler.go +++ b/management/internals/modules/agentnetwork/handlers/settings_handler.go @@ -12,15 +12,54 @@ import ( "github.com/netbirdio/netbird/shared/management/http/util" ) -// addSettingsEndpoints registers the Agent Network settings routes. The -// settings row is bootstrapped server-side on first provider create or on the -// first PUT carrying a cluster; GET reads it and PUT applies a partial update -// of the mutable collection toggles (cluster/subdomain stay immutable). +// addSettingsEndpoints registers the Agent Network settings routes. POST +// bootstraps the settings row, assigning the account's immutable endpoint; +// GET reads it (defaults with an empty endpoint before bootstrap) and PUT +// replaces the mutable collection toggles. The identity fields are not part +// of the PUT schema — immutability by shape, not by rejection. func (h *handler) addSettingsEndpoints(router *mux.Router) { router.HandleFunc("/agent-network/settings", h.getSettings).Methods("GET", "OPTIONS") + router.HandleFunc("/agent-network/settings", h.createSettings).Methods("POST", "OPTIONS") router.HandleFunc("/agent-network/settings", h.updateSettings).Methods("PUT", "OPTIONS") } +// createSettings bootstraps the account's settings row. Exactly one of +// proxy_address (labeled endpoint; the server allocates the label) and +// endpoint (self-addressed, claimed verbatim) must be provided; optional +// collection toggles ride along with defaults for omitted fields. +func (h *handler) createSettings(w http.ResponseWriter, r *http.Request) { + userAuth, err := nbcontext.GetUserAuthFromContext(r.Context()) + if err != nil { + util.WriteError(r.Context(), err, w) + return + } + + var req api.AgentNetworkSettingsCreateRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + util.WriteErrorResponse("couldn't parse JSON request", http.StatusBadRequest, w) + return + } + + settings := types.DefaultSettings(userAuth.AccountId) + settings.FromAPICreateRequest(&req) + + proxyAddress := "" + if req.ProxyAddress != nil { + proxyAddress = *req.ProxyAddress + } + endpoint := "" + if req.Endpoint != nil { + endpoint = *req.Endpoint + } + + created, err := h.manager.CreateSettings(r.Context(), userAuth.UserId, settings, proxyAddress, endpoint) + if err != nil { + util.WriteError(r.Context(), err, w) + return + } + util.WriteJSONObject(r.Context(), w, created.ToAPIResponse()) +} + // updateSettings replaces the mutable settings fields on the account's row. // A request carrying a cluster bootstraps the row when the account doesn't // have one yet. diff --git a/management/internals/modules/agentnetwork/handlers/settings_handler_test.go b/management/internals/modules/agentnetwork/handlers/settings_handler_test.go index 636ec5b26..deefbe2c6 100644 --- a/management/internals/modules/agentnetwork/handlers/settings_handler_test.go +++ b/management/internals/modules/agentnetwork/handlers/settings_handler_test.go @@ -3,6 +3,7 @@ package handlers import ( "encoding/json" "net/http" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -14,7 +15,7 @@ import ( // TestSettingsHandler_GetUnbootstrappedReturnsDefaults pins the settings-read // convention shared with the account and DNS settings endpoints: settings // always read as a JSON object. Before bootstrap that object carries the -// defaults with an empty cluster/subdomain/endpoint (the "not bootstrapped" +// defaults with an empty endpoint/proxy_address (the "not bootstrapped" // signal) and no timestamps — never a 404 and never the legacy null body. func TestSettingsHandler_GetUnbootstrappedReturnsDefaults(t *testing.T) { f := newAgentNetworkHandlerFixture(t) @@ -27,9 +28,9 @@ func TestSettingsHandler_GetUnbootstrappedReturnsDefaults(t *testing.T) { var got api.AgentNetworkSettings require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) - assert.Empty(t, got.Cluster, "cluster must be empty until bootstrapped") - assert.Empty(t, got.Subdomain, "subdomain must be empty until bootstrapped") - assert.Empty(t, got.Endpoint, "endpoint must be empty until bootstrapped, not a bare dot") + assert.Empty(t, got.Endpoint, "endpoint must be empty until bootstrapped") + assert.Empty(t, got.ProxyAddress, "proxy address must be empty until bootstrapped") + assert.False(t, got.Dedicated, "an unbootstrapped account has no serving shape") assert.True(t, got.EnableLogCollection, "defaults must show log collection on, matching bootstrap") assert.False(t, got.EnablePromptCollection, "defaults must show prompt collection off") assert.False(t, got.RedactPii, "defaults must show redaction off") @@ -39,56 +40,142 @@ func TestSettingsHandler_GetUnbootstrappedReturnsDefaults(t *testing.T) { assert.Nil(t, got.UpdatedAt, "no timestamps before a row exists") } -// TestSettingsHandler_PutBootstrapsWithCluster covers the settings-first -// bootstrap path: a PUT carrying a cluster on an unbootstrapped account -// creates the row (cluster pinned, subdomain assigned) and applies the -// mutable fields from the same request. -func TestSettingsHandler_PutBootstrapsWithCluster(t *testing.T) { +// TestSettingsHandler_PostBootstrapsLabeled covers the labeled bootstrap +// shape: a POST carrying a proxy_address allocates a label beneath it, so the +// endpoint hangs one label under the shared cluster's address and the pin is +// not dedicated. Toggles riding along apply; omitted ones keep defaults. +func TestSettingsHandler_PostBootstrapsLabeled(t *testing.T) { f := newAgentNetworkHandlerFixture(t) - rec := f.do(t, http.MethodPut, "/agent-network/settings", - `{"cluster": "eu.proxy.netbird.io", "enable_log_collection": true, "enable_prompt_collection": true, "redact_pii": false, "access_log_retention_days": 30}`) - require.Equal(t, http.StatusOK, rec.Code, "bootstrap PUT must succeed: %s", rec.Body.String()) + rec := f.do(t, http.MethodPost, "/agent-network/settings", + `{"proxy_address": "eu.proxy.netbird.io", "enable_prompt_collection": true, "access_log_retention_days": 14}`) + require.Equal(t, http.StatusOK, rec.Code, "bootstrap POST must succeed: %s", rec.Body.String()) var got api.AgentNetworkSettings require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) - assert.Equal(t, "eu.proxy.netbird.io", got.Cluster, "cluster must be pinned from the request") - assert.NotEmpty(t, got.Subdomain, "subdomain must be assigned at bootstrap") - assert.Equal(t, got.Subdomain+".eu.proxy.netbird.io", got.Endpoint, "endpoint must combine subdomain and cluster") - assert.True(t, got.EnableLogCollection, "toggle from the bootstrap request must apply") + assert.Equal(t, "eu.proxy.netbird.io", got.ProxyAddress, "proxy address must be pinned from the request") + require.NotEmpty(t, got.Endpoint, "endpoint must be allocated at bootstrap") + assert.True(t, strings.HasSuffix(got.Endpoint, ".eu.proxy.netbird.io"), + "labeled endpoint must hang off the proxy address: %s", got.Endpoint) + label := strings.TrimSuffix(got.Endpoint, ".eu.proxy.netbird.io") + assert.NotContains(t, label, ".", "the allocated label must be a single DNS label: %s", label) + assert.False(t, got.Dedicated, "a labeled pin is not dedicated") + assert.True(t, got.EnableLogCollection, "omitted toggle must keep its default") assert.True(t, got.EnablePromptCollection, "toggle from the bootstrap request must apply") require.NotNil(t, got.AccessLogRetentionDays) - assert.Equal(t, 30, *got.AccessLogRetentionDays, "retention from the bootstrap request must apply") + assert.Equal(t, 14, *got.AccessLogRetentionDays, "retention from the bootstrap request must apply") + assert.NotNil(t, got.CreatedAt, "a persisted row carries timestamps") // The row is now readable via GET. rec = f.do(t, http.MethodGet, "/agent-network/settings", "") require.Equal(t, http.StatusOK, rec.Code, "GET after bootstrap must succeed") + var read api.AgentNetworkSettings + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &read)) + assert.Equal(t, got.Endpoint, read.Endpoint, "GET must return the bootstrapped endpoint") } -// TestSettingsHandler_PutWithoutClusterOnUnbootstrapped pins that a PUT -// without a cluster cannot conjure a settings row out of nothing — there is -// no cluster to pin — and surfaces as 404 like the GET. -func TestSettingsHandler_PutWithoutClusterOnUnbootstrapped(t *testing.T) { +// TestSettingsHandler_PostBootstrapsSelfAddressed covers the dedicated shape: +// a POST carrying an endpoint claims the hostname verbatim, the proxy address +// equals it, and the pin reads as dedicated. The claim is legitimate before +// any proxy declares the address (address-first). +func TestSettingsHandler_PostBootstrapsSelfAddressed(t *testing.T) { + f := newAgentNetworkHandlerFixture(t) + + rec := f.do(t, http.MethodPost, "/agent-network/settings", + `{"endpoint": "Brave-Otter.Gateway.Example.com"}`) + require.Equal(t, http.StatusOK, rec.Code, "bootstrap POST must succeed: %s", rec.Body.String()) + + var got api.AgentNetworkSettings + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) + assert.Equal(t, "brave-otter.gateway.example.com", got.Endpoint, + "endpoint must be claimed verbatim, lowercased") + assert.Equal(t, got.Endpoint, got.ProxyAddress, "self-addressed: the proxy address is the endpoint") + assert.True(t, got.Dedicated, "a self-addressed pin is dedicated") + assert.True(t, got.EnableLogCollection, "omitted toggles must keep their defaults") +} + +// TestSettingsHandler_PostRequiresExactlyOneIdentityField pins the request +// contract: proxy_address and endpoint are mutually exclusive and one is +// required — both or neither is a validation error, not a guess. +func TestSettingsHandler_PostRequiresExactlyOneIdentityField(t *testing.T) { + f := newAgentNetworkHandlerFixture(t) + + rec := f.do(t, http.MethodPost, "/agent-network/settings", `{}`) + assert.Equal(t, http.StatusUnprocessableEntity, rec.Code, + "empty POST must be rejected: got %d body=%s", rec.Code, rec.Body.String()) + + rec = f.do(t, http.MethodPost, "/agent-network/settings", + `{"proxy_address": "eu.proxy.netbird.io", "endpoint": "brave-otter.gateway.example.com"}`) + assert.Equal(t, http.StatusUnprocessableEntity, rec.Code, + "POST with both identity fields must be rejected: got %d body=%s", rec.Code, rec.Body.String()) +} + +// TestSettingsHandler_PostRejectsMalformedHostnames pins per-write input +// validation: shapes canonicalization cannot repair — trailing dots, embedded +// whitespace, empty labels — are rejected with a validation error instead of +// landing in an immutable column. +func TestSettingsHandler_PostRejectsMalformedHostnames(t *testing.T) { + f := newAgentNetworkHandlerFixture(t) + + for name, body := range map[string]string{ + "trailing dot": `{"endpoint": "gateway.example.com."}`, + "leading dot": `{"endpoint": ".gateway.example.com"}`, + "inner whitespace": `{"endpoint": "gate way.example.com"}`, + "empty label": `{"proxy_address": "eu..proxy.netbird.io"}`, + } { + rec := f.do(t, http.MethodPost, "/agent-network/settings", body) + assert.Equal(t, http.StatusUnprocessableEntity, rec.Code, + "%s must be rejected: got %d body=%s", name, rec.Code, rec.Body.String()) + } +} + +// TestSettingsHandler_PostConflictsOnSecondBootstrap pins that bootstrap is a +// one-time create: a second POST returns 409 and leaves the row untouched. +func TestSettingsHandler_PostConflictsOnSecondBootstrap(t *testing.T) { + f := newAgentNetworkHandlerFixture(t) + + rec := f.do(t, http.MethodPost, "/agent-network/settings", `{"proxy_address": "eu.proxy.netbird.io"}`) + require.Equal(t, http.StatusOK, rec.Code, "first bootstrap must succeed: %s", rec.Body.String()) + var first api.AgentNetworkSettings + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &first)) + + rec = f.do(t, http.MethodPost, "/agent-network/settings", `{"proxy_address": "us.proxy.netbird.io"}`) + assert.Equal(t, http.StatusConflict, rec.Code, + "second bootstrap must 409: got %d body=%s", rec.Code, rec.Body.String()) + + rec = f.do(t, http.MethodGet, "/agent-network/settings", "") + require.Equal(t, http.StatusOK, rec.Code) + var got api.AgentNetworkSettings + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) + assert.Equal(t, first.Endpoint, got.Endpoint, "the original endpoint must survive the rejected bootstrap") + assert.Equal(t, first.ProxyAddress, got.ProxyAddress, "the original proxy address must survive") +} + +// TestSettingsHandler_PutBeforeBootstrapIs404 pins that a PUT cannot conjure a +// settings row out of nothing — bootstrap is the explicit POST — and the +// error points the caller there. +func TestSettingsHandler_PutBeforeBootstrapIs404(t *testing.T) { f := newAgentNetworkHandlerFixture(t) rec := f.do(t, http.MethodPut, "/agent-network/settings", `{"enable_log_collection": false, "enable_prompt_collection": false, "redact_pii": false}`) assert.Equal(t, http.StatusNotFound, rec.Code, - "cluster-less PUT on an unbootstrapped account must 404: got %d body=%s", rec.Code, rec.Body.String()) - assert.Contains(t, rec.Body.String(), "cluster", - "the error must point the caller at the bootstrap paths: %s", rec.Body.String()) + "PUT on an unbootstrapped account must 404: got %d body=%s", rec.Code, rec.Body.String()) + assert.Contains(t, rec.Body.String(), "/api/agent-network/settings", + "the error must point the caller at the bootstrap POST: %s", rec.Body.String()) } // TestSettingsHandler_PutReplacesMutableFields pins the update contract shared // with the other PUT endpoints: the request replaces every mutable field, so a // toggle absent from the JSON lands as its zero value rather than being -// preserved. Cluster and subdomain survive untouched. +// preserved. The identity fields are not part of the PUT schema at all, so +// the endpoint and proxy address survive updates by construction. func TestSettingsHandler_PutReplacesMutableFields(t *testing.T) { f := newAgentNetworkHandlerFixture(t) - rec := f.do(t, http.MethodPut, "/agent-network/settings", - `{"cluster": "eu.proxy.netbird.io", "enable_log_collection": true, "enable_prompt_collection": true, "redact_pii": true, "access_log_retention_days": 14}`) - require.Equal(t, http.StatusOK, rec.Code, "bootstrap PUT must succeed: %s", rec.Body.String()) + rec := f.do(t, http.MethodPost, "/agent-network/settings", + `{"proxy_address": "eu.proxy.netbird.io", "enable_prompt_collection": true, "redact_pii": true, "access_log_retention_days": 14}`) + require.Equal(t, http.StatusOK, rec.Code, "bootstrap POST must succeed: %s", rec.Body.String()) var before api.AgentNetworkSettings require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &before)) @@ -105,33 +192,6 @@ func TestSettingsHandler_PutReplacesMutableFields(t *testing.T) { require.NotNil(t, got.AccessLogRetentionDays) assert.Equal(t, 0, *got.AccessLogRetentionDays, "retention absent from the request must land as the zero value — PUT replaces all mutable fields") - assert.Equal(t, before.Cluster, got.Cluster, "cluster must survive updates untouched") - assert.Equal(t, before.Subdomain, got.Subdomain, "subdomain must survive updates untouched") -} - -// TestSettingsHandler_PutRejectsClusterChange pins cluster immutability: once -// assigned, a differing cluster is rejected as a validation error instead of -// being silently ignored, so callers never observe a value other than the one -// they sent. Echoing the assigned cluster back stays valid, which lets -// declarative clients send their full desired state idempotently. -func TestSettingsHandler_PutRejectsClusterChange(t *testing.T) { - f := newAgentNetworkHandlerFixture(t) - - rec := f.do(t, http.MethodPut, "/agent-network/settings", - `{"cluster": "eu.proxy.netbird.io", "enable_log_collection": true, "enable_prompt_collection": false, "redact_pii": false}`) - require.Equal(t, http.StatusOK, rec.Code, "bootstrap PUT must succeed: %s", rec.Body.String()) - - rec = f.do(t, http.MethodPut, "/agent-network/settings", - `{"cluster": "us.proxy.netbird.io", "enable_log_collection": true, "enable_prompt_collection": false, "redact_pii": false}`) - assert.Equal(t, http.StatusUnprocessableEntity, rec.Code, - "cluster change must be rejected as a validation error: got %d body=%s", rec.Code, rec.Body.String()) - - rec = f.do(t, http.MethodPut, "/agent-network/settings", - `{"cluster": "eu.proxy.netbird.io", "enable_log_collection": true, "enable_prompt_collection": false, "redact_pii": true}`) - require.Equal(t, http.StatusOK, rec.Code, "echoing the assigned cluster must stay valid: %s", rec.Body.String()) - - var got api.AgentNetworkSettings - require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) - assert.Equal(t, "eu.proxy.netbird.io", got.Cluster, "cluster must be unchanged") - assert.True(t, got.RedactPii, "toggle sent alongside the echoed cluster must apply") + assert.Equal(t, before.Endpoint, got.Endpoint, "endpoint must survive updates untouched") + assert.Equal(t, before.ProxyAddress, got.ProxyAddress, "proxy address must survive updates untouched") } diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go index 8c43d5748..62f38822e 100644 --- a/management/internals/modules/agentnetwork/manager.go +++ b/management/internals/modules/agentnetwork/manager.go @@ -47,7 +47,7 @@ func ensureSessionKeys(p *types.Provider) error { type Manager interface { GetAllProviders(ctx context.Context, accountID, userID string) ([]*types.Provider, error) GetProvider(ctx context.Context, accountID, userID, providerID string) (*types.Provider, error) - CreateProvider(ctx context.Context, userID string, provider *types.Provider, bootstrapCluster string) (*types.Provider, error) + CreateProvider(ctx context.Context, userID string, provider *types.Provider) (*types.Provider, error) UpdateProvider(ctx context.Context, userID string, provider *types.Provider) (*types.Provider, error) DeleteProvider(ctx context.Context, accountID, userID, providerID string) error @@ -70,6 +70,7 @@ type Manager interface { DeleteBudgetRule(ctx context.Context, accountID, userID, ruleID string) error GetSettings(ctx context.Context, accountID, userID string) (*types.Settings, error) + CreateSettings(ctx context.Context, userID string, settings *types.Settings, proxyAddress, endpoint string) (*types.Settings, error) UpdateSettings(ctx context.Context, userID string, settings *types.Settings) (*types.Settings, error) ListConsumption(ctx context.Context, accountID, userID string) ([]*types.Consumption, error) @@ -168,19 +169,14 @@ func (m *managerImpl) GetProvider(ctx context.Context, accountID, userID, provid return m.store.GetAgentNetworkProviderByID(ctx, store.LockingStrengthNone, accountID, providerID) } -// CreateProvider persists a new provider for the account. bootstrapCluster -// is used only when the per-account agent-network Settings row hasn't -// been created yet; otherwise it is ignored (the cluster is pinned on -// Settings and every provider in the account routes through it). -func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provider *types.Provider, bootstrapCluster string) (*types.Provider, error) { +// CreateProvider persists a new provider for the account. Providers have no +// settings side effects: the account's endpoint is bootstrapped separately and +// explicitly via CreateSettings, and every provider in the account routes +// through it. +func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provider *types.Provider) (*types.Provider, error) { if err := m.requirePermission(ctx, provider.AccountID, userID, modules.AgentNetworkProviders, operations.Create); err != nil { return nil, err } - if strings.TrimSpace(bootstrapCluster) != "" { - if err := m.requireSettingsBootstrapPermission(ctx, provider.AccountID, userID); err != nil { - return nil, err - } - } // An empty api_key would silently produce a synthesised service // that 401s on every upstream request. Surface the misconfiguration @@ -204,16 +200,6 @@ func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provide return nil, fmt.Errorf("save agent network provider: %w", err) } - if strings.TrimSpace(bootstrapCluster) != "" { - if _, err := m.bootstrapSettingsIfNeeded(ctx, m.store, provider.AccountID, bootstrapCluster); err != nil { - // The provider create has already succeeded; logging the - // bootstrap miss matches the plan's PoC behaviour. The synth - // path treats a missing settings row as a no-op, and the next - // provider create retries the bootstrap. - log.WithContext(ctx).Debugf("agent-network bootstrap settings for account %s on cluster %s: %v", provider.AccountID, bootstrapCluster, err) - } - } - m.accountManager.StoreEvent(ctx, userID, provider.ID, provider.AccountID, activity.AgentNetworkProviderCreated, provider.EventMeta()) m.reconcile(ctx, provider.AccountID) @@ -558,48 +544,28 @@ func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, r } // UpdateSettings replaces the mutable account-level settings — the collection -// toggles and retention — on the account's row. When the account has no -// settings row yet, a non-empty settings.Cluster bootstraps one (same path as -// first provider create); without it the update fails with NotFound. On an -// existing row the cluster and subdomain are immutable: a differing -// settings.Cluster is rejected rather than silently ignored so callers never -// observe a value other than what they sent. Because the collection toggles -// change the synthesised service config (prompt-capture gating, access-log -// emission), a reconcile is triggered so the proxy and peer network maps -// converge on the new state. +// toggles and retention — on the account's row. The identity fields (Domain, +// ProxyAddress) are assigned at bootstrap (CreateSettings) and are not part of +// the update surface at all; when the account has no settings row yet the +// update fails with NotFound. Because the collection toggles change the +// synthesised service config (prompt-capture gating, access-log emission), a +// reconcile is triggered so the proxy and peer network maps converge on the +// new state. func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, settings *types.Settings) (*types.Settings, error) { if err := m.requirePermission(ctx, settings.AccountID, userID, modules.AgentNetworkSettings, operations.Update); err != nil { return nil, err } - requestedCluster := strings.TrimSpace(settings.Cluster) - // The row lock from LockingStrengthUpdate only holds for the duration of - // the surrounding transaction, so the read, the cluster-immutability - // check, and the save must share one — otherwise concurrent PUTs could - // interleave between them. + // the surrounding transaction, so the read and the save must share one — + // otherwise concurrent PUTs could interleave between them. var updated *types.Settings err := m.store.ExecuteInTransaction(ctx, func(tx store.Store) error { existing, err := tx.GetAgentNetworkSettings(ctx, store.LockingStrengthUpdate, settings.AccountID) switch { case err == nil: - if requestedCluster != "" && requestedCluster != existing.Cluster { - return status.Errorf(status.InvalidArgument, "cluster is immutable once assigned (current: %s)", existing.Cluster) - } case isNotFound(err): - if requestedCluster == "" { - return status.Errorf(status.NotFound, "agent network settings have not been bootstrapped yet; pass cluster to bootstrap them, or create a provider with bootstrap_cluster set") - } - // Bootstrapping pins the cluster and subdomain — a settings - // create on top of the update the caller already passed, matching - // the gate on the provider-create bootstrap path. - if err := m.requirePermission(ctx, settings.AccountID, userID, modules.AgentNetworkSettings, operations.Create); err != nil { - return err - } - existing, err = m.bootstrapSettingsIfNeeded(ctx, tx, settings.AccountID, requestedCluster) - if err != nil { - return err - } + return status.Errorf(status.NotFound, "agent network settings have not been bootstrapped yet; POST /api/agent-network/settings to bootstrap them") default: return fmt.Errorf("get agent network settings: %w", err) } @@ -676,74 +642,162 @@ func (m *managerImpl) GetSettings(ctx context.Context, accountID, userID string) } } -// requireSettingsBootstrapPermission gates the one-time settings bootstrap a -// first provider create performs. Pinning the account's cluster and subdomain -// is a settings write, so it needs the settings permission on top of the -// provider one. No-op once the settings row exists. -func (m *managerImpl) requireSettingsBootstrapPermission(ctx context.Context, accountID, userID string) error { - _, err := m.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID) - if err == nil { - return nil - } - if !isNotFound(err) { - return fmt.Errorf("get agent network settings: %w", err) - } - return m.requirePermission(ctx, accountID, userID, modules.AgentNetworkSettings, operations.Create) -} +// maxDomainAllocationAttempts bounds the label search when bootstrapping a +// labeled endpoint. Package-level (rather than function-local) so tests can +// assert on the exhaustion path without duplicating the literal. +const maxDomainAllocationAttempts = 10 -// bootstrapSettingsIfNeeded creates the per-account agent-network -// settings row when missing. The cluster comes from the create-time -// hint the dashboard sends (auto-picked from the active cluster list); -// the subdomain is picked from the curated wordlist avoiding -// collisions on the same cluster. Idempotent: if a row already exists -// it is returned untouched and the hint is ignored. st is the store to -// operate on — pass the transaction store when calling from within one. -func (m *managerImpl) bootstrapSettingsIfNeeded(ctx context.Context, st store.Store, accountID, providerCluster string) (*types.Settings, error) { - if accountID == "" { - return nil, fmt.Errorf("bootstrap settings: account id is required") +// CreateSettings bootstraps the per-account settings row, assigning the +// account's immutable endpoint. Exactly one of proxyAddress and endpoint must +// be non-empty: proxyAddress allocates a labeled endpoint one label beneath +// the given cluster address; endpoint claims the given hostname verbatim as a +// self-addressed (dedicated) endpoint — a legitimate claim before any proxy +// declares the address (address-first). settings carries the account ID and +// the initial collection toggles; its identity fields are assigned here. +func (m *managerImpl) CreateSettings(ctx context.Context, userID string, settings *types.Settings, proxyAddress, endpoint string) (*types.Settings, error) { + if settings == nil || settings.AccountID == "" { + return nil, status.Errorf(status.InvalidArgument, "account id is required") } - if strings.TrimSpace(providerCluster) == "" { - return nil, fmt.Errorf("bootstrap settings: provider cluster is required") + if err := m.requirePermission(ctx, settings.AccountID, userID, modules.AgentNetworkSettings, operations.Create); err != nil { + return nil, err } - existing, err := st.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID) - if err == nil { - return existing, nil + hasProxyAddress := strings.TrimSpace(proxyAddress) != "" + hasEndpoint := strings.TrimSpace(endpoint) != "" + if hasProxyAddress == hasEndpoint { + return nil, status.Errorf(status.InvalidArgument, "exactly one of proxy_address and endpoint is required") } - if !isNotFound(err) { + + // Fail fast on an existing row for a clean 409; the insert below stays + // the authority against concurrent bootstraps (the primary key wins). + if _, err := m.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, settings.AccountID); err == nil { + return nil, status.Errorf(status.AlreadyExists, "agent network settings already bootstrapped for account %s", settings.AccountID) + } else if !isNotFound(err) { return nil, fmt.Errorf("get agent network settings: %w", err) } - siblings, err := st.GetAgentNetworkSettingsByCluster(ctx, store.LockingStrengthNone, providerCluster) - if err != nil { - return nil, fmt.Errorf("list agent network settings on cluster: %w", err) - } - taken := make(map[string]struct{}, len(siblings)) - for _, s := range siblings { - taken[s.Subdomain] = struct{}{} - } - - suffix := accountID - if len(suffix) > 4 { - suffix = suffix[:4] - } - - m.labelRngMu.Lock() - subdomain := labelgen.PickUnique(m.labelRng, taken, suffix) - m.labelRngMu.Unlock() - now := time.Now().UTC() - settings := types.DefaultSettings(accountID) - settings.Cluster = providerCluster - settings.Subdomain = subdomain settings.CreatedAt = now settings.UpdatedAt = now - if err := st.SaveAgentNetworkSettings(ctx, settings); err != nil { - return nil, fmt.Errorf("save agent network settings: %w", err) + + var err error + if hasEndpoint { + err = m.bootstrapSelfAddressed(ctx, settings, endpoint) + } else { + err = m.bootstrapLabeled(ctx, settings, proxyAddress) } + if err != nil { + return nil, err + } + + m.accountManager.StoreEvent(ctx, userID, settings.AccountID, settings.AccountID, activity.AgentNetworkSettingsUpdated, map[string]any{ + "bootstrapped": true, + "endpoint": settings.Domain, + "dedicated": settings.Dedicated(), + }) + m.reconcile(ctx, settings.AccountID) + return settings, nil } +// bootstrapSelfAddressed claims the given hostname as the account's endpoint, +// served only by a proxy declaring exactly that address (Domain == +// ProxyAddress). The domain unique index is the arbiter of availability. +func (m *managerImpl) bootstrapSelfAddressed(ctx context.Context, settings *types.Settings, endpoint string) error { + hostname, err := types.NormalizeHostname(endpoint) + if err != nil { + return status.Errorf(status.InvalidArgument, "invalid endpoint: %s", err) + } + + settings.Domain = hostname + settings.ProxyAddress = hostname + if err := m.store.CreateAgentNetworkSettings(ctx, settings); err != nil { + if isUniqueConstraintError(err) { + // The violation is either the account primary key (a concurrent + // bootstrap for the same account won) or the domain index + // (another account holds the hostname). Distinguish by re-read. + if _, getErr := m.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, settings.AccountID); getErr == nil { + return status.Errorf(status.AlreadyExists, "agent network settings already bootstrapped for account %s", settings.AccountID) + } + return status.Errorf(status.AlreadyExists, "endpoint %s is already taken", hostname) + } + return fmt.Errorf("create agent network settings: %w", err) + } + return nil +} + +// bootstrapLabeled allocates a labeled endpoint one label beneath the given +// cluster address: Domain =