mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-23 13:59:09 +02:00
[client] Address review comments on DNS lazy-connection warm-up
- Use netip.Addr instead of string IPs in the local resolver's PeerConnectivity and PeerActivator interfaces; extract record addresses as netip.Addr (v4-mapped forms unmapped) and convert to string only at the peer.Status boundary. - Scope warm-up to match-only (non-authoritative) zones so the synthesized private-service records keep warming while plain peer-name lookups in the authoritative peer zone no longer wake every idle peer. - Parse NB_DNS_LAZY_WARMUP_TIMEOUT once in the resolver constructor instead of on every request, and log invalid values. - Stop reusing engine.syncMsgMux on the DNS path: ConnMgr.ActivatePeer is now safe for concurrent use (lazy manager pointer guarded by a dedicated RWMutex; the manager itself is internally synchronized), so network-map processing no longer blocks DNS resolution. - Add SetPeerActivator to the dns.Server interface (no-op on the mock) and drop the *dns.DefaultServer type assertion in the engine. - Drop WireGuard wording from dns/local comments; the layer is transport-agnostic. Tests: authoritative zones never trigger warm-up; warm-up timeout env parsing (valid/invalid/non-positive); extractRecordAddr unmaps v4-mapped record data; dnsPeerActivator skips connected/unknown/ conn-less peers, returns on connect, and releases at the budget when the peer stays idle; ConnMgr.ActivatePeer races the manager lifecycle cleanly under -race.
This commit is contained in:
@@ -34,6 +34,8 @@ const (
|
|||||||
// - Handling connection establishment based on peer signaling
|
// - Handling connection establishment based on peer signaling
|
||||||
//
|
//
|
||||||
// The implementation is not thread-safe; it is protected by engine.syncMsgMux.
|
// The implementation is not thread-safe; it is protected by engine.syncMsgMux.
|
||||||
|
// The only exception is ActivatePeer, which is safe for concurrent use so the
|
||||||
|
// DNS warm-up path can call it without contending on the engine mutex.
|
||||||
type ConnMgr struct {
|
type ConnMgr struct {
|
||||||
peerStore *peerstore.Store
|
peerStore *peerstore.Store
|
||||||
statusRecorder *peer.Status
|
statusRecorder *peer.Status
|
||||||
@@ -42,6 +44,10 @@ type ConnMgr struct {
|
|||||||
rosenpassEnabled bool
|
rosenpassEnabled bool
|
||||||
|
|
||||||
lazyConnMgr *manager.Manager
|
lazyConnMgr *manager.Manager
|
||||||
|
// lazyConnMgrMu guards the lazyConnMgr pointer for readers outside the
|
||||||
|
// engine loop (ActivatePeer). Writers hold it in addition to
|
||||||
|
// engine.syncMsgMux; all other reads stay under engine.syncMsgMux only.
|
||||||
|
lazyConnMgrMu sync.RWMutex
|
||||||
|
|
||||||
wg sync.WaitGroup
|
wg sync.WaitGroup
|
||||||
lazyCtx context.Context
|
lazyCtx context.Context
|
||||||
@@ -238,12 +244,20 @@ func (e *ConnMgr) RemovePeerConn(peerKey string) {
|
|||||||
conn.Log.Infof("removed peer from lazy conn manager")
|
conn.Log.Infof("removed peer from lazy conn manager")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ActivatePeer wakes an idle lazy connection. Unlike the rest of ConnMgr it is
|
||||||
|
// safe for concurrent use: the lazy manager pointer is read under lazyConnMgrMu
|
||||||
|
// and the manager itself is internally synchronized, so callers outside the
|
||||||
|
// engine loop (DNS warm-up) do not need engine.syncMsgMux.
|
||||||
func (e *ConnMgr) ActivatePeer(ctx context.Context, conn *peer.Conn) {
|
func (e *ConnMgr) ActivatePeer(ctx context.Context, conn *peer.Conn) {
|
||||||
if !e.isStartedWithLazyMgr() {
|
e.lazyConnMgrMu.RLock()
|
||||||
|
lazyConnMgr := e.lazyConnMgr
|
||||||
|
started := lazyConnMgr != nil && e.lazyCtxCancel != nil
|
||||||
|
e.lazyConnMgrMu.RUnlock()
|
||||||
|
if !started {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if found := e.lazyConnMgr.ActivatePeer(conn.GetKey()); found {
|
if found := lazyConnMgr.ActivatePeer(conn.GetKey()); found {
|
||||||
if err := conn.Open(ctx); err != nil {
|
if err := conn.Open(ctx); err != nil {
|
||||||
conn.Log.Errorf("failed to open connection: %v", err)
|
conn.Log.Errorf("failed to open connection: %v", err)
|
||||||
}
|
}
|
||||||
@@ -268,16 +282,21 @@ func (e *ConnMgr) Close() {
|
|||||||
|
|
||||||
e.lazyCtxCancel()
|
e.lazyCtxCancel()
|
||||||
e.wg.Wait()
|
e.wg.Wait()
|
||||||
|
|
||||||
|
e.lazyConnMgrMu.Lock()
|
||||||
e.lazyConnMgr = nil
|
e.lazyConnMgr = nil
|
||||||
|
e.lazyConnMgrMu.Unlock()
|
||||||
}
|
}
|
||||||
|
|
||||||
func (e *ConnMgr) initLazyManager(engineCtx context.Context) {
|
func (e *ConnMgr) initLazyManager(engineCtx context.Context) {
|
||||||
cfg := manager.Config{
|
cfg := manager.Config{
|
||||||
InactivityThreshold: inactivityThresholdEnv(),
|
InactivityThreshold: inactivityThresholdEnv(),
|
||||||
}
|
}
|
||||||
e.lazyConnMgr = manager.NewManager(cfg, engineCtx, e.peerStore, e.iface)
|
|
||||||
|
|
||||||
|
e.lazyConnMgrMu.Lock()
|
||||||
|
e.lazyConnMgr = manager.NewManager(cfg, engineCtx, e.peerStore, e.iface)
|
||||||
e.lazyCtx, e.lazyCtxCancel = context.WithCancel(engineCtx)
|
e.lazyCtx, e.lazyCtxCancel = context.WithCancel(engineCtx)
|
||||||
|
e.lazyConnMgrMu.Unlock()
|
||||||
|
|
||||||
e.wg.Add(1)
|
e.wg.Add(1)
|
||||||
go func() {
|
go func() {
|
||||||
@@ -316,7 +335,10 @@ func (e *ConnMgr) closeManager(ctx context.Context) {
|
|||||||
|
|
||||||
e.lazyCtxCancel()
|
e.lazyCtxCancel()
|
||||||
e.wg.Wait()
|
e.wg.Wait()
|
||||||
|
|
||||||
|
e.lazyConnMgrMu.Lock()
|
||||||
e.lazyConnMgr = nil
|
e.lazyConnMgr = nil
|
||||||
|
e.lazyConnMgrMu.Unlock()
|
||||||
|
|
||||||
for _, peerID := range e.peerStore.PeersPubKey() {
|
for _, peerID := range e.peerStore.PeersPubKey() {
|
||||||
e.peerStore.PeerConnOpen(ctx, peerID)
|
e.peerStore.PeerConnOpen(ctx, peerID)
|
||||||
|
|||||||
@@ -1,10 +1,21 @@
|
|||||||
package internal
|
package internal
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
|
"net"
|
||||||
|
"net/netip"
|
||||||
"os"
|
"os"
|
||||||
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
|
||||||
|
|
||||||
|
"github.com/netbirdio/netbird/client/iface/wgaddr"
|
||||||
"github.com/netbirdio/netbird/client/internal/lazyconn"
|
"github.com/netbirdio/netbird/client/internal/lazyconn"
|
||||||
|
"github.com/netbirdio/netbird/client/internal/peer"
|
||||||
|
"github.com/netbirdio/netbird/client/internal/peerstore"
|
||||||
|
"github.com/netbirdio/netbird/monotime"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestResolveLazyForce(t *testing.T) {
|
func TestResolveLazyForce(t *testing.T) {
|
||||||
@@ -38,3 +49,58 @@ func TestResolveLazyForce(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type mockLazyWGIface struct{}
|
||||||
|
|
||||||
|
func (mockLazyWGIface) RemovePeer(string) error { return nil }
|
||||||
|
func (mockLazyWGIface) UpdatePeer(string, []netip.Prefix, time.Duration, *net.UDPAddr, *wgtypes.Key) error {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
func (mockLazyWGIface) IsUserspaceBind() bool { return false }
|
||||||
|
func (mockLazyWGIface) Address() wgaddr.Address { return wgaddr.Address{} }
|
||||||
|
func (mockLazyWGIface) LastActivities() map[string]monotime.Time { return nil }
|
||||||
|
func (mockLazyWGIface) MTU() uint16 { return 1280 }
|
||||||
|
|
||||||
|
// TestConnMgr_ActivatePeerConcurrentWithLifecycle exercises ActivatePeer from
|
||||||
|
// non-engine goroutines (the DNS warm-up path) racing the manager lifecycle,
|
||||||
|
// which stays on the engine loop. Run with -race: it fails if ActivatePeer
|
||||||
|
// still requires engine.syncMsgMux for safety.
|
||||||
|
func TestConnMgr_ActivatePeerConcurrentWithLifecycle(t *testing.T) {
|
||||||
|
t.Setenv(lazyconn.EnvLazyConn, "on")
|
||||||
|
|
||||||
|
status := peer.NewRecorder("https://mgm")
|
||||||
|
store := peerstore.NewConnStore()
|
||||||
|
connMgr := NewConnMgr(&EngineConfig{}, status, store, mockLazyWGIface{})
|
||||||
|
|
||||||
|
conn := newTestPeerConn(t, "peerA")
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
defer cancel()
|
||||||
|
connMgr.Start(ctx)
|
||||||
|
|
||||||
|
done := make(chan struct{})
|
||||||
|
var wg sync.WaitGroup
|
||||||
|
for range 4 {
|
||||||
|
wg.Add(1)
|
||||||
|
go func() {
|
||||||
|
defer wg.Done()
|
||||||
|
for {
|
||||||
|
select {
|
||||||
|
case <-done:
|
||||||
|
return
|
||||||
|
default:
|
||||||
|
connMgr.ActivatePeer(ctx, conn)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
}
|
||||||
|
|
||||||
|
// Let the activators spin against the started manager, then tear it down
|
||||||
|
// underneath them and let them spin against the stopped manager.
|
||||||
|
time.Sleep(100 * time.Millisecond)
|
||||||
|
connMgr.Close()
|
||||||
|
time.Sleep(50 * time.Millisecond)
|
||||||
|
|
||||||
|
close(done)
|
||||||
|
wg.Wait()
|
||||||
|
}
|
||||||
|
|||||||
@@ -37,31 +37,43 @@ type resolver interface {
|
|||||||
// record is left alone (it points at something outside our mesh, e.g.
|
// record is left alone (it points at something outside our mesh, e.g.
|
||||||
// a non-peer upstream).
|
// a non-peer upstream).
|
||||||
type PeerConnectivity interface {
|
type PeerConnectivity interface {
|
||||||
IsConnectedByIP(ip string) (known, connected bool)
|
IsConnectedByIP(ip netip.Addr) (known, connected bool)
|
||||||
}
|
}
|
||||||
|
|
||||||
// PeerActivator wakes lazy-connection peers on demand. The local resolver calls
|
// PeerActivator wakes lazy-connection peers on demand. The local resolver calls
|
||||||
// it with the tunnel IPs an answer points at, so a peer that is idle (lazily
|
// it with the tunnel IPs an answer points at, so a peer that is idle (lazily
|
||||||
// disconnected) begins its WireGuard handshake at DNS-resolution time rather
|
// disconnected) starts connecting at DNS-resolution time rather than racing the
|
||||||
// than racing the client's first request packet. nil disables warm-up.
|
// client's first request packet. nil disables warm-up.
|
||||||
type PeerActivator interface {
|
type PeerActivator interface {
|
||||||
// ActivatePeersByIP triggers wake-up for the peer(s) owning ips and blocks
|
// ActivatePeersByIP triggers wake-up for the peer(s) owning addrs and blocks
|
||||||
// until one is connected or ctx (a short per-query budget) expires. It is a
|
// until one is connected or ctx (a short per-query budget) expires. It is a
|
||||||
// fast no-op for unknown or already-connected IPs.
|
// fast no-op for unknown or already-connected addresses.
|
||||||
ActivatePeersByIP(ctx context.Context, ips []string)
|
ActivatePeersByIP(ctx context.Context, addrs []netip.Addr)
|
||||||
}
|
}
|
||||||
|
|
||||||
const defaultLazyWarmupTimeout = 2 * time.Second
|
const (
|
||||||
|
defaultLazyWarmupTimeout = 2 * time.Second
|
||||||
|
envLazyWarmupTimeout = "NB_DNS_LAZY_WARMUP_TIMEOUT"
|
||||||
|
)
|
||||||
|
|
||||||
// lazyWarmupTimeout is the per-query budget for waking a lazy-connection peer a
|
// lazyWarmupTimeoutFromEnv returns the per-query budget for waking a
|
||||||
// DNS answer points at. Tunable via NB_DNS_LAZY_WARMUP_TIMEOUT (a Go duration).
|
// lazy-connection peer a DNS answer points at. Tunable via
|
||||||
func lazyWarmupTimeout() time.Duration {
|
// NB_DNS_LAZY_WARMUP_TIMEOUT (a Go duration). Parsed once at construction time.
|
||||||
if v := os.Getenv("NB_DNS_LAZY_WARMUP_TIMEOUT"); v != "" {
|
func lazyWarmupTimeoutFromEnv() time.Duration {
|
||||||
if d, err := time.ParseDuration(v); err == nil && d > 0 {
|
v := os.Getenv(envLazyWarmupTimeout)
|
||||||
return d
|
if v == "" {
|
||||||
}
|
return defaultLazyWarmupTimeout
|
||||||
}
|
}
|
||||||
return defaultLazyWarmupTimeout
|
d, err := time.ParseDuration(v)
|
||||||
|
if err != nil {
|
||||||
|
log.Warnf("invalid %s value %q, using default %s: %v", envLazyWarmupTimeout, v, defaultLazyWarmupTimeout, err)
|
||||||
|
return defaultLazyWarmupTimeout
|
||||||
|
}
|
||||||
|
if d <= 0 {
|
||||||
|
log.Warnf("non-positive %s value %q, using default %s", envLazyWarmupTimeout, v, defaultLazyWarmupTimeout)
|
||||||
|
return defaultLazyWarmupTimeout
|
||||||
|
}
|
||||||
|
return d
|
||||||
}
|
}
|
||||||
|
|
||||||
type Resolver struct {
|
type Resolver struct {
|
||||||
@@ -79,6 +91,9 @@ type Resolver struct {
|
|||||||
// peerActivator, when non-nil, is called at resolution time to warm the
|
// peerActivator, when non-nil, is called at resolution time to warm the
|
||||||
// lazy connection to the peer(s) an answer points at. nil disables warm-up.
|
// lazy connection to the peer(s) an answer points at. nil disables warm-up.
|
||||||
peerActivator PeerActivator
|
peerActivator PeerActivator
|
||||||
|
// warmupTimeout is the per-query budget for the lazy-connection warm-up
|
||||||
|
// wait, resolved from the environment once at construction time.
|
||||||
|
warmupTimeout time.Duration
|
||||||
|
|
||||||
ctx context.Context
|
ctx context.Context
|
||||||
cancel context.CancelFunc
|
cancel context.CancelFunc
|
||||||
@@ -87,11 +102,12 @@ type Resolver struct {
|
|||||||
func NewResolver() *Resolver {
|
func NewResolver() *Resolver {
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
return &Resolver{
|
return &Resolver{
|
||||||
records: make(map[dns.Question][]dns.RR),
|
records: make(map[dns.Question][]dns.RR),
|
||||||
domains: make(map[domain.Domain]struct{}),
|
domains: make(map[domain.Domain]struct{}),
|
||||||
zones: make(map[domain.Domain]bool),
|
zones: make(map[domain.Domain]bool),
|
||||||
ctx: ctx,
|
warmupTimeout: lazyWarmupTimeoutFromEnv(),
|
||||||
cancel: cancel,
|
ctx: ctx,
|
||||||
|
cancel: cancel,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -160,7 +176,7 @@ func (d *Resolver) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
|
|||||||
result := d.lookupRecords(logger, question)
|
result := d.lookupRecords(logger, question)
|
||||||
// Warm before filtering: activation flips a lazily-idle target to connected,
|
// Warm before filtering: activation flips a lazily-idle target to connected,
|
||||||
// which then lets it survive the disconnected-peer filter below.
|
// which then lets it survive the disconnected-peer filter below.
|
||||||
d.warmLazyPeers(result.records)
|
d.warmLazyPeers(question, result.records)
|
||||||
result.records = d.filterDisconnectedPeerAnswers(logger, question, result.records)
|
result.records = d.filterDisconnectedPeerAnswers(logger, question, result.records)
|
||||||
replyMessage.Authoritative = !result.hasExternalData
|
replyMessage.Authoritative = !result.hasExternalData
|
||||||
replyMessage.Answer = result.records
|
replyMessage.Answer = result.records
|
||||||
@@ -534,8 +550,8 @@ func (d *Resolver) filterDisconnectedPeerAnswers(logger *log.Entry, question dns
|
|||||||
kept := make([]dns.RR, 0, len(records))
|
kept := make([]dns.RR, 0, len(records))
|
||||||
var dropped int
|
var dropped int
|
||||||
for _, rr := range records {
|
for _, rr := range records {
|
||||||
ip := extractRecordIP(rr)
|
ip, ok := extractRecordAddr(rr)
|
||||||
if ip == "" {
|
if !ok {
|
||||||
kept = append(kept, rr)
|
kept = append(kept, rr)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -559,47 +575,52 @@ func (d *Resolver) filterDisconnectedPeerAnswers(logger *log.Entry, question dns
|
|||||||
|
|
||||||
// warmLazyPeers triggers lazy-connection wake-up for the peers a resolved
|
// warmLazyPeers triggers lazy-connection wake-up for the peers a resolved
|
||||||
// answer points at and waits briefly for one to connect, so the caller's first
|
// answer points at and waits briefly for one to connect, so the caller's first
|
||||||
// request doesn't race the WireGuard handshake. No-op when no activator is
|
// request doesn't race the connection establishment. Warm-up is scoped to
|
||||||
// wired (lazy connections disabled) or the answer carries no peer IPs.
|
// match-only (non-authoritative) zones — the synthesized private-service zones
|
||||||
func (d *Resolver) warmLazyPeers(records []dns.RR) {
|
// and user-created zones whose records point at specific peers. The account's
|
||||||
|
// peer zone is authoritative, so plain peer-name lookups never trigger warm-up;
|
||||||
|
// otherwise resolving any peer's name would wake its idle connection, defeating
|
||||||
|
// laziness mesh-wide. No-op when no activator is wired (lazy connections
|
||||||
|
// disabled) or the answer carries no peer IPs.
|
||||||
|
func (d *Resolver) warmLazyPeers(question dns.Question, records []dns.RR) {
|
||||||
d.mu.RLock()
|
d.mu.RLock()
|
||||||
activator := d.peerActivator
|
activator := d.peerActivator
|
||||||
|
var nonAuth, found bool
|
||||||
|
if activator != nil {
|
||||||
|
nonAuth, found = d.findZone(question.Name)
|
||||||
|
}
|
||||||
d.mu.RUnlock()
|
d.mu.RUnlock()
|
||||||
if activator == nil {
|
if activator == nil || !found || !nonAuth {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
var ips []string
|
var addrs []netip.Addr
|
||||||
for _, rr := range records {
|
for _, rr := range records {
|
||||||
if ip := extractRecordIP(rr); ip != "" {
|
if addr, ok := extractRecordAddr(rr); ok {
|
||||||
ips = append(ips, ip)
|
addrs = append(addrs, addr)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if len(ips) == 0 {
|
if len(addrs) == 0 {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
ctx, cancel := context.WithTimeout(d.ctx, lazyWarmupTimeout())
|
ctx, cancel := context.WithTimeout(d.ctx, d.warmupTimeout)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
activator.ActivatePeersByIP(ctx, ips)
|
activator.ActivatePeersByIP(ctx, addrs)
|
||||||
}
|
}
|
||||||
|
|
||||||
// extractRecordIP returns the dotted-decimal / colon-hex IP carried by
|
// extractRecordAddr returns the IP address carried by an A or AAAA record.
|
||||||
// an A or AAAA record, or "" for any other record type.
|
// ok is false for any other record type or a record with no address.
|
||||||
func extractRecordIP(rr dns.RR) string {
|
func extractRecordAddr(rr dns.RR) (netip.Addr, bool) {
|
||||||
switch r := rr.(type) {
|
switch r := rr.(type) {
|
||||||
case *dns.A:
|
case *dns.A:
|
||||||
if r.A == nil {
|
addr, ok := netip.AddrFromSlice(r.A)
|
||||||
return ""
|
return addr.Unmap(), ok
|
||||||
}
|
|
||||||
return r.A.String()
|
|
||||||
case *dns.AAAA:
|
case *dns.AAAA:
|
||||||
if r.AAAA == nil {
|
addr, ok := netip.AddrFromSlice(r.AAAA)
|
||||||
return ""
|
return addr.Unmap(), ok
|
||||||
}
|
|
||||||
return r.AAAA.String()
|
|
||||||
}
|
}
|
||||||
return ""
|
return netip.Addr{}, false
|
||||||
}
|
}
|
||||||
|
|
||||||
// Update replaces all zones and their records
|
// Update replaces all zones and their records
|
||||||
|
|||||||
@@ -37,8 +37,8 @@ type mockPeerConnectivity struct {
|
|||||||
byIP map[string]struct{ known, connected bool }
|
byIP map[string]struct{ known, connected bool }
|
||||||
}
|
}
|
||||||
|
|
||||||
func (m mockPeerConnectivity) IsConnectedByIP(ip string) (known, connected bool) {
|
func (m mockPeerConnectivity) IsConnectedByIP(ip netip.Addr) (known, connected bool) {
|
||||||
v, ok := m.byIP[ip]
|
v, ok := m.byIP[ip.String()]
|
||||||
if !ok {
|
if !ok {
|
||||||
return false, false
|
return false, false
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,8 +2,11 @@ package local
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"net"
|
||||||
|
"net/netip"
|
||||||
"sync"
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
"github.com/miekg/dns"
|
"github.com/miekg/dns"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
@@ -13,19 +16,19 @@ import (
|
|||||||
nbdns "github.com/netbirdio/netbird/dns"
|
nbdns "github.com/netbirdio/netbird/dns"
|
||||||
)
|
)
|
||||||
|
|
||||||
// recordingActivator records the IPs it was asked to warm and returns
|
// recordingActivator records the addresses it was asked to warm and returns
|
||||||
// immediately, so ServeDNS is not blocked by the test.
|
// immediately, so ServeDNS is not blocked by the test.
|
||||||
type recordingActivator struct {
|
type recordingActivator struct {
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
called bool
|
called bool
|
||||||
ips []string
|
addrs []netip.Addr
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *recordingActivator) ActivatePeersByIP(_ context.Context, ips []string) {
|
func (r *recordingActivator) ActivatePeersByIP(_ context.Context, addrs []netip.Addr) {
|
||||||
r.mu.Lock()
|
r.mu.Lock()
|
||||||
defer r.mu.Unlock()
|
defer r.mu.Unlock()
|
||||||
r.called = true
|
r.called = true
|
||||||
r.ips = append(r.ips, ips...)
|
r.addrs = append(r.addrs, addrs...)
|
||||||
}
|
}
|
||||||
|
|
||||||
func serveA(t *testing.T, resolver *Resolver, name string) *dns.Msg {
|
func serveA(t *testing.T, resolver *Resolver, name string) *dns.Msg {
|
||||||
@@ -36,10 +39,21 @@ func serveA(t *testing.T, resolver *Resolver, name string) *dns.Msg {
|
|||||||
return resp
|
return resp
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// serviceZone registers rec in a match-only (non-authoritative) zone, the shape
|
||||||
|
// the synthesized private-service zones arrive in.
|
||||||
|
func serviceZone(t *testing.T, resolver *Resolver, zone string, records ...nbdns.SimpleRecord) {
|
||||||
|
t.Helper()
|
||||||
|
resolver.Update([]nbdns.CustomZone{{
|
||||||
|
Domain: zone,
|
||||||
|
Records: records,
|
||||||
|
NonAuthoritative: true,
|
||||||
|
}})
|
||||||
|
}
|
||||||
|
|
||||||
func TestLocalResolver_WarmsLazyPeerOnResolve(t *testing.T) {
|
func TestLocalResolver_WarmsLazyPeerOnResolve(t *testing.T) {
|
||||||
rec := nbdns.SimpleRecord{Name: "svc.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}
|
rec := nbdns.SimpleRecord{Name: "svc.proxy.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}
|
||||||
resolver := NewResolver()
|
resolver := NewResolver()
|
||||||
require.NoError(t, resolver.RegisterRecord(rec))
|
serviceZone(t, resolver, "proxy.netbird.cloud", rec)
|
||||||
|
|
||||||
act := &recordingActivator{}
|
act := &recordingActivator{}
|
||||||
resolver.SetPeerActivator(act)
|
resolver.SetPeerActivator(act)
|
||||||
@@ -50,15 +64,15 @@ func TestLocalResolver_WarmsLazyPeerOnResolve(t *testing.T) {
|
|||||||
|
|
||||||
act.mu.Lock()
|
act.mu.Lock()
|
||||||
defer act.mu.Unlock()
|
defer act.mu.Unlock()
|
||||||
assert.True(t, act.called, "activator must be invoked for an overlay A answer")
|
assert.True(t, act.called, "activator must be invoked for a service-zone A answer")
|
||||||
assert.Contains(t, act.ips, "100.64.0.7", "activator must receive the answer's peer IP")
|
assert.Contains(t, act.addrs, netip.MustParseAddr("100.64.0.7"), "activator must receive the answer's peer IP")
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestLocalResolver_NoActivatorNoWarmup(t *testing.T) {
|
func TestLocalResolver_NoActivatorNoWarmup(t *testing.T) {
|
||||||
// With no activator wired the resolver behaves exactly as before.
|
// With no activator wired the resolver behaves exactly as before.
|
||||||
rec := nbdns.SimpleRecord{Name: "svc.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}
|
rec := nbdns.SimpleRecord{Name: "svc.proxy.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}
|
||||||
resolver := NewResolver()
|
resolver := NewResolver()
|
||||||
require.NoError(t, resolver.RegisterRecord(rec))
|
serviceZone(t, resolver, "proxy.netbird.cloud", rec)
|
||||||
|
|
||||||
resp := serveA(t, resolver, rec.Name)
|
resp := serveA(t, resolver, rec.Name)
|
||||||
require.NotNil(t, resp, "resolver must still answer without an activator")
|
require.NotNil(t, resp, "resolver must still answer without an activator")
|
||||||
@@ -68,14 +82,91 @@ func TestLocalResolver_NoActivatorNoWarmup(t *testing.T) {
|
|||||||
func TestLocalResolver_NoWarmupForMissingRecord(t *testing.T) {
|
func TestLocalResolver_NoWarmupForMissingRecord(t *testing.T) {
|
||||||
// A query that resolves to nothing must not invoke the activator (no IPs).
|
// A query that resolves to nothing must not invoke the activator (no IPs).
|
||||||
resolver := NewResolver()
|
resolver := NewResolver()
|
||||||
require.NoError(t, resolver.RegisterRecord(nbdns.SimpleRecord{Name: "svc.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}))
|
serviceZone(t, resolver, "proxy.netbird.cloud",
|
||||||
|
nbdns.SimpleRecord{Name: "svc.proxy.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"})
|
||||||
|
|
||||||
act := &recordingActivator{}
|
act := &recordingActivator{}
|
||||||
resolver.SetPeerActivator(act)
|
resolver.SetPeerActivator(act)
|
||||||
|
|
||||||
serveA(t, resolver, "absent.netbird.cloud.")
|
serveA(t, resolver, "absent.proxy.netbird.cloud.")
|
||||||
|
|
||||||
act.mu.Lock()
|
act.mu.Lock()
|
||||||
defer act.mu.Unlock()
|
defer act.mu.Unlock()
|
||||||
assert.False(t, act.called, "activator must not be invoked when there is no answer")
|
assert.False(t, act.called, "activator must not be invoked when there is no answer")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestLocalResolver_NoWarmupInAuthoritativeZone(t *testing.T) {
|
||||||
|
// The account's peer zone is authoritative; resolving a peer's name there
|
||||||
|
// must not wake its lazy connection — warm-up is scoped to match-only
|
||||||
|
// (non-authoritative) zones such as the synthesized private-service zones.
|
||||||
|
rec := nbdns.SimpleRecord{Name: "peer.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.9"}
|
||||||
|
resolver := NewResolver()
|
||||||
|
resolver.Update([]nbdns.CustomZone{{
|
||||||
|
Domain: "netbird.cloud",
|
||||||
|
Records: []nbdns.SimpleRecord{rec},
|
||||||
|
}})
|
||||||
|
|
||||||
|
act := &recordingActivator{}
|
||||||
|
resolver.SetPeerActivator(act)
|
||||||
|
|
||||||
|
resp := serveA(t, resolver, rec.Name)
|
||||||
|
require.NotNil(t, resp, "resolver must answer")
|
||||||
|
require.NotEmpty(t, resp.Answer, "answer must carry the A record")
|
||||||
|
|
||||||
|
act.mu.Lock()
|
||||||
|
defer act.mu.Unlock()
|
||||||
|
assert.False(t, act.called, "activator must not be invoked for authoritative-zone answers")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLazyWarmupTimeoutFromEnv(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
value string
|
||||||
|
envSet bool
|
||||||
|
want time.Duration
|
||||||
|
}{
|
||||||
|
{name: "unset uses default", want: defaultLazyWarmupTimeout},
|
||||||
|
{name: "valid overrides", value: "5s", envSet: true, want: 5 * time.Second},
|
||||||
|
{name: "invalid falls back", value: "not-a-duration", envSet: true, want: defaultLazyWarmupTimeout},
|
||||||
|
{name: "negative falls back", value: "-1s", envSet: true, want: defaultLazyWarmupTimeout},
|
||||||
|
{name: "zero falls back", value: "0s", envSet: true, want: defaultLazyWarmupTimeout},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
if tt.envSet {
|
||||||
|
t.Setenv(envLazyWarmupTimeout, tt.value)
|
||||||
|
}
|
||||||
|
assert.Equal(t, tt.want, lazyWarmupTimeoutFromEnv())
|
||||||
|
assert.Equal(t, tt.want, NewResolver().warmupTimeout, "constructor must resolve the timeout once")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestExtractRecordAddr(t *testing.T) {
|
||||||
|
t.Run("A record yields unmapped v4", func(t *testing.T) {
|
||||||
|
// net.ParseIP returns the 16-byte v4-in-v6 form, the same shape
|
||||||
|
// miekg/dns stores after parsing an A record; the extracted address
|
||||||
|
// must compare equal to a plain v4 netip.Addr.
|
||||||
|
addr, ok := extractRecordAddr(&dns.A{A: net.ParseIP("100.64.0.7")})
|
||||||
|
require.True(t, ok)
|
||||||
|
assert.True(t, addr.Is4())
|
||||||
|
assert.Equal(t, netip.MustParseAddr("100.64.0.7"), addr)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("AAAA record yields v6", func(t *testing.T) {
|
||||||
|
addr, ok := extractRecordAddr(&dns.AAAA{AAAA: net.ParseIP("fd00::1")})
|
||||||
|
require.True(t, ok)
|
||||||
|
assert.Equal(t, netip.MustParseAddr("fd00::1"), addr)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("A record without address", func(t *testing.T) {
|
||||||
|
_, ok := extractRecordAddr(&dns.A{})
|
||||||
|
assert.False(t, ok)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("non-address record", func(t *testing.T) {
|
||||||
|
_, ok := extractRecordAddr(&dns.CNAME{Target: "target.netbird.cloud."})
|
||||||
|
assert.False(t, ok)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import (
|
|||||||
"github.com/miekg/dns"
|
"github.com/miekg/dns"
|
||||||
|
|
||||||
dnsconfig "github.com/netbirdio/netbird/client/internal/dns/config"
|
dnsconfig "github.com/netbirdio/netbird/client/internal/dns/config"
|
||||||
|
"github.com/netbirdio/netbird/client/internal/dns/local"
|
||||||
nbdns "github.com/netbirdio/netbird/dns"
|
nbdns "github.com/netbirdio/netbird/dns"
|
||||||
"github.com/netbirdio/netbird/route"
|
"github.com/netbirdio/netbird/route"
|
||||||
"github.com/netbirdio/netbird/shared/management/domain"
|
"github.com/netbirdio/netbird/shared/management/domain"
|
||||||
@@ -92,6 +93,11 @@ func (m *MockServer) SetFirewall(Firewall) {
|
|||||||
// Mock implementation - no-op
|
// Mock implementation - no-op
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SetPeerActivator mock implementation of SetPeerActivator from Server interface
|
||||||
|
func (m *MockServer) SetPeerActivator(local.PeerActivator) {
|
||||||
|
// Mock implementation - no-op
|
||||||
|
}
|
||||||
|
|
||||||
// BeginBatch mock implementation of BeginBatch from Server interface
|
// BeginBatch mock implementation of BeginBatch from Server interface
|
||||||
func (m *MockServer) BeginBatch() {
|
func (m *MockServer) BeginBatch() {
|
||||||
// Mock implementation - no-op
|
// Mock implementation - no-op
|
||||||
|
|||||||
@@ -82,6 +82,7 @@ type Server interface {
|
|||||||
PopulateManagementDomain(mgmtURL *url.URL) error
|
PopulateManagementDomain(mgmtURL *url.URL) error
|
||||||
SetRouteSources(selected, active func() route.HAMap)
|
SetRouteSources(selected, active func() route.HAMap)
|
||||||
SetFirewall(Firewall)
|
SetFirewall(Firewall)
|
||||||
|
SetPeerActivator(local.PeerActivator)
|
||||||
}
|
}
|
||||||
|
|
||||||
type nsGroupsByDomain struct {
|
type nsGroupsByDomain struct {
|
||||||
@@ -1442,11 +1443,11 @@ type localPeerConnectivity struct {
|
|||||||
|
|
||||||
// IsConnectedByIP looks the IP up in the peerstore and surfaces both
|
// IsConnectedByIP looks the IP up in the peerstore and surfaces both
|
||||||
// the known and connected bits. Used by Resolver.filterDisconnectedPeerAnswers.
|
// the known and connected bits. Used by Resolver.filterDisconnectedPeerAnswers.
|
||||||
func (l localPeerConnectivity) IsConnectedByIP(ip string) (known, connected bool) {
|
func (l localPeerConnectivity) IsConnectedByIP(ip netip.Addr) (known, connected bool) {
|
||||||
if l.status == nil {
|
if l.status == nil {
|
||||||
return false, false
|
return false, false
|
||||||
}
|
}
|
||||||
state, ok := l.status.PeerStateByIP(ip)
|
state, ok := l.status.PeerStateByIP(ip.String())
|
||||||
if !ok {
|
if !ok {
|
||||||
return false, false
|
return false, false
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ package internal
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"sync"
|
"net/netip"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/netbirdio/netbird/client/internal/peer"
|
"github.com/netbirdio/netbird/client/internal/peer"
|
||||||
@@ -12,33 +12,33 @@ import (
|
|||||||
const dnsActivationPollInterval = 50 * time.Millisecond
|
const dnsActivationPollInterval = 50 * time.Millisecond
|
||||||
|
|
||||||
// dnsPeerActivator wakes lazy-connection peers from the DNS resolution path. It
|
// dnsPeerActivator wakes lazy-connection peers from the DNS resolution path. It
|
||||||
// implements dns/local.PeerActivator. ConnMgr is not thread-safe (guarded by
|
// implements dns/local.PeerActivator. DNS queries run on their own goroutines,
|
||||||
// the engine's syncMsgMux) while DNS queries run on their own goroutines, so
|
// so it only touches state that is safe for concurrent use — ConnMgr.ActivatePeer,
|
||||||
// activation runs under that mutex; the connection wait runs without it.
|
// peerstore.Store and peer.Status — and never takes the engine's syncMsgMux,
|
||||||
|
// keeping DNS resolution from contending with network-map processing.
|
||||||
type dnsPeerActivator struct {
|
type dnsPeerActivator struct {
|
||||||
connMgr *ConnMgr
|
connMgr *ConnMgr
|
||||||
peerStore *peerstore.Store
|
peerStore *peerstore.Store
|
||||||
status *peer.Status
|
status *peer.Status
|
||||||
mu *sync.Mutex
|
|
||||||
// ctx is the engine's long-lived context. The connection dial is tied to it
|
// ctx is the engine's long-lived context. The connection dial is tied to it
|
||||||
// (not the per-query DNS wait budget) so a handshake that outlasts the wait
|
// (not the per-query DNS wait budget) so a handshake that outlasts the wait
|
||||||
// still completes in the background rather than being cancelled at the deadline.
|
// still completes in the background rather than being cancelled at the deadline.
|
||||||
ctx context.Context
|
ctx context.Context
|
||||||
}
|
}
|
||||||
|
|
||||||
// ActivatePeersByIP triggers wake-up for the peer(s) owning ips and waits until
|
// ActivatePeersByIP triggers wake-up for the peer(s) owning addrs and waits
|
||||||
// one is connected or ctx (the per-query DNS wait budget) expires. Activation
|
// until one is connected or ctx (the per-query DNS wait budget) expires.
|
||||||
// itself is tied to the engine's long-lived context so the dial survives a wait
|
// Activation itself is tied to the engine's long-lived context so the dial
|
||||||
// that times out. Unknown or already-connected IPs are skipped, so the
|
// survives a wait that times out. Unknown or already-connected addresses are
|
||||||
// steady-state (warm) path adds no latency.
|
// skipped, so the steady-state (warm) path adds no latency.
|
||||||
func (a *dnsPeerActivator) ActivatePeersByIP(ctx context.Context, ips []string) {
|
func (a *dnsPeerActivator) ActivatePeersByIP(ctx context.Context, addrs []netip.Addr) {
|
||||||
if a == nil || a.connMgr == nil {
|
if a == nil || a.connMgr == nil {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
var pending []string
|
var pending []string
|
||||||
a.mu.Lock()
|
for _, addr := range addrs {
|
||||||
for _, ip := range ips {
|
ip := addr.String()
|
||||||
st, ok := a.status.PeerStateByIP(ip)
|
st, ok := a.status.PeerStateByIP(ip)
|
||||||
if !ok || st.ConnStatus == peer.StatusConnected {
|
if !ok || st.ConnStatus == peer.StatusConnected {
|
||||||
continue
|
continue
|
||||||
@@ -50,7 +50,6 @@ func (a *dnsPeerActivator) ActivatePeersByIP(ctx context.Context, ips []string)
|
|||||||
a.connMgr.ActivatePeer(a.ctx, conn)
|
a.connMgr.ActivatePeer(a.ctx, conn)
|
||||||
pending = append(pending, ip)
|
pending = append(pending, ip)
|
||||||
}
|
}
|
||||||
a.mu.Unlock()
|
|
||||||
|
|
||||||
if len(pending) == 0 {
|
if len(pending) == 0 {
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -0,0 +1,129 @@
|
|||||||
|
package internal
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"net/netip"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"github.com/netbirdio/netbird/client/internal/peer"
|
||||||
|
"github.com/netbirdio/netbird/client/internal/peerstore"
|
||||||
|
)
|
||||||
|
|
||||||
|
func newTestPeerConn(t *testing.T, key string) *peer.Conn {
|
||||||
|
t.Helper()
|
||||||
|
conn, err := peer.NewConn(peer.ConnConfig{
|
||||||
|
Key: key,
|
||||||
|
LocalKey: "local",
|
||||||
|
WgConfig: peer.WgConfig{
|
||||||
|
AllowedIps: []netip.Prefix{netip.MustParsePrefix("100.64.0.1/32")},
|
||||||
|
},
|
||||||
|
}, peer.ServiceDependencies{})
|
||||||
|
require.NoError(t, err)
|
||||||
|
return conn
|
||||||
|
}
|
||||||
|
|
||||||
|
func newTestDNSPeerActivator(t *testing.T) (*dnsPeerActivator, *peer.Status, *peerstore.Store) {
|
||||||
|
t.Helper()
|
||||||
|
status := peer.NewRecorder("https://mgm")
|
||||||
|
store := peerstore.NewConnStore()
|
||||||
|
// ConnMgr without Start: the lazy manager is nil, so ActivatePeer is a
|
||||||
|
// no-op — these tests exercise the activator's skip/wait logic.
|
||||||
|
connMgr := NewConnMgr(&EngineConfig{}, status, store, nil)
|
||||||
|
return &dnsPeerActivator{
|
||||||
|
connMgr: connMgr,
|
||||||
|
peerStore: store,
|
||||||
|
status: status,
|
||||||
|
ctx: context.Background(),
|
||||||
|
}, status, store
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDNSPeerActivator_NilSafe(t *testing.T) {
|
||||||
|
var a *dnsPeerActivator
|
||||||
|
a.ActivatePeersByIP(context.Background(), []netip.Addr{netip.MustParseAddr("100.64.0.1")})
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDNSPeerActivator_SkipsUnknownAndConnectedPeers verifies the steady-state
|
||||||
|
// (warm) path adds no latency: already-connected and unknown addresses never
|
||||||
|
// enter the wait loop.
|
||||||
|
func TestDNSPeerActivator_SkipsUnknownAndConnectedPeers(t *testing.T) {
|
||||||
|
a, status, store := newTestDNSPeerActivator(t)
|
||||||
|
|
||||||
|
require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", "fd00::1"))
|
||||||
|
require.NoError(t, status.UpdatePeerState(peer.State{PubKey: "peerA", ConnStatus: peer.StatusConnected}))
|
||||||
|
store.AddPeerConn("peerA", newTestPeerConn(t, "peerA"))
|
||||||
|
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
start := time.Now()
|
||||||
|
a.ActivatePeersByIP(ctx, []netip.Addr{
|
||||||
|
netip.MustParseAddr("100.64.0.1"), // known, connected -> skipped
|
||||||
|
netip.MustParseAddr("fd00::1"), // known via IPv6, connected -> skipped
|
||||||
|
netip.MustParseAddr("100.64.0.99"), // unknown -> skipped
|
||||||
|
})
|
||||||
|
require.Less(t, time.Since(start), time.Second, "no pending peer must mean no wait")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDNSPeerActivator_WaitsForPendingPeerToConnect verifies the wait loop
|
||||||
|
// returns as soon as a pending peer reports connected, well before the
|
||||||
|
// per-query budget expires.
|
||||||
|
func TestDNSPeerActivator_WaitsForPendingPeerToConnect(t *testing.T) {
|
||||||
|
a, status, store := newTestDNSPeerActivator(t)
|
||||||
|
|
||||||
|
require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", ""))
|
||||||
|
store.AddPeerConn("peerA", newTestPeerConn(t, "peerA"))
|
||||||
|
|
||||||
|
go func() {
|
||||||
|
time.Sleep(150 * time.Millisecond)
|
||||||
|
_ = status.UpdatePeerState(peer.State{PubKey: "peerA", ConnStatus: peer.StatusConnected})
|
||||||
|
}()
|
||||||
|
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
start := time.Now()
|
||||||
|
a.ActivatePeersByIP(ctx, []netip.Addr{netip.MustParseAddr("100.64.0.1")})
|
||||||
|
elapsed := time.Since(start)
|
||||||
|
|
||||||
|
require.GreaterOrEqual(t, elapsed, 100*time.Millisecond, "must wait for the pending peer")
|
||||||
|
require.Less(t, elapsed, 5*time.Second, "must return on connect, not at the deadline")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDNSPeerActivator_ReturnsAtBudgetWhenPeerStaysIdle verifies a peer that
|
||||||
|
// never connects releases the DNS response at the per-query budget instead of
|
||||||
|
// blocking it indefinitely.
|
||||||
|
func TestDNSPeerActivator_ReturnsAtBudgetWhenPeerStaysIdle(t *testing.T) {
|
||||||
|
a, status, store := newTestDNSPeerActivator(t)
|
||||||
|
|
||||||
|
require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", ""))
|
||||||
|
store.AddPeerConn("peerA", newTestPeerConn(t, "peerA"))
|
||||||
|
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond)
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
start := time.Now()
|
||||||
|
a.ActivatePeersByIP(ctx, []netip.Addr{netip.MustParseAddr("100.64.0.1")})
|
||||||
|
elapsed := time.Since(start)
|
||||||
|
|
||||||
|
require.GreaterOrEqual(t, elapsed, 250*time.Millisecond, "must wait out the budget for a pending peer")
|
||||||
|
require.Less(t, elapsed, 5*time.Second, "must not block past the budget")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDNSPeerActivator_NoWaitWithoutPeerConn verifies a known-but-idle peer
|
||||||
|
// with no connection object in the store is not waited on: there is nothing to
|
||||||
|
// activate, so waiting could only ever time out.
|
||||||
|
func TestDNSPeerActivator_NoWaitWithoutPeerConn(t *testing.T) {
|
||||||
|
a, status, _ := newTestDNSPeerActivator(t)
|
||||||
|
|
||||||
|
require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", ""))
|
||||||
|
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
start := time.Now()
|
||||||
|
a.ActivatePeersByIP(ctx, []netip.Addr{netip.MustParseAddr("100.64.0.1")})
|
||||||
|
require.Less(t, time.Since(start), time.Second, "peer without a conn must not be waited on")
|
||||||
|
}
|
||||||
@@ -657,15 +657,12 @@ func (e *Engine) Start(netbirdConfig *mgmProto.NetbirdConfig, mgmtURL *url.URL)
|
|||||||
// Wire DNS-time lazy-connection warm-up now that the connection manager
|
// Wire DNS-time lazy-connection warm-up now that the connection manager
|
||||||
// exists (it does not at DNS-server construction time). A DNS answer that
|
// exists (it does not at DNS-server construction time). A DNS answer that
|
||||||
// points at an idle peer then wakes it before the client's first request.
|
// points at an idle peer then wakes it before the client's first request.
|
||||||
if ds, ok := e.dnsServer.(*dns.DefaultServer); ok {
|
e.dnsServer.SetPeerActivator(&dnsPeerActivator{
|
||||||
ds.SetPeerActivator(&dnsPeerActivator{
|
connMgr: e.connMgr,
|
||||||
connMgr: e.connMgr,
|
peerStore: e.peerStore,
|
||||||
peerStore: e.peerStore,
|
status: e.statusRecorder,
|
||||||
status: e.statusRecorder,
|
ctx: e.ctx,
|
||||||
mu: e.syncMsgMux,
|
})
|
||||||
ctx: e.ctx,
|
|
||||||
})
|
|
||||||
}
|
|
||||||
|
|
||||||
e.srWatcher = guard.NewSRWatcher(e.signal, e.relayManager, e.mobileDep.IFaceDiscover, iceCfg)
|
e.srWatcher = guard.NewSRWatcher(e.signal, e.relayManager, e.mobileDep.IFaceDiscover, iceCfg)
|
||||||
e.srWatcher.Start(peer.IsForceRelayed())
|
e.srWatcher.Start(peer.IsForceRelayed())
|
||||||
|
|||||||
Reference in New Issue
Block a user