state: fold primary route election into NodeStore snapshot

primaryRoutes lived as a separate write-then-read store guarded by an
external mutex. Each Connect, Disconnect, SetApprovedRoutes, and HA
prober write touched both NodeStore and primaryRoutes in sequence,
opening a TOCTOU window in which a stale Disconnect could overwrite a
fresh Connect's route assignment (issue #3203).

The election algorithm is a pure function of the snapshot: every node
contributes its IsOnline + AllApprovedRoutes + a runtime Unhealthy
bit. Move the computation into snapshotFromNodes so it runs in the
NodeStore writer goroutine and recomputes from immutable input on
every batch — no separate primaryRoutes mutation is required.

Caller-side changes are local. GetNodePrimaryRoutes, RoutesForPeer,
the HA prober and the debug endpoint all read PrimaryRoutesForNode /
HANodes / DebugRoutes off NodeStore. Health writes flow through
State.SetNodeUnhealthy, which captures the snapshot's primaries
before/after and signals back whether failover happened. The
SetApprovedRoutes and UpdateNodeFromMapRequest paths use the same
prevPrimaries / maps.Equal pattern to detect when announced/approved
changes shifted a primary.

The per-node lock and connectGen counter that closed the original
TOCTOU window are gone too: Connect bumps a SessionEpoch field on
the node inside its UpdateNode closure, Disconnect re-checks it in
its closure and no-ops on mismatch. The NodeStore writer goroutine
serialises both batches so the check + mutation are atomic by
construction.

Updates #3203
This commit is contained in:
Kristoffer Dalby committed 2026-04-29 08:02:45 +00:00
1 parent c68e763e62
commit 5dd622a31b
8 files changed
+224 -208

No files matched your search

+11 -7
View File
@@ -9,7 +9,6 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/juanfont/headscale/hscontrol/routes"
"github.com/juanfont/headscale/hscontrol/types"
"tailscale.com/net/tsaddr"
"tailscale.com/tailcfg"
@@ -209,25 +208,30 @@ func TestTailNode(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
primary := routes.New()
cfg := &types.Config{
BaseDomain: tt.baseDomain,
TailcfgDNSConfig: tt.dnsConfig,
RandomizeClientPort: false,
Taildrop: types.TaildropConfig{Enabled: true},
}
_ = primary.SetRoutes(tt.node.ID, tt.node.SubnetRoutes()...)
// This is a hack to avoid having a second node to test the primary route.
// This should be baked into the test case proper if it is extended in the future.
_ = primary.SetRoutes(2, netip.MustParsePrefix("192.168.0.0/24"))
// Stub primary-route lookup: tt.node owns its SubnetRoutes,
// node ID 2 owns 192.168.0.0/24 (a hack carried over from
// the original routes-package-driven version of this test —
// avoids spinning up a second node just to validate that
// other nodes' primaries don't leak into tt.node's TailNode
// output).
primaries := map[types.NodeID][]netip.Prefix{
tt.node.ID: tt.node.SubnetRoutes(),
2: {netip.MustParsePrefix("192.168.0.0/24")},
}
nv := tt.node.View()
got, err := nv.TailNode(
0,
func(id types.NodeID) []netip.Prefix {
// Route function returns primaries + exit routes
// (matching the real caller contract).
return slices.Concat(primary.PrimaryRoutes(id), nv.ExitRoutes())
return slices.Concat(primaries[id], nv.ExitRoutes())
},
cfg,
)