From 87c0d3694213f432f0b5b44584c9aad006541a05 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 14 Jul 2026 20:00:08 +0000 Subject: [PATCH] net/routemanager,ipn/ipnlocal: name the changed-allowed-IPs map type The map[key.NodePublic][]netip.Prefix that flows from route manager commits to WireGuard device syncs has subtle semantics (a nil value means the peer was removed or no longer contributes any prefixes) that were documented on routemanager.Result.AllowedIPs and then re-documented, or not, at each signature that passed it along. Give it a named type, PeersWithRouteChanges, and document the nil semantics once on it. Updates #12542 Change-Id: I2566361a5331eb11b2b70a5bcdb497cc20ee561d Signed-off-by: Brad Fitzpatrick --- ipn/ipnlocal/node_backend.go | 18 +++++++----------- net/routemanager/routemanager.go | 23 ++++++++++++++--------- net/routemanager/routemanager_test.go | 2 +- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index e86fcec41..811d0802f 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/ipn/ipnlocal/node_backend.go @@ -918,10 +918,9 @@ type routePrefs struct { // updateRouteManagerPrefs pushes p into the route manager. // // It returns the peers whose allowed source prefixes changed as a -// result (for example the old and new exit node when the selection -// changes), as described by [routemanager.Result.AllowedIPs]. -// In particular, the value for a key will be nil when that peer was removed. -func (nb *nodeBackend) updateRouteManagerPrefs(p routePrefs) (changedAllowedIPs map[key.NodePublic][]netip.Prefix) { +// result, for example the old and new exit node when the selection +// changes. +func (nb *nodeBackend) updateRouteManagerPrefs(p routePrefs) routemanager.PeersWithRouteChanges { nb.mu.Lock() defer nb.mu.Unlock() var exitID tailcfg.NodeID @@ -950,8 +949,8 @@ func (nb *nodeBackend) updateRouteManagerPrefs(p routePrefs) (changedAllowedIPs // public key. // // It returns the peers whose allowed source prefixes changed as a -// result, as described by [routemanager.Result.AllowedIPs]. -func (nb *nodeBackend) updateRouteManagerExtras(fn func(key.NodePublic) views.Slice[netip.Prefix]) (changedAllowedIPs map[key.NodePublic][]netip.Prefix) { +// result. +func (nb *nodeBackend) updateRouteManagerExtras(fn func(key.NodePublic) views.Slice[netip.Prefix]) routemanager.PeersWithRouteChanges { nb.mu.Lock() defer nb.mu.Unlock() var extras map[tailcfg.NodeID][]netip.Prefix @@ -1010,11 +1009,8 @@ func (nb *nodeBackend) mergeUserProfiles(profiles map[tailcfg.UserID]tailcfg.Use // delta mutations that the caller must propagate. type netmapDeltaResult struct { // ChangedAllowedIPs are the peers whose allowed source prefixes - // changed, as described by [routemanager.Result.AllowedIPs]; the - // caller syncs those peers to the WireGuard device. In - // particular, the value for a key will be nil when that peer was - // removed. - ChangedAllowedIPs map[key.NodePublic][]netip.Prefix + // changed; the caller syncs those peers to the WireGuard device. + ChangedAllowedIPs routemanager.PeersWithRouteChanges // DiscoChanged is the set of peers whose disco key changed in a // way that requires a WireGuard session reset (see diff --git a/net/routemanager/routemanager.go b/net/routemanager/routemanager.go index e750fea8f..a6055c91f 100644 --- a/net/routemanager/routemanager.go +++ b/net/routemanager/routemanager.go @@ -324,17 +324,22 @@ type Result struct { Outbound *bart.Table[*PeerRoute] OSRoutes *bart.Lite - // AllowedIPs maps the public key of each peer whose allowed - // source prefixes changed in this commit to its new sorted - // prefix list, as [RouteManager.PeerAllowedIPs] would now - // return it. A nil value means the peer no longer has any - // allowed prefixes, because it was removed or now contributes - // nothing. When a peer's key changes, the old key maps to nil - // and the new key to the peer's prefixes. The map is nil when - // no peer's allowed prefixes changed. - AllowedIPs map[key.NodePublic][]netip.Prefix + // AllowedIPs describes the peers whose allowed source prefixes + // changed in this commit. The map is nil when no peer's allowed + // prefixes changed. + AllowedIPs PeersWithRouteChanges } +// PeersWithRouteChanges maps the public key of each peer whose +// allowed source prefixes changed to its new sorted prefix list, as +// [RouteManager.PeerAllowedIPs] would now return it. +// +// A nil value means the peer no longer has any allowed prefixes, +// because it was removed or now contributes nothing; consumers +// should treat such peers as deleted. When a peer's key changes, the +// old key maps to nil and the new key to the peer's prefixes. +type PeersWithRouteChanges map[key.NodePublic][]netip.Prefix + type opKind uint8 const ( diff --git a/net/routemanager/routemanager_test.go b/net/routemanager/routemanager_test.go index 108b42870..6b7c2c917 100644 --- a/net/routemanager/routemanager_test.go +++ b/net/routemanager/routemanager_test.go @@ -655,7 +655,7 @@ func TestUpsertPeerNodeViewIneligible(t *testing.T) { // "no allowed prefixes"). func wantChangedAllowedIPs(t *testing.T, res Result, want map[key.NodePublic][]string) { t.Helper() - wantMap := make(map[key.NodePublic][]netip.Prefix) + wantMap := make(PeersWithRouteChanges) for k, ss := range want { var pfxs []netip.Prefix for _, s := range ss {