state, policy/v2: classify only effective route changes

Compare announced-and-approved routes only when immutable route inputs
change. Publish Hostinfo without unnecessary peer adjacency rebuilds.
This commit is contained in:
Kristoffer Dalby committed 2026-11-02 19:15:40 +01:00
1 parent 4342883bd4
commit 0e7515eab9
8 files changed
+215 -68

No files matched your search

+2 -10
View File
@@ -1182,7 +1182,7 @@ func (pm *PolicyManager) NodesGeneration() uint64 {
func (pm *PolicyManager) cacheableLocked(node types.NodeView) bool {
own, ok := pm.nodesByID[node.ID()]
return ok && !node.HasPolicyChange(own) && !node.HasNetworkChanges(own)
return ok && !node.HasPolicyChange(own)
}
// nodeIDViewMap indexes a slice of node views by node ID. On duplicate IDs the
@@ -1212,14 +1212,6 @@ func (pm *PolicyManager) nodesHavePolicyAffectingChanges(newNodes views.Slice[ty
if newNode.HasPolicyChange(oldNode) {
return true
}
// Via grants and autogroup:self compile filter rules per-node
// that depend on the node's route state (SubnetRoutes, ExitRoutes).
// Route changes are policy-affecting in this context because they
// alter which filter rules get generated for the via-designated node.
if pm.needsPerNodeFilter && newNode.HasNetworkChanges(oldNode) {
return true
}
}
return false
@@ -2005,7 +1997,7 @@ func (pm *PolicyManager) invalidateGlobalPolicyCache(newNodes views.Slice[types.
continue
}
if newNode.HasNetworkChanges(oldNode) {
if newNode.HasPolicyChange(oldNode) {
pm.filterRulesMap.Delete(nodeID)
pm.matchersForNodeMap.Delete(nodeID)
}
+59
View File
@@ -4749,3 +4749,62 @@ func TestResolvedPolicyKeepsTagIndex(t *testing.T) {
require.NotNil(t, pm.resolvedPol.nodesByTag)
require.NotNil(t, pm.resolvedPol.resolvedGroups)
}
func TestSetNodesUnapprovedRoutesKeepGeneration(t *testing.T) {
users := types.Users{{ID: 1, Name: "user1"}}
route := netip.MustParsePrefix("10.77.0.0/24")
for _, tt := range []struct {
name string
policy string
}{
{name: "global", policy: `{"acls": [{"action": "accept",
"src": ["user1@"], "dst": ["10.77.0.0/24:*"]}]}`},
{name: "self", policy: `{"acls": [{"action": "accept",
"src": ["user1@"], "dst": ["autogroup:self:*"]}]}`},
{name: "via", policy: `{
"tagOwners": {"tag:router": ["user1@"]},
"grants": [{"src": ["user1@"], "dst": ["10.77.0.0/24"],
"ip": ["*"], "via": ["tag:router"]}]
}`},
} {
t.Run(tt.name, func(t *testing.T) {
router := node("router", "100.64.0.1", "fd7a:115c:a1e0::1", users[0])
router.ID = 1
client := node("client", "100.64.0.2", "fd7a:115c:a1e0::2", users[0])
client.ID = 2
if tt.name == "via" {
router.Tags = []string{"tag:router"}
}
pm, err := NewPolicyManager(
[]byte(tt.policy),
users,
types.Nodes{router, client}.ViewSlice(),
)
require.NoError(t, err)
require.Equal(t, tt.name != "global", pm.needsPerNodeFilter)
gen := pm.NodesGeneration()
announced := router.Clone()
announced.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{route}}
changed, err := pm.SetNodes(types.Nodes{announced, client}.ViewSlice())
require.NoError(t, err)
require.False(t, changed, "unapproved routes are not policy inputs")
require.Equal(t, gen, pm.NodesGeneration())
require.Equal(
t,
[]netip.Prefix{route},
pm.nodesByID[router.ID].AnnouncedRoutes(),
)
approved := announced.Clone()
approved.ApprovedRoutes = []netip.Prefix{route}
changed, err = pm.SetNodes(types.Nodes{approved, client}.ViewSlice())
require.NoError(t, err)
require.True(t, changed, "approval changes served routes")
require.Equal(t, gen+1, pm.NodesGeneration())
})
}
}
@@ -114,11 +114,6 @@ var mapStreamSkipReasons = map[string]string{
"route-announce/RA9/srv/presence": policyResponseToEveryNode,
"route-announce/RA9/wit/presence": policyResponseToEveryNode,
"route-announce/RA7/wit/presence": announcePolicyResend,
"route-announce/RA8/wit/presence": announcePolicyResend,
"route-announce/RA10/wit/presence": announcePolicyResend,
"route-announce/RA11/wit/presence": announcePolicyResend,
"route-announce/RA0/rtr/self-routes": selfRoutesReduced,
"route-announce/RA1/rtr/self-routes": selfRoutesReduced,
"route-announce/RA2/rtr/self-routes": selfRoutesReduced,
@@ -177,7 +172,6 @@ const (
peerHostinfoWhole = "PEER_HOSTINFO_WHOLE: headscale sends peers a node's whole reported Hostinfo; SaaS sends only Hostname, OS, Services, the SSH host keys and AppConnector"
peerServicesWhole = "PEER_SERVICES_WHOLE: headscale sends peers every service a node reports; SaaS leaves out peerapi-dns-proxy"
removalSentToAll = "REMOVAL_SENT_TO_ALL: headscale tells every connected node that a deleted node is gone; SaaS tells only the nodes that had it as a peer"
announcePolicyResend = "ANNOUNCE_POLICY_RESEND: under an autogroup:self or via policy, headscale sends every connected node a policy response when a router changes only routes nobody approved; SaaS sends nothing to nodes that cannot see the router"
withdrawnRouteEnabled = "WITHDRAWN_ROUTE_ENABLED: headscale's routes API keeps a route the device stopped advertising in enabledRoutes; SaaS lists only advertised routes as enabled"
taggedDeviceUser = "TAGGED_DEVICE_USER: SaaS shows a tagged device's user as the login that made its key; headscale shows tagged-devices"
)
+5 -3
View File
@@ -211,7 +211,7 @@ type work struct {
// before the write reads the same on both sides.
func updateChanges(pre, post *types.Node) (bool, bool) {
preView, postView := pre.View(), post.View()
if postView.HasPolicyChange(preView) || postView.HasNetworkChanges(preView) {
if postView.HasPolicyChange(preView) {
return true, true
}
@@ -259,6 +259,8 @@ func (s *NodeStore) PutNode(n types.Node) types.NodeView {
}
// UpdateNodeFunc is a function type that takes a pointer to a [types.Node] and modifies it.
// Referenced data, including Hostinfo and slices, is shared with published
// snapshots. Replace it instead of mutating it in place.
type UpdateNodeFunc func(n *types.Node)
// UpdateNode applies a function to modify a specific node in the
@@ -537,7 +539,7 @@ func (s *NodeStore) applyBatch(batch []work) {
}
oldGivenName := n.GivenName
pre := n.Clone()
pre := n
fn(&n)
if n.GivenName != oldGivenName {
@@ -550,7 +552,7 @@ func (s *NodeStore) applyBatch(batch []work) {
w.pre[id] = pre.View()
}
relation, election := updateChanges(pre, &n)
relation, election := updateChanges(&pre, &n)
relationChanged = relationChanged || relation
electionChanged = electionChanged || election
}
+80 -15
View File
@@ -1580,8 +1580,8 @@ func TestRebuildPeerMapsAfterStopReturns(t *testing.T) {
}
// TestUpdateNodeRecomputesPeersOnlyForRelationInputs pins which fields make a
// write recompute peer adjacency: the inputs that force a peer-map rebuild
// (an announced but unapproved route is included on purpose).
// write recompute peer adjacency: only inputs read by the policy require
// a peer-map rebuild.
func TestUpdateNodeRecomputesPeersOnlyForRelationInputs(t *testing.T) {
subnet := netip.MustParsePrefix("10.77.0.0/24")
@@ -1615,6 +1615,13 @@ func TestUpdateNodeRecomputesPeersOnlyForRelationInputs(t *testing.T) {
mutate: func(n *types.Node) {
n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{subnet}}
},
},
{
name: "announced and approved route",
mutate: func(n *types.Node) {
n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{subnet}}
n.ApprovedRoutes = []netip.Prefix{subnet}
},
wantRecompute: true,
},
{name: "user association", mutate: func(n *types.Node) { n.User = nil }, wantRecompute: true},
@@ -1675,6 +1682,53 @@ func TestUpdateNodeDiff(t *testing.T) {
assert.False(t, before.Valid())
}
func TestUpdateNodeDiffSharesUnchangedRoutes(t *testing.T) {
node := createTestNode(1, 1, "user1", "node1")
peer := createTestNode(2, 2, "user2", "node2")
announced := make([]netip.Prefix, 2048)
ip := netip.MustParseAddr("10.0.0.0")
for i := range announced {
announced[i] = netip.PrefixFrom(ip, 32)
ip = ip.Next()
}
node.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: announced}
node.ApprovedRoutes = []netip.Prefix{announced[0]}
lastSeen := time.Now().Add(-time.Minute)
node.LastSeen = &lastSeen
var peerBuilds atomic.Int64
store := NewNodeStore(types.Nodes{&node, &peer},
func(nodes []types.NodeView) map[types.NodeID][]types.NodeID {
peerBuilds.Add(1)
return allowAllPeersFunc(nodes)
}, TestBatchSize, TestBatchTimeout)
store.Start()
defer store.Stop()
peerBuilds.Store(0)
published := store.data.Load()
now := time.Now()
before, after, ok := store.UpdateNodeDiff(1, func(n *types.Node) {
n.LastSeen = &now
})
require.True(t, ok)
assert.Same(t, &before.AnnouncedRoutes()[0], &after.AnnouncedRoutes()[0])
assert.Same(t, &published.nodesByID[1].Hostinfo.RoutableIPs[0],
&after.AnnouncedRoutes()[0])
assert.Equal(t, lastSeen, before.LastSeen().Get())
assert.Equal(t, lastSeen, *published.nodesByID[1].LastSeen)
assert.Equal(t, now, after.LastSeen().Get())
assert.Zero(t, peerBuilds.Load())
assert.Same(t, &published.peersByNode[1][0], &store.data.Load().peersByNode[1][0])
assert.Equal(t, now, store.ListPeers(2).At(0).LastSeen().Get())
}
// TestListPeersExcludesSelf proves a node is never returned among its own
// peers, on both the snapshot path and the explicit peer-ID path.
//
@@ -1899,9 +1953,9 @@ func TestNodeStoreAdjacencyMatchesFullBuild(t *testing.T) {
// - "payload": neither; a write can reuse the previous peer adjacency.
//
// A field missing here fails TestUpdateChangesCoversEveryNodeField:
// classify it, and if it is a relation input, teach HasPolicyChange or
// HasNetworkChanges about it before updateChanges can let NodeStore
// reuse peer adjacency across the write.
// classify it, and if it is a relation input, teach HasPolicyChange
// about it before updateChanges can let NodeStore reuse peer adjacency
// across the write.
var nodeFieldImpact = map[string]string{
"ID": "relation", "IPv4": "relation", "IPv6": "relation",
"UserID": "relation", "User": "relation", "Tags": "relation",
@@ -1953,16 +2007,11 @@ func TestUpdateChangesReportsRelationFields(t *testing.T) {
n.ApprovedRoutes = []netip.Prefix{pfx}
}, wantRelation: true, wantElection: true},
{
// Announcing a route the policy has not approved does not
// change who can access it (SubnetRoutes/ExitRoutes, which
// gate HasPolicyChange, stay empty), but HasNetworkChanges
// tracks the raw announcement so a later approval sees a
// fresh Hostinfo rather than one NodeStore decided to reuse.
// Policy reads served routes; a later approval triggers its own rebuild.
name: "announced but not approved route",
mutate: func(n *types.Node) {
n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{pfx}}
},
wantRelation: true, wantElection: true,
},
{name: "online", mutate: func(n *types.Node) { n.IsOnline = new(true) }, wantElection: true},
{name: "unhealthy", mutate: func(n *types.Node) { n.Unhealthy = true }, wantElection: true},
@@ -2030,15 +2079,20 @@ func benchNodes(n int) ([]types.User, types.Nodes) {
// (lastseen) should cost far fewer builds than a relation-changing one
// (route, tag).
func BenchmarkNodeStoreWrite(b *testing.B) {
users, nodes := benchNodes(617)
for _, kind := range []struct {
name string
mutate func(i int, n *types.Node)
name string
announced int
mutate func(i int, n *types.Node)
}{
{name: "lastseen", mutate: func(_ int, n *types.Node) {
n.LastSeen = new(time.Now())
}},
{
name: "lastseen-many-routes", announced: 4096,
mutate: func(_ int, n *types.Node) {
n.LastSeen = new(time.Now())
},
},
{name: "route", mutate: func(i int, n *types.Node) {
subnet := netip.PrefixFrom(netip.AddrFrom4([4]byte{10, byte(200 + i%50), 0, 0}), 24) //nolint:gosec
n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{subnet}}
@@ -2054,6 +2108,17 @@ func BenchmarkNodeStoreWrite(b *testing.B) {
n.UserID, n.User = nil, nil
}},
} {
users, nodes := benchNodes(617)
if kind.announced > 0 {
nodes[0].Hostinfo.RoutableIPs = make([]netip.Prefix, kind.announced)
ip := netip.MustParseAddr("10.0.0.0")
for i := range kind.announced {
nodes[0].Hostinfo.RoutableIPs[i] = netip.PrefixFrom(ip, 32)
ip = ip.Next()
}
}
b.Run(fmt.Sprintf("%s/n=%d", kind.name, len(nodes)), func(b *testing.B) {
var calls atomic.Int64
+1 -1
View File
@@ -3876,7 +3876,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
// update that shrinks the announced set can drop the node
// out of HA candidacy without touching ApprovedRoutes.
// Clear any stale Unhealthy bit in that case.
if len(currentNode.AllApprovedRoutes()) == 0 {
if currentNode.Unhealthy && len(currentNode.AllApprovedRoutes()) == 0 {
currentNode.Unhealthy = false
}
})
+12 -25
View File
@@ -1198,31 +1198,6 @@ func (nv NodeView) IPsAsString() []string {
return nv.ж.IPsAsString()
}
// HasNetworkChanges checks if the node has network-related changes.
// Returns true if IPs, announced routes, or approved routes changed.
// This is primarily used for policy cache invalidation. Route slices
// are compared order-insensitively since clients may re-advertise the
// same routes in a different order.
func (nv NodeView) HasNetworkChanges(other NodeView) bool {
if !slices.Equal(nv.IPs(), other.IPs()) {
return true
}
if !equalPrefixesUnordered(nv.AnnouncedRoutes(), other.AnnouncedRoutes()) {
return true
}
if !equalPrefixesUnordered(nv.SubnetRoutes(), other.SubnetRoutes()) {
return true
}
if !equalPrefixesUnordered(nv.ExitRoutes(), other.ExitRoutes()) {
return true
}
return false
}
// equalPrefixesUnordered reports whether a and b contain the same
// prefixes, order-independent. Inputs are cloned before sorting so
// callers' slices are not mutated.
@@ -1247,6 +1222,13 @@ func equalUnordered[E comparable](a, b []E, cmp func(E, E) int) bool {
return slices.Equal(ac, bc)
}
// Unchanged snapshots share immutable slices, so pointer equality avoids a scan.
func sameSlice[E comparable](a, b []E) bool {
aView, bView := views.SliceOf(a), views.SliceOf(b)
return aView.MapKey() == bView.MapKey() || views.SliceEqual(aView, bView)
}
// HasPolicyChange reports whether the node has changes that affect
// policy evaluation. Includes approved subnet routes because they act
// as source identity in [Node.CanAccess] for subnet-to-subnet ACLs,
@@ -1272,6 +1254,11 @@ func (nv NodeView) HasPolicyChange(other NodeView) bool {
return true
}
if sameSlice(nv.AnnouncedRoutes(), other.AnnouncedRoutes()) &&
sameSlice(nv.ж.ApprovedRoutes, other.ж.ApprovedRoutes) {
return false
}
if !equalPrefixesUnordered(nv.SubnetRoutes(), other.SubnetRoutes()) {
return true
}
+56 -8
View File
@@ -687,9 +687,9 @@ func TestApplyPeerChange(t *testing.T) {
}
}
// TestHasNetworkChanges tests the [NodeView] method for detecting
// when a node's network properties have changed.
func TestHasNetworkChanges(t *testing.T) {
// TestHasPolicyChangeAddressesAndRoutes covers the addresses and served
// routes that policy evaluation reads.
func TestHasPolicyChangeAddressesAndRoutes(t *testing.T) {
mustIPPtr := func(s string) *netip.Addr {
ip := netip.MustParseAddr(s)
return &ip
@@ -759,7 +759,7 @@ func TestHasNetworkChanges(t *testing.T) {
IPv4: mustIPPtr("100.64.0.1"),
Hostinfo: &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/24")}},
},
changed: true,
changed: false,
},
{
name: "RoutableIPs removed",
@@ -773,7 +773,7 @@ func TestHasNetworkChanges(t *testing.T) {
IPv4: mustIPPtr("100.64.0.1"),
Hostinfo: &tailcfg.Hostinfo{},
},
changed: true,
changed: false,
},
{
name: "RoutableIPs changed",
@@ -787,7 +787,7 @@ func TestHasNetworkChanges(t *testing.T) {
IPv4: mustIPPtr("100.64.0.1"),
Hostinfo: &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{netip.MustParsePrefix("192.168.0.0/24")}},
},
changed: true,
changed: false,
},
{
name: "SubnetRoutes added",
@@ -886,9 +886,9 @@ func TestHasNetworkChanges(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := tt.new.View().HasNetworkChanges(tt.old.View())
got := tt.new.View().HasPolicyChange(tt.old.View())
if got != tt.changed {
t.Errorf("HasNetworkChanges() = %v, want %v", got, tt.changed)
t.Errorf("HasPolicyChange() = %v, want %v", got, tt.changed)
}
})
}
@@ -1076,6 +1076,54 @@ func TestNodeIsExpiredAt(t *testing.T) {
})
}
func TestHasPolicyChangeSharedRouteInputs(t *testing.T) {
subnet := netip.MustParsePrefix("10.0.0.0/24")
exit := netip.MustParsePrefix("::/0")
base := Node{
Hostinfo: &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{subnet, exit}},
ApprovedRoutes: []netip.Prefix{subnet, exit},
}
for _, tt := range []struct {
name string
mutate func(*Node)
want bool
}{
{name: "shared slices", mutate: func(*Node) {}},
{name: "equal slices", mutate: func(n *Node) {
n.Hostinfo = n.Hostinfo.Clone()
n.ApprovedRoutes = []netip.Prefix{subnet, exit}
}},
{name: "reordered slices", mutate: func(n *Node) {
n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{exit, subnet}}
n.ApprovedRoutes = []netip.Prefix{exit, subnet}
}},
{name: "approval removed", mutate: func(n *Node) {
n.ApprovedRoutes = []netip.Prefix{subnet}
}, want: true},
{name: "announcement removed", mutate: func(n *Node) {
n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{subnet}}
}, want: true},
} {
t.Run(tt.name, func(t *testing.T) {
other := base
tt.mutate(&other)
require.Equal(t, tt.want, base.View().HasPolicyChange(other.View()))
})
}
shared := base
var changed bool
allocs := testing.AllocsPerRun(10, func() {
changed = base.View().HasPolicyChange(shared.View())
})
require.False(t, changed)
require.Zero(t, allocs, "unchanged route inputs should not derive route lists")
}
func TestValidateHostinfo(t *testing.T) {
t.Parallel()