types/netmap: start phasing out Addresses, add GetAddresses method

NetworkMap.Addresses is redundant with the SelfNode.Addresses. This
works towards a TODO to delete NetworkMap.Addresses and replace it
with a method.

This is similar to #9389.

Updates #cleanup

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

ipn/ipnlocal/local.go

@@ -541,7 +541,7 @@ func (b *LocalBackend) linkChange(delta *netmon.ChangeDelta) {
 	b.updateFilterLocked(b.netMap, b.pm.CurrentPrefs())
 
 	if peerAPIListenAsync && b.netMap != nil && b.state == ipn.Running {
-		want := len(b.netMap.Addresses)
+		want := b.netMap.GetAddresses().Len()
 		if len(b.peerAPIListeners) < want {
 			b.logf("linkChange: peerAPIListeners too low; trying again")
 			go b.initPeerAPIListener()
@@ -711,8 +711,9 @@ func (b *LocalBackend) UpdateStatus(sb *ipnstate.StatusBuilder) {
 
 	var tailscaleIPs []netip.Addr
 	if b.netMap != nil {
-		for _, addr := range b.netMap.Addresses {
-			if addr.IsSingleIP() {
+		addrs := b.netMap.GetAddresses()
+		for i := range addrs.LenIter() {
+			if addr := addrs.At(i); addr.IsSingleIP() {
 				sb.AddTailscaleIP(addr.Addr())
 				tailscaleIPs = append(tailscaleIPs, addr.Addr())
 			}
@@ -872,7 +873,9 @@ func (b *LocalBackend) peerCapsLocked(src netip.Addr) tailcfg.PeerCapMap {
 	if filt == nil {
 		return nil
 	}
-	for _, a := range b.netMap.Addresses {
+	addrs := b.netMap.GetAddresses()
+	for i := range addrs.LenIter() {
+		a := addrs.At(i)
 		if !a.IsSingleIP() {
 			continue
 		}
@@ -1566,7 +1569,7 @@ func (b *LocalBackend) updateFilterLocked(netMap *netmap.NetworkMap, prefs ipn.P
 	// quite hard to debug, so save yourself the trouble.
 	var (
 		haveNetmap   = netMap != nil
-		addrs        []netip.Prefix
+		addrs        views.Slice[netip.Prefix]
 		packetFilter []filter.Match
 		localNetsB   netipx.IPSetBuilder
 		logNetsB     netipx.IPSetBuilder
@@ -1577,9 +1580,9 @@ func (b *LocalBackend) updateFilterLocked(netMap *netmap.NetworkMap, prefs ipn.P
 	logNetsB.AddPrefix(tsaddr.TailscaleULARange())
 	logNetsB.RemovePrefix(tsaddr.ChromeOSVMRange())
 	if haveNetmap {
-		addrs = netMap.Addresses
-		for _, p := range addrs {
-			localNetsB.AddPrefix(p)
+		addrs = netMap.GetAddresses()
+		for i := range addrs.LenIter() {
+			localNetsB.AddPrefix(addrs.At(i))
 		}
 		packetFilter = netMap.PacketFilter
 
@@ -1631,7 +1634,7 @@ func (b *LocalBackend) updateFilterLocked(netMap *netmap.NetworkMap, prefs ipn.P
 
 	changed := deephash.Update(&b.filterHash, &struct {
 		HaveNetmap  bool
-		Addrs       []netip.Prefix
+		Addrs       views.Slice[netip.Prefix]
 		FilterMatch []filter.Match
 		LocalNets   []netipx.IPRange
 		LogNets     []netipx.IPRange
@@ -2893,7 +2896,7 @@ func (b *LocalBackend) handlePeerAPIConn(remote, local netip.AddrPort, c net.Con
 
 func (b *LocalBackend) isLocalIP(ip netip.Addr) bool {
 	nm := b.NetMap()
-	return nm != nil && slices.Contains(nm.Addresses, netip.PrefixFrom(ip, ip.BitLen()))
+	return nm != nil && views.SliceContains(nm.GetAddresses(), netip.PrefixFrom(ip, ip.BitLen()))
 }
 
 var (
@@ -3399,10 +3402,11 @@ func (b *LocalBackend) initPeerAPIListener() {
 		return
 	}
 
-	if len(b.netMap.Addresses) == len(b.peerAPIListeners) {
+	addrs := b.netMap.GetAddresses()
+	if addrs.Len() == len(b.peerAPIListeners) {
 		allSame := true
 		for i, pln := range b.peerAPIListeners {
-			if pln.ip != b.netMap.Addresses[i].Addr() {
+			if pln.ip != addrs.At(i).Addr() {
 				allSame = false
 				break
 			}
@@ -3416,7 +3420,7 @@ func (b *LocalBackend) initPeerAPIListener() {
 	b.closePeerAPIListenersLocked()
 
 	selfNode := b.netMap.SelfNode
-	if len(b.netMap.Addresses) == 0 || !selfNode.Valid() {
+	if !selfNode.Valid() || b.netMap.GetAddresses().Len() == 0 {
 		return
 	}
 
@@ -3437,7 +3441,8 @@ func (b *LocalBackend) initPeerAPIListener() {
 	b.peerAPIServer = ps
 
 	isNetstack := b.sys.IsNetstack()
-	for i, a := range b.netMap.Addresses {
+	for i := range addrs.LenIter() {
+		a := addrs.At(i)
 		var ln net.Listener
 		var err error
 		skipListen := i > 0 && isNetstack
@@ -3721,11 +3726,12 @@ func (b *LocalBackend) enterStateLockedOnEntry(newState ipn.State) {
 		// Needed so that UpdateEndpoints can run
 		b.e.RequestStatus()
 	case ipn.Running:
-		var addrs []string
-		for _, addr := range netMap.Addresses {
-			addrs = append(addrs, addr.Addr().String())
+		var addrStrs []string
+		addrs := netMap.GetAddresses()
+		for i := range addrs.LenIter() {
+			addrStrs = append(addrStrs, addrs.At(i).Addr().String())
 		}
-		systemd.Status("Connected; %s; %s", activeLogin, strings.Join(addrs, " "))
+		systemd.Status("Connected; %s; %s", activeLogin, strings.Join(addrStrs, " "))
 	case ipn.NoState:
 		// Do nothing.
 	default:
@@ -4838,13 +4844,14 @@ func (b *LocalBackend) handleQuad100Port80Conn(w http.ResponseWriter, r *http.Re
 		io.WriteString(w, "No netmap.\n")
 		return
 	}
-	if len(b.netMap.Addresses) == 0 {
+	addrs := b.netMap.GetAddresses()
+	if addrs.Len() == 0 {
 		io.WriteString(w, "No local addresses.\n")
 		return
 	}
 	io.WriteString(w, "<p>Local addresses:</p><ul>\n")
-	for _, ipp := range b.netMap.Addresses {
-		fmt.Fprintf(w, "<li>%v</li>\n", ipp.Addr())
+	for i := range addrs.LenIter() {
+		fmt.Fprintf(w, "<li>%v</li>\n", addrs.At(i).Addr())
 	}
 	io.WriteString(w, "</ul>\n")
 }

ipn/ipnlocal/serve.go

@@ -205,7 +205,9 @@ func (b *LocalBackend) updateServeTCPPortNetMapAddrListenersLocked(ports []uint1
 		return
 	}
 
-	for _, a := range nm.Addresses {
+	addrs := nm.GetAddresses()
+	for i := range addrs.LenIter() {
+		a := addrs.At(i)
 		for _, p := range ports {
 			addrPort := netip.AddrPortFrom(a.Addr(), p)
 			if _, ok := b.serveListeners[addrPort]; ok {

net/tsdial/dnsmap.go

@@ -35,14 +35,15 @@ func dnsMapFromNetworkMap(nm *netmap.NetworkMap) dnsMap {
 	ret := make(dnsMap)
 	suffix := nm.MagicDNSSuffix()
 	have4 := false
-	if nm.Name != "" && len(nm.Addresses) > 0 {
-		ip := nm.Addresses[0].Addr()
+	addrs := nm.GetAddresses()
+	if nm.Name != "" && addrs.Len() > 0 {
+		ip := addrs.At(0).Addr()
 		ret[canonMapKey(nm.Name)] = ip
 		if dnsname.HasSuffix(nm.Name, suffix) {
 			ret[canonMapKey(dnsname.TrimSuffix(nm.Name, suffix))] = ip
 		}
-		for _, a := range nm.Addresses {
-			if a.Addr().Is4() {
+		for i := range addrs.LenIter() {
+			if addrs.At(i).Addr().Is4() {
 				have4 = true
 			}
 		}

tsnet/tsnet.go

@@ -408,7 +408,9 @@ func (s *Server) TailscaleIPs() (ip4, ip6 netip.Addr) {
 	if nm == nil {
 		return
 	}
-	for _, addr := range nm.Addresses {
+	addrs := nm.GetAddresses()
+	for i := range addrs.LenIter() {
+		addr := addrs.At(i)
 		ip := addr.Addr()
 		if ip.Is6() {
 			ip6 = ip

types/netmap/netmap.go

@@ -35,7 +35,8 @@ type NetworkMap struct {
 
 	// Addresses is SelfNode.Addresses. (IP addresses of this Node directly)
 	//
-	// TODO(bradfitz): remove this field and make this a method.
+	// Deprecated: use GetAddresses instead. As of 2023-09-17, this field
+	// is being phased out.
 	Addresses []netip.Prefix
 
 	// MachineStatus is either tailcfg.MachineAuthorized or tailcfg.MachineUnauthorized,
@@ -99,6 +100,22 @@ func (nm *NetworkMap) User() tailcfg.UserID {
 	return 0
 }
 
+// GetAddresses returns the self node's addresses, or the zero value
+// if SelfNode is invalid.
+func (nm *NetworkMap) GetAddresses() views.Slice[netip.Prefix] {
+	var zero views.Slice[netip.Prefix]
+	if nm.Addresses != nil {
+		// For now (2023-09-17), let the deprecated Addressees field take
+		// precedence. This is a migration mechanism while we update tests &
+		// code cross-repo.
+		return views.SliceOf(nm.Addresses)
+	}
+	if !nm.SelfNode.Valid() {
+		return zero
+	}
+	return nm.SelfNode.Addresses()
+}
+
 // AnyPeersAdvertiseRoutes reports whether any peer is advertising non-exit node routes.
 func (nm *NetworkMap) AnyPeersAdvertiseRoutes() bool {
 	for _, p := range nm.Peers {

wgengine/magicsock/magicsock.go

@@ -1816,8 +1816,8 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) {
 	c.peers = curPeers
 
 	flags := c.debugFlagsLocked()
-	if len(nm.Addresses) > 0 {
-		c.firstAddrForTest = nm.Addresses[0].Addr()
+	if addrs := nm.GetAddresses(); addrs.Len() > 0 {
+		c.firstAddrForTest = addrs.At(0).Addr()
 	} else {
 		c.firstAddrForTest = netip.Addr{}
 	}