mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-14 02:39:06 +02:00
[management] Name private capabilities, not an embedded proxy, in the refusal
The requirement is a cluster with private capabilities, which is what the proxy reports and what the dashboard renders as supports_private. Framing the refusal around an embedded proxy named one way of getting there as if it were the requirement, and told an API user to fix the wrong thing. The message, the comments and the test fixtures now speak of private capabilities throughout. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sa3DsBDP3VciAi4PPG17L6
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
2ee84e475a
commit
e5dc66b65b
@@ -19,18 +19,17 @@ import (
|
||||
// on the picked cluster, end to end against a real proxy.
|
||||
//
|
||||
// The synthesised gateway service is always private: agents reach it over the
|
||||
// WireGuard tunnel and are authorised by their peer identity. Only a proxy
|
||||
// running embedded in a netbird client (`netbird proxy --private`) can serve
|
||||
// that, and management reports it per cluster as the `private` capability —
|
||||
// the same supports_private flag the dashboard reads to decide which clusters
|
||||
// it may offer. The endpoint assigned at bootstrap is immutable, so pinning
|
||||
// WireGuard tunnel and are authorised by their peer identity. Only a cluster
|
||||
// with private capabilities can serve that, and management reports it per
|
||||
// cluster as the `private` capability — the same supports_private flag the
|
||||
// dashboard reads to decide which clusters it may offer. The endpoint assigned at bootstrap is immutable, so pinning
|
||||
// to a cluster that cannot serve it has to be refused up front rather than
|
||||
// leaving the account with a dead gateway.
|
||||
//
|
||||
// One combined server and one cluster address, walked through three states:
|
||||
// a live centralised proxy (refused), that proxy stopped so nothing in the
|
||||
// cluster is live any more (still refused — the record of what the cluster is
|
||||
// outlives its heartbeats), and finally an embedded proxy (accepted, the
|
||||
// outlives its heartbeats), and finally a private-capable proxy (accepted, the
|
||||
// capability being any-true across the cluster's live proxies). Same account,
|
||||
// same address, so nothing but the cluster's state accounts for the different
|
||||
// answers.
|
||||
@@ -45,8 +44,8 @@ func TestSettingsBootstrapValidatesProxyCluster(t *testing.T) {
|
||||
|
||||
const cluster = harness.AgentNetworkCluster
|
||||
|
||||
// A centralised proxy: connected and serving the cluster, but not
|
||||
// embedded in a netbird client, so it cannot authenticate tunnel peers.
|
||||
// A centralised proxy: connected and serving the cluster, but without
|
||||
// private capabilities, so it cannot serve a private service.
|
||||
central, err := harness.StartProxy(ctx, fresh, proxyToken, map[string]string{
|
||||
"NB_PROXY_PRIVATE": "false",
|
||||
})
|
||||
@@ -59,9 +58,9 @@ func TestSettingsBootstrapValidatesProxyCluster(t *testing.T) {
|
||||
_, err = fresh.CreateSettings(ctx, api.AgentNetworkSettingsCreateRequest{
|
||||
ProxyAddress: ptr(cluster),
|
||||
})
|
||||
require.Error(t, err, "bootstrap onto a cluster with no embedded proxy must be refused")
|
||||
require.Error(t, err, "bootstrap onto a cluster without private capabilities must be refused")
|
||||
requireClientError(t, err)
|
||||
assert.Contains(t, err.Error(), "embedded proxy",
|
||||
assert.Contains(t, err.Error(), "private capabilities",
|
||||
"the refusal must name what the cluster is missing: %v", err)
|
||||
|
||||
after, err := fresh.GetSettings(ctx)
|
||||
@@ -81,14 +80,14 @@ func TestSettingsBootstrapValidatesProxyCluster(t *testing.T) {
|
||||
_, err = fresh.CreateSettings(ctx, api.AgentNetworkSettingsCreateRequest{
|
||||
ProxyAddress: ptr(cluster),
|
||||
})
|
||||
require.Error(t, err, "an offline cluster with no embedded proxy on record must stay refused")
|
||||
require.Error(t, err, "an offline cluster without private capabilities on record must stay refused")
|
||||
requireClientError(t, err)
|
||||
|
||||
// Add an embedded proxy to the same cluster: now it can serve a private
|
||||
// Add a private-capable proxy to the same cluster: now it can serve a private
|
||||
// service, and the very same request must go through.
|
||||
embedded, err := harness.StartProxy(ctx, fresh, proxyToken)
|
||||
require.NoError(t, err, "start embedded proxy")
|
||||
t.Cleanup(func() { _ = embedded.Terminate(context.Background()) })
|
||||
privateProxy, err := harness.StartProxy(ctx, fresh, proxyToken)
|
||||
require.NoError(t, err, "start private-capable proxy")
|
||||
t.Cleanup(func() { _ = privateProxy.Terminate(context.Background()) })
|
||||
|
||||
waitClusterPrivate(ctx, t, fresh, cluster, true)
|
||||
|
||||
|
||||
@@ -32,7 +32,7 @@ const (
|
||||
testAccountID = "acc-1"
|
||||
testUserID = "user-bob"
|
||||
// testClusterAddress is the shared proxy cluster the settings tests pin
|
||||
// their gateway to; the fixture seeds a connected embedded proxy for it.
|
||||
// their gateway to; the fixture seeds a connected private-capable proxy for it.
|
||||
testClusterAddress = "eu.proxy.netbird.io"
|
||||
)
|
||||
|
||||
@@ -84,7 +84,7 @@ func newAgentNetworkHandlerFixture(t *testing.T) *agentNetworkHandlerFixture {
|
||||
// clusters, so seed the shared cluster these tests pin to as a real,
|
||||
// private-capable one — the wire-shape assertions then run through the
|
||||
// validated path rather than the "nothing connected yet" carve-out.
|
||||
seedSharedEmbeddedCluster(t, st, testClusterAddress)
|
||||
seedSharedPrivateCluster(t, st, testClusterAddress)
|
||||
|
||||
router := mux.NewRouter()
|
||||
router.HandleFunc("/agent-network/providers", h.createProvider).Methods("POST")
|
||||
@@ -280,10 +280,10 @@ func TestConsumptionHandler_PopulatedAccountListsRows(t *testing.T) {
|
||||
"rows recorded in the same window must share the aligned window_start_utc")
|
||||
}
|
||||
|
||||
// seedSharedEmbeddedCluster registers a connected, NetBird-operated proxy
|
||||
// running embedded in a netbird client (the `private` capability) so
|
||||
// seedSharedPrivateCluster registers a connected, NetBird-operated proxy
|
||||
// with private capabilities (the `private` capability) so
|
||||
// clusterAddr is a cluster any account may pin its agent-network gateway to.
|
||||
func seedSharedEmbeddedCluster(t *testing.T, st store.Store, clusterAddr string) {
|
||||
func seedSharedPrivateCluster(t *testing.T, st store.Store, clusterAddr string) {
|
||||
t.Helper()
|
||||
private := true
|
||||
now := time.Now().UTC()
|
||||
|
||||
@@ -1074,17 +1074,15 @@ func (m *managerImpl) bootstrapSelfAddressed(ctx context.Context, settings *type
|
||||
// The synthesised gateway service is unconditionally private
|
||||
// (buildAccountService): agents reach it over the WireGuard tunnel and are
|
||||
// authorised by ValidateTunnelPeer against the policies' source groups, and
|
||||
// its single target is the cluster itself with DirectUpstream. Only a proxy
|
||||
// running embedded in a netbird client (`netbird proxy`) can serve that — a
|
||||
// centralised proxy has no tunnel identity to authenticate against and no
|
||||
// WireGuard endpoint to be reached on. Management reports that per cluster as
|
||||
// the `private` capability, the same flag the dashboard renders as
|
||||
// its single target is the cluster itself with DirectUpstream. Only a cluster
|
||||
// with private capabilities can serve that. Management reports it per cluster
|
||||
// as the `private` capability, the same flag the dashboard renders as
|
||||
// supports_private when it gates NetBird-only services.
|
||||
//
|
||||
// Without this check the bootstrap happily pins to any cluster the caller
|
||||
// names, including one with no embedded proxy — and the endpoint it allocates
|
||||
// is immutable, so the account is left with a dead gateway that only a
|
||||
// DeleteSettings/re-bootstrap can undo.
|
||||
// names, including one without private capabilities — and the endpoint it
|
||||
// allocates is immutable, so the account is left with a dead gateway that only
|
||||
// a DeleteSettings/re-bootstrap can undo.
|
||||
//
|
||||
// Whether management knows the cluster is decided on the proxy rows
|
||||
// themselves, never on how fresh their heartbeats are: a cluster's rows
|
||||
@@ -1107,7 +1105,7 @@ func (m *managerImpl) validateGatewayCluster(ctx context.Context, accountID, clu
|
||||
}
|
||||
|
||||
// A cluster management knows has to prove it can serve the gateway, and
|
||||
// only a live embedded proxy proves that. Both an explicit false and an
|
||||
// only a live proxy reporting the capability proves that. Both an explicit false and an
|
||||
// unreported capability (nothing live in the cluster, or proxies predating
|
||||
// capability reporting) fail here: unusable and unproven are the same
|
||||
// answer for a decision that cannot be revisited later.
|
||||
@@ -1123,8 +1121,8 @@ func (m *managerImpl) validateGatewayCluster(ctx context.Context, accountID, clu
|
||||
}
|
||||
|
||||
return status.Errorf(status.InvalidArgument,
|
||||
"proxy cluster %s cannot serve the agent network gateway: the gateway is reachable only from connected peers, "+
|
||||
"which needs at least one connected embedded proxy (netbird proxy) in the cluster", clusterAddr)
|
||||
"proxy cluster %s has no private capabilities: the agent network gateway requires a reverse proxy cluster "+
|
||||
"with private capabilities", clusterAddr)
|
||||
}
|
||||
|
||||
// accountClusterSpellings returns every proxy cluster address in the account's
|
||||
|
||||
@@ -77,7 +77,7 @@ func ptrTo[T any](v T) *T { return &v }
|
||||
// seedProxy registers a proxy in clusterAddr, heartbeating now, so the labeled
|
||||
// bootstrap path has a real cluster to validate against. accountID empty makes
|
||||
// it a shared (NetBird-operated) cluster; private mirrors the capability an
|
||||
// embedded `netbird proxy` reports, nil an unreported one.
|
||||
// proxy with private capabilities reports, nil an unreported one.
|
||||
func (f *bootstrapFixture) seedProxy(t *testing.T, proxyID, accountID, clusterAddr string, private *bool) {
|
||||
t.Helper()
|
||||
f.seedProxyAt(t, proxyID, accountID, clusterAddr, private, time.Now().UTC())
|
||||
@@ -101,9 +101,9 @@ func (f *bootstrapFixture) seedProxyAt(t *testing.T, proxyID, accountID, cluster
|
||||
require.NoError(t, f.store.SaveProxy(context.Background(), p), "seeding a proxy must succeed")
|
||||
}
|
||||
|
||||
// seedEmbeddedCluster is the common case: a shared cluster with a connected
|
||||
// embedded proxy, which is what the labeled bootstrap requires.
|
||||
func (f *bootstrapFixture) seedEmbeddedCluster(t *testing.T, clusterAddr string) {
|
||||
// seedPrivateCluster is the common case: a shared cluster with a connected
|
||||
// proxy that has private capabilities, which is what a bootstrap requires.
|
||||
func (f *bootstrapFixture) seedPrivateCluster(t *testing.T, clusterAddr string) {
|
||||
t.Helper()
|
||||
f.seedProxy(t, "proxy-"+clusterAddr, "", clusterAddr, ptrTo(true))
|
||||
}
|
||||
@@ -147,7 +147,7 @@ func TestCreateSettingsRequiresPermission(t *testing.T) {
|
||||
func TestCreateSettingsLabeled(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
f := newBootstrapFixture(t)
|
||||
f.seedEmbeddedCluster(t, "cluster1.example.com")
|
||||
f.seedPrivateCluster(t, "cluster1.example.com")
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true)
|
||||
|
||||
created, err := f.createSettings(ctx, "account1", "user1", "Cluster1.Example.com", "")
|
||||
@@ -221,7 +221,7 @@ func TestCreateSettingsIdentityFieldValidation(t *testing.T) {
|
||||
func TestCreateSettingsConflictsOnSecondBootstrap(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
f := newBootstrapFixture(t)
|
||||
f.seedEmbeddedCluster(t, "cluster1.example.com")
|
||||
f.seedPrivateCluster(t, "cluster1.example.com")
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true)
|
||||
|
||||
first, err := f.createSettings(ctx, "account1", "user1", "cluster1.example.com", "")
|
||||
@@ -301,7 +301,7 @@ func TestCreateSettingsRejectsOfflineCluster(t *testing.T) {
|
||||
// A cluster that could serve the gateway still has to have something
|
||||
// live in it to prove so at bootstrap: refusing is the safe direction
|
||||
// (reconnect the proxy and retry) where accepting is permanent.
|
||||
"embedded proxy gone quiet": ptrTo(true),
|
||||
"private proxy gone quiet": ptrTo(true),
|
||||
}
|
||||
for name, private := range cases {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
@@ -315,8 +315,8 @@ func TestCreateSettingsRejectsOfflineCluster(t *testing.T) {
|
||||
var sErr *status.Error
|
||||
require.ErrorAs(t, err, &sErr)
|
||||
assert.Equal(t, status.InvalidArgument, sErr.Type(), "rejection must be a validation error")
|
||||
assert.Contains(t, err.Error(), "connected embedded proxy",
|
||||
"the error must say a live embedded proxy is what is missing")
|
||||
assert.Contains(t, err.Error(), "private capabilities",
|
||||
"the error must say private capabilities are what is missing")
|
||||
|
||||
_, err = f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, "account1")
|
||||
assert.Error(t, err, "no row may be left behind by a rejected bootstrap")
|
||||
@@ -326,7 +326,7 @@ func TestCreateSettingsRejectsOfflineCluster(t *testing.T) {
|
||||
|
||||
// TestCreateSettingsRequiresPrivateCluster pins the capability gate: the
|
||||
// synthesised gateway service is always private, so a live cluster whose
|
||||
// proxies are not embedded in a netbird client cannot serve it and must not
|
||||
// proxies lack private capabilities cannot serve it and must not
|
||||
// become the account's immutable endpoint.
|
||||
func TestCreateSettingsRequiresPrivateCluster(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
@@ -336,18 +336,18 @@ func TestCreateSettingsRequiresPrivateCluster(t *testing.T) {
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true)
|
||||
|
||||
_, err := f.createSettings(ctx, "account1", "user1", "central.example.com", "")
|
||||
require.Error(t, err, "a cluster without an embedded proxy must be rejected")
|
||||
require.Error(t, err, "a cluster without private capabilities must be rejected")
|
||||
var sErr *status.Error
|
||||
require.ErrorAs(t, err, &sErr)
|
||||
assert.Equal(t, status.InvalidArgument, sErr.Type(), "rejection must be a validation error")
|
||||
assert.Contains(t, err.Error(), "embedded proxy", "the error must name what the cluster is missing")
|
||||
assert.Contains(t, err.Error(), "private capabilities", "the error must name what the cluster is missing")
|
||||
|
||||
_, err = f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, "account1")
|
||||
assert.Error(t, err, "no row may be left behind by a rejected bootstrap")
|
||||
}
|
||||
|
||||
// TestCreateSettingsAcceptsOwnPrivateCluster pins the BYOP happy path: the
|
||||
// account's own cluster with a connected embedded proxy is a valid pin.
|
||||
// account's own cluster with a connected private-capable proxy is a valid pin.
|
||||
func TestCreateSettingsAcceptsOwnPrivateCluster(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
f := newBootstrapFixture(t)
|
||||
@@ -385,7 +385,7 @@ func TestCreateSettingsMatchesClusterCasing(t *testing.T) {
|
||||
|
||||
_, err := f.createSettings(ctx, "account1", "user1", "central.example.com", "")
|
||||
require.Error(t, err, "casing must not become a way past the capability check")
|
||||
assert.Contains(t, err.Error(), "embedded proxy")
|
||||
assert.Contains(t, err.Error(), "private capabilities")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -530,7 +530,7 @@ func TestCreateSettingsRejectsHostAnotherAccountPinned(t *testing.T) {
|
||||
// TestCreateSettingsSelfAddressedRequiresPrivateCluster pins that the
|
||||
// capability gate applies to a self-addressed endpoint too: the service behind
|
||||
// it is the same private one, so a proxy that already declares the hostname
|
||||
// must be an embedded one, whether the account's own or a shared cluster's. A
|
||||
// must have private capabilities, whether the account's own or a shared cluster's. A
|
||||
// hostname no proxy declares yet stays claimable (TestCreateSettingsSelfAddressed).
|
||||
func TestCreateSettingsSelfAddressedRequiresPrivateCluster(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
@@ -545,15 +545,15 @@ func TestCreateSettingsSelfAddressedRequiresPrivateCluster(t *testing.T) {
|
||||
var sErr *status.Error
|
||||
require.ErrorAs(t, err, &sErr)
|
||||
assert.Equal(t, status.InvalidArgument, sErr.Type())
|
||||
assert.Contains(t, err.Error(), "embedded proxy")
|
||||
assert.Contains(t, err.Error(), "private capabilities")
|
||||
|
||||
_, err = f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, "account1")
|
||||
assert.Error(t, err, "no row may be left behind by a rejected bootstrap")
|
||||
})
|
||||
|
||||
t.Run("embedded proxy at the hostname is accepted", func(t *testing.T) {
|
||||
t.Run("private proxy at the hostname is accepted", func(t *testing.T) {
|
||||
f := newBootstrapFixture(t)
|
||||
f.seedProxy(t, "embedded", "", "gw.example.com", ptrTo(true))
|
||||
f.seedProxy(t, "private", "", "gw.example.com", ptrTo(true))
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true)
|
||||
|
||||
created, err := f.createSettings(ctx, "account1", "user1", "", "gw.example.com")
|
||||
|
||||
@@ -89,7 +89,7 @@ func TestAgentNetwork_UpdateSettings_PreservesImmutableAndTogglesCollection(t *t
|
||||
|
||||
// Bootstrap is an explicit settings create; providers have no settings
|
||||
// side effects anymore.
|
||||
seedEmbeddedProxyCluster(t, am.Store, clusterAddr)
|
||||
seedPrivateProxyCluster(t, am.Store, clusterAddr)
|
||||
before, err := mgr.CreateSettings(ctx, adminUserID, agenttypes.DefaultSettings(accountID), clusterAddr, "")
|
||||
require.NoError(t, err, "CreateSettings must bootstrap the row")
|
||||
require.Equal(t, clusterAddr, before.ProxyAddress, "proxy address pinned at bootstrap")
|
||||
|
||||
@@ -93,7 +93,7 @@ func TestAgentNetwork_ProviderCRUD_FansOutToProxyAndClientPeers(t *testing.T) {
|
||||
// UpdateAccountPeers, which is the path under test.
|
||||
agentMgr := agentnetwork.NewManager(am.Store, permissions.NewManager(am.Store), am, nil)
|
||||
|
||||
seedEmbeddedProxyCluster(t, am.Store, clusterAddr)
|
||||
seedPrivateProxyCluster(t, am.Store, clusterAddr)
|
||||
_, err = agentMgr.CreateSettings(ctx, adminUserID, agenttypes.DefaultSettings(accountID), clusterAddr, "")
|
||||
require.NoError(t, err, "CreateSettings must bootstrap the endpoint")
|
||||
// The bootstrap itself reconciles and queues updates on both channels;
|
||||
@@ -225,11 +225,11 @@ func synthZoneRData(sync *nbproto.SyncResponse, clusterAddr, fqdn string) string
|
||||
return ""
|
||||
}
|
||||
|
||||
// seedEmbeddedProxyCluster registers a connected proxy running embedded in a
|
||||
// seedPrivateProxyCluster registers a connected proxy with private capabilities in a
|
||||
// netbird client for clusterAddr, matching what a real deployment looks like
|
||||
// when the account bootstraps: the agent-network gateway service is always
|
||||
// private, so its cluster has to be one that can serve private services.
|
||||
func seedEmbeddedProxyCluster(t *testing.T, st store.Store, clusterAddr string) {
|
||||
func seedPrivateProxyCluster(t *testing.T, st store.Store, clusterAddr string) {
|
||||
t.Helper()
|
||||
private := true
|
||||
now := time.Now().UTC()
|
||||
|
||||
Reference in New Issue
Block a user