diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index b3a9f61ac..e1aa7958b 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -770,12 +770,12 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*Netw c.mu.Unlock() nm := &NetworkMap{ + SelfNode: resp.Node, NodeKey: tailcfg.NodeKey(persist.PrivateNodeKey.Public()), PrivateKey: persist.PrivateNodeKey, MachineKey: machinePubKey, Expiry: resp.Node.KeyExpiry, Name: resp.Node.Name, - DisplayName: resp.Node.DisplayName, Addresses: resp.Node.Addresses, Peers: resp.Peers, LocalPort: localPort, @@ -799,10 +799,10 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*Netw } } addUserProfile(nm.User) + magicDNSSuffix := nm.MagicDNSSuffix() + nm.SelfNode.InitDisplayNames(magicDNSSuffix) for _, peer := range resp.Peers { - if peer.DisplayName == "" { - peer.DisplayName = peer.DefaultDisplayName() - } + peer.InitDisplayNames(magicDNSSuffix) if !peer.Sharer.IsZero() { if c.keepSharerAndUserSplit { addUserProfile(peer.Sharer) @@ -812,9 +812,6 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*Netw } addUserProfile(peer.User) } - if resp.Node.DisplayName == "" { - nm.DisplayName = resp.Node.DefaultDisplayName() - } if resp.Node.MachineAuthorized { nm.MachineStatus = tailcfg.MachineAuthorized } else { diff --git a/control/controlclient/netmap.go b/control/controlclient/netmap.go index fe8553b66..c3007c275 100644 --- a/control/controlclient/netmap.go +++ b/control/controlclient/netmap.go @@ -24,13 +24,12 @@ type NetworkMap struct { // Core networking + SelfNode *tailcfg.Node NodeKey tailcfg.NodeKey PrivateKey wgkey.Private Expiry time.Time // Name is the DNS name assigned to this node. - Name string - // DisplayName is the title to show for the node in client UIs. - DisplayName string + Name string Addresses []netaddr.IPPrefix LocalPort uint16 // used for debugging MachineStatus tailcfg.MachineStatus diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 098c95d56..4e7c51f79 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -161,12 +161,6 @@ type Node struct { StableID StableNodeID Name string // DNS - // DisplayName is the title to show for the node in client - // UIs. This field is assigned by default in controlclient, - // but can be overriden by providing this field non-empty - // in a MapResponse. - DisplayName string `json:",omitempty"` - // User is the user who created the node. If ACL tags are in // use for the node then it doesn't reflect the ACL identity // that the node is running as. @@ -190,21 +184,98 @@ type Node struct { KeepAlive bool `json:",omitempty"` // open and keep open a connection to this peer MachineAuthorized bool `json:",omitempty"` // TODO(crawshaw): replace with MachineStatus + + // The following three computed fields hold the various names that can + // be used for this node in UIs. They are populated from controlclient + // (not from control) by calling node.InitDisplayNames. These can be + // used directly or accessed via node.DisplayName or node.DisplayNames. + + ComputedName string `json:",omitempty"` // MagicDNS base name (for normal non-shared-in nodes), FQDN (without trailing dot, for shared-in nodes), or Hostname (if no MagicDNS) + computedHostIfDifferent string // hostname, if different than ComputedName, otherwise empty + ComputedNameWithHost string `json:",omitempty"` // either "ComputedName" or "ComputedName (computedHostIfDifferent)", if computedHostIfDifferent is set } -// DefaultDisplayName returns a value suitable -// for using as the default value for n.DisplayName. -func (n *Node) DefaultDisplayName() string { - if n.Name != "" { - // Use the Magic DNS prefix as the default display name. - return dnsname.ToBaseName(n.Name) +// DisplayName returns the user-facing name for a node which should +// be shown in client UIs. +// +// Parameter forOwner specifies whether the name is requested by +// the owner of the node. When forOwner is false, the hostname is +// never included in the return value. +// +// Return value is either either "Name" or "Name (Hostname)", where +// Name is the node's MagicDNS base name (for normal non-shared-in +// nodes), FQDN (without trailing dot, for shared-in nodes), or +// Hostname (if no MagicDNS). Hostname is only included in the +// return value if it varies from Name and forOwner is provided true. +// +// DisplayName is only valid if InitDisplayNames has been called. +func (n *Node) DisplayName(forOwner bool) string { + if forOwner { + return n.ComputedNameWithHost } - if n.Hostinfo.Hostname != "" { - // When no Magic DNS name is present, use the hostname. - return n.Hostinfo.Hostname + return n.ComputedName +} + +// DisplayName returns the decomposed user-facing name for a node. +// +// Parameter forOwner specifies whether the name is requested by +// the owner of the node. When forOwner is false, hostIfDifferent +// is always returned empty. +// +// Return value name is the node's primary name, populated with the +// node's MagicDNS base name (for normal non-shared-in nodes), FQDN +// (without trailing dot, for shared-in nodes), or Hostname (if no +// MagicDNS). +// +// Return value hostIfDifferent, when non-empty, is the node's +// hostname. hostIfDifferent is only populated when the hostname +// varies from name and forOwner is provided as true. +// +// DisplayNames is only valid if InitDisplayNames has been called. +func (n *Node) DisplayNames(forOwner bool) (name, hostIfDifferent string) { + if forOwner { + return n.ComputedName, n.computedHostIfDifferent } - // When we've exhausted all other name options, use the node's ID. - return n.ID.String() + return n.ComputedName, "" +} + +// InitDisplayNames computes and populates n's display name +// fields: n.ComputedName, n.computedHostIfDifferent, and +// n.ComputedNameWithHost. +func (n *Node) InitDisplayNames(networkMagicDNSSuffix string) { + dnsName := n.Name + if dnsName != "" { + dnsName = strings.TrimRight(dnsName, ".") + if i := strings.Index(dnsName, "."); i != -1 && dnsname.HasSuffix(dnsName, networkMagicDNSSuffix) { + dnsName = dnsName[:i] + } + } + + name := dnsName + hostIfDifferent := n.Hostinfo.Hostname + + if strings.EqualFold(name, hostIfDifferent) { + hostIfDifferent = "" + } + if name == "" { + if hostIfDifferent != "" { + name = hostIfDifferent + hostIfDifferent = "" + } else { + name = n.Key.String() + } + } + + var nameWithHost string + if hostIfDifferent != "" { + nameWithHost = fmt.Sprintf("%s (%s)", name, hostIfDifferent) + } else { + nameWithHost = name + } + + n.ComputedName = name + n.computedHostIfDifferent = hostIfDifferent + n.ComputedNameWithHost = nameWithHost } type MachineStatus int @@ -818,7 +889,6 @@ func (n *Node) Equal(n2 *Node) bool { n.ID == n2.ID && n.StableID == n2.StableID && n.Name == n2.Name && - n.DisplayName == n2.DisplayName && n.User == n2.User && n.Sharer == n2.Sharer && n.Key == n2.Key && @@ -832,7 +902,10 @@ func (n *Node) Equal(n2 *Node) bool { n.Hostinfo.Equal(&n2.Hostinfo) && n.Created.Equal(n2.Created) && eqTimePtr(n.LastSeen, n2.LastSeen) && - n.MachineAuthorized == n2.MachineAuthorized + n.MachineAuthorized == n2.MachineAuthorized && + n.ComputedName == n2.ComputedName && + n.computedHostIfDifferent == n2.computedHostIfDifferent && + n.ComputedNameWithHost == n2.ComputedNameWithHost } func eqStrings(a, b []string) bool { diff --git a/tailcfg/tailcfg_clone.go b/tailcfg/tailcfg_clone.go index 3101048b4..87bbb2484 100644 --- a/tailcfg/tailcfg_clone.go +++ b/tailcfg/tailcfg_clone.go @@ -61,25 +61,27 @@ func (src *Node) Clone() *Node { // A compilation failure here means this code must be regenerated, with command: // tailscale.com/cmd/cloner -type User,Node,Hostinfo,NetInfo,Group,Role,Capability,Login,DNSConfig,RegisterResponse var _NodeNeedsRegeneration = Node(struct { - ID NodeID - StableID StableNodeID - Name string - DisplayName string - User UserID - Sharer UserID - Key NodeKey - KeyExpiry time.Time - Machine MachineKey - DiscoKey DiscoKey - Addresses []netaddr.IPPrefix - AllowedIPs []netaddr.IPPrefix - Endpoints []string - DERP string - Hostinfo Hostinfo - Created time.Time - LastSeen *time.Time - KeepAlive bool - MachineAuthorized bool + ID NodeID + StableID StableNodeID + Name string + User UserID + Sharer UserID + Key NodeKey + KeyExpiry time.Time + Machine MachineKey + DiscoKey DiscoKey + Addresses []netaddr.IPPrefix + AllowedIPs []netaddr.IPPrefix + Endpoints []string + DERP string + Hostinfo Hostinfo + Created time.Time + LastSeen *time.Time + KeepAlive bool + MachineAuthorized bool + ComputedName string + computedHostIfDifferent string + ComputedNameWithHost string }{}) // Clone makes a deep copy of Hostinfo. diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 794f7d4a1..a6c843db5 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -189,10 +189,11 @@ func TestHostinfoEqual(t *testing.T) { func TestNodeEqual(t *testing.T) { nodeHandles := []string{ - "ID", "StableID", "Name", "DisplayName", "User", "Sharer", + "ID", "StableID", "Name", "User", "Sharer", "Key", "KeyExpiry", "Machine", "DiscoKey", "Addresses", "AllowedIPs", "Endpoints", "DERP", "Hostinfo", "Created", "LastSeen", "KeepAlive", "MachineAuthorized", + "ComputedName", "computedHostIfDifferent", "ComputedNameWithHost", } if have := fieldsOf(reflect.TypeOf(Node{})); !reflect.DeepEqual(have, nodeHandles) { t.Errorf("Node.Equal check might be out of sync\nfields: %q\nhandled: %q\n", diff --git a/util/dnsname/dnsname.go b/util/dnsname/dnsname.go index 633471e2f..1488272a4 100644 --- a/util/dnsname/dnsname.go +++ b/util/dnsname/dnsname.go @@ -17,11 +17,3 @@ func HasSuffix(name, suffix string) bool { nameBase := strings.TrimSuffix(name, suffix) return len(nameBase) < len(name) && strings.HasSuffix(nameBase, ".") } - -// ToBaseName removes the domain ending from a DNS name of a node. -func ToBaseName(name string) string { - if i := strings.Index(name, "."); i != -1 { - return name[:i] - } - return name -} diff --git a/util/dnsname/dnsname_test.go b/util/dnsname/dnsname_test.go index a8c97ed8b..da4e51384 100644 --- a/util/dnsname/dnsname_test.go +++ b/util/dnsname/dnsname_test.go @@ -26,21 +26,3 @@ func TestHasSuffix(t *testing.T) { } } } - -func TestToBaseName(t *testing.T) { - tests := []struct { - name string - want string - }{ - {"foo", "foo"}, - {"foo.com", "foo"}, - {"foo.example.com.beta.tailscale.net", "foo"}, - {"computer-a.test.gmail.com.beta.tailscale.net", "computer-a"}, - } - for _, tt := range tests { - got := ToBaseName(tt.name) - if got != tt.want { - t.Errorf("ToBaseName(%q) = %q; want %q", tt.name, got, tt.want) - } - } -}