mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-21 12:59:08 +02:00
[management] Fix agent-network proxy-peer fan-out on affected-peer recompute
The affected-peers resolver loaded only persisted reverse-proxy services, but agent-network services are synthesized on demand and never persisted. As a result the embedded proxy peer was never folded into the affected set when a client's group changed, so the proxy received no network-map update for a newly authorised client and rejected its handshake until a full resync (restart). loadProxyServices now merges the synthesized agent-network services (injected via a registration hook to avoid an import cycle), so proxy peers learn newly authorised clients immediately.
This commit is contained in:
@@ -116,6 +116,24 @@ func (c *Controller) OnPeerDisconnected(ctx context.Context, accountID string, p
|
|||||||
c.EphemeralPeersManager.OnPeerDisconnected(ctx, peer)
|
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 {
|
func (c *Controller) CountStreams() int {
|
||||||
return c.peersUpdateManager.CountStreams()
|
return c.peersUpdateManager.CountStreams()
|
||||||
}
|
}
|
||||||
@@ -150,7 +168,7 @@ func (c *Controller) sendUpdateAccountPeers(ctx context.Context, accountID strin
|
|||||||
var wg sync.WaitGroup
|
var wg sync.WaitGroup
|
||||||
semaphore := make(chan struct{}, 10)
|
semaphore := make(chan struct{}, 10)
|
||||||
|
|
||||||
account.InjectProxyPolicies(ctx)
|
c.injectAllProxyPolicies(ctx, account)
|
||||||
dnsCache := &cache.DNSConfigCache{}
|
dnsCache := &cache.DNSConfigCache{}
|
||||||
dnsDomain := c.GetDNSDomain(account.Settings)
|
dnsDomain := c.GetDNSDomain(account.Settings)
|
||||||
peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain)
|
peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain)
|
||||||
@@ -281,7 +299,15 @@ func (c *Controller) sendUpdateForAffectedPeers(ctx context.Context, accountID s
|
|||||||
var wg sync.WaitGroup
|
var wg sync.WaitGroup
|
||||||
semaphore := make(chan struct{}, 10)
|
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{}
|
dnsCache := &cache.DNSConfigCache{}
|
||||||
dnsDomain := c.GetDNSDomain(account.Settings)
|
dnsDomain := c.GetDNSDomain(account.Settings)
|
||||||
peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain)
|
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)
|
return fmt.Errorf("failed to get validated peers: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
account.InjectProxyPolicies(ctx)
|
c.injectAllProxyPolicies(ctx, account)
|
||||||
dnsCache := &cache.DNSConfigCache{}
|
dnsCache := &cache.DNSConfigCache{}
|
||||||
dnsDomain := c.GetDNSDomain(account.Settings)
|
dnsDomain := c.GetDNSDomain(account.Settings)
|
||||||
peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain)
|
peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain)
|
||||||
@@ -603,7 +629,7 @@ func (c *Controller) GetValidatedPeerWithMap(ctx context.Context, isRequiresAppr
|
|||||||
return nil, nil, 0, err
|
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)
|
approvedPeersMap, err := c.integratedPeerValidator.GetValidatedPeers(ctx, account.Id, maps.Values(account.Groups), maps.Values(account.Peers), account.Settings.Extra)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -874,7 +900,7 @@ func (c *Controller) GetNetworkMap(ctx context.Context, peerID string) (*types.N
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
account.InjectProxyPolicies(ctx)
|
c.injectAllProxyPolicies(ctx, account)
|
||||||
resourcePolicies := account.GetResourcePoliciesMap()
|
resourcePolicies := account.GetResourcePoliciesMap()
|
||||||
routers := account.GetResourceRoutersMap()
|
routers := account.GetResourceRoutersMap()
|
||||||
groupIDToUserIDs := account.GetActiveGroupUsers()
|
groupIDToUserIDs := account.GetActiveGroupUsers()
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
@@ -29,6 +29,19 @@ import (
|
|||||||
"github.com/netbirdio/netbird/route"
|
"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.
|
// 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
|
// Loaded in-tx, walked by Expand after commit. Only the collections the Change
|
||||||
// can touch are loaded; the rest stay nil (see Load).
|
// 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
|
// 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 {
|
func (snap *Snapshot) loadProxyServices(ctx context.Context, s store.Store, accountID string) error {
|
||||||
var err error
|
var err error
|
||||||
if snap.proxyByCluster, err = s.GetEmbeddedProxyPeerIDsByCluster(ctx, accountID); err != nil {
|
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 {
|
if len(snap.proxyByCluster) == 0 {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
snap.services, err = s.GetAccountServices(ctx, store.LockingStrengthNone, accountID)
|
if snap.services, err = s.GetAccountServices(ctx, store.LockingStrengthNone, accountID); err != nil {
|
||||||
return err
|
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
|
// loadGroupIndex loads all groups (for group.Resources) and builds the
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user