all: use iterators in more places instead of Range funcs

And misc cleanup along the way.

Updates #12912

Change-Id: I0cab148b49efc668c6f5cdf09c740b84a713e388
Signed-off-by: Brad Fitzpatrick <[email protected]>

control/controlclient/map.go

@@ -663,21 +663,23 @@ func peerChangeDiff(was tailcfg.NodeView, n *tailcfg.Node) (_ *tailcfg.PeerChang
 			}
 		case "CapMap":
 			if len(n.CapMap) != was.CapMap().Len() {
+				// If they have different lengths, they're different.
 				if n.CapMap == nil {
 					pc().CapMap = make(tailcfg.NodeCapMap)
 				} else {
 					pc().CapMap = maps.Clone(n.CapMap)
 				}
-				break
-			}
-			was.CapMap().Range(func(k tailcfg.NodeCapability, v views.Slice[tailcfg.RawMessage]) bool {
-				nv, ok := n.CapMap[k]
-				if !ok || !views.SliceEqual(v, views.SliceOf(nv)) {
-					pc().CapMap = maps.Clone(n.CapMap)
-					return false
+			} else {
+				// If they have the same length, check that all their keys
+				// have the same values.
+				for k, v := range was.CapMap().All() {
+					nv, ok := n.CapMap[k]
+					if !ok || !views.SliceEqual(v, views.SliceOf(nv)) {
+						pc().CapMap = maps.Clone(n.CapMap)
+						break
+					}
 				}
-				return true
-			})
+			}
 		case "Tags":
 			if !views.SliceEqual(was.Tags(), views.SliceOf(n.Tags)) {
 				return nil, false

envknob/logknob/logknob.go

@@ -11,7 +11,6 @@ import (
 	"tailscale.com/envknob"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/logger"
-	"tailscale.com/types/views"
 )
 
 // TODO(andrew-d): should we have a package-global registry of logknobs? It
@@ -59,7 +58,7 @@ func (lk *LogKnob) Set(v bool) {
 // about; we use this rather than a concrete type to avoid a circular
 // dependency.
 type NetMap interface {
-	SelfCapabilities() views.Slice[tailcfg.NodeCapability]
+	HasSelfCapability(tailcfg.NodeCapability) bool
 }
 
 // UpdateFromNetMap will enable logging if the SelfNode in the provided NetMap
@@ -68,8 +67,7 @@ func (lk *LogKnob) UpdateFromNetMap(nm NetMap) {
 	if lk.capName == "" {
 		return
 	}
-
-	lk.cap.Store(views.SliceContains(nm.SelfCapabilities(), lk.capName))
+	lk.cap.Store(nm.HasSelfCapability(lk.capName))
 }
 
 // Do will call log with the provided format and arguments if any of the

envknob/logknob/logknob_test.go

@@ -11,6 +11,7 @@ import (
 	"tailscale.com/envknob"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/netmap"
+	"tailscale.com/util/set"
 )
 
 var testKnob = NewLogKnob(
@@ -63,11 +64,7 @@ func TestLogKnob(t *testing.T) {
 		}
 
 		testKnob.UpdateFromNetMap(&netmap.NetworkMap{
-			SelfNode: (&tailcfg.Node{
-				Capabilities: []tailcfg.NodeCapability{
-					"https://tailscale.com/cap/testing",
-				},
-			}).View(),
+			AllCaps: set.Of(tailcfg.NodeCapability("https://tailscale.com/cap/testing")),
 		})
 		if !testKnob.shouldLog() {
 			t.Errorf("expected shouldLog()=true")

ipn/ipnlocal/local.go

@@ -1126,11 +1126,10 @@ func (b *LocalBackend) UpdateStatus(sb *ipnstate.StatusBuilder) {
 					ss.Capabilities = make([]tailcfg.NodeCapability, 1, cm.Len()+1)
 					ss.Capabilities[0] = "HTTPS://TAILSCALE.COM/s/DEPRECATED-NODE-CAPS#see-https://github.com/tailscale/tailscale/issues/11508"
 					ss.CapMap = make(tailcfg.NodeCapMap, sn.CapMap().Len())
-					cm.Range(func(k tailcfg.NodeCapability, v views.Slice[tailcfg.RawMessage]) bool {
+					for k, v := range cm.All() {
 						ss.CapMap[k] = v.AsSlice()
 						ss.Capabilities = append(ss.Capabilities, k)
-						return true
-					})
+					}
 					slices.Sort(ss.Capabilities[1:])
 				}
 			}
@@ -1192,10 +1191,9 @@ func (b *LocalBackend) populatePeerStatusLocked(sb *ipnstate.StatusBuilder) {
 		}
 		if cm := p.CapMap(); cm.Len() > 0 {
 			ps.CapMap = make(tailcfg.NodeCapMap, cm.Len())
-			cm.Range(func(k tailcfg.NodeCapability, v views.Slice[tailcfg.RawMessage]) bool {
+			for k, v := range cm.All() {
 				ps.CapMap[k] = v.AsSlice()
-				return true
-			})
+			}
 		}
 		peerStatusFromNode(ps, p)
 
@@ -5918,15 +5916,15 @@ func (b *LocalBackend) setServeProxyHandlersLocked() {
 	}
 	var backends map[string]bool
 	b.serveConfig.RangeOverWebs(func(_ ipn.HostPort, conf ipn.WebServerConfigView) (cont bool) {
-		conf.Handlers().Range(func(_ string, h ipn.HTTPHandlerView) (cont bool) {
+		for _, h := range conf.Handlers().All() {
 			backend := h.Proxy()
 			if backend == "" {
 				// Only create proxy handlers for servers with a proxy backend.
-				return true
+				continue
 			}
 			mak.Set(&backends, backend, true)
 			if _, ok := b.serveProxyHandlers.Load(backend); ok {
-				return true
+				continue
 			}
 
 			b.logf("serve: creating a new proxy handler for %s", backend)
@@ -5935,11 +5933,10 @@ func (b *LocalBackend) setServeProxyHandlersLocked() {
 				// The backend endpoint (h.Proxy) should have been validated by expandProxyTarget
 				// in the CLI, so just log the error here.
 				b.logf("[unexpected] could not create proxy for %v: %s", backend, err)
-				return true
+				continue
 			}
 			b.serveProxyHandlers.Store(backend, p)
-			return true
-		})
+		}
 		return true
 	})
 

ipn/ipnlocal/serve.go

@@ -326,7 +326,7 @@ func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig, etag string
 		if b.serveConfig.Valid() {
 			has = b.serveConfig.Foreground().Contains
 		}
-		prevConfig.Foreground().Range(func(k string, v ipn.ServeConfigView) (cont bool) {
+		for k := range prevConfig.Foreground().All() {
 			if !has(k) {
 				for _, sess := range b.notifyWatchers {
 					if sess.sessionID == k {
@@ -334,8 +334,7 @@ func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig, etag string
 					}
 				}
 			}
-			return true
-		})
+		}
 	}
 
 	return nil

tailcfg/tailcfg.go

@@ -1457,7 +1457,7 @@ const (
 
 // NodeCapMap is a map of capabilities to their optional values. It is valid for
 // a capability to have no values (nil slice); such capabilities can be tested
-// for by using the Contains method.
+// for by using the [NodeCapMap.Contains] method.
 //
 // See [NodeCapability] for more information on keys.
 type NodeCapMap map[NodeCapability][]RawMessage
@@ -1873,7 +1873,7 @@ type MapResponse struct {
 
 	// PeersChangedPatch, if non-nil, means that node(s) have changed.
 	// This is a lighter version of the older PeersChanged support that
-	// only supports certain types of updates
+	// only supports certain types of updates.
 	//
 	// These are applied after Peers* above, but in practice the
 	// control server should only send these on their own, without

types/netmap/netmap.go

@@ -197,21 +197,11 @@ func (nm *NetworkMap) DomainName() string {
 	return nm.Domain
 }
 
-// SelfCapabilities returns SelfNode.Capabilities if nm and nm.SelfNode are
-// non-nil. This is a method so we can use it in envknob/logknob without a
-// circular dependency.
-func (nm *NetworkMap) SelfCapabilities() views.Slice[tailcfg.NodeCapability] {
-	var zero views.Slice[tailcfg.NodeCapability]
-	if nm == nil || !nm.SelfNode.Valid() {
-		return zero
-	}
-	out := nm.SelfNode.Capabilities().AsSlice()
-	nm.SelfNode.CapMap().Range(func(k tailcfg.NodeCapability, _ views.Slice[tailcfg.RawMessage]) (cont bool) {
-		out = append(out, k)
-		return true
-	})
-
-	return views.SliceOf(out)
+// HasSelfCapability reports whether nm.SelfNode contains capability c.
+//
+// It exists to satisify an unused (as of 2025-01-04) interface in the logknob package.
+func (nm *NetworkMap) HasSelfCapability(c tailcfg.NodeCapability) bool {
+	return nm.AllCaps.Contains(c)
 }
 
 func (nm *NetworkMap) String() string {