From 72d8672ef75d1618caf1395fa3d1e61d7f72bb87 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Tue, 15 Feb 2022 08:19:44 -0800 Subject: [PATCH] tailcfg: make Node.Hostinfo a HostinfoView Signed-off-by: Maisem Ali --- cmd/cloner/cloner.go | 23 ++++++++++++++++--- cmd/hello/hello.go | 2 +- cmd/tailscale/depaware.txt | 1 + cmd/tailscaled/depaware.txt | 1 + control/controlclient/map.go | 4 +++- ipn/ipnlocal/local.go | 17 +++++++++----- ipn/ipnlocal/local_test.go | 16 ++++++------- tailcfg/tailcfg.go | 15 +++++++----- tailcfg/tailcfg_clone.go | 4 ++-- tailcfg/tailcfg_test.go | 8 +++---- tstest/integration/testcontrol/testcontrol.go | 10 ++++---- types/netmap/netmap.go | 7 +++--- wgengine/magicsock/debughttp.go | 2 +- wgengine/magicsock/magicsock.go | 2 +- wgengine/wgcfg/nmcfg/nmcfg.go | 2 +- 15 files changed, 72 insertions(+), 42 deletions(-) diff --git a/cmd/cloner/cloner.go b/cmd/cloner/cloner.go index f913c5cde..a825dc214 100644 --- a/cmd/cloner/cloner.go +++ b/cmd/cloner/cloner.go @@ -172,9 +172,15 @@ func gen(buf *bytes.Buffer, imports map[string]struct{}, typ *types.Named, thisP if !codegen.ContainsPointers(ft) { continue } - if named, _ := ft.(*types.Named); named != nil && !hasBasicUnderlying(ft) { - writef("dst.%s = *src.%s.Clone()", fname, fname) - continue + if named, _ := ft.(*types.Named); named != nil { + if isViewType(ft) { + writef("dst.%s = src.%s", fname, fname) + continue + } + if !hasBasicUnderlying(ft) { + writef("dst.%s = *src.%s.Clone()", fname, fname) + continue + } } switch ft := ft.Underlying().(type) { case *types.Slice: @@ -234,6 +240,17 @@ func gen(buf *bytes.Buffer, imports map[string]struct{}, typ *types.Named, thisP buf.Write(codegen.AssertStructUnchanged(t, thisPkg, name, "Clone", imports)) } +func isViewType(typ types.Type) bool { + t, ok := typ.Underlying().(*types.Struct) + if !ok { + return false + } + if t.NumFields() != 1 { + return false + } + return t.Field(0).Name() == "ж" +} + func hasBasicUnderlying(typ types.Type) bool { switch typ.Underlying().(type) { case *types.Slice, *types.Map: diff --git a/cmd/hello/hello.go b/cmd/hello/hello.go index 97b98be32..ef863e24a 100644 --- a/cmd/hello/hello.go +++ b/cmd/hello/hello.go @@ -196,7 +196,7 @@ func root(w http.ResponseWriter, r *http.Request) { LoginName: who.UserProfile.LoginName, ProfilePicURL: who.UserProfile.ProfilePicURL, MachineName: firstLabel(who.Node.ComputedName), - MachineOS: who.Node.Hostinfo.OS, + MachineOS: who.Node.Hostinfo.OS(), IP: tailscaleIP(who), } } diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 30e7aa87f..e3e84e3ce 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -75,6 +75,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/types/persist from tailscale.com/ipn tailscale.com/types/preftype from tailscale.com/cmd/tailscale/cli+ tailscale.com/types/structs from tailscale.com/ipn+ + tailscale.com/types/views from tailscale.com/tailcfg tailscale.com/util/clientmetric from tailscale.com/net/netcheck+ tailscale.com/util/dnsname from tailscale.com/cmd/tailscale/cli+ W tailscale.com/util/endian from tailscale.com/net/netns diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index d6e6e96db..8cd617db9 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -240,6 +240,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/types/persist from tailscale.com/control/controlclient+ tailscale.com/types/preftype from tailscale.com/ipn+ tailscale.com/types/structs from tailscale.com/control/controlclient+ + tailscale.com/types/views from tailscale.com/tailcfg tailscale.com/util/clientmetric from tailscale.com/ipn/localapi+ L tailscale.com/util/cmpver from tailscale.com/net/dns 💣 tailscale.com/util/deephash from tailscale.com/ipn/ipnlocal+ diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 02308b364..9109f1602 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -138,7 +138,9 @@ func (ms *mapSession) netmapForResponse(resp *tailcfg.MapResponse) *netmap.Netwo nm.Name = node.Name nm.Addresses = filterSelfAddresses(node.Addresses) nm.User = node.User - nm.Hostinfo = node.Hostinfo + if node.Hostinfo.Valid() { + nm.Hostinfo = *node.Hostinfo.AsStruct() + } if node.MachineAuthorized { nm.MachineStatus = tailcfg.MachineAuthorized } else { diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 15f7f5698..75b3f6149 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -446,14 +446,14 @@ func (b *LocalBackend) populatePeerStatusLocked(sb *ipnstate.StatusBuilder) { ID: p.StableID, UserID: p.User, TailscaleIPs: tailscaleIPs, - HostName: p.Hostinfo.Hostname, + HostName: p.Hostinfo.Hostname(), DNSName: p.Name, - OS: p.Hostinfo.OS, + OS: p.Hostinfo.OS(), KeepAlive: p.KeepAlive, Created: p.Created, LastSeen: lastSeen, Online: p.Online != nil && *p.Online, - ShareeNode: p.Hostinfo.ShareeNode, + ShareeNode: p.Hostinfo.ShareeNode(), ExitNode: p.StableID != "" && p.StableID == b.prefs.ExitNodeID, ExitNodeOption: exitNodeOption, }) @@ -2956,9 +2956,10 @@ func (b *LocalBackend) registerIncomingFile(inf *incomingFile, active bool) { // It returns the empty string if the peer doesn't support the peerapi // or there's no matching address family based on the netmap's own addresses. func peerAPIBase(nm *netmap.NetworkMap, peer *tailcfg.Node) string { - if nm == nil || peer == nil { + if nm == nil || peer == nil || !peer.Hostinfo.Valid() { return "" } + var have4, have6 bool for _, a := range nm.Addresses { if !a.IsSingleIP() { @@ -2972,7 +2973,9 @@ func peerAPIBase(nm *netmap.NetworkMap, peer *tailcfg.Node) string { } } var p4, p6 uint16 - for _, s := range peer.Hostinfo.Services { + svcs := peer.Hostinfo.Services() + for i, n := 0, svcs.Len(); i < n; i++ { + s := svcs.At(i) switch s.Proto { case tailcfg.PeerAPI4: p4 = s.Port @@ -3178,7 +3181,9 @@ func exitNodeCanProxyDNS(nm *netmap.NetworkMap, exitNodeID tailcfg.StableNodeID) if p.StableID != exitNodeID { continue } - for _, s := range p.Hostinfo.Services { + services := p.Hostinfo.Services() + for i, n := 0, services.Len(); i < n; i++ { + s := services.At(i) if s.Proto == tailcfg.PeerAPIDNS && s.Port >= 1 { return peerAPIBase(nm, p) + "/dns-query", true } diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 271b83bd7..280476048 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -346,12 +346,12 @@ func TestPeerAPIBase(t *testing.T) { netaddr.MustParseIPPrefix("100.64.1.2/32"), netaddr.MustParseIPPrefix("fe70::2/128"), }, - Hostinfo: tailcfg.Hostinfo{ + Hostinfo: (&tailcfg.Hostinfo{ Services: []tailcfg.Service{ {Proto: "peerapi4", Port: 444}, {Proto: "peerapi6", Port: 666}, }, - }, + }).View(), }, want: "http://100.64.1.2:444", }, @@ -367,12 +367,12 @@ func TestPeerAPIBase(t *testing.T) { netaddr.MustParseIPPrefix("100.64.1.2/32"), netaddr.MustParseIPPrefix("fe70::2/128"), }, - Hostinfo: tailcfg.Hostinfo{ + Hostinfo: (&tailcfg.Hostinfo{ Services: []tailcfg.Service{ {Proto: "peerapi4", Port: 444}, {Proto: "peerapi6", Port: 666}, }, - }, + }).View(), }, want: "http://[fe70::2]:666", }, @@ -389,11 +389,11 @@ func TestPeerAPIBase(t *testing.T) { netaddr.MustParseIPPrefix("100.64.1.2/32"), netaddr.MustParseIPPrefix("fe70::2/128"), }, - Hostinfo: tailcfg.Hostinfo{ + Hostinfo: (&tailcfg.Hostinfo{ Services: []tailcfg.Service{ {Proto: "peerapi4", Port: 444}, }, - }, + }).View(), }, want: "http://100.64.1.2:444", }, @@ -410,11 +410,11 @@ func TestPeerAPIBase(t *testing.T) { netaddr.MustParseIPPrefix("100.64.1.2/32"), netaddr.MustParseIPPrefix("fe70::2/128"), }, - Hostinfo: tailcfg.Hostinfo{ + Hostinfo: (&tailcfg.Hostinfo{ Services: []tailcfg.Service{ {Proto: "peerapi6", Port: 666}, }, - }, + }).View(), }, want: "http://[fe70::2]:666", }, diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 7756345d0..6f4133764 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -157,7 +157,7 @@ type Node struct { AllowedIPs []netaddr.IPPrefix // range of IP addresses to route to this node Endpoints []string `json:",omitempty"` // IP+port (public via STUN, and local LANs) DERP string `json:",omitempty"` // DERP-in-IP:port ("127.3.3.40:N") endpoint - Hostinfo Hostinfo + Hostinfo HostinfoView Created time.Time // Tags are the list of ACL tags applied to this node. @@ -256,7 +256,10 @@ func (n *Node) DisplayNames(forOwner bool) (name, hostIfDifferent string) { // n.ComputedNameWithHost. func (n *Node) InitDisplayNames(networkMagicDNSSuffix string) { name := dnsname.TrimSuffix(n.Name, networkMagicDNSSuffix) - hostIfDifferent := dnsname.SanitizeHostname(n.Hostinfo.Hostname) + var hostIfDifferent string + if n.Hostinfo.Valid() { + hostIfDifferent = dnsname.SanitizeHostname(n.Hostinfo.Hostname()) + } if strings.EqualFold(name, hostIfDifferent) { hostIfDifferent = "" @@ -456,7 +459,7 @@ type Hostinfo struct { // require changes to Hostinfo.Equal. } -// View returns a read-only accessor for the Hostinfo object. +// View returns a read-only accessor for hi. func (hi *Hostinfo) View() HostinfoView { return HostinfoView{hi} } // HostinfoView is a read-only accessor for Hostinfo. @@ -503,7 +506,7 @@ func (v HostinfoView) Hostname() string { return v.ж.Hostname } func (v HostinfoView) ShieldsUp() bool { return v.ж.ShieldsUp } func (v HostinfoView) ShareeNode() bool { return v.ж.ShareeNode } func (v HostinfoView) GoArch() string { return v.ж.GoArch } -func (v HostinfoView) Equal(h2 HostinfoView) bool { return v.ж.Equal(h2.ж) } +func (v HostinfoView) Equal(v2 HostinfoView) bool { return v.ж.Equal(v2.ж) } func (v HostinfoView) RoutableIPs() views.IPPrefixSlice { return views.IPPrefixSliceOf(v.ж.RoutableIPs) @@ -656,7 +659,7 @@ func (ni *NetInfo) portMapSummary() string { return prefix + conciseOptBool(ni.UPnP, "U") + conciseOptBool(ni.PMP, "M") + conciseOptBool(ni.PCP, "C") } -// View returns a read-only accessor for the NetInfo object. +// View returns a read-only accessor for ni. func (ni *NetInfo) View() NetInfoView { return NetInfoView{ni} } func conciseOptBool(b opt.Bool, trueVal string) string { @@ -1390,7 +1393,7 @@ func (n *Node) Equal(n2 *Node) bool { eqCIDRs(n.PrimaryRoutes, n2.PrimaryRoutes) && eqStrings(n.Endpoints, n2.Endpoints) && n.DERP == n2.DERP && - n.Hostinfo.Equal(&n2.Hostinfo) && + n.Hostinfo.Equal(n2.Hostinfo) && n.Created.Equal(n2.Created) && eqTimePtr(n.LastSeen, n2.LastSeen) && n.MachineAuthorized == n2.MachineAuthorized && diff --git a/tailcfg/tailcfg_clone.go b/tailcfg/tailcfg_clone.go index 2f162bf53..d3e3c7f28 100644 --- a/tailcfg/tailcfg_clone.go +++ b/tailcfg/tailcfg_clone.go @@ -50,7 +50,7 @@ func (src *Node) Clone() *Node { dst.Addresses = append(src.Addresses[:0:0], src.Addresses...) dst.AllowedIPs = append(src.AllowedIPs[:0:0], src.AllowedIPs...) dst.Endpoints = append(src.Endpoints[:0:0], src.Endpoints...) - dst.Hostinfo = *src.Hostinfo.Clone() + dst.Hostinfo = src.Hostinfo dst.Tags = append(src.Tags[:0:0], src.Tags...) dst.PrimaryRoutes = append(src.PrimaryRoutes[:0:0], src.PrimaryRoutes...) if dst.LastSeen != nil { @@ -80,7 +80,7 @@ var _NodeCloneNeedsRegeneration = Node(struct { AllowedIPs []netaddr.IPPrefix Endpoints []string DERP string - Hostinfo Hostinfo + Hostinfo HostinfoView Created time.Time Tags []string PrimaryRoutes []netaddr.IPPrefix diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 37275c59a..a75de335b 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -400,13 +400,13 @@ func TestNodeEqual(t *testing.T) { true, }, { - &Node{Hostinfo: Hostinfo{Hostname: "alice"}}, - &Node{Hostinfo: Hostinfo{Hostname: "bob"}}, + &Node{Hostinfo: (&Hostinfo{Hostname: "alice"}).View()}, + &Node{Hostinfo: (&Hostinfo{Hostname: "bob"}).View()}, false, }, { - &Node{Hostinfo: Hostinfo{}}, - &Node{Hostinfo: Hostinfo{}}, + &Node{Hostinfo: (&Hostinfo{}).View()}, + &Node{Hostinfo: (&Hostinfo{}).View()}, true, }, { diff --git a/tstest/integration/testcontrol/testcontrol.go b/tstest/integration/testcontrol/testcontrol.go index 77f237089..4eb0446af 100644 --- a/tstest/integration/testcontrol/testcontrol.go +++ b/tstest/integration/testcontrol/testcontrol.go @@ -474,7 +474,7 @@ func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey key. MachineAuthorized: machineAuthorized, Addresses: allowedIPs, AllowedIPs: allowedIPs, - Hostinfo: *req.Hostinfo, + Hostinfo: req.Hostinfo.View(), } requireAuth := s.RequireAuth if requireAuth && s.nodeKeyAuthed[nk] { @@ -611,10 +611,10 @@ func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey key.Machi node.Endpoints = endpoints node.DiscoKey = req.DiscoKey if req.Hostinfo != nil { - node.Hostinfo = *req.Hostinfo.Clone() - if ni := node.Hostinfo.NetInfo; ni != nil { - if ni.PreferredDERP != 0 { - node.DERP = fmt.Sprintf("127.3.3.40:%d", ni.PreferredDERP) + node.Hostinfo = req.Hostinfo.View() + if ni := node.Hostinfo.NetInfo(); ni.Valid() { + if ni.PreferredDERP() != 0 { + node.DERP = fmt.Sprintf("127.3.3.40:%d", ni.PreferredDERP()) } } } diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index 8871eeebb..315c62ca7 100644 --- a/types/netmap/netmap.go +++ b/types/netmap/netmap.go @@ -37,9 +37,10 @@ type NetworkMap struct { MachineKey key.MachinePublic Peers []*tailcfg.Node // sorted by Node.ID DNS tailcfg.DNSConfig - Hostinfo tailcfg.Hostinfo - PacketFilter []filter.Match - SSHPolicy *tailcfg.SSHPolicy // or nil, if not enabled/allowed + // TODO(maisem) : replace with View. + Hostinfo tailcfg.Hostinfo + PacketFilter []filter.Match + SSHPolicy *tailcfg.SSHPolicy // or nil, if not enabled/allowed // CollectServices reports whether this node's Tailnet has // requested that info about services be included in HostInfo. diff --git a/wgengine/magicsock/debughttp.go b/wgengine/magicsock/debughttp.go index bde862c1b..672616271 100644 --- a/wgengine/magicsock/debughttp.go +++ b/wgengine/magicsock/debughttp.go @@ -191,7 +191,7 @@ func peerDebugName(p *tailcfg.Node) string { if i := strings.Index(n, "."); i != -1 { return n[:i] } - return p.Hostinfo.Hostname + return p.Hostinfo.Hostname() } func ipPortLess(a, b netaddr.IPPort) bool { diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 2cc2471e0..1d621f753 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -901,7 +901,7 @@ func (c *Conn) Ping(peer *tailcfg.Node, res *ipnstate.PingResult, cb func(*ipnst } res.NodeName = peer.Name // prefer DNS name if res.NodeName == "" { - res.NodeName = peer.Hostinfo.Hostname // else hostname + res.NodeName = peer.Hostinfo.Hostname() // else hostname } else { if i := strings.Index(res.NodeName, "."); i != -1 { res.NodeName = res.NodeName[:i] diff --git a/wgengine/wgcfg/nmcfg/nmcfg.go b/wgengine/wgcfg/nmcfg/nmcfg.go index e8cdd16ee..16014c465 100644 --- a/wgengine/wgcfg/nmcfg/nmcfg.go +++ b/wgengine/wgcfg/nmcfg/nmcfg.go @@ -21,7 +21,7 @@ import ( func nodeDebugName(n *tailcfg.Node) string { name := n.Name if name == "" { - name = n.Hostinfo.Hostname + name = n.Hostinfo.Hostname() } if i := strings.Index(name, "."); i != -1 { name = name[:i]