From b104688e04797162fb854eb42158f983d05dc6cc Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 22 Mar 2024 14:33:41 -0700 Subject: [PATCH] ipn/ipnlocal, types/netmap: replace hasCapability with set lookup on NetworkMap When node attributes were super rare, the O(n) slice scans looking for node attributes was more acceptable. But now more code and more users are using increasingly more node attributes. Time to make it a map. Noticed while working on tailscale/corp#17879 Updates #cleanup Change-Id: Ic17c80341f418421002fbceb47490729048756d2 Signed-off-by: Brad Fitzpatrick --- control/controlclient/map.go | 12 ++++++++++++ ipn/ipnlocal/local.go | 32 ++++++++++++-------------------- types/netmap/netmap.go | 7 +++++++ 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 5b7cb46d4..482779b36 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -31,6 +31,7 @@ import ( "tailscale.com/types/views" "tailscale.com/util/clientmetric" "tailscale.com/util/mak" + "tailscale.com/util/set" "tailscale.com/wgengine/filter" ) @@ -74,6 +75,7 @@ type mapSession struct { // Fields storing state over the course of multiple MapResponses. lastPrintMap time.Time lastNode tailcfg.NodeView + lastCapSet set.Set[tailcfg.NodeCapability] peers map[tailcfg.NodeID]*tailcfg.NodeView // pointer to view (oddly). same pointers as sortedPeers. sortedPeers []*tailcfg.NodeView // same pointers as peers, but sorted by Node.ID lastDNSConfig *tailcfg.DNSConfig @@ -239,6 +241,15 @@ func (ms *mapSession) updateStateFromResponse(resp *tailcfg.MapResponse) { if resp.Node != nil { ms.lastNode = resp.Node.View() + + capSet := set.Set[tailcfg.NodeCapability]{} + for _, c := range resp.Node.Capabilities { + capSet.Add(c) + } + for c := range resp.Node.CapMap { + capSet.Add(c) + } + ms.lastCapSet = capSet } for _, up := range resp.UserProfiles { @@ -803,6 +814,7 @@ func (ms *mapSession) netmap() *netmap.NetworkMap { nm.SelfNode = node nm.Expiry = node.KeyExpiry() nm.Name = node.Name() + nm.AllCaps = ms.lastCapSet } ms.addUserProfile(nm, nm.User()) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 47a892c11..9f6bcf17d 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1164,7 +1164,7 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control // Perform all reconfiguration based on the netmap here. if st.NetMap != nil { - b.capTailnetLock = hasCapability(st.NetMap, tailcfg.CapabilityTailnetLock) + b.capTailnetLock = st.NetMap.HasCap(tailcfg.CapabilityTailnetLock) b.setWebClientAtomicBoolLocked(st.NetMap) b.mu.Unlock() // respect locking rules for tkaSyncIfNeeded @@ -1201,7 +1201,7 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control } if st.NetMap != nil { - if envknob.NoLogsNoSupport() && hasCapability(st.NetMap, tailcfg.CapabilityDataPlaneAuditLogs) { + if envknob.NoLogsNoSupport() && st.NetMap.HasCap(tailcfg.CapabilityDataPlaneAuditLogs) { msg := "tailnet requires logging to be enabled. Remove --no-logs-no-support from tailscaled command line." health.SetLocalLogConfigHealth(errors.New(msg)) // Connecting to this tailnet without logging is forbidden; boot us outta here. @@ -3068,7 +3068,7 @@ func (b *LocalBackend) checkSSHPrefsLocked(p *ipn.Prefs) error { return nil } if b.netMap != nil { - if !hasCapability(b.netMap, tailcfg.CapabilitySSH) { + if !b.netMap.HasCap(tailcfg.CapabilitySSH) { if b.isDefaultServerLocked() { return errors.New("Unable to enable local Tailscale SSH server; not enabled on Tailnet. See https://tailscale.com/s/ssh") } @@ -3093,9 +3093,8 @@ func (b *LocalBackend) sshOnButUnusableHealthCheckMessageLocked() (healthMessage return "" } isDefault := b.isDefaultServerLocked() - isAdmin := hasCapability(nm, tailcfg.CapabilityAdmin) - if !isAdmin { + if !nm.HasCap(tailcfg.CapabilityAdmin) { return healthmsg.TailscaleSSHOnBut + "access controls don't allow anyone to access this device. Ask your admin to update your tailnet's ACLs to allow access." } if !isDefault { @@ -3565,7 +3564,7 @@ func (b *LocalBackend) authReconfig() { prefs := b.pm.CurrentPrefs() nm := b.netMap hasPAC := b.prevIfState.HasPAC() - disableSubnetsIfPAC := hasCapability(nm, tailcfg.NodeAttrDisableSubnetsIfPAC) + disableSubnetsIfPAC := nm.HasCap(tailcfg.NodeAttrDisableSubnetsIfPAC) dohURL, dohURLOK := exitNodeCanProxyDNS(nm, b.peers, prefs.ExitNodeID()) dcfg := dnsConfigForNetmap(nm, b.peers, prefs, b.logf, version.OS()) // If the current node is an app connector, ensure the app connector machine is started @@ -4533,7 +4532,7 @@ func (b *LocalBackend) ShouldExposeRemoteWebClient() bool { // // b.mu must be held. func (b *LocalBackend) setWebClientAtomicBoolLocked(nm *netmap.NetworkMap) { - shouldRun := !hasCapability(nm, tailcfg.NodeAttrDisableWebClient) + shouldRun := !nm.HasCap(tailcfg.NodeAttrDisableWebClient) wasRunning := b.webClientAtomicBool.Swap(shouldRun) if wasRunning && !shouldRun { go b.webClientShutdown() // stop web client @@ -4630,13 +4629,6 @@ func (b *LocalBackend) setNetInfo(ni *tailcfg.NetInfo) { cc.SetNetInfo(ni) } -func hasCapability(nm *netmap.NetworkMap, cap tailcfg.NodeCapability) bool { - if nm != nil { - return nm.SelfNode.HasCap(cap) - } - return false -} - // setNetMapLocked updates the LocalBackend state to reflect the newly // received nm. If nm is nil, it resets all configuration as though // Tailscale is turned off. @@ -4665,15 +4657,15 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { } // Determine if file sharing is enabled - fs := hasCapability(nm, tailcfg.CapabilityFileSharing) + fs := nm.HasCap(tailcfg.CapabilityFileSharing) if fs != b.capFileSharing { osshare.SetFileSharingEnabled(fs, b.logf) } b.capFileSharing = fs - if hasCapability(nm, tailcfg.NodeAttrLinuxMustUseIPTables) { + if nm.HasCap(tailcfg.NodeAttrLinuxMustUseIPTables) { b.capForcedNetfilter = "iptables" - } else if hasCapability(nm, tailcfg.NodeAttrLinuxMustUseNfTables) { + } else if nm.HasCap(tailcfg.NodeAttrLinuxMustUseNfTables) { b.capForcedNetfilter = "nftables" } else { b.capForcedNetfilter = "" // empty string means client can auto-detect @@ -4685,8 +4677,8 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { b.setDebugLogsByCapabilityLocked(nm) // See the netns package for documentation on what this capability does. - netns.SetBindToInterfaceByRoute(hasCapability(nm, tailcfg.CapabilityBindToInterfaceByRoute)) - netns.SetDisableBindConnToInterface(hasCapability(nm, tailcfg.CapabilityDebugDisableBindConnToInterface)) + netns.SetBindToInterfaceByRoute(nm.HasCap(tailcfg.CapabilityBindToInterfaceByRoute)) + netns.SetDisableBindConnToInterface(nm.HasCap(tailcfg.CapabilityDebugDisableBindConnToInterface)) b.setTCPPortsInterceptedFromNetmapAndPrefsLocked(b.pm.CurrentPrefs()) if nm == nil { @@ -4777,7 +4769,7 @@ func (t *tailFSTransport) RoundTrip(req *http.Request) (*http.Response, error) { func (b *LocalBackend) setDebugLogsByCapabilityLocked(nm *netmap.NetworkMap) { // These are sufficiently cheap (atomic bools) that we don't need to // store state and compare. - if hasCapability(nm, tailcfg.CapabilityDebugTSDNSResolution) { + if nm.HasCap(tailcfg.CapabilityDebugTSDNSResolution) { dnscache.SetDebugLoggingEnabled(true) } else { dnscache.SetDebugLoggingEnabled(false) diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index 693701bb5..de187f5f8 100644 --- a/types/netmap/netmap.go +++ b/types/netmap/netmap.go @@ -17,6 +17,7 @@ import ( "tailscale.com/tka" "tailscale.com/types/key" "tailscale.com/types/views" + "tailscale.com/util/set" "tailscale.com/wgengine/filter" ) @@ -26,6 +27,7 @@ import ( // alias parts of previous NetworkMap values. type NetworkMap struct { SelfNode tailcfg.NodeView + AllCaps set.Set[tailcfg.NodeCapability] // set version of SelfNode.Capabilities + SelfNode.CapMap NodeKey key.NodePublic PrivateKey key.NodePrivate Expiry time.Time @@ -120,6 +122,11 @@ func (nm *NetworkMap) GetMachineStatus() tailcfg.MachineStatus { return tailcfg.MachineUnauthorized } +// HasCap reports whether nm is non-nil and nm.AllCaps contains c. +func (nm *NetworkMap) HasCap(c tailcfg.NodeCapability) bool { + return nm != nil && nm.AllCaps.Contains(c) +} + // PeerByTailscaleIP returns a peer's Node based on its Tailscale IP. // // If nm is nil or no peer is found, ok is false.