mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-17 04:09:07 +02:00
[management] Align agent-network API contracts for API clients (#7026)
## Describe your changes Work on the Terraform provider (terraform-provider-netbird #177–#183) surfaced places where the agent-network API broke its own contracts or deviated from the conventions the rest of the management API follows, forcing client-side workarounds. Settings reads now follow the settings-endpoint convention: GET always answers with a JSON object. Before bootstrap it returns the defaults with an empty cluster/subdomain/endpoint (previously 200 with a JSON `null` body, while the spec said 404). The settings PUT can bootstrap the account by carrying a `cluster` — previously the row could only come into existence through the first provider create, and a settings-first setup was impossible; a differing cluster on a bootstrapped account is rejected instead of silently ignored. PUT remains full-state. The provider PUT schema promised omit-preserves semantics for several operator-editable fields that the handler never delivered (it builds the row from the request, like every other update handler). The schema wording now matches the shipped full-state behavior; only the api_key (secret) and session keys stay preserved by the manager. Identity headers are always present in provider responses so an explicitly cleared value round-trips as an empty string. The Go REST client gains the full agent-network surface (catalog, providers, policies, guardrails, budget rules, settings), including a shim translating the legacy 200+`null` settings body from older servers into an `IsNotFound` error. Note for reviewers: the dashboard special-cased the `null` settings body; it needs a small follow-up for the new defaults response (in progress).
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,
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
//go:build e2e
|
||||
|
||||
package agentnetwork
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/netbirdio/netbird/e2e/harness"
|
||||
"github.com/netbirdio/netbird/shared/management/http/api"
|
||||
)
|
||||
|
||||
// harnessStartFresh boots a dedicated combined server with its own fresh
|
||||
// account and registers its teardown on t.
|
||||
func harnessStartFresh(ctx context.Context, t *testing.T) (*harness.Combined, error) {
|
||||
t.Helper()
|
||||
fresh, err := harness.StartCombined(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
t.Cleanup(func() { _ = fresh.Terminate(context.Background()) })
|
||||
if _, err := fresh.Bootstrap(ctx); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return fresh, nil
|
||||
}
|
||||
|
||||
// TestSettingsBootstrapViaPut covers the settings-first bootstrap path on an
|
||||
// account that has never been bootstrapped: the GET reads as the defaults
|
||||
// with an empty cluster/subdomain/endpoint, a PUT without a cluster has
|
||||
// nothing to pin and fails, and a PUT carrying a cluster creates the row and
|
||||
// pins it immutably. The shared srv cannot provide that starting state (any
|
||||
// provider-creating test bootstraps it, and test order is deliberately not
|
||||
// relied on), so this boots a dedicated combined server — the image is
|
||||
// already built and cached by TestMain's StartCombined, so the extra cost is
|
||||
// one container start.
|
||||
func TestSettingsBootstrapViaPut(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
fresh, err := harnessStartFresh(ctx, t)
|
||||
require.NoError(t, err, "start dedicated combined server")
|
||||
|
||||
// Before agent-network bootstrap the settings read as the defaults, not
|
||||
// as an error and not as a null body.
|
||||
before, err := fresh.GetSettings(ctx)
|
||||
require.NoError(t, err, "get settings on a fresh account must succeed")
|
||||
assert.Empty(t, before.Cluster, "cluster must be empty before bootstrap")
|
||||
assert.Empty(t, before.Subdomain, "subdomain must be empty before bootstrap")
|
||||
assert.Empty(t, before.Endpoint, "endpoint must be empty before bootstrap, not a bare dot")
|
||||
assert.True(t, before.EnableLogCollection, "defaults must show log collection on, matching bootstrap")
|
||||
assert.False(t, before.EnablePromptCollection, "defaults must show prompt collection off")
|
||||
|
||||
// A PUT without a cluster has nothing to pin the account to.
|
||||
_, err = fresh.UpdateSettings(ctx, api.AgentNetworkSettingsRequest{
|
||||
EnableLogCollection: true,
|
||||
})
|
||||
requireClientError(t, err)
|
||||
|
||||
// A PUT carrying a cluster bootstraps the account and applies the
|
||||
// mutable fields from the same request. Every toggle is set away from
|
||||
// its bootstrap default so each assertion can actually fail.
|
||||
const cluster = "e2e.bootstrap.netbird.selfhosted"
|
||||
bootstrapped, err := fresh.UpdateSettings(ctx, api.AgentNetworkSettingsRequest{
|
||||
Cluster: ptr(cluster),
|
||||
EnableLogCollection: false,
|
||||
EnablePromptCollection: true,
|
||||
RedactPii: true,
|
||||
})
|
||||
require.NoError(t, err, "bootstrap settings via PUT must succeed")
|
||||
assert.Equal(t, cluster, bootstrapped.Cluster, "cluster must be pinned from the request")
|
||||
require.NotEmpty(t, bootstrapped.Subdomain, "subdomain must be assigned at bootstrap")
|
||||
assert.Equal(t, bootstrapped.Subdomain+"."+cluster, bootstrapped.Endpoint, "endpoint must combine subdomain and cluster")
|
||||
assert.False(t, bootstrapped.EnableLogCollection, "log collection from the bootstrap request must override the default")
|
||||
assert.True(t, bootstrapped.EnablePromptCollection, "prompt collection from the bootstrap request must apply")
|
||||
assert.True(t, bootstrapped.RedactPii, "redact toggle from the bootstrap request must apply")
|
||||
|
||||
// The row is persisted: an independent read agrees on every field.
|
||||
after, err := fresh.GetSettings(ctx)
|
||||
require.NoError(t, err, "get settings after bootstrap must succeed")
|
||||
assert.Equal(t, bootstrapped.Endpoint, after.Endpoint, "bootstrap must persist across reads")
|
||||
assert.Equal(t, bootstrapped.EnableLogCollection, after.EnableLogCollection, "log collection must persist")
|
||||
assert.Equal(t, bootstrapped.EnablePromptCollection, after.EnablePromptCollection, "prompt collection must persist")
|
||||
assert.Equal(t, bootstrapped.RedactPii, after.RedactPii, "redact toggle must persist")
|
||||
|
||||
// Once bootstrapped, later updates may omit the cluster entirely.
|
||||
persisted, err := fresh.UpdateSettings(ctx, api.AgentNetworkSettingsRequest{
|
||||
EnableLogCollection: true,
|
||||
EnablePromptCollection: false,
|
||||
RedactPii: true,
|
||||
})
|
||||
require.NoError(t, err, "post-bootstrap update without cluster must succeed")
|
||||
assert.Equal(t, cluster, persisted.Cluster, "omitted cluster must keep the pinned value")
|
||||
assert.True(t, persisted.EnableLogCollection, "post-bootstrap toggle must apply")
|
||||
assert.False(t, persisted.EnablePromptCollection, "post-bootstrap toggle must apply")
|
||||
|
||||
// The cluster is immutable: a different value is rejected rather than
|
||||
// silently ignored, and the rejected update must not disturb anything.
|
||||
_, err = fresh.UpdateSettings(ctx, api.AgentNetworkSettingsRequest{
|
||||
Cluster: ptr("other.cluster.invalid"),
|
||||
EnableLogCollection: false,
|
||||
})
|
||||
requireClientError(t, err)
|
||||
|
||||
final, err := fresh.GetSettings(ctx)
|
||||
require.NoError(t, err, "get settings after the rejected cluster change must succeed")
|
||||
assert.Equal(t, persisted.Cluster, final.Cluster, "rejected update must not change the cluster")
|
||||
assert.Equal(t, persisted.Endpoint, final.Endpoint, "rejected update must not change the endpoint")
|
||||
assert.Equal(t, persisted.EnableLogCollection, final.EnableLogCollection, "rejected update must not apply its toggles")
|
||||
assert.Equal(t, persisted.EnablePromptCollection, final.EnablePromptCollection, "rejected update must not apply its toggles")
|
||||
assert.Equal(t, persisted.RedactPii, final.RedactPii, "rejected update must not apply its toggles")
|
||||
}
|
||||
Reference in New Issue
Block a user