From 926c990a092d82d060fff3194fcc786bc5fa539b Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 17 Sep 2023 12:53:23 -0500 Subject: [PATCH] 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 --- ipn/ipnlocal/local.go | 49 +++++++++++++++++++-------------- ipn/ipnlocal/serve.go | 4 ++- net/tsdial/dnsmap.go | 9 +++--- tsnet/tsnet.go | 4 ++- types/netmap/netmap.go | 19 ++++++++++++- wgengine/magicsock/magicsock.go | 4 +-- 6 files changed, 59 insertions(+), 30 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 7088b35e3..612e75e0c 100644 --- a/ipn/ipnlocal/local.go +++ b/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, "

Local addresses:

\n") } diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index b15e90601..c28ca0239 100644 --- a/ipn/ipnlocal/serve.go +++ b/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 { diff --git a/net/tsdial/dnsmap.go b/net/tsdial/dnsmap.go index c5ed7e01f..a549b5203 100644 --- a/net/tsdial/dnsmap.go +++ b/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 } } diff --git a/tsnet/tsnet.go b/tsnet/tsnet.go index 2015ead21..d92b4ede3 100644 --- a/tsnet/tsnet.go +++ b/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 diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index 7c6302ff3..f3723ee72 100644 --- a/types/netmap/netmap.go +++ b/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 { diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 7c7eb358c..ad646fd99 100644 --- a/wgengine/magicsock/magicsock.go +++ b/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{} }