mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-02 13:51:28 +02:00
[management] Fix agent-network settings and provider update contracts
Declarative API clients (the Terraform provider, scripted PUTs) hit three places where the agent-network API broke its own contract or left users stuck, forcing client-side workarounds. The settings GET answered 200 with a JSON null body for unbootstrapped accounts while the OpenAPI spec documented 404; it now returns the 404. The settings PUT still replaces every mutable field like the other PUT endpoints, but a request may now carry the cluster: on an account without a settings row it bootstraps one (assigning the subdomain), so settings can be managed before the first provider exists; on a bootstrapped account a differing cluster is rejected instead of silently ignored. The provider PUT handler built a fresh row from the request, so the fields the schema documents as omit-preserves (extra_values, metadata_disabled, enabled, skip_tls_verification, identity headers) silently reset to zero when omitted. The handler now overlays the request onto the stored row, making the documented nil-gating real. Identity headers are always present in provider responses so an explicitly cleared header round-trips as an empty string rather than vanishing.
This commit is contained in:
@@ -160,8 +160,19 @@ func TestSettingsRoundTrip(t *testing.T) {
|
||||
assert.Equal(t, before.Cluster, flipped.Cluster, "cluster must be immutable across updates")
|
||||
assert.Equal(t, before.Subdomain, flipped.Subdomain, "subdomain must be immutable across updates")
|
||||
|
||||
// A cluster different from the pinned one must be rejected; echoing the
|
||||
// pinned one back is valid.
|
||||
_, err = srv.UpdateSettings(ctx, api.AgentNetworkSettingsRequest{
|
||||
Cluster: ptr("attacker.cluster.invalid"),
|
||||
EnableLogCollection: before.EnableLogCollection,
|
||||
EnablePromptCollection: before.EnablePromptCollection,
|
||||
RedactPii: before.RedactPii,
|
||||
})
|
||||
requireClientError(t, err)
|
||||
|
||||
// Restore the original toggles.
|
||||
_, err = srv.UpdateSettings(ctx, api.AgentNetworkSettingsRequest{
|
||||
Cluster: ptr(before.Cluster),
|
||||
EnableLogCollection: before.EnableLogCollection,
|
||||
EnablePromptCollection: before.EnablePromptCollection,
|
||||
RedactPii: before.RedactPii,
|
||||
|
||||
@@ -17,6 +17,7 @@ import (
|
||||
|
||||
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
|
||||
agentNetworkTypes "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types"
|
||||
"github.com/netbirdio/netbird/management/server/account"
|
||||
nbcontext "github.com/netbirdio/netbird/management/server/context"
|
||||
"github.com/netbirdio/netbird/management/server/permissions"
|
||||
"github.com/netbirdio/netbird/management/server/store"
|
||||
@@ -61,10 +62,23 @@ func newAgentNetworkHandlerFixture(t *testing.T) *agentNetworkHandlerFixture {
|
||||
Return(true, context.Background(), nil).
|
||||
AnyTimes()
|
||||
|
||||
manager := agentnetwork.NewManager(st, perms, nil, nil)
|
||||
// Swallow activity events so the mutation paths (create/update/delete)
|
||||
// are exercisable through the HTTP layer.
|
||||
accounts := account.NewMockManager(ctrl)
|
||||
accounts.EXPECT().
|
||||
StoreEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
|
||||
AnyTimes()
|
||||
accounts.EXPECT().
|
||||
UpdateAccountPeers(gomock.Any(), gomock.Any(), gomock.Any()).
|
||||
AnyTimes()
|
||||
|
||||
manager := agentnetwork.NewManager(st, perms, accounts, nil)
|
||||
h := &handler{manager: manager}
|
||||
|
||||
router := mux.NewRouter()
|
||||
router.HandleFunc("/agent-network/providers", h.createProvider).Methods("POST")
|
||||
router.HandleFunc("/agent-network/providers/{providerId}", h.getProvider).Methods("GET")
|
||||
router.HandleFunc("/agent-network/providers/{providerId}", h.updateProvider).Methods("PUT")
|
||||
h.addPolicyEndpoints(router)
|
||||
h.addConsumptionEndpoints(router)
|
||||
h.addBudgetRuleEndpoints(router)
|
||||
|
||||
@@ -193,10 +193,19 @@ func (h *handler) updateProvider(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
provider := &types.Provider{
|
||||
ID: providerID,
|
||||
AccountID: userAuth.AccountId,
|
||||
// Overlay the request onto the stored row so omitted optional fields
|
||||
// (models, extra_values, toggles, identity headers) keep their persisted
|
||||
// values instead of silently resetting to zero — FromAPIRequest's
|
||||
// nil-gating only works against the existing state.
|
||||
existing, err := h.manager.GetProvider(r.Context(), userAuth.AccountId, userAuth.UserId, providerID)
|
||||
if err != nil {
|
||||
util.WriteError(r.Context(), err, w)
|
||||
return
|
||||
}
|
||||
provider := existing.Copy()
|
||||
// Blank the key so it is set only when the caller rotates it; the manager
|
||||
// preserves the stored key for an empty value.
|
||||
provider.APIKey = ""
|
||||
provider.FromAPIRequest(&req)
|
||||
|
||||
updated, err := h.manager.UpdateProvider(r.Context(), userAuth.UserId, provider)
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"math"
|
||||
nethttp "net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -51,3 +53,57 @@ func TestValidate_ModelRates(t *testing.T) {
|
||||
assert.Error(t, validate(base(m), true), "case %q must be rejected", name)
|
||||
}
|
||||
}
|
||||
|
||||
// TestProviderHandler_UpdatePreservesOmittedFields is the anti-clobber guard
|
||||
// for PUT /agent-network/providers/{id}: the fields the OpenAPI schema
|
||||
// documents as omit-preserves — extra_values, metadata_disabled, identity
|
||||
// headers, enabled, skip_tls_verification, api_key — must keep their stored
|
||||
// values when absent from the request. Before the handler merged onto the
|
||||
// existing row, such an update silently wiped dashboard-configured
|
||||
// extra_values and reset the toggles account-wide. Models carry no such
|
||||
// contract: like the rest of the PUT payload they are replaced with what the
|
||||
// request says, so an omitted list clears.
|
||||
func TestProviderHandler_UpdatePreservesOmittedFields(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
|
||||
create := `{
|
||||
"provider_id": "openai_api",
|
||||
"name": "openai",
|
||||
"upstream_url": "https://api.openai.com",
|
||||
"api_key": "sk-test",
|
||||
"enabled": true,
|
||||
"metadata_disabled": true,
|
||||
"skip_tls_verification": true,
|
||||
"extra_values": {"x-portkey-config": "pc-prod-3f2a"},
|
||||
"identity_header_user_id": "x-bf-dim-netbird_user_id",
|
||||
"models": [{"id": "gpt-4o", "input_per_1k": 0.0025, "output_per_1k": 0.01}]
|
||||
}`
|
||||
rec := f.do(t, nethttp.MethodPost, "/agent-network/providers", create)
|
||||
require.Equal(t, nethttp.StatusOK, rec.Code, "create must succeed: %s", rec.Body.String())
|
||||
|
||||
var created api.AgentNetworkProvider
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &created))
|
||||
|
||||
// Minimal update: only the required fields (a rename), nothing optional.
|
||||
update := `{"provider_id": "openai_api", "name": "openai-renamed", "upstream_url": "https://api.openai.com"}`
|
||||
rec = f.do(t, nethttp.MethodPut, "/agent-network/providers/"+created.Id, update)
|
||||
require.Equal(t, nethttp.StatusOK, rec.Code, "update must succeed: %s", rec.Body.String())
|
||||
|
||||
var updated api.AgentNetworkProvider
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &updated))
|
||||
assert.Equal(t, "openai-renamed", updated.Name, "sent field must apply")
|
||||
assert.True(t, updated.MetadataDisabled, "omitted metadata_disabled must be preserved")
|
||||
assert.True(t, updated.SkipTlsVerification, "omitted skip_tls_verification must be preserved")
|
||||
assert.True(t, updated.Enabled, "omitted enabled must be preserved")
|
||||
require.NotNil(t, updated.ExtraValues, "omitted extra_values must be preserved")
|
||||
assert.Equal(t, "pc-prod-3f2a", (*updated.ExtraValues)["x-portkey-config"], "stored extra value must survive the update")
|
||||
assert.Equal(t, "x-bf-dim-netbird_user_id", updated.IdentityHeaderUserId, "omitted identity header must be preserved")
|
||||
assert.Empty(t, updated.Models, "models carry no omit-preserves contract: an omitted list is replaced with empty")
|
||||
|
||||
// An explicit "" clears the identity header and stays on the wire.
|
||||
clearHeader := `{"provider_id": "openai_api", "name": "openai-renamed", "upstream_url": "https://api.openai.com", "identity_header_user_id": ""}`
|
||||
rec = f.do(t, nethttp.MethodPut, "/agent-network/providers/"+created.Id, clearHeader)
|
||||
require.Equal(t, nethttp.StatusOK, rec.Code, "clearing update must succeed: %s", rec.Body.String())
|
||||
assert.Contains(t, rec.Body.String(), `"identity_header_user_id":""`,
|
||||
"cleared identity header must round-trip as an explicit empty string")
|
||||
}
|
||||
|
||||
@@ -2,7 +2,6 @@ package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
|
||||
"github.com/gorilla/mux"
|
||||
@@ -11,19 +10,20 @@ import (
|
||||
nbcontext "github.com/netbirdio/netbird/management/server/context"
|
||||
"github.com/netbirdio/netbird/shared/management/http/api"
|
||||
"github.com/netbirdio/netbird/shared/management/http/util"
|
||||
"github.com/netbirdio/netbird/shared/management/status"
|
||||
)
|
||||
|
||||
// addSettingsEndpoints registers the Agent Network settings routes. The
|
||||
// settings row is bootstrapped server-side on first provider create; GET reads
|
||||
// it and PUT updates the mutable collection toggles (cluster/subdomain stay
|
||||
// immutable).
|
||||
// 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).
|
||||
func (h *handler) addSettingsEndpoints(router *mux.Router) {
|
||||
router.HandleFunc("/agent-network/settings", h.getSettings).Methods("GET", "OPTIONS")
|
||||
router.HandleFunc("/agent-network/settings", h.updateSettings).Methods("PUT", "OPTIONS")
|
||||
}
|
||||
|
||||
// updateSettings applies the collection toggles to the account's settings row.
|
||||
// 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.
|
||||
func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
|
||||
userAuth, err := nbcontext.GetUserAuthFromContext(r.Context())
|
||||
if err != nil {
|
||||
@@ -48,11 +48,10 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
|
||||
util.WriteJSONObject(r.Context(), w, updated.ToAPIResponse())
|
||||
}
|
||||
|
||||
// getSettings returns the account's agent-network settings. The settings
|
||||
// row is bootstrapped on first provider create, so freshly-onboarded
|
||||
// accounts have nothing to read. Rather than 404-ing in that case (which
|
||||
// the dashboard would have to special-case), return a JSON null with 200
|
||||
// so consumers can branch on the body alone.
|
||||
// getSettings returns the account's agent-network settings. Freshly-onboarded
|
||||
// accounts have no settings row until a provider create (or a settings PUT
|
||||
// with a cluster) bootstraps one; per the OpenAPI contract that reads as a
|
||||
// plain 404.
|
||||
func (h *handler) getSettings(w http.ResponseWriter, r *http.Request) {
|
||||
userAuth, err := nbcontext.GetUserAuthFromContext(r.Context())
|
||||
if err != nil {
|
||||
@@ -62,11 +61,6 @@ func (h *handler) getSettings(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
settings, err := h.manager.GetSettings(r.Context(), userAuth.AccountId, userAuth.UserId)
|
||||
if err != nil {
|
||||
var sErr *status.Error
|
||||
if errors.As(err, &sErr) && sErr.Type() == status.NotFound {
|
||||
util.WriteJSONObject(r.Context(), w, nil)
|
||||
return
|
||||
}
|
||||
util.WriteError(r.Context(), err, w)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/netbirdio/netbird/shared/management/http/api"
|
||||
)
|
||||
|
||||
// TestSettingsHandler_GetUnbootstrappedReturns404 pins the OpenAPI contract:
|
||||
// an account with no settings row answers a plain 404, never 200 with a
|
||||
// null/zero body, so API clients can rely on the status code alone.
|
||||
func TestSettingsHandler_GetUnbootstrappedReturns404(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
|
||||
rec := f.do(t, http.MethodGet, "/agent-network/settings", "")
|
||||
assert.Equal(t, http.StatusNotFound, rec.Code,
|
||||
"unbootstrapped account must read as 404: got %d body=%s", rec.Code, rec.Body.String())
|
||||
assert.NotEqual(t, "null", trimSpace(rec.Body.String()),
|
||||
"the legacy 200+null shape must not come back")
|
||||
}
|
||||
|
||||
// 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) {
|
||||
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())
|
||||
|
||||
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.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")
|
||||
|
||||
// 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")
|
||||
}
|
||||
|
||||
// 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) {
|
||||
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())
|
||||
}
|
||||
|
||||
// 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.
|
||||
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())
|
||||
|
||||
var before api.AgentNetworkSettings
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &before))
|
||||
|
||||
rec = f.do(t, http.MethodPut, "/agent-network/settings",
|
||||
`{"enable_log_collection": true, "enable_prompt_collection": false, "redact_pii": false}`)
|
||||
require.Equal(t, http.StatusOK, rec.Code, "update PUT must succeed: %s", rec.Body.String())
|
||||
|
||||
var got api.AgentNetworkSettings
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got))
|
||||
assert.True(t, got.EnableLogCollection, "sent toggle must apply")
|
||||
assert.False(t, got.EnablePromptCollection, "sent toggle must apply")
|
||||
assert.False(t, got.RedactPii, "sent toggle must apply")
|
||||
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")
|
||||
}
|
||||
@@ -554,19 +554,38 @@ func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, r
|
||||
return nil
|
||||
}
|
||||
|
||||
// UpdateSettings applies the mutable account-level settings — the collection
|
||||
// toggles — onto the existing row. Cluster and Subdomain are immutable and are
|
||||
// preserved from the persisted row regardless of the input. 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.
|
||||
// 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.
|
||||
func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, settings *types.Settings) (*types.Settings, error) {
|
||||
if err := m.requirePermission(ctx, settings.AccountID, userID, operations.Update); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
requestedCluster := strings.TrimSpace(settings.Cluster)
|
||||
|
||||
existing, err := m.store.GetAgentNetworkSettings(ctx, store.LockingStrengthUpdate, settings.AccountID)
|
||||
if err != nil {
|
||||
switch {
|
||||
case err == nil:
|
||||
if requestedCluster != "" && requestedCluster != existing.Cluster {
|
||||
return nil, status.Errorf(status.InvalidArgument, "cluster is immutable once assigned (current: %s)", existing.Cluster)
|
||||
}
|
||||
case isNotFound(err):
|
||||
if requestedCluster == "" {
|
||||
return nil, 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")
|
||||
}
|
||||
existing, err = m.bootstrapSettingsIfNeeded(ctx, settings.AccountID, requestedCluster)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
default:
|
||||
return nil, fmt.Errorf("get agent network settings: %w", err)
|
||||
}
|
||||
|
||||
@@ -590,6 +609,12 @@ func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, setting
|
||||
return existing, nil
|
||||
}
|
||||
|
||||
// isNotFound reports whether err is a status.NotFound error.
|
||||
func isNotFound(err error) bool {
|
||||
var sErr *status.Error
|
||||
return errors.As(err, &sErr) && sErr.Type() == status.NotFound
|
||||
}
|
||||
|
||||
// validateProviderRefs ensures every destination provider id refers to a
|
||||
// provider that exists in the same account.
|
||||
func (m *managerImpl) validateProviderRefs(ctx context.Context, accountID string, providerIDs []string) error {
|
||||
|
||||
@@ -192,16 +192,20 @@ func (p *Provider) ToAPIResponse() *api.AgentNetworkProvider {
|
||||
created := p.CreatedAt
|
||||
updated := p.UpdatedAt
|
||||
resp := &api.AgentNetworkProvider{
|
||||
Id: p.ID,
|
||||
ProviderId: p.ProviderID,
|
||||
Name: p.Name,
|
||||
UpstreamUrl: p.UpstreamURL,
|
||||
Models: models,
|
||||
Enabled: p.Enabled,
|
||||
SkipTlsVerification: p.SkipTLSVerification,
|
||||
MetadataDisabled: p.MetadataDisabled,
|
||||
CreatedAt: &created,
|
||||
UpdatedAt: &updated,
|
||||
Id: p.ID,
|
||||
ProviderId: p.ProviderID,
|
||||
Name: p.Name,
|
||||
UpstreamUrl: p.UpstreamURL,
|
||||
Models: models,
|
||||
// Always present on the wire so an explicitly cleared header
|
||||
// round-trips as "" instead of vanishing from the response.
|
||||
IdentityHeaderUserId: p.IdentityHeaderUserID,
|
||||
IdentityHeaderGroups: p.IdentityHeaderGroups,
|
||||
Enabled: p.Enabled,
|
||||
SkipTlsVerification: p.SkipTLSVerification,
|
||||
MetadataDisabled: p.MetadataDisabled,
|
||||
CreatedAt: &created,
|
||||
UpdatedAt: &updated,
|
||||
}
|
||||
if len(p.ExtraValues) > 0 {
|
||||
out := make(map[string]string, len(p.ExtraValues))
|
||||
@@ -210,14 +214,6 @@ func (p *Provider) ToAPIResponse() *api.AgentNetworkProvider {
|
||||
}
|
||||
resp.ExtraValues = &out
|
||||
}
|
||||
if p.IdentityHeaderUserID != "" {
|
||||
v := p.IdentityHeaderUserID
|
||||
resp.IdentityHeaderUserId = &v
|
||||
}
|
||||
if p.IdentityHeaderGroups != "" {
|
||||
v := p.IdentityHeaderGroups
|
||||
resp.IdentityHeaderGroups = &v
|
||||
}
|
||||
return resp
|
||||
}
|
||||
|
||||
|
||||
@@ -77,3 +77,41 @@ func TestProvider_MetadataDisabled_RoundTrip(t *testing.T) {
|
||||
assert.False(t, p.MetadataDisabled, "explicit false must clear metadata_disabled")
|
||||
assert.False(t, p.ToAPIResponse().MetadataDisabled, "response must reflect the cleared value")
|
||||
}
|
||||
|
||||
// TestProvider_IdentityHeaders_AlwaysOnWire pins that the identity header
|
||||
// fields are always present in the API response — an explicitly cleared
|
||||
// ("") header must round-trip as "" rather than vanish, so API consumers
|
||||
// (e.g. the Terraform provider) never observe a value other than the one
|
||||
// they wrote.
|
||||
func TestProvider_IdentityHeaders_AlwaysOnWire(t *testing.T) {
|
||||
set := "x-bf-dim-netbird_user_id"
|
||||
empty := ""
|
||||
|
||||
base := func() *api.AgentNetworkProviderRequest {
|
||||
return &api.AgentNetworkProviderRequest{
|
||||
ProviderId: "custom",
|
||||
Name: "bifrost",
|
||||
UpstreamUrl: "https://bifrost.internal",
|
||||
}
|
||||
}
|
||||
|
||||
p := NewProvider("acc-1")
|
||||
resp := p.ToAPIResponse()
|
||||
assert.Equal(t, "", resp.IdentityHeaderUserId, "unset header must surface as empty string, not be omitted")
|
||||
assert.Equal(t, "", resp.IdentityHeaderGroups, "unset header must surface as empty string, not be omitted")
|
||||
|
||||
req := base()
|
||||
req.IdentityHeaderUserId = &set
|
||||
p.FromAPIRequest(req)
|
||||
assert.Equal(t, set, p.ToAPIResponse().IdentityHeaderUserId, "configured header must round-trip")
|
||||
|
||||
// Omitting the field preserves it.
|
||||
p.FromAPIRequest(base())
|
||||
assert.Equal(t, set, p.ToAPIResponse().IdentityHeaderUserId, "omitted header must preserve the stored value")
|
||||
|
||||
// An explicit "" clears it AND stays visible on the wire.
|
||||
req = base()
|
||||
req.IdentityHeaderUserId = &empty
|
||||
p.FromAPIRequest(req)
|
||||
assert.Equal(t, "", p.ToAPIResponse().IdentityHeaderUserId, "cleared header must round-trip as empty string")
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package types
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/netbirdio/netbird/shared/management/http/api"
|
||||
@@ -66,9 +67,15 @@ func (s *Settings) ToAPIResponse() *api.AgentNetworkSettings {
|
||||
}
|
||||
}
|
||||
|
||||
// FromAPIRequest applies the mutable settings fields from the request. Cluster
|
||||
// and Subdomain are immutable and intentionally not touched here.
|
||||
// FromAPIRequest applies the request onto the receiver. The mutable
|
||||
// collection fields are always replaced with the request values. Cluster
|
||||
// participates only in bootstrap and the immutability check (see
|
||||
// Manager.UpdateSettings); Subdomain is server-assigned and never taken
|
||||
// from a request.
|
||||
func (s *Settings) FromAPIRequest(req *api.AgentNetworkSettingsRequest) {
|
||||
if req.Cluster != nil {
|
||||
s.Cluster = strings.TrimSpace(*req.Cluster)
|
||||
}
|
||||
s.EnableLogCollection = req.EnableLogCollection
|
||||
s.EnablePromptCollection = req.EnablePromptCollection
|
||||
s.RedactPii = req.RedactPii
|
||||
|
||||
@@ -102,11 +102,20 @@ func TestAgentNetwork_UpdateSettings_PreservesImmutableAndTogglesCollection(t *t
|
||||
require.NotEmpty(t, before.Subdomain, "subdomain pinned at bootstrap")
|
||||
assert.False(t, before.EnablePromptCollection, "prompt collection defaults off")
|
||||
|
||||
// Attempt to flip toggles AND smuggle a different cluster/subdomain — the
|
||||
// immutable fields must be ignored.
|
||||
// A cluster different from the one pinned at bootstrap must be rejected
|
||||
// outright — never silently swapped or ignored.
|
||||
_, err = mgr.UpdateSettings(ctx, adminUserID, &agenttypes.Settings{
|
||||
AccountID: accountID,
|
||||
Cluster: "attacker.cluster",
|
||||
EnableLogCollection: true,
|
||||
})
|
||||
require.Error(t, err, "UpdateSettings with a mismatched cluster must fail")
|
||||
|
||||
// Flipping the toggles works with the pinned cluster echoed back (and
|
||||
// with it omitted); the subdomain is never taken from the request.
|
||||
updated, err := mgr.UpdateSettings(ctx, adminUserID, &agenttypes.Settings{
|
||||
AccountID: accountID,
|
||||
Cluster: "attacker.cluster",
|
||||
Cluster: clusterAddr,
|
||||
Subdomain: "evil",
|
||||
EnableLogCollection: true,
|
||||
EnablePromptCollection: true,
|
||||
|
||||
@@ -5149,12 +5149,12 @@ components:
|
||||
identity_header_user_id:
|
||||
type: string
|
||||
description: |
|
||||
Wire header name the proxy stamps with the caller's display identity (user email or peer name) when the catalog entry's HeaderPair is `customizable`. Empty disables stamping for this dimension. Ignored when the catalog entry has a fixed HeaderPair (e.g. LiteLLM, Portkey). Used today by Bifrost: typical values are `x-bf-lh-netbird_user_id` (always-on log metadata) or `x-bf-dim-netbird_user_id` (Prometheus / OTEL — requires the label to be pre-declared in the gateway's `client.prometheus_labels` config).
|
||||
Wire header name the proxy stamps with the caller's display identity (user email or peer name) when the catalog entry's HeaderPair is `customizable`. Always present in responses; empty disables stamping for this dimension. Ignored when the catalog entry has a fixed HeaderPair (e.g. LiteLLM, Portkey). Used today by Bifrost: typical values are `x-bf-lh-netbird_user_id` (always-on log metadata) or `x-bf-dim-netbird_user_id` (Prometheus / OTEL — requires the label to be pre-declared in the gateway's `client.prometheus_labels` config).
|
||||
example: "x-bf-dim-netbird_user_id"
|
||||
identity_header_groups:
|
||||
type: string
|
||||
description: |
|
||||
Wire header name the proxy stamps with the caller's NetBird groups as a comma-separated list (sorted) when the catalog entry's HeaderPair is `customizable`. Empty disables stamping for this dimension. Same per-catalog semantics as `identity_header_user_id`.
|
||||
Wire header name the proxy stamps with the caller's NetBird groups as a comma-separated list (sorted) when the catalog entry's HeaderPair is `customizable`. Always present in responses; empty disables stamping for this dimension. Same per-catalog semantics as `identity_header_user_id`.
|
||||
example: "x-bf-dim-netbird_groups"
|
||||
enabled:
|
||||
type: boolean
|
||||
@@ -5186,6 +5186,8 @@ components:
|
||||
- name
|
||||
- upstream_url
|
||||
- models
|
||||
- identity_header_user_id
|
||||
- identity_header_groups
|
||||
- enabled
|
||||
- skip_tls_verification
|
||||
- metadata_disabled
|
||||
@@ -5222,7 +5224,7 @@ components:
|
||||
extra_values:
|
||||
type: object
|
||||
description: |
|
||||
Operator-typed values for catalog-declared extra headers (see AgentNetworkProvider.extra_values). When present on a request, the whole map replaces the stored values. Empty strings drop the corresponding key.
|
||||
Operator-typed values for catalog-declared extra headers (see AgentNetworkProvider.extra_values). When present on a request, the whole map replaces the stored values; when omitted the stored values are left unchanged. Empty strings drop the corresponding key.
|
||||
additionalProperties:
|
||||
type: string
|
||||
example:
|
||||
@@ -6244,8 +6246,12 @@ components:
|
||||
- updated_at
|
||||
AgentNetworkSettingsRequest:
|
||||
type: object
|
||||
description: Mutable account-level Agent Network settings. Cluster and subdomain are immutable and not accepted here.
|
||||
description: Account-level Agent Network settings update. The request replaces every mutable field. `cluster` additionally bootstraps the per-account settings row when the account does not have one yet; the subdomain is always server-assigned.
|
||||
properties:
|
||||
cluster:
|
||||
type: string
|
||||
description: Address of the NetBird proxy cluster fronting this account's agent-network endpoint. When the account has no settings row yet, providing it bootstraps the row (assigning the subdomain that forms the agent endpoint). The cluster is immutable once assigned — later updates must omit it or send the assigned value; any other value is rejected.
|
||||
example: "eu.proxy.netbird.io"
|
||||
enable_log_collection:
|
||||
type: boolean
|
||||
description: Whether per-request access-log entries are collected for this account's agent-network traffic.
|
||||
@@ -13690,7 +13696,7 @@ paths:
|
||||
/api/agent-network/settings:
|
||||
get:
|
||||
summary: Retrieve Agent Network settings
|
||||
description: Returns the per-account Agent Network gateway settings (cluster, subdomain, endpoint). Returns 404 when no provider has been created yet — settings are lazily bootstrapped on first provider create.
|
||||
description: Returns the per-account Agent Network gateway settings (cluster, subdomain, endpoint). Returns 404 when the account has not been bootstrapped yet — settings are bootstrapped on first provider create (`bootstrap_cluster`) or via PUT with `cluster`.
|
||||
tags: [ Agent Network ]
|
||||
security:
|
||||
- BearerAuth: [ ]
|
||||
@@ -13712,7 +13718,7 @@ paths:
|
||||
"$ref": "#/components/responses/internal_error"
|
||||
put:
|
||||
summary: Update Agent Network settings
|
||||
description: Updates the mutable account-level Agent Network settings (collection toggles). Cluster and subdomain are immutable and ignored if sent. Returns 404 when settings have not been bootstrapped (no provider created yet).
|
||||
description: Updates the account-level Agent Network settings; the request replaces every mutable field (collection toggles and retention). When the account has no settings row yet, providing `cluster` bootstraps it (assigning the subdomain that forms the agent endpoint); without `cluster` the request returns 404. Sending a `cluster` different from the assigned one is rejected (the cluster is immutable once assigned). The subdomain is always server-assigned and immutable.
|
||||
tags: [ Agent Network ]
|
||||
security:
|
||||
- BearerAuth: [ ]
|
||||
@@ -13738,6 +13744,8 @@ paths:
|
||||
"$ref": "#/components/responses/forbidden"
|
||||
'404':
|
||||
"$ref": "#/components/responses/not_found"
|
||||
'422':
|
||||
"$ref": "#/components/responses/validation_failed"
|
||||
'500':
|
||||
"$ref": "#/components/responses/internal_error"
|
||||
/api/agent-network/budget-rules:
|
||||
|
||||
@@ -2275,11 +2275,11 @@ type AgentNetworkProvider struct {
|
||||
// Id Provider ID
|
||||
Id string `json:"id"`
|
||||
|
||||
// IdentityHeaderGroups Wire header name the proxy stamps with the caller's NetBird groups as a comma-separated list (sorted) when the catalog entry's HeaderPair is `customizable`. Empty disables stamping for this dimension. Same per-catalog semantics as `identity_header_user_id`.
|
||||
IdentityHeaderGroups *string `json:"identity_header_groups,omitempty"`
|
||||
// IdentityHeaderGroups Wire header name the proxy stamps with the caller's NetBird groups as a comma-separated list (sorted) when the catalog entry's HeaderPair is `customizable`. Always present in responses; empty disables stamping for this dimension. Same per-catalog semantics as `identity_header_user_id`.
|
||||
IdentityHeaderGroups string `json:"identity_header_groups"`
|
||||
|
||||
// IdentityHeaderUserId Wire header name the proxy stamps with the caller's display identity (user email or peer name) when the catalog entry's HeaderPair is `customizable`. Empty disables stamping for this dimension. Ignored when the catalog entry has a fixed HeaderPair (e.g. LiteLLM, Portkey). Used today by Bifrost: typical values are `x-bf-lh-netbird_user_id` (always-on log metadata) or `x-bf-dim-netbird_user_id` (Prometheus / OTEL — requires the label to be pre-declared in the gateway's `client.prometheus_labels` config).
|
||||
IdentityHeaderUserId *string `json:"identity_header_user_id,omitempty"`
|
||||
// IdentityHeaderUserId Wire header name the proxy stamps with the caller's display identity (user email or peer name) when the catalog entry's HeaderPair is `customizable`. Always present in responses; empty disables stamping for this dimension. Ignored when the catalog entry has a fixed HeaderPair (e.g. LiteLLM, Portkey). Used today by Bifrost: typical values are `x-bf-lh-netbird_user_id` (always-on log metadata) or `x-bf-dim-netbird_user_id` (Prometheus / OTEL — requires the label to be pre-declared in the gateway's `client.prometheus_labels` config).
|
||||
IdentityHeaderUserId string `json:"identity_header_user_id"`
|
||||
|
||||
// MetadataDisabled Whether identity metadata injection is disabled for this provider. When enabled (the default), the proxy stamps the caller's user and authorizing group onto upstream requests as provider-specific metadata (e.g. AWS Bedrock's X-Amzn-Bedrock-Request-Metadata header). Set true to suppress it.
|
||||
MetadataDisabled bool `json:"metadata_disabled"`
|
||||
@@ -2335,7 +2335,7 @@ type AgentNetworkProviderRequest struct {
|
||||
// Enabled Whether the provider is enabled. Defaults to true on create.
|
||||
Enabled *bool `json:"enabled,omitempty"`
|
||||
|
||||
// ExtraValues Operator-typed values for catalog-declared extra headers (see AgentNetworkProvider.extra_values). When present on a request, the whole map replaces the stored values. Empty strings drop the corresponding key.
|
||||
// ExtraValues Operator-typed values for catalog-declared extra headers (see AgentNetworkProvider.extra_values). When present on a request, the whole map replaces the stored values; when omitted the stored values are left unchanged. Empty strings drop the corresponding key.
|
||||
ExtraValues *map[string]string `json:"extra_values,omitempty"`
|
||||
|
||||
// IdentityHeaderGroups Wire header name for the caller's groups CSV. See AgentNetworkProvider.identity_header_groups. Same omit / empty semantics as `identity_header_user_id`.
|
||||
@@ -2393,11 +2393,14 @@ type AgentNetworkSettings struct {
|
||||
UpdatedAt *time.Time `json:"updated_at,omitempty"`
|
||||
}
|
||||
|
||||
// AgentNetworkSettingsRequest Mutable account-level Agent Network settings. Cluster and subdomain are immutable and not accepted here.
|
||||
// AgentNetworkSettingsRequest Account-level Agent Network settings update. The request replaces every mutable field. `cluster` additionally bootstraps the per-account settings row when the account does not have one yet; the subdomain is always server-assigned.
|
||||
type AgentNetworkSettingsRequest struct {
|
||||
// AccessLogRetentionDays Days to retain full access-log rows; older rows are swept. 0 or less means keep indefinitely.
|
||||
AccessLogRetentionDays *int `json:"access_log_retention_days,omitempty"`
|
||||
|
||||
// Cluster Address of the NetBird proxy cluster fronting this account's agent-network endpoint. When the account has no settings row yet, providing it bootstraps the row (assigning the subdomain that forms the agent endpoint). The cluster is immutable once assigned — later updates must omit it or send the assigned value; any other value is rejected.
|
||||
Cluster *string `json:"cluster,omitempty"`
|
||||
|
||||
// EnableLogCollection Whether per-request access-log entries are collected for this account's agent-network traffic.
|
||||
EnableLogCollection bool `json:"enable_log_collection"`
|
||||
|
||||
|
||||
Reference in New Issue
Block a user