mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-20 14:41:29 +02:00
Compare commits
4 Commits
sync-seria
...
modify-pee
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
65b8a2089c | ||
|
|
d8937a61da | ||
|
|
5ae19bc0e4 | ||
|
|
787d07b57f |
@@ -863,19 +863,40 @@ func (e *Engine) modifyPeers(peersUpdate []*mgmProto.RemotePeerConfig) error {
|
||||
}
|
||||
}
|
||||
|
||||
// second, close all modified connections and remove them from the state map
|
||||
// second, look up the activation state of all modified peers before removing
|
||||
// any of them, so an unavailable state leaves the current connections intact
|
||||
active := make(map[string]bool, len(modified))
|
||||
for _, p := range modified {
|
||||
err := e.removePeer(p.GetWgPubKey())
|
||||
peerPubKey := p.GetWgPubKey()
|
||||
state, err := e.statusRecorder.GetPeer(peerPubKey)
|
||||
if err != nil {
|
||||
return fmt.Errorf("get status of modified peer %s: %w", peerPubKey, err)
|
||||
}
|
||||
active[peerPubKey] = state.ConnStatus != peer.StatusIdle
|
||||
}
|
||||
// then close all modified connections and remove them from the state map
|
||||
for _, p := range modified {
|
||||
if err := e.removePeer(p.GetWgPubKey()); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
// third, add the peer connections again
|
||||
// third, add the peer connections again, restoring each peer's activation
|
||||
// state: under lazy connections a re-added peer starts idle, but the remote
|
||||
// side of an established connection keeps its state and sends no further
|
||||
// offers, so a previously active peer left idle cannot reconnect until the
|
||||
// remote's connection expires.
|
||||
for _, p := range modified {
|
||||
err := e.addNewPeer(p)
|
||||
if err != nil {
|
||||
if err := e.addNewPeer(p); err != nil {
|
||||
return err
|
||||
}
|
||||
if !active[p.GetWgPubKey()] {
|
||||
continue
|
||||
}
|
||||
conn, ok := e.peerStore.PeerConn(p.GetWgPubKey())
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
e.connMgr.ActivatePeer(e.ctx, conn)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -26,6 +26,7 @@ import (
|
||||
"github.com/netbirdio/netbird/client/iface/wgaddr"
|
||||
"github.com/netbirdio/netbird/client/iface/wgproxy"
|
||||
"github.com/netbirdio/netbird/client/internal/dns"
|
||||
"github.com/netbirdio/netbird/client/internal/lazyconn"
|
||||
"github.com/netbirdio/netbird/client/internal/peer"
|
||||
"github.com/netbirdio/netbird/client/internal/peer/guard"
|
||||
icemaker "github.com/netbirdio/netbird/client/internal/peer/ice"
|
||||
@@ -466,6 +467,163 @@ func TestEngine_UpdateNetworkMap(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestEngine_ModifiedPeerKeepsActivationState verifies that a peer re-added by
|
||||
// modifyPeers keeps its previous activation state under lazy connections. A
|
||||
// modified peer is removed and re-added, and a re-add defaults to idle; the
|
||||
// remote side of an established connection keeps its state and sends no further
|
||||
// offers, so a previously active peer parked idle leaves the pair unable to
|
||||
// reconnect until the remote's connection expires.
|
||||
func TestEngine_ModifiedPeerKeepsActivationState(t *testing.T) {
|
||||
key, err := wgtypes.GeneratePrivateKey()
|
||||
require.NoError(t, err)
|
||||
|
||||
ctx, cancel := context.WithCancel(CtxInitState(context.Background()))
|
||||
t.Cleanup(cancel)
|
||||
|
||||
relayMgr := relayClient.NewManager(ctx, nil, key.PublicKey().String(), iface.DefaultMTU)
|
||||
engine := NewEngine(ctx, cancel, &EngineConfig{
|
||||
WgIfaceName: "utun103",
|
||||
WgAddr: wgaddr.MustParseWGAddress("100.64.0.1/24"),
|
||||
WgPrivateKey: key,
|
||||
WgPort: 33101,
|
||||
MTU: iface.DefaultMTU,
|
||||
LazyConnection: lazyconn.StateOn,
|
||||
}, EngineServices{
|
||||
SignalClient: &signal.MockClient{},
|
||||
MgmClient: &mgmt.MockClient{},
|
||||
RelayManager: relayMgr,
|
||||
StatusRecorder: peer.NewRecorder("https://mgm"),
|
||||
}, MobileDependency{})
|
||||
|
||||
wgIface := &MockWGIface{
|
||||
NameFunc: func() string { return "utun103" },
|
||||
IsUserspaceBindFunc: func() bool {
|
||||
return false
|
||||
},
|
||||
RemovePeerFunc: func(peerKey string) error {
|
||||
return nil
|
||||
},
|
||||
AddressFunc: func() wgaddr.Address {
|
||||
return wgaddr.Address{
|
||||
IP: netip.MustParseAddr("10.20.0.1"),
|
||||
Network: netip.MustParsePrefix("10.20.0.0/24"),
|
||||
}
|
||||
},
|
||||
UpdatePeerFunc: func(peerKey string, allowedIps []netip.Prefix, keepAlive time.Duration, endpoint *net.UDPAddr, preSharedKey *wgtypes.Key) error {
|
||||
return nil
|
||||
},
|
||||
}
|
||||
engine.wgInterface = wgIface
|
||||
engine.routeManager = routemanager.NewManager(routemanager.ManagerConfig{
|
||||
Context: ctx,
|
||||
PublicKey: key.PublicKey().String(),
|
||||
DNSRouteInterval: time.Minute,
|
||||
WGInterface: engine.wgInterface,
|
||||
StatusRecorder: engine.statusRecorder,
|
||||
RelayManager: relayMgr,
|
||||
})
|
||||
require.NoError(t, engine.routeManager.Init())
|
||||
engine.dnsServer = &dns.MockServer{
|
||||
UpdateDNSServerFunc: func(serial uint64, update nbdns.Config) error { return nil },
|
||||
}
|
||||
udpConn, err := net.ListenUDP("udp4", nil)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() {
|
||||
if err := udpConn.Close(); err != nil {
|
||||
t.Errorf("close UDP listener: %v", err)
|
||||
}
|
||||
})
|
||||
engine.udpMux = udpmux.NewUniversalUDPMuxDefault(udpmux.UniversalUDPMuxParams{UDPConn: udpConn, MTU: 1280})
|
||||
engine.ctx = ctx
|
||||
engine.srWatcher = guard.NewSRWatcher(nil, nil, nil, icemaker.Config{})
|
||||
engine.connMgr = NewConnMgr(engine.config, engine.statusRecorder, engine.peerStore, wgIface)
|
||||
engine.connMgr.Start(ctx)
|
||||
t.Cleanup(engine.connMgr.Close)
|
||||
|
||||
// No agent version: not lazy-capable, so the connection opens permanently.
|
||||
activePeer := &mgmtProto.RemotePeerConfig{
|
||||
WgPubKey: "RRHf3Ma6z6mdLbriAJbqhX7+nM/B71lgw2+91q3LfhU=",
|
||||
AllowedIps: []string{"100.64.0.10/24"},
|
||||
}
|
||||
// Lazy-capable, never activated: managed as idle.
|
||||
idlePeer := &mgmtProto.RemotePeerConfig{
|
||||
WgPubKey: "LLHf3Ma6z6mdLbriAJbqhX7+nM/B71lgw2+91q3LfhU=",
|
||||
AllowedIps: []string{"100.64.0.11/24"},
|
||||
AgentVersion: "development",
|
||||
}
|
||||
|
||||
err = engine.updateNetworkMap(&mgmtProto.NetworkMap{
|
||||
Serial: 1,
|
||||
RemotePeers: []*mgmtProto.RemotePeerConfig{activePeer, idlePeer},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
state, err := engine.statusRecorder.GetPeer(activePeer.WgPubKey)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, peer.StatusConnecting, state.ConnStatus, "peer without lazy support should open a permanent connection")
|
||||
|
||||
state, err = engine.statusRecorder.GetPeer(idlePeer.WgPubKey)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, peer.StatusIdle, state.ConnStatus, "lazy-capable peer should be managed as idle")
|
||||
|
||||
// The active peer's agent version changes, as when a peer registered over the
|
||||
// API logs in and fills in its meta; the idle peer's allowed IPs change. Both
|
||||
// count as modified and are removed and re-added.
|
||||
err = engine.updateNetworkMap(&mgmtProto.NetworkMap{
|
||||
Serial: 2,
|
||||
RemotePeers: []*mgmtProto.RemotePeerConfig{
|
||||
{
|
||||
WgPubKey: activePeer.WgPubKey,
|
||||
AllowedIps: activePeer.AllowedIps,
|
||||
AgentVersion: "development",
|
||||
},
|
||||
{
|
||||
WgPubKey: idlePeer.WgPubKey,
|
||||
AllowedIps: []string{"100.64.0.21/24"},
|
||||
AgentVersion: "development",
|
||||
},
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
state, err = engine.statusRecorder.GetPeer(activePeer.WgPubKey)
|
||||
require.NoError(t, err)
|
||||
assert.NotEqual(t, peer.StatusIdle, state.ConnStatus, "previously active peer should stay active after a modify")
|
||||
|
||||
state, err = engine.statusRecorder.GetPeer(idlePeer.WgPubKey)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, peer.StatusIdle, state.ConnStatus, "previously idle peer should stay idle after a modify")
|
||||
|
||||
// A missing status entry fails the modify before any connection is removed.
|
||||
require.NoError(t, engine.statusRecorder.RemovePeer(activePeer.WgPubKey))
|
||||
err = engine.updateNetworkMap(&mgmtProto.NetworkMap{
|
||||
Serial: 3,
|
||||
RemotePeers: []*mgmtProto.RemotePeerConfig{
|
||||
{
|
||||
WgPubKey: activePeer.WgPubKey,
|
||||
AllowedIps: []string{"100.64.0.30/24"},
|
||||
AgentVersion: "development",
|
||||
},
|
||||
{
|
||||
WgPubKey: idlePeer.WgPubKey,
|
||||
AllowedIps: []string{"100.64.0.31/24"},
|
||||
AgentVersion: "development",
|
||||
},
|
||||
},
|
||||
})
|
||||
require.ErrorContains(t, err, "get status of modified peer", "a modify with an unavailable peer state should fail")
|
||||
|
||||
activeConn, ok := engine.peerStore.PeerConn(activePeer.WgPubKey)
|
||||
require.True(t, ok, "peer with unavailable state should keep its connection")
|
||||
assert.True(t, compareNetIPLists(activeConn.WgConfig().AllowedIps, activePeer.AllowedIps),
|
||||
"peer with unavailable state should keep its allowed IPs")
|
||||
|
||||
idleConn, ok := engine.peerStore.PeerConn(idlePeer.WgPubKey)
|
||||
require.True(t, ok, "the other modified peer should keep its connection")
|
||||
assert.True(t, compareNetIPLists(idleConn.WgConfig().AllowedIps, []string{"100.64.0.21/24"}),
|
||||
"the other modified peer should keep its allowed IPs")
|
||||
}
|
||||
|
||||
func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
|
||||
Reference in New Issue
Block a user