mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-04 04:34:07 +09:00
state, policy/v2: stop treating announced-only routes as policy changes
Policy evaluation reads only announced-and-approved routes. Keep publishing Hostinfo updates without rebuilding peer adjacency.
This commit is contained in:
1 parent
2377e060f2
commit
8735976f26
7 files changed
+83
-61
No files matched your search
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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},
|
||||
@@ -1899,9 +1906,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 +1960,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},
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user