diff --git a/management/internals/controllers/network_map/controller/controller.go b/management/internals/controllers/network_map/controller/controller.go index 0d8fb3c47..f1b1832d2 100644 --- a/management/internals/controllers/network_map/controller/controller.go +++ b/management/internals/controllers/network_map/controller/controller.go @@ -116,6 +116,24 @@ func (c *Controller) OnPeerDisconnected(ctx context.Context, accountID string, p c.EphemeralPeersManager.OnPeerDisconnected(ctx, peer) } +// injectAllProxyPolicies prepares an account for the per-peer network-map +// computation. It prepends the in-memory agent-network services synthesised +// from the account's current provider/policy state to account.Services so +// the existing InjectProxyPolicies + injectPrivateServicePolicies walks pick +// them up alongside persisted reverse-proxy services. Synthesised services +// are never persisted; the account is loaded fresh per cycle so re-prepending +// is safe and idempotent. Accounts without agent-network providers get an +// empty synth slice — no behaviour change. +func (c *Controller) injectAllProxyPolicies(ctx context.Context, account *types.Account) { + synth, err := c.repo.SynthesizeAgentNetworkServices(ctx, account.Id) + if err != nil { + log.WithContext(ctx).Warnf("synthesise agent-network services for account %s: %v", account.Id, err) + } else if len(synth) > 0 { + account.Services = append(synth, account.Services...) + } + account.InjectProxyPolicies(ctx) +} + func (c *Controller) CountStreams() int { return c.peersUpdateManager.CountStreams() } @@ -150,7 +168,7 @@ func (c *Controller) sendUpdateAccountPeers(ctx context.Context, accountID strin var wg sync.WaitGroup semaphore := make(chan struct{}, 10) - account.InjectProxyPolicies(ctx) + c.injectAllProxyPolicies(ctx, account) dnsCache := &cache.DNSConfigCache{} dnsDomain := c.GetDNSDomain(account.Settings) peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain) @@ -281,7 +299,15 @@ func (c *Controller) sendUpdateForAffectedPeers(ctx context.Context, accountID s var wg sync.WaitGroup semaphore := make(chan struct{}, 10) - account.InjectProxyPolicies(ctx) + // The affected-peer path MUST mirror sendUpdateAccountPeers (line 171) + // here: injectAllProxyPolicies prepends the synthesised agent-network + // services BEFORE InjectProxyPolicies + private-service policies run. + // Previously this path called only account.InjectProxyPolicies, which + // skipped the synth-services prepend — so peer-level changes + // (proxy restart, embedded peer connect/disconnect) propagated a + // network map that omitted the synth DNS zone, and the agent kept + // resolving against the stale or absent record. + c.injectAllProxyPolicies(ctx, account) dnsCache := &cache.DNSConfigCache{} dnsDomain := c.GetDNSDomain(account.Settings) peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain) @@ -399,7 +425,7 @@ func (c *Controller) UpdateAccountPeer(ctx context.Context, accountId string, pe return fmt.Errorf("failed to get validated peers: %v", err) } - account.InjectProxyPolicies(ctx) + c.injectAllProxyPolicies(ctx, account) dnsCache := &cache.DNSConfigCache{} dnsDomain := c.GetDNSDomain(account.Settings) peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain) @@ -603,7 +629,7 @@ func (c *Controller) GetValidatedPeerWithMap(ctx context.Context, isRequiresAppr return nil, nil, 0, err } - account.InjectProxyPolicies(ctx) + c.injectAllProxyPolicies(ctx, account) approvedPeersMap, err := c.integratedPeerValidator.GetValidatedPeers(ctx, account.Id, maps.Values(account.Groups), maps.Values(account.Peers), account.Settings.Extra) if err != nil { @@ -874,7 +900,7 @@ func (c *Controller) GetNetworkMap(ctx context.Context, peerID string) (*types.N return nil, err } - account.InjectProxyPolicies(ctx) + c.injectAllProxyPolicies(ctx, account) resourcePolicies := account.GetResourcePoliciesMap() routers := account.GetResourceRoutersMap() groupIDToUserIDs := account.GetActiveGroupUsers() diff --git a/management/server/affectedpeers/proxy_synth_test.go b/management/server/affectedpeers/proxy_synth_test.go new file mode 100644 index 000000000..07273b18b --- /dev/null +++ b/management/server/affectedpeers/proxy_synth_test.go @@ -0,0 +1,95 @@ +package affectedpeers + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" + "github.com/netbirdio/netbird/management/server/store" +) + +// fakeProxyStore implements only the two store methods loadProxyServices calls; +// the embedded nil store.Store panics if anything else is invoked, which keeps +// the test honest about the surface under test. +type fakeProxyStore struct { + store.Store + proxyByCluster map[string][]string + persisted []*rpservice.Service +} + +func (f *fakeProxyStore) GetEmbeddedProxyPeerIDsByCluster(_ context.Context, _ string) (map[string][]string, error) { + return f.proxyByCluster, nil +} + +func (f *fakeProxyStore) GetAccountServices(_ context.Context, _ store.LockingStrength, _ string) ([]*rpservice.Service, error) { + return f.persisted, nil +} + +func serviceIDs(svcs []*rpservice.Service) []string { + ids := make([]string, 0, len(svcs)) + for _, s := range svcs { + ids = append(ids, s.ID) + } + return ids +} + +// loadProxyServices must merge the synthesised agent-network services (which are +// never persisted) with the persisted ones, so the proxy-affected expansion can +// see agent-network AccessGroups. Without this the embedded proxy peer is never +// flagged on a client group change and only a full resync (restart) recovers. +func TestLoadProxyServices_MergesSynthesizedAgentNetworkServices(t *testing.T) { + prev := agentNetworkSynthesizer + t.Cleanup(func() { agentNetworkSynthesizer = prev }) + SetAgentNetworkSynthesizer(func(_ context.Context, _ store.Store, _ string) ([]*rpservice.Service, error) { + return []*rpservice.Service{ + {ID: "agent-net-svc-acc", ProxyCluster: "proxy.netbird.local", Private: true, AccessGroups: []string{"gB"}}, + }, nil + }) + + s := &fakeProxyStore{ + proxyByCluster: map[string][]string{"proxy.netbird.local": {"proxy-peer-1"}}, + persisted: []*rpservice.Service{{ID: "persisted-rp-svc", ProxyCluster: "proxy.netbird.local"}}, + } + snap := &Snapshot{} + require.NoError(t, snap.loadProxyServices(context.Background(), s, "acc")) + + ids := serviceIDs(snap.services) + assert.Contains(t, ids, "persisted-rp-svc", "persisted services must be kept") + assert.Contains(t, ids, "agent-net-svc-acc", "synthesised agent-network service must be merged in") +} + +// With no synthesiser registered, loadProxyServices falls back to persisted +// services only (no panic, no behaviour change for non-agent-network builds). +func TestLoadProxyServices_NoSynthesizerRegistered(t *testing.T) { + prev := agentNetworkSynthesizer + t.Cleanup(func() { agentNetworkSynthesizer = prev }) + agentNetworkSynthesizer = nil + + s := &fakeProxyStore{ + proxyByCluster: map[string][]string{"c": {"proxy-1"}}, + persisted: []*rpservice.Service{{ID: "persisted"}}, + } + snap := &Snapshot{} + require.NoError(t, snap.loadProxyServices(context.Background(), s, "acc")) + assert.Equal(t, []string{"persisted"}, serviceIDs(snap.services)) +} + +// No embedded proxy peers → skip entirely (don't even call the synthesiser). +func TestLoadProxyServices_NoEmbeddedProxyPeersSkips(t *testing.T) { + prev := agentNetworkSynthesizer + t.Cleanup(func() { agentNetworkSynthesizer = prev }) + called := false + SetAgentNetworkSynthesizer(func(_ context.Context, _ store.Store, _ string) ([]*rpservice.Service, error) { + called = true + return nil, nil + }) + + s := &fakeProxyStore{proxyByCluster: map[string][]string{}} + snap := &Snapshot{} + require.NoError(t, snap.loadProxyServices(context.Background(), s, "acc")) + assert.False(t, called, "synthesiser must not run for accounts without embedded proxy peers") + assert.Empty(t, snap.services) +} diff --git a/management/server/affectedpeers/resolver.go b/management/server/affectedpeers/resolver.go index 94e24ced6..16a795539 100644 --- a/management/server/affectedpeers/resolver.go +++ b/management/server/affectedpeers/resolver.go @@ -29,6 +29,19 @@ import ( "github.com/netbirdio/netbird/route" ) +// agentNetworkSynthesizer returns the account's synthesised (never-persisted) +// agent-network reverse-proxy services. It is registered at boot via +// SetAgentNetworkSynthesizer to avoid an import cycle (agentnetwork → account → +// affectedpeers). nil when agent-network is not wired, in which case only +// persisted services are considered. +var agentNetworkSynthesizer func(ctx context.Context, s store.Store, accountID string) ([]*rpservice.Service, error) + +// SetAgentNetworkSynthesizer registers the agent-network service synthesiser. +// Called once during boot, before any request is served. +func SetAgentNetworkSynthesizer(fn func(ctx context.Context, s store.Store, accountID string) ([]*rpservice.Service, error)) { + agentNetworkSynthesizer = fn +} + // Snapshot is an in-memory view of the collections needed to expand a Change. // Loaded in-tx, walked by Expand after commit. Only the collections the Change // can touch are loaded; the rest stay nil (see Load). @@ -124,7 +137,12 @@ func (snap *Snapshot) loadDNS(ctx context.Context, s store.Store, accountID stri } // loadProxyServices loads the embedded-proxy cluster index, and the services only -// when the account actually has embedded proxy peers. +// when the account actually has embedded proxy peers. Both the persisted +// reverse-proxy services and the synthesised agent-network services are loaded: +// agent-network services are never persisted, so without synthesising them here +// collectFromProxyServices can't fold the embedded proxy peer into the affected +// set when a client's group changes, and the proxy never learns a newly +// authorised client until it reconnects (full network-map resync). func (snap *Snapshot) loadProxyServices(ctx context.Context, s store.Store, accountID string) error { var err error if snap.proxyByCluster, err = s.GetEmbeddedProxyPeerIDsByCluster(ctx, accountID); err != nil { @@ -133,8 +151,21 @@ func (snap *Snapshot) loadProxyServices(ctx context.Context, s store.Store, acco if len(snap.proxyByCluster) == 0 { return nil } - snap.services, err = s.GetAccountServices(ctx, store.LockingStrengthNone, accountID) - return err + if snap.services, err = s.GetAccountServices(ctx, store.LockingStrengthNone, accountID); err != nil { + return err + } + if agentNetworkSynthesizer == nil { + return nil + } + synth, serr := agentNetworkSynthesizer(ctx, s, accountID) + if serr != nil { + // Non-fatal: fall back to persisted services. The next full + // network-map resync still converges the proxy. + log.WithContext(ctx).Warnf("affectedpeers: synthesise agent-network services for account %s: %v", accountID, serr) + return nil + } + snap.services = append(snap.services, synth...) + return nil } // loadGroupIndex loads all groups (for group.Resources) and builds the diff --git a/management/server/agentnetwork/affectedpeers_hook.go b/management/server/agentnetwork/affectedpeers_hook.go new file mode 100644 index 000000000..58347666d --- /dev/null +++ b/management/server/agentnetwork/affectedpeers_hook.go @@ -0,0 +1,15 @@ +package agentnetwork + +import "github.com/netbirdio/netbird/management/server/affectedpeers" + +// init registers the agent-network service synthesiser with the affectedpeers +// resolver. Agent-network reverse-proxy services are synthesised on demand and +// never persisted, so the resolver can't load them from the store; without them +// it can't fold the embedded proxy peer into the affected set on a client +// group/peer change, and the proxy never learns a newly authorised client until +// it reconnects. Registered here (rather than via a direct +// affectedpeers→agentnetwork import) to avoid an import cycle +// (agentnetwork → account → affectedpeers). +func init() { + affectedpeers.SetAgentNetworkSynthesizer(SynthesizeServices) +}