mirror of
https://github.com/juanfont/headscale.git
synced 2026-09-26 02:04:53 +09:00
8973347d22
Each request is reduced to the narrowest change it justifies, so a
keepalive or endpoint bump no longer resends the whole node to peers.
Updates #3417
0.29 pins an older tailscale where Hostinfo.NetInfo.PreferredDERP is an int,
so the DERPRegionID types are int here.
(cherry picked from commit 59f3ff12a7)
1011 lines
31 KiB
Go
1011 lines
31 KiB
Go
package state
|
|
|
|
import (
|
|
"net/netip"
|
|
"strings"
|
|
"sync"
|
|
"sync/atomic"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/juanfont/headscale/hscontrol/types"
|
|
"github.com/juanfont/headscale/hscontrol/types/change"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"gorm.io/gorm"
|
|
"tailscale.com/tailcfg"
|
|
"tailscale.com/types/key"
|
|
"tailscale.com/types/opt"
|
|
)
|
|
|
|
func TestNetInfoFromMapRequest(t *testing.T) {
|
|
nodeID := types.NodeID(1)
|
|
|
|
tests := []struct {
|
|
name string
|
|
currentHostinfo *tailcfg.Hostinfo
|
|
reqHostinfo *tailcfg.Hostinfo
|
|
expectNetInfo *tailcfg.NetInfo
|
|
}{
|
|
{
|
|
name: "no current NetInfo - return nil",
|
|
currentHostinfo: nil,
|
|
reqHostinfo: &tailcfg.Hostinfo{
|
|
Hostname: "test-node",
|
|
},
|
|
expectNetInfo: nil,
|
|
},
|
|
{
|
|
name: "current has NetInfo, request has NetInfo - use request",
|
|
currentHostinfo: &tailcfg.Hostinfo{
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
},
|
|
reqHostinfo: &tailcfg.Hostinfo{
|
|
Hostname: "test-node",
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 2},
|
|
},
|
|
expectNetInfo: &tailcfg.NetInfo{PreferredDERP: 2},
|
|
},
|
|
{
|
|
name: "current has NetInfo, request has no NetInfo - use current",
|
|
currentHostinfo: &tailcfg.Hostinfo{
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 3},
|
|
},
|
|
reqHostinfo: &tailcfg.Hostinfo{
|
|
Hostname: "test-node",
|
|
},
|
|
expectNetInfo: &tailcfg.NetInfo{PreferredDERP: 3},
|
|
},
|
|
{
|
|
name: "current has NetInfo, no request Hostinfo - use current",
|
|
currentHostinfo: &tailcfg.Hostinfo{
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 4},
|
|
},
|
|
reqHostinfo: nil,
|
|
expectNetInfo: &tailcfg.NetInfo{PreferredDERP: 4},
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
result := netInfoFromMapRequest(nodeID, tt.currentHostinfo, tt.reqHostinfo)
|
|
|
|
if tt.expectNetInfo == nil {
|
|
assert.Nil(t, result, "expected nil NetInfo")
|
|
} else {
|
|
require.NotNil(t, result, "expected non-nil NetInfo")
|
|
assert.Equal(t, tt.expectNetInfo.PreferredDERP, result.PreferredDERP, "DERP mismatch")
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestNetInfoPreservationInRegistrationFlow(t *testing.T) {
|
|
nodeID := types.NodeID(1)
|
|
|
|
// This test reproduces the bug in registration flows where NetInfo was lost
|
|
// because we used the wrong hostinfo reference when calling [netInfoFromMapRequest]
|
|
t.Run("registration_flow_bug_reproduction", func(t *testing.T) {
|
|
// Simulate existing node with NetInfo (before re-registration)
|
|
existingNodeHostinfo := &tailcfg.Hostinfo{
|
|
Hostname: "test-node",
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 5},
|
|
}
|
|
|
|
// Simulate new registration request (no NetInfo)
|
|
newRegistrationHostinfo := &tailcfg.Hostinfo{
|
|
Hostname: "test-node",
|
|
OS: "linux",
|
|
// NetInfo is nil - this is what comes from the registration request
|
|
}
|
|
|
|
// Simulate what was happening in the bug: we passed the "current node being modified"
|
|
// hostinfo (which has no NetInfo) instead of the existing node's hostinfo
|
|
nodeBeingModifiedHostinfo := &tailcfg.Hostinfo{
|
|
Hostname: "test-node",
|
|
// NetInfo is nil because this node is being modified/reset
|
|
}
|
|
|
|
// BUG: Using the node being modified (no NetInfo) instead of existing node (has NetInfo)
|
|
buggyResult := netInfoFromMapRequest(nodeID, nodeBeingModifiedHostinfo, newRegistrationHostinfo)
|
|
assert.Nil(t, buggyResult, "Bug: Should return nil when using wrong hostinfo reference")
|
|
|
|
// CORRECT: Using the existing node's hostinfo (has NetInfo)
|
|
correctResult := netInfoFromMapRequest(nodeID, existingNodeHostinfo, newRegistrationHostinfo)
|
|
assert.NotNil(t, correctResult, "Fix: Should preserve NetInfo when using correct hostinfo reference")
|
|
assert.Equal(t, 5, correctResult.PreferredDERP, "Should preserve the DERP region from existing node")
|
|
})
|
|
|
|
t.Run("new_node_creation_for_different_user_should_preserve_netinfo", func(t *testing.T) {
|
|
// This test covers the scenario where:
|
|
// 1. A node exists for user1 with NetInfo
|
|
// 2. The same machine logs in as user2 (different user)
|
|
// 3. A NEW node is created for user2 (pre-auth key flow)
|
|
// 4. The new node should preserve NetInfo from the old node
|
|
|
|
// Existing node for user1 with NetInfo
|
|
existingNodeUser1Hostinfo := &tailcfg.Hostinfo{
|
|
Hostname: "test-node",
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 7},
|
|
}
|
|
|
|
// New registration request for user2 (no NetInfo yet)
|
|
newNodeUser2Hostinfo := &tailcfg.Hostinfo{
|
|
Hostname: "test-node",
|
|
OS: "linux",
|
|
// NetInfo is nil - registration request doesn't include it
|
|
}
|
|
|
|
// When creating a new node for user2, we should preserve NetInfo from user1's node
|
|
result := netInfoFromMapRequest(types.NodeID(2), existingNodeUser1Hostinfo, newNodeUser2Hostinfo)
|
|
assert.NotNil(t, result, "New node for user2 should preserve NetInfo from user1's node")
|
|
assert.Equal(t, 7, result.PreferredDERP, "Should preserve DERP region from existing node")
|
|
})
|
|
}
|
|
|
|
// TestNoOpMapRequestSkipsPersist ensures an identical, no-op MapRequest does
|
|
// not issue a database UPDATE (nor the O(n) policy SetNodes scan that follows
|
|
// persistNodeAndRefreshPolicy). The node state is unchanged, so persisting is pure waste on
|
|
// the hot map-request path.
|
|
func TestNoOpMapRequestSkipsPersist(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
var nodeUpdateCount atomic.Int64
|
|
|
|
gdb := s.DB().DB
|
|
cbName := "noop_count_node_updates"
|
|
err := gdb.Callback().Update().After("gorm:update").Register(cbName, func(tx *gorm.DB) {
|
|
if tx.Statement == nil {
|
|
return
|
|
}
|
|
|
|
if tx.Statement.Table == "nodes" ||
|
|
strings.Contains(strings.ToLower(tx.Statement.SQL.String()), "update \"nodes\"") {
|
|
nodeUpdateCount.Add(1)
|
|
}
|
|
})
|
|
require.NoError(t, err)
|
|
t.Cleanup(func() { _ = gdb.Callback().Update().Remove(cbName) })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok, "node should exist in NodeStore")
|
|
|
|
stored := nv.AsStruct()
|
|
|
|
req := tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
},
|
|
}
|
|
|
|
// First request establishes the Hostinfo/DERP state (expected to persist).
|
|
_, err = s.UpdateNodeFromMapRequest(nodeID, req)
|
|
require.NoError(t, err)
|
|
|
|
nodeUpdateCount.Store(0)
|
|
|
|
// Second request is value-identical: a no-op.
|
|
req2 := tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
},
|
|
}
|
|
|
|
_, err = s.UpdateNodeFromMapRequest(nodeID, req2)
|
|
require.NoError(t, err)
|
|
|
|
require.Equalf(t, int64(0), nodeUpdateCount.Load(),
|
|
"no-op MapRequest should not issue any nodes-table UPDATE, got %d",
|
|
nodeUpdateCount.Load())
|
|
}
|
|
|
|
// TestNoOpMapRequestEmitsNoPeerChange ensures an identical, no-op MapRequest
|
|
// does not emit a peer-visible change.
|
|
func TestNoOpMapRequestEmitsNoPeerChange(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok, "node should exist in NodeStore")
|
|
|
|
stored := nv.AsStruct()
|
|
|
|
req := func() tailcfg.MapRequest {
|
|
return tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
},
|
|
}
|
|
}
|
|
|
|
// First request establishes the Hostinfo/DERP state.
|
|
_, err := s.UpdateNodeFromMapRequest(nodeID, req())
|
|
require.NoError(t, err)
|
|
|
|
beforeSeen := mustLastSeen(t, s, nodeID)
|
|
|
|
// Second request is value-identical: a no-op.
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, req())
|
|
require.NoError(t, err)
|
|
|
|
require.Truef(t, c.IsEmpty(),
|
|
"no-op MapRequest must not emit a change, got reason=%q type=%q peersChanged=%v",
|
|
c.Reason, c.Type(), c.PeersChanged)
|
|
|
|
require.True(t, mustLastSeen(t, s, nodeID).After(beforeSeen),
|
|
"no-op request must still stamp LastSeen in the NodeStore")
|
|
}
|
|
|
|
// TestSTUNOnlyEndpointUpdateEmitsNoPeerChange ensures endpoint churn that
|
|
// endpointBroadcastWorthy suppresses stays suppressed instead of escalating
|
|
// to a whole-peer change.
|
|
func TestSTUNOnlyEndpointUpdateEmitsNoPeerChange(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok, "node should exist in NodeStore")
|
|
|
|
stored := nv.AsStruct()
|
|
|
|
hi := func() *tailcfg.Hostinfo {
|
|
return &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
}
|
|
}
|
|
|
|
local := netip.MustParseAddrPort("192.168.1.5:41641")
|
|
|
|
// First request establishes the Hostinfo/DERP state and a useful
|
|
// endpoint, so the delta below is churn on a set peers already hold
|
|
// rather than the node's first endpoints.
|
|
_, err := s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: hi(),
|
|
Endpoints: []netip.AddrPort{local},
|
|
EndpointTypes: []tailcfg.EndpointType{tailcfg.EndpointLocal},
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
// Second request adds a single STUN-derived endpoint and nothing else.
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: hi(),
|
|
Endpoints: []netip.AddrPort{local, netip.MustParseAddrPort("198.51.100.7:41641")},
|
|
EndpointTypes: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN},
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
require.Truef(t, c.IsEmpty(),
|
|
"suppressed STUN-only endpoint delta must not emit a change, got reason=%q type=%q peersChanged=%v",
|
|
c.Reason, c.Type(), c.PeersChanged)
|
|
|
|
after, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
require.Contains(t, after.Endpoints().AsSlice(), netip.MustParseAddrPort("198.51.100.7:41641"),
|
|
"suppressed endpoint must still be stored")
|
|
}
|
|
|
|
// TestFirstEndpointsReachPeersEvenWhenSTUNOnly pins that the first endpoint
|
|
// set a node announces is always broadcast. Peers hold no endpoints for it
|
|
// yet, and a suppressed delta is never resent, so suppressing the first set
|
|
// leaves peers with no direct path for the life of the node.
|
|
func TestFirstEndpointsReachPeersEvenWhenSTUNOnly(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok, "node should exist in NodeStore")
|
|
require.Empty(t, nv.Endpoints().AsSlice(), "precondition: node starts with no endpoints")
|
|
|
|
stored := nv.AsStruct()
|
|
stun := netip.MustParseAddrPort("198.51.100.7:41641")
|
|
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Endpoints: []netip.AddrPort{stun},
|
|
EndpointTypes: []tailcfg.EndpointType{tailcfg.EndpointSTUN},
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
require.Falsef(t, c.IsEmpty(),
|
|
"a node's first endpoints must reach peers, got reason=%q type=%q", c.Reason, c.Type())
|
|
require.Len(t, c.PeerPatches, 1, "expected a single endpoint patch")
|
|
require.Contains(t, c.PeerPatches[0].Endpoints, stun,
|
|
"the patch must carry the announced endpoint")
|
|
}
|
|
|
|
// TestMapRequestDERPLatencyJitterEmitsNoPeerChange pins the wiring between
|
|
// [State.UpdateNodeFromMapRequest] and the Hostinfo comparison: DERP latency
|
|
// is transport diagnostics that no peer reads, so jitter must not reach the
|
|
// classifier as a change. hostinfoEqual covers the comparison itself; this
|
|
// covers the path.
|
|
func TestMapRequestDERPLatencyJitterEmitsNoPeerChange(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok, "node should exist in NodeStore")
|
|
|
|
stored := nv.AsStruct()
|
|
|
|
hi := func(latency float64) *tailcfg.Hostinfo {
|
|
return &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
NetInfo: &tailcfg.NetInfo{
|
|
PreferredDERP: 1,
|
|
DERPLatency: map[string]float64{"1-v4": latency},
|
|
},
|
|
}
|
|
}
|
|
|
|
// Establish the Hostinfo, DERP region and a first latency sample.
|
|
_, err := s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: hi(0.010),
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
// Only the latency sample moves.
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: hi(0.025),
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
require.Truef(t, c.IsEmpty(),
|
|
"DERP latency jitter must not emit a change, got reason=%q type=%q peersChanged=%v",
|
|
c.Reason, c.Type(), c.PeersChanged)
|
|
|
|
after, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
require.Equal(t, 1, after.Hostinfo().NetInfo().PreferredDERP(),
|
|
"the DERP region must survive a latency-only request")
|
|
}
|
|
|
|
// TestMapRequestOmittedNetInfoIsNoChange pins that a MapRequest carrying
|
|
// Hostinfo with NetInfo omitted is compared against the preserved NetInfo, not
|
|
// against the bare request. Tailscale clients send NetInfo only when it
|
|
// changed, so classifying the omission as a Hostinfo change turns
|
|
// every routine map request into a database write, an O(n) policy rescan and a
|
|
// whole-peer broadcast.
|
|
func TestMapRequestOmittedNetInfoIsNoChange(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
var nodeUpdateCount atomic.Int64
|
|
|
|
gdb := s.DB().DB
|
|
cbName := "omitted_netinfo_count_node_updates"
|
|
err := gdb.Callback().Update().After("gorm:update").Register(cbName, func(tx *gorm.DB) {
|
|
if tx.Statement != nil && tx.Statement.Table == "nodes" {
|
|
nodeUpdateCount.Add(1)
|
|
}
|
|
})
|
|
require.NoError(t, err)
|
|
t.Cleanup(func() { _ = gdb.Callback().Update().Remove(cbName) })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
|
|
stored := nv.AsStruct()
|
|
|
|
// Establish Hostinfo with NetInfo.
|
|
_, err = s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
OS: "linux",
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
},
|
|
})
|
|
require.NoError(t, err)
|
|
require.Positive(t, nodeUpdateCount.Load(), "first request must persist")
|
|
|
|
nodeUpdateCount.Store(0)
|
|
|
|
// Same Hostinfo, NetInfo omitted: the client is saying "unchanged".
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
OS: "linux",
|
|
},
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
require.True(t, c.IsEmpty(), "omitted NetInfo must not broadcast, got %+v", c)
|
|
require.Equalf(t, int64(0), nodeUpdateCount.Load(),
|
|
"omitted NetInfo must not persist, got %d updates", nodeUpdateCount.Load())
|
|
|
|
after, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
require.Equal(t, int(1), hostinfoDERP(after.AsStruct().Hostinfo),
|
|
"stored NetInfo must survive a request that omits it")
|
|
}
|
|
|
|
// TestMapRequestDERPClearToZeroIsStoredAndBroadcast pins that a node reporting
|
|
// PreferredDERP 0 has its home region cleared. tailcfg.PeerChange.DERPRegion
|
|
// zero means "unchanged" on the wire, so the clear cannot ride a patch and has
|
|
// to escalate to a whole-peer update.
|
|
func TestMapRequestDERPClearToZeroIsStoredAndBroadcast(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
var nodeUpdateCount atomic.Int64
|
|
|
|
gdb := s.DB().DB
|
|
cbName := "derp_clear_count_node_updates"
|
|
err := gdb.Callback().Update().After("gorm:update").Register(cbName, func(tx *gorm.DB) {
|
|
if tx.Statement != nil && tx.Statement.Table == "nodes" {
|
|
nodeUpdateCount.Add(1)
|
|
}
|
|
})
|
|
require.NoError(t, err)
|
|
t.Cleanup(func() { _ = gdb.Callback().Update().Remove(cbName) })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
|
|
stored := nv.AsStruct()
|
|
|
|
_, err = s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
},
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
nodeUpdateCount.Store(0)
|
|
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 0},
|
|
},
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
after, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
require.Equal(t, int(0), hostinfoDERP(after.AsStruct().Hostinfo),
|
|
"clearing PreferredDERP must be stored")
|
|
|
|
require.Contains(t, c.PeersChanged, nodeID,
|
|
"clearing PreferredDERP cannot be a patch, got %+v", c)
|
|
require.Empty(t, c.PeerPatches,
|
|
"clearing PreferredDERP must not emit a DERP patch, got %+v", c)
|
|
require.Positive(t, nodeUpdateCount.Load(), "clearing PreferredDERP must be persisted")
|
|
}
|
|
|
|
// TestMapRequestDERPOnlyChangeKeepsGivenName pins that a request changing only
|
|
// PreferredDERP does not re-derive GivenName. Peers only receive a DERP patch
|
|
// for such a request, so a rename here would never reach them.
|
|
func TestMapRequestDERPOnlyChangeKeepsGivenName(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
|
|
stored := nv.AsStruct()
|
|
|
|
req := func(derp int) tailcfg.MapRequest {
|
|
return tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: derp},
|
|
},
|
|
}
|
|
}
|
|
|
|
_, err := s.UpdateNodeFromMapRequest(nodeID, req(1))
|
|
require.NoError(t, err)
|
|
|
|
// A collision-bumped name whose base is free again is exactly what the
|
|
// auto-derive path rewrites.
|
|
bumped := stored.Hostname + "-1"
|
|
_, ok = s.nodeStore.UpdateNode(nodeID, func(n *types.Node) { n.GivenName = bumped })
|
|
require.True(t, ok)
|
|
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, req(2))
|
|
require.NoError(t, err)
|
|
require.NotEmpty(t, c.PeerPatches, "DERP-only change must be a patch, got %+v", c)
|
|
|
|
after, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
require.Equal(t, bumped, after.GivenName(), "DERP-only change must not touch GivenName")
|
|
}
|
|
|
|
func TestConcurrentMapRequestDERPUsesPersistedState(t *testing.T) {
|
|
for _, rotateKey := range []bool{false, true} {
|
|
for _, tt := range []struct {
|
|
name string
|
|
firstDERP, lastDERP int
|
|
}{
|
|
{name: "newer region", firstDERP: 2, lastDERP: 3},
|
|
{name: "newer clear", firstDERP: 2, lastDERP: 0},
|
|
{name: "superseded clear", firstDERP: 0, lastDERP: 3},
|
|
} {
|
|
name := tt.name
|
|
if rotateKey {
|
|
name += " with key rotation"
|
|
}
|
|
|
|
t.Run(name, func(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
|
|
req := tailcfg.MapRequest{
|
|
NodeKey: nv.NodeKey(),
|
|
DiscoKey: nv.DiscoKey(),
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: nv.Hostname(),
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
},
|
|
}
|
|
_, err := s.UpdateNodeFromMapRequest(nodeID, req)
|
|
require.NoError(t, err)
|
|
|
|
if rotateKey {
|
|
req.NodeKey = key.NewNode().Public()
|
|
req.DiscoKey = key.NewDisco().Public()
|
|
}
|
|
|
|
// Let both requests publish their NodeStore writes before
|
|
// either can persist. Both must then use the last region,
|
|
// regardless of which response reaches peers first.
|
|
s.persistMu.Lock()
|
|
unlockPersist := sync.OnceFunc(s.persistMu.Unlock)
|
|
|
|
var wg sync.WaitGroup
|
|
|
|
t.Cleanup(func() {
|
|
unlockPersist()
|
|
wg.Wait()
|
|
})
|
|
|
|
type result struct {
|
|
change change.Change
|
|
err error
|
|
}
|
|
|
|
results := make(chan result, 2)
|
|
|
|
for _, derp := range []int{tt.firstDERP, tt.lastDERP} {
|
|
request := req
|
|
request.Hostinfo = req.Hostinfo.Clone()
|
|
request.Hostinfo.NetInfo.PreferredDERP = derp
|
|
|
|
wg.Go(func() {
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, request)
|
|
results <- result{change: c, err: err}
|
|
})
|
|
|
|
require.EventuallyWithT(t, func(c *assert.CollectT) {
|
|
stored, exists := s.GetNodeByID(nodeID)
|
|
require.True(c, exists)
|
|
require.Equal(c, derp, stored.Hostinfo().NetInfo().PreferredDERP())
|
|
}, 5*time.Second, time.Millisecond, "request must update NodeStore before persisting")
|
|
}
|
|
|
|
unlockPersist()
|
|
wg.Wait()
|
|
close(results)
|
|
|
|
for result := range results {
|
|
require.NoError(t, result.err)
|
|
|
|
if tt.lastDERP == 0 {
|
|
require.Contains(t, result.change.PeersChanged, nodeID,
|
|
"the current region is zero, so neither response can use a DERP patch")
|
|
require.Empty(t, result.change.PeerPatches)
|
|
|
|
continue
|
|
}
|
|
|
|
require.Empty(t, result.change.PeersChanged)
|
|
require.Len(t, result.change.PeerPatches, 1)
|
|
require.Equal(t, tt.lastDERP, result.change.PeerPatches[0].DERPRegion,
|
|
"neither response may restore a superseded DERP region")
|
|
}
|
|
|
|
persisted, err := s.DB().GetNodeByID(nodeID)
|
|
require.NoError(t, err)
|
|
require.Equal(t, tt.lastDERP, persisted.Hostinfo.NetInfo.PreferredDERP)
|
|
|
|
// A resend is still a no-op; correct delivery cannot depend
|
|
// on a future identical request repairing the peer's state.
|
|
req.Hostinfo = req.Hostinfo.Clone()
|
|
req.Hostinfo.NetInfo.PreferredDERP = tt.lastDERP
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, req)
|
|
require.NoError(t, err)
|
|
require.True(t, c.IsEmpty())
|
|
})
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestMapRequestPeerInvisibleHostinfoChangeIsStoredNotBroadcast pins that
|
|
// a Hostinfo field no peer reads is stored and persisted but does not
|
|
// resend the whole node to every peer.
|
|
func TestMapRequestPeerInvisibleHostinfoChangeIsStoredNotBroadcast(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
var nodeUpdateCount atomic.Int64
|
|
|
|
gdb := s.DB().DB
|
|
cbName := "peer_invisible_count_node_updates"
|
|
err := gdb.Callback().Update().After("gorm:update").Register(cbName, func(tx *gorm.DB) {
|
|
if tx.Statement != nil && tx.Statement.Table == "nodes" {
|
|
nodeUpdateCount.Add(1)
|
|
}
|
|
})
|
|
require.NoError(t, err)
|
|
t.Cleanup(func() { _ = gdb.Callback().Update().Remove(cbName) })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
|
|
stored := nv.AsStruct()
|
|
|
|
req := func(model string) tailcfg.MapRequest {
|
|
return tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
OS: "linux",
|
|
DeviceModel: model,
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
},
|
|
}
|
|
}
|
|
|
|
_, err = s.UpdateNodeFromMapRequest(nodeID, req("a"))
|
|
require.NoError(t, err)
|
|
|
|
nodeUpdateCount.Store(0)
|
|
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, req("b"))
|
|
require.NoError(t, err)
|
|
require.True(t, c.IsEmpty(), "peers read no DeviceModel, got %+v", c)
|
|
require.Positive(t, nodeUpdateCount.Load(), "the new DeviceModel must be persisted")
|
|
|
|
after, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
require.Equal(t, "b", after.Hostinfo().DeviceModel(), "the new DeviceModel must be stored")
|
|
}
|
|
|
|
// TestMapRequestPeerVisibleHostinfoChangeIsBroadcast pins that a Hostinfo
|
|
// field peers read still resends the whole node.
|
|
func TestMapRequestPeerVisibleHostinfoChangeIsBroadcast(t *testing.T) {
|
|
_, s, nodeID := persistTestSetup(t)
|
|
t.Cleanup(func() { _ = s.Close() })
|
|
|
|
nv, ok := s.GetNodeByID(nodeID)
|
|
require.True(t, ok)
|
|
|
|
stored := nv.AsStruct()
|
|
|
|
req := func(os string) tailcfg.MapRequest {
|
|
return tailcfg.MapRequest{
|
|
NodeKey: stored.NodeKey,
|
|
DiscoKey: stored.DiscoKey,
|
|
Hostinfo: &tailcfg.Hostinfo{
|
|
Hostname: stored.Hostname,
|
|
OS: os,
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
},
|
|
}
|
|
}
|
|
|
|
_, err := s.UpdateNodeFromMapRequest(nodeID, req("linux"))
|
|
require.NoError(t, err)
|
|
|
|
c, err := s.UpdateNodeFromMapRequest(nodeID, req("windows"))
|
|
require.NoError(t, err)
|
|
require.Contains(t, c.PeersChanged, nodeID, "peers read OS, got %+v", c)
|
|
}
|
|
|
|
func mustLastSeen(t *testing.T, s *State, id types.NodeID) time.Time {
|
|
t.Helper()
|
|
|
|
nv, ok := s.GetNodeByID(id)
|
|
require.True(t, ok)
|
|
|
|
seen, ok := nv.LastSeen().GetOk()
|
|
require.True(t, ok, "LastSeen must be stamped")
|
|
|
|
return seen
|
|
}
|
|
|
|
func TestHostinfoEqual(t *testing.T) {
|
|
t.Run("both nil", func(t *testing.T) {
|
|
require.True(t, hostinfoEqual(nil, nil))
|
|
})
|
|
|
|
t.Run("one nil", func(t *testing.T) {
|
|
require.False(t, hostinfoEqual(&tailcfg.Hostinfo{}, nil))
|
|
require.False(t, hostinfoEqual(nil, &tailcfg.Hostinfo{}))
|
|
})
|
|
|
|
t.Run("identical", func(t *testing.T) {
|
|
a := &tailcfg.Hostinfo{Hostname: "node1", OS: "linux"}
|
|
b := &tailcfg.Hostinfo{Hostname: "node1", OS: "linux"}
|
|
require.True(t, hostinfoEqual(a, b))
|
|
})
|
|
|
|
t.Run("DERPLatency jitter is not a change", func(t *testing.T) {
|
|
a := &tailcfg.Hostinfo{
|
|
Hostname: "node1",
|
|
NetInfo: &tailcfg.NetInfo{
|
|
PreferredDERP: 1,
|
|
DERPLatency: map[string]float64{"1-v4": 0.010},
|
|
},
|
|
}
|
|
b := &tailcfg.Hostinfo{
|
|
Hostname: "node1",
|
|
NetInfo: &tailcfg.NetInfo{
|
|
PreferredDERP: 1,
|
|
DERPLatency: map[string]float64{"1-v4": 0.025}, // jitter
|
|
},
|
|
}
|
|
require.True(t, hostinfoEqual(a, b),
|
|
"DERPLatency jitter must not count as a change")
|
|
})
|
|
|
|
t.Run("PreferredDERP change is not a change here", func(t *testing.T) {
|
|
a := &tailcfg.Hostinfo{
|
|
Hostname: "node1",
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
|
|
}
|
|
b := &tailcfg.Hostinfo{
|
|
Hostname: "node1",
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 2},
|
|
}
|
|
require.True(t, hostinfoEqual(a, b),
|
|
"PreferredDERP change is tracked separately")
|
|
})
|
|
|
|
t.Run("hostname change", func(t *testing.T) {
|
|
a := &tailcfg.Hostinfo{Hostname: "node1"}
|
|
b := &tailcfg.Hostinfo{Hostname: "node2"}
|
|
require.False(t, hostinfoEqual(a, b))
|
|
})
|
|
|
|
t.Run("route change", func(t *testing.T) {
|
|
a := &tailcfg.Hostinfo{Hostname: "node1", RoutableIPs: nil}
|
|
b := &tailcfg.Hostinfo{Hostname: "node1"}
|
|
// RoutableIPs nil vs nil: equal
|
|
require.True(t, hostinfoEqual(a, b))
|
|
|
|
// Add a route.
|
|
// (also tracked separately as routesChangedInput)
|
|
b.RoutableIPs = []netip.Prefix{netip.MustParsePrefix("10.0.0.0/24")}
|
|
require.False(t, hostinfoEqual(a, b))
|
|
})
|
|
}
|
|
|
|
func TestPeerHostinfoEqual(t *testing.T) {
|
|
base := func() *tailcfg.Hostinfo {
|
|
return &tailcfg.Hostinfo{
|
|
Hostname: "node",
|
|
OS: "linux",
|
|
DeviceModel: "laptop",
|
|
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1, WorkingUDP: opt.NewBool(true)},
|
|
}
|
|
}
|
|
|
|
tests := []struct {
|
|
name string
|
|
mutate func(*tailcfg.Hostinfo)
|
|
want bool
|
|
}{
|
|
{name: "no change", mutate: func(*tailcfg.Hostinfo) {}, want: true},
|
|
{name: "device model", mutate: func(hi *tailcfg.Hostinfo) { hi.DeviceModel = "desktop" }, want: true},
|
|
{name: "shields up", mutate: func(hi *tailcfg.Hostinfo) { hi.ShieldsUp = true }, want: true},
|
|
{name: "ipn version", mutate: func(hi *tailcfg.Hostinfo) { hi.IPNVersion = "1.99" }, want: true},
|
|
{name: "netinfo udp", mutate: func(hi *tailcfg.Hostinfo) { hi.NetInfo.WorkingUDP = opt.NewBool(false) }, want: true},
|
|
{name: "netinfo dropped", mutate: func(hi *tailcfg.Hostinfo) { hi.NetInfo = nil }, want: true},
|
|
{name: "hostname", mutate: func(hi *tailcfg.Hostinfo) { hi.Hostname = "other" }, want: false},
|
|
{name: "os", mutate: func(hi *tailcfg.Hostinfo) { hi.OS = "windows" }, want: false},
|
|
{
|
|
name: "services",
|
|
mutate: func(hi *tailcfg.Hostinfo) { hi.Services = []tailcfg.Service{{Proto: "peerapi4", Port: 1}} },
|
|
want: false,
|
|
},
|
|
{name: "ssh host keys", mutate: func(hi *tailcfg.Hostinfo) { hi.SSH_HostKeys = []string{"ssh-ed25519 AAAA"} }, want: false},
|
|
{name: "location", mutate: func(hi *tailcfg.Hostinfo) { hi.Location = &tailcfg.Location{Priority: 5} }, want: false},
|
|
{name: "app connector", mutate: func(hi *tailcfg.Hostinfo) { hi.AppConnector = opt.NewBool(true) }, want: false},
|
|
{
|
|
name: "routes",
|
|
mutate: func(hi *tailcfg.Hostinfo) { hi.RoutableIPs = []netip.Prefix{netip.MustParsePrefix("10.0.0.0/24")} },
|
|
want: false,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
changed := base()
|
|
tt.mutate(changed)
|
|
require.Equal(t, tt.want, peerHostinfoEqual(base(), changed))
|
|
})
|
|
}
|
|
|
|
require.True(t, peerHostinfoEqual(nil, nil))
|
|
require.False(t, peerHostinfoEqual(nil, base()))
|
|
}
|
|
|
|
func TestNetInfoEqualIgnoringDERP(t *testing.T) {
|
|
t.Run("both nil", func(t *testing.T) {
|
|
require.True(t, netInfoEqualIgnoringDERP(nil, nil))
|
|
})
|
|
|
|
t.Run("ignores PreferredDERP", func(t *testing.T) {
|
|
a := &tailcfg.NetInfo{PreferredDERP: 1, WorkingUDP: opt.NewBool(true)}
|
|
b := &tailcfg.NetInfo{PreferredDERP: 9, WorkingUDP: opt.NewBool(true)}
|
|
require.True(t, netInfoEqualIgnoringDERP(a, b))
|
|
})
|
|
|
|
t.Run("ignores DERPLatency", func(t *testing.T) {
|
|
a := &tailcfg.NetInfo{DERPLatency: map[string]float64{"1": 0.01}}
|
|
b := &tailcfg.NetInfo{DERPLatency: map[string]float64{"1": 0.99}}
|
|
require.True(t, netInfoEqualIgnoringDERP(a, b))
|
|
})
|
|
|
|
t.Run("catches WorkingUDP change", func(t *testing.T) {
|
|
a := &tailcfg.NetInfo{WorkingUDP: opt.NewBool(true)}
|
|
b := &tailcfg.NetInfo{WorkingUDP: opt.NewBool(false)}
|
|
require.False(t, netInfoEqualIgnoringDERP(a, b))
|
|
})
|
|
}
|
|
|
|
func TestHostinfoDERP(t *testing.T) {
|
|
require.Equal(t, int(0), hostinfoDERP(nil))
|
|
require.Equal(t, int(0), hostinfoDERP(&tailcfg.Hostinfo{}))
|
|
require.Equal(t, int(0), hostinfoDERP(&tailcfg.Hostinfo{NetInfo: &tailcfg.NetInfo{}}))
|
|
require.Equal(t, int(5), hostinfoDERP(&tailcfg.Hostinfo{NetInfo: &tailcfg.NetInfo{PreferredDERP: 5}}))
|
|
}
|
|
|
|
func TestBuildMapRequestChangeResponse(t *testing.T) {
|
|
node := types.Node{
|
|
ID: 1,
|
|
NodeKey: key.NewNode().Public(),
|
|
DiscoKey: key.NewDisco().Public(),
|
|
Endpoints: []netip.AddrPort{netip.MustParseAddrPort("203.0.113.9:41641")},
|
|
Hostinfo: &tailcfg.Hostinfo{NetInfo: &tailcfg.NetInfo{PreferredDERP: 2}},
|
|
}
|
|
tests := []struct {
|
|
name string
|
|
delta mapRequestDelta
|
|
nodeDERP int
|
|
|
|
wantEmpty bool
|
|
wantWholePeer bool
|
|
wantKeys bool
|
|
wantEndpoints bool
|
|
wantDERP int
|
|
}{
|
|
{
|
|
name: "nothing peer visible",
|
|
delta: mapRequestDelta{},
|
|
|
|
wantEmpty: true,
|
|
},
|
|
{
|
|
name: "peer visible hostinfo",
|
|
delta: mapRequestDelta{peerHostinfoChanged: true, endpointBroadcast: true},
|
|
|
|
wantWholePeer: true,
|
|
},
|
|
{
|
|
name: "derp clear to zero",
|
|
delta: mapRequestDelta{derpChanged: true, oldDERP: 1, newDERP: 0},
|
|
|
|
wantWholePeer: true,
|
|
},
|
|
{
|
|
name: "disco key and derp clear to zero",
|
|
delta: mapRequestDelta{discoKeyChanged: true, derpChanged: true, oldDERP: 1, newDERP: 0},
|
|
|
|
wantWholePeer: true,
|
|
},
|
|
{
|
|
name: "disco key only",
|
|
delta: mapRequestDelta{discoKeyChanged: true},
|
|
|
|
wantKeys: true,
|
|
wantEndpoints: true,
|
|
},
|
|
{
|
|
name: "disco key and derp move",
|
|
delta: mapRequestDelta{discoKeyChanged: true, derpChanged: true, oldDERP: 1, newDERP: 2},
|
|
nodeDERP: 2,
|
|
|
|
wantKeys: true,
|
|
wantEndpoints: true,
|
|
wantDERP: 2,
|
|
},
|
|
{
|
|
name: "useful endpoint only",
|
|
delta: mapRequestDelta{endpointBroadcast: true},
|
|
|
|
wantEndpoints: true,
|
|
},
|
|
{
|
|
name: "derp move only",
|
|
delta: mapRequestDelta{derpChanged: true, oldDERP: 1, newDERP: 2},
|
|
nodeDERP: 2,
|
|
|
|
wantDERP: 2,
|
|
},
|
|
{
|
|
name: "endpoint and derp move",
|
|
delta: mapRequestDelta{endpointBroadcast: true, derpChanged: true, oldDERP: 1, newDERP: 2},
|
|
nodeDERP: 2,
|
|
|
|
wantEndpoints: true,
|
|
wantDERP: 2,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
current := node.Clone()
|
|
current.Hostinfo.NetInfo.PreferredDERP = tt.nodeDERP
|
|
c := buildMapRequestChangeResponse(node.ID, current.View(), tt.delta)
|
|
|
|
if tt.wantEmpty {
|
|
require.True(t, c.IsEmpty(), "got %+v", c)
|
|
|
|
return
|
|
}
|
|
|
|
if tt.wantWholePeer {
|
|
require.Contains(t, c.PeersChanged, node.ID, "got %+v", c)
|
|
require.Empty(t, c.PeerPatches)
|
|
|
|
return
|
|
}
|
|
|
|
require.Empty(t, c.PeersChanged, "got %+v", c)
|
|
require.Len(t, c.PeerPatches, 1)
|
|
|
|
patch := c.PeerPatches[0]
|
|
require.Equal(t, tt.wantKeys, patch.Key != nil && patch.DiscoKey != nil, "keys on patch")
|
|
require.Equal(t, tt.wantEndpoints, patch.Endpoints != nil, "endpoints on patch")
|
|
require.Equal(t, tt.wantDERP, patch.DERPRegion, "DERP on patch")
|
|
})
|
|
}
|
|
}
|