From 159cf8707a0293e58c6e23360240b9b62f72294f Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 30 Apr 2026 16:27:55 +0000 Subject: [PATCH] ipn/ipnlocal, all: split LocalBackend.NetMap into NetMapNoPeers / NetMapWithPeers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two narrower accessors alongside the existing [LocalBackend.NetMap], with docs that distinguish their semantics: - NetMapNoPeers: cheap (returns the cached *netmap.NetworkMap with a possibly-stale Peers slice). For callers that only read non-Peers fields like SelfNode, DNS, PacketFilter, capabilities. - NetMapWithPeers: documented as returning an up-to-date Peers slice. For callers that genuinely need to iterate Peers or call PeerByXxx. Mark the existing NetMap deprecated and point readers at the two new accessors. NetMap, NetMapNoPeers, and NetMapWithPeers all currently return the same value (b.currentNode().NetMap()): this commit is a no-op behaviorally, just a renaming and migration of in-tree callers. A subsequent change in the same series will switch NetMapWithPeers to actually rebuild the Peers slice from the live per-node-backend peers map (O(N) per call), at which point the distinction between the two new accessors becomes load-bearing. Migrate in-tree callers to the appropriate accessor based on what fields they read: - NetMapNoPeers (most common): localapi handlers, peerapi accept, GetCertPEMWithValidity, web client noise request, doctor DNS resolver check, tsnet CertDomains/TailscaleIPs, ssh/tailssh SSH-policy/cap reads, several LocalBackend internals (isLocalIP, allowExitNodeDNSProxyToServeName, pauseForNetwork nil-check, serve config). - NetMapWithPeers: writeNetmapToDiskLocked (persist full netmap to disk for fast restart), PeerByTailscaleIP lookup. Tests still call the legacy NetMap; they'll see the deprecation warning but otherwise behave identically. Also add two pieces of plumbing the next change in this series will need, but which are already useful on their own: - [client/local.GetDebugResultJSON]: a generic [Client.DebugResultJSON] that decodes directly into a target type T, avoiding the marshal/unmarshal roundtrip callers otherwise need. - localapi "current-netmap" debug action: returns the current netmap (with peers) as JSON. Documented as debug-only — the netmap.NetworkMap shape is internal and may change without notice. This commit is part of a series breaking up a larger change for review; on its own it is a no-op refactor. Updates #12542 Change-Id: Idbb30707414f8da3149c44ca0273262708375b02 Signed-off-by: Brad Fitzpatrick --- client/local/local.go | 18 +++++++++ feature/doctor/doctor.go | 2 +- ipn/ipnlocal/cert.go | 2 +- ipn/ipnlocal/local.go | 49 ++++++++++++++++++++++--- ipn/ipnlocal/peerapi.go | 2 +- ipn/ipnlocal/serve.go | 4 +- ipn/ipnlocal/web_client.go | 2 +- ipn/localapi/debug.go | 21 ++++++++++- ipn/localapi/localapi.go | 8 ++-- ssh/tailssh/incubator.go | 2 +- ssh/tailssh/incubator_plan9.go | 2 +- ssh/tailssh/tailssh.go | 5 ++- ssh/tailssh/tailssh_integration_test.go | 2 + ssh/tailssh/tailssh_test.go | 2 + tsnet/tsnet.go | 4 +- 15 files changed, 102 insertions(+), 23 deletions(-) diff --git a/client/local/local.go b/client/local/local.go index 6508af29e..f27257576 100644 --- a/client/local/local.go +++ b/client/local/local.go @@ -607,6 +607,24 @@ func (lc *Client) DebugResultJSON(ctx context.Context, action string) (any, erro return x, nil } +// GetDebugResultJSON invokes a debug action and decodes the JSON response +// into a value of type T. It avoids the marshal/unmarshal roundtrip that +// callers of [Client.DebugResultJSON] otherwise need to do to get a typed +// value. +// +// These are development tools and subject to change or removal over time. +func GetDebugResultJSON[T any](ctx context.Context, lc *Client, action string) (T, error) { + var v T + body, err := lc.send(ctx, "POST", "/localapi/v0/debug?action="+url.QueryEscape(action), 200, nil) + if err != nil { + return v, fmt.Errorf("error %w: %s", err, body) + } + if err := json.Unmarshal(body, &v); err != nil { + return v, err + } + return v, nil +} + // QueryOptionalFeatures queries the optional features supported by the Tailscale daemon. func (lc *Client) QueryOptionalFeatures(ctx context.Context) (*apitype.OptionalFeatures, error) { body, err := lc.send(ctx, "POST", "/localapi/v0/debug-optional-features", 200, nil) diff --git a/feature/doctor/doctor.go b/feature/doctor/doctor.go index db061311b..01897f0a6 100644 --- a/feature/doctor/doctor.go +++ b/feature/doctor/doctor.go @@ -63,7 +63,7 @@ func visitDoctor(ctx context.Context, b *ipnlocal.LocalBackend, logf logger.Logf // IPs; this can interfere with our ability to connect to the Tailscale // controlplane. checks = append(checks, doctor.CheckFunc("dns-resolvers", func(_ context.Context, logf logger.Logf) error { - nm := b.NetMap() + nm := b.NetMapNoPeers() if nm == nil { return nil } diff --git a/ipn/ipnlocal/cert.go b/ipn/ipnlocal/cert.go index efab9db7a..eab70b295 100644 --- a/ipn/ipnlocal/cert.go +++ b/ipn/ipnlocal/cert.go @@ -909,7 +909,7 @@ func (b *LocalBackend) resolveCertDomain(domain string) (string, error) { } // Read the netmap once to get both CertDomains and capabilities atomically. - nm := b.NetMap() + nm := b.NetMapNoPeers() if nm == nil { return "", errors.New("no netmap available") } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 891558a48..e5f76711f 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -701,7 +701,9 @@ func (b *LocalBackend) onHomeDERPUpdateLocked(du magicsock.HomeDERPChanged) { return } - if err := b.writeNetmapToDiskLocked(b.NetMap()); err != nil { + // Persist the full netmap (including up-to-date Peers) to disk for + // fast restart. + if err := b.writeNetmapToDiskLocked(b.NetMapWithPeers()); err != nil { b.logf("write netmap to cache: %v", err) } } @@ -1023,7 +1025,7 @@ func (b *LocalBackend) pauseOrResumeControlClientLocked() { return } networkUp := b.interfaceState.AnyInterfaceUp() - pauseForNetwork := (b.state == ipn.Stopped && b.NetMap() != nil) || (!networkUp && !testenv.InTest() && !envknob.AssumeNetworkUp()) + pauseForNetwork := (b.state == ipn.Stopped && b.NetMapNoPeers() != nil) || (!networkUp && !testenv.InTest() && !envknob.AssumeNetworkUp()) prefs := b.pm.CurrentPrefs() pauseForSyncPref := prefs.Valid() && prefs.Sync().EqualBool(false) @@ -4057,7 +4059,8 @@ func (b *LocalBackend) pingPeerAPI(ctx context.Context, ip netip.Addr) (peer tai var zero tailcfg.NodeView ctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() - nm := b.NetMap() + // PeerByTailscaleIP needs an up-to-date Peers slice. + nm := b.NetMapWithPeers() if nm == nil { return zero, "", errors.New("no netmap") } @@ -4967,7 +4970,7 @@ func (b *LocalBackend) handlePeerAPIConn(remote, local netip.AddrPort, c net.Con } func (b *LocalBackend) isLocalIP(ip netip.Addr) bool { - nm := b.NetMap() + nm := b.NetMapNoPeers() return nm != nil && views.SliceContains(nm.GetAddresses(), netip.PrefixFrom(ip, ip.BitLen())) } @@ -5119,10 +5122,46 @@ func extractPeerAPIPorts(services []tailcfg.Service) portPair { // NetMap returns the latest cached network map received from // controlclient, or nil if no network map was received yet. +// +// Deprecated: callers should declare their needs explicitly by calling +// either [LocalBackend.NetMapNoPeers] (cheap; for code that reads +// non-Peers fields like SelfNode, DNS, PacketFilter, capabilities) or +// [LocalBackend.NetMapWithPeers] (currently the same; will be made to +// return an up-to-date Peers slice in a follow-up change, at the cost of +// O(N) work per call). NetMap will eventually be removed. func (b *LocalBackend) NetMap() *netmap.NetworkMap { return b.currentNode().NetMap() } +// NetMapNoPeers returns the latest cached network map received from +// controlclient WITHOUT a freshly-built Peers slice. +// +// On a tailnet with frequent peer churn the cached netmap's Peers slice +// can be stale relative to the live per-node-backend peers map; non-Peers +// fields (SelfNode, DNS, PacketFilter, capabilities, ...) are always +// current. Use this for any caller that does not need to iterate Peers, +// since it's O(1) regardless of tailnet size. +// +// Returns nil if no network map has been received yet. +func (b *LocalBackend) NetMapNoPeers() *netmap.NetworkMap { + return b.currentNode().NetMap() +} + +// NetMapWithPeers returns the latest network map with the Peers slice +// populated. +// +// Currently this is the same as [LocalBackend.NetMapNoPeers]: the cached +// netmap's Peers slice may be stale relative to the live per-node-backend +// peers map. A follow-up change will switch this method to return a +// freshly-built netmap with up-to-date Peers, at O(N) cost per call. +// Callers that genuinely need the up-to-date peer set should use this +// method (and document why) so the upcoming change reaches them. +// +// Returns nil if no network map has been received yet. +func (b *LocalBackend) NetMapWithPeers() *netmap.NetworkMap { + return b.currentNode().NetMap() +} + // lookupPeerByIP returns the node public key for the peer that owns the // given IP address. It is the fast path for [Engine.SetPeerByIPPacketFunc], // handling exact-IP matches against node addresses; subnet routes and exit @@ -6978,7 +7017,7 @@ func (b *LocalBackend) AppConnector() *appc.AppConnector { func (b *LocalBackend) allowExitNodeDNSProxyToServeName(name string) bool { b.mu.Lock() defer b.mu.Unlock() - nm := b.NetMap() + nm := b.NetMapNoPeers() if nm == nil { return false } diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index 322884fc7..d72a519ab 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -192,7 +192,7 @@ func (pln *peerAPIListener) ServeConn(src netip.AddrPort, c net.Conn) { c.Close() return } - nm := pln.lb.NetMap() + nm := pln.lb.NetMapNoPeers() if nm == nil || !nm.SelfNode.Valid() { logf("peerapi: no netmap") c.Close() diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index 9460896ad..83b8027d7 100644 --- a/ipn/ipnlocal/serve.go +++ b/ipn/ipnlocal/serve.go @@ -276,7 +276,7 @@ func (b *LocalBackend) updateServeTCPPortNetMapAddrListenersLocked(ports []uint1 } } - nm := b.NetMap() + nm := b.NetMapNoPeers() if nm == nil { b.logf("netMap is nil") return @@ -333,7 +333,7 @@ func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig, etag string return errors.New("can't reconfigure tailscaled when using a config file; config file is locked") } - nm := b.NetMap() + nm := b.NetMapNoPeers() if nm == nil { return errors.New("netMap is nil") } diff --git a/ipn/ipnlocal/web_client.go b/ipn/ipnlocal/web_client.go index 37dba93d0..6ab68858e 100644 --- a/ipn/ipnlocal/web_client.go +++ b/ipn/ipnlocal/web_client.go @@ -173,7 +173,7 @@ func (b *LocalBackend) waitWebClientAuthURL(ctx context.Context, id string, src // one to be completed, based on the presence or absence of the // provided id value. func (b *LocalBackend) doWebClientNoiseRequest(ctx context.Context, id string, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) { - nm := b.NetMap() + nm := b.NetMapNoPeers() if nm == nil || !nm.SelfNode.Valid() { return nil, errors.New("[unexpected] no self node") } diff --git a/ipn/localapi/debug.go b/ipn/localapi/debug.go index dac39195a..6f222bef0 100644 --- a/ipn/localapi/debug.go +++ b/ipn/localapi/debug.go @@ -9,6 +9,7 @@ "cmp" "context" "encoding/json" + "errors" "fmt" "io" "net" @@ -249,6 +250,22 @@ func (h *Handler) serveDebug(w http.ResponseWriter, r *http.Request) { } case "clear-netmap-cache": h.b.ClearNetmapCache(r.Context()) + case "current-netmap": + // Return the current netmap (with peers populated) as JSON. This + // is a debug-only path: the netmap.NetworkMap shape is an + // internal type and may change without notice. Production + // callers should fetch the narrower bits they need via their + // own LocalAPI methods instead. + nm := h.b.NetMapWithPeers() + if nm == nil { + err = errors.New("no netmap") + break + } + w.Header().Set("Content-Type", "application/json") + err = json.NewEncoder(w).Encode(nm) + if err == nil { + return + } case "": err = fmt.Errorf("missing parameter 'action'") default: @@ -284,7 +301,7 @@ func (h *Handler) serveDebugPacketFilterRules(w http.ResponseWriter, r *http.Req http.Error(w, "debug access denied", http.StatusForbidden) return } - nm := h.b.NetMap() + nm := h.b.NetMapNoPeers() if nm == nil { http.Error(w, "no netmap", http.StatusNotFound) return @@ -301,7 +318,7 @@ func (h *Handler) serveDebugPacketFilterMatches(w http.ResponseWriter, r *http.R http.Error(w, "debug access denied", http.StatusForbidden) return } - nm := h.b.NetMap() + nm := h.b.NetMapNoPeers() if nm == nil { http.Error(w, "no netmap", http.StatusNotFound) return diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index f16fab2aa..16366d994 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -347,7 +347,7 @@ func (h *Handler) serveIDToken(w http.ResponseWriter, r *http.Request) { http.Error(w, "id-token access denied", http.StatusForbidden) return } - nm := h.b.NetMap() + nm := h.b.NetMapNoPeers() if nm == nil { http.Error(w, "no netmap", http.StatusServiceUnavailable) return @@ -417,7 +417,7 @@ func (h *Handler) serveBugReport(w http.ResponseWriter, r *http.Request) { } // Information about the current node from the netmap - if nm := h.b.NetMap(); nm != nil { + if nm := h.b.NetMapNoPeers(); nm != nil { if self := nm.SelfNode; self.Valid() { h.logf("user bugreport node info: nodeid=%q stableid=%q expiry=%q", self.ID(), self.StableID(), self.KeyExpiry().Format(time.RFC3339)) } @@ -1476,7 +1476,7 @@ func (h *Handler) serveQueryFeature(w http.ResponseWriter, r *http.Request) { http.Error(w, "missing feature", http.StatusInternalServerError) return } - nm := h.b.NetMap() + nm := h.b.NetMapNoPeers() if nm == nil { http.Error(w, "no netmap", http.StatusServiceUnavailable) return @@ -1731,7 +1731,7 @@ func (h *Handler) serveServices(w http.ResponseWriter, r *http.Request) { http.Error(w, "only GET allowed", http.StatusMethodNotAllowed) return } - nm := h.b.NetMap() + nm := h.b.NetMapNoPeers() if nm == nil { http.Error(w, "no netmap", http.StatusServiceUnavailable) return diff --git a/ssh/tailssh/incubator.go b/ssh/tailssh/incubator.go index c20b18d3e..48c65e8e5 100644 --- a/ssh/tailssh/incubator.go +++ b/ssh/tailssh/incubator.go @@ -202,7 +202,7 @@ func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err incubatorArgs = append(incubatorArgs, "--is-selinux-enforcing") } - nm := ss.conn.srv.lb.NetMap() + nm := ss.conn.srv.lb.NetMapNoPeers() forceV1Behavior := nm.HasCap(tailcfg.NodeAttrSSHBehaviorV1) && !nm.HasCap(tailcfg.NodeAttrSSHBehaviorV2) if forceV1Behavior { incubatorArgs = append(incubatorArgs, "--force-v1-behavior") diff --git a/ssh/tailssh/incubator_plan9.go b/ssh/tailssh/incubator_plan9.go index 69112635f..8d0031413 100644 --- a/ssh/tailssh/incubator_plan9.go +++ b/ssh/tailssh/incubator_plan9.go @@ -92,7 +92,7 @@ func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err "--tty-name=", // updated in-place by startWithPTY } - nm := ss.conn.srv.lb.NetMap() + nm := ss.conn.srv.lb.NetMapNoPeers() forceV1Behavior := nm.HasCap(tailcfg.NodeAttrSSHBehaviorV1) && !nm.HasCap(tailcfg.NodeAttrSSHBehaviorV2) if forceV1Behavior { incubatorArgs = append(incubatorArgs, "--force-v1-behavior") diff --git a/ssh/tailssh/tailssh.go b/ssh/tailssh/tailssh.go index c13d3d29e..e01f78eb3 100644 --- a/ssh/tailssh/tailssh.go +++ b/ssh/tailssh/tailssh.go @@ -76,6 +76,7 @@ type ipnLocalBackend interface { ShouldRunSSH() bool NetMap() *netmap.NetworkMap + NetMapNoPeers() *netmap.NetworkMap WhoIs(proto string, ipp netip.AddrPort) (n tailcfg.NodeView, u tailcfg.UserProfile, ok bool) DoNoiseRequest(req *http.Request) (*http.Response, error) Dialer() *tsdial.Dialer @@ -598,7 +599,7 @@ func (c *conn) sshPolicy() (_ *tailcfg.SSHPolicy, ok bool) { if !lb.ShouldRunSSH() { return nil, false } - nm := lb.NetMap() + nm := lb.NetMapNoPeers() if nm == nil { return nil, false } @@ -717,7 +718,7 @@ func (c *conn) handleSessionPostSSHAuth(s gliderssh.Session) { } func (c *conn) expandDelegateURLLocked(actionURL string) string { - nm := c.srv.lb.NetMap() + nm := c.srv.lb.NetMapNoPeers() ci := c.info lu := c.localUser var dstNodeID string diff --git a/ssh/tailssh/tailssh_integration_test.go b/ssh/tailssh/tailssh_integration_test.go index 8b34836c0..7b70a6d51 100644 --- a/ssh/tailssh/tailssh_integration_test.go +++ b/ssh/tailssh/tailssh_integration_test.go @@ -910,6 +910,8 @@ func (tb *testBackend) NetMap() *netmap.NetworkMap { } } +func (tb *testBackend) NetMapNoPeers() *netmap.NetworkMap { return tb.NetMap() } + func (tb *testBackend) WhoIs(_ string, ipp netip.AddrPort) (n tailcfg.NodeView, u tailcfg.UserProfile, ok bool) { return (&tailcfg.Node{}).View(), tailcfg.UserProfile{ LoginName: tb.localUser + "@example.com", diff --git a/ssh/tailssh/tailssh_test.go b/ssh/tailssh/tailssh_test.go index 9f80a028e..04c9cd2f5 100644 --- a/ssh/tailssh/tailssh_test.go +++ b/ssh/tailssh/tailssh_test.go @@ -422,6 +422,8 @@ func (ts *localState) NetMap() *netmap.NetworkMap { } } +func (ts *localState) NetMapNoPeers() *netmap.NetworkMap { return ts.NetMap() } + func (ts *localState) WhoIs(proto string, ipp netip.AddrPort) (n tailcfg.NodeView, u tailcfg.UserProfile, ok bool) { if proto != "tcp" { return tailcfg.NodeView{}, tailcfg.UserProfile{}, false diff --git a/tsnet/tsnet.go b/tsnet/tsnet.go index 4d6318018..6811c3d87 100644 --- a/tsnet/tsnet.go +++ b/tsnet/tsnet.go @@ -668,7 +668,7 @@ func (s *Server) doInit() { // Server. // If the server is not running, it returns nil. func (s *Server) CertDomains() []string { - nm := s.lb.NetMap() + nm := s.lb.NetMapNoPeers() if nm == nil { return nil } @@ -679,7 +679,7 @@ func (s *Server) CertDomains() []string { // has not yet joined a tailnet or is otherwise unaware of its own IP addresses, // the returned ip4, ip6 will be !netip.IsValid(). func (s *Server) TailscaleIPs() (ip4, ip6 netip.Addr) { - nm := s.lb.NetMap() + nm := s.lb.NetMapNoPeers() if nm == nil { return }