tailcfg, control/controlclient: accept nil MapResponse.Node (mapver 18)

All MapResponse fields can not be omitted and are tagged "omitempty".

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

control/controlclient/direct.go

@@ -798,6 +798,10 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm
 		}
 
 		nm := sess.netmapForResponse(&resp)
+		if nm.SelfNode == nil {
+			c.logf("MapResponse lacked node")
+			return errors.New("MapResponse lacked node")
+		}
 
 		// Temporarily (2020-06-29) support removing all but
 		// discovery-supporting nodes during development, for

control/controlclient/map.go

@@ -32,6 +32,7 @@ type mapSession struct {
 	keepSharerAndUserSplit bool // see Options.KeepSharerAndUserSplit
 
 	// Fields storing state over the the coards of multiple MapResponses.
+	lastNode               *tailcfg.Node
 	lastDNSConfig          *tailcfg.DNSConfig
 	lastDERPMap            *tailcfg.DERPMap
 	lastUserProfile        map[tailcfg.UserID]tailcfg.UserProfile
@@ -102,19 +103,13 @@ func (ms *mapSession) netmapForResponse(resp *tailcfg.MapResponse) *netmap.Netwo
 	}
 
 	nm := &netmap.NetworkMap{
-		SelfNode:        resp.Node,
 		NodeKey:         tailcfg.NodeKey(ms.privateNodeKey.Public()),
 		PrivateKey:      ms.privateNodeKey,
 		MachineKey:      ms.machinePubKey,
-		Expiry:          resp.Node.KeyExpiry,
-		Name:            resp.Node.Name,
-		Addresses:       resp.Node.Addresses,
 		Peers:           resp.Peers,
-		User:            resp.Node.User,
 		UserProfiles:    make(map[tailcfg.UserID]tailcfg.UserProfile),
 		Domain:          ms.lastDomain,
 		DNS:             *ms.lastDNSConfig,
-		Hostinfo:        resp.Node.Hostinfo,
 		PacketFilter:    ms.lastParsedPacketFilter,
 		CollectServices: ms.collectServices,
 		DERPMap:         ms.lastDERPMap,
@@ -122,9 +117,28 @@ func (ms *mapSession) netmapForResponse(resp *tailcfg.MapResponse) *netmap.Netwo
 	}
 	ms.netMapBuilding = nm
 
+	if resp.Node != nil {
+		ms.lastNode = resp.Node
+	}
+	if node := ms.lastNode.Clone(); node != nil {
+		nm.SelfNode = node
+		nm.Expiry = node.KeyExpiry
+		nm.Name = node.Name
+		nm.Addresses = node.Addresses
+		nm.User = node.User
+		nm.Hostinfo = node.Hostinfo
+		if node.MachineAuthorized {
+			nm.MachineStatus = tailcfg.MachineAuthorized
+		} else {
+			nm.MachineStatus = tailcfg.MachineUnauthorized
+		}
+	}
+
 	ms.addUserProfile(nm.User)
 	magicDNSSuffix := nm.MagicDNSSuffix()
-	nm.SelfNode.InitDisplayNames(magicDNSSuffix)
+	if nm.SelfNode != nil {
+		nm.SelfNode.InitDisplayNames(magicDNSSuffix)
+	}
 	for _, peer := range resp.Peers {
 		peer.InitDisplayNames(magicDNSSuffix)
 		if !peer.Sharer.IsZero() {
@@ -136,11 +150,6 @@ func (ms *mapSession) netmapForResponse(resp *tailcfg.MapResponse) *netmap.Netwo
 		}
 		ms.addUserProfile(peer.User)
 	}
-	if resp.Node.MachineAuthorized {
-		nm.MachineStatus = tailcfg.MachineAuthorized
-	} else {
-		nm.MachineStatus = tailcfg.MachineUnauthorized
-	}
 	if len(resp.DNS) > 0 {
 		nm.DNS.Nameservers = resp.DNS
 	}

control/controlclient/map_test.go

@@ -5,6 +5,7 @@
 package controlclient
 
 import (
+	"encoding/json"
 	"fmt"
 	"reflect"
 	"strings"
@@ -276,4 +277,35 @@ func TestNetmapForResponse(t *testing.T) {
 		})
 		want("foo.com")
 	})
+	t.Run("implicit_node", func(t *testing.T) {
+		someNode := &tailcfg.Node{
+			Name: "foo",
+		}
+		wantNode := &tailcfg.Node{
+			Name:                 "foo",
+			ComputedName:         "foo",
+			ComputedNameWithHost: "foo",
+		}
+		ms := newTestMapSession(t)
+
+		nm1 := ms.netmapForResponse(&tailcfg.MapResponse{
+			Node: someNode,
+		})
+		if nm1.SelfNode == nil {
+			t.Fatal("nil Node in 1st netmap")
+		}
+		if !reflect.DeepEqual(nm1.SelfNode, wantNode) {
+			j, _ := json.Marshal(nm1.SelfNode)
+			t.Errorf("Node mismatch in 1st netmap; got: %s", j)
+		}
+
+		nm2 := ms.netmapForResponse(&tailcfg.MapResponse{})
+		if nm2.SelfNode == nil {
+			t.Fatal("nil Node in 1st netmap")
+		}
+		if !reflect.DeepEqual(nm2.SelfNode, wantNode) {
+			j, _ := json.Marshal(nm2.SelfNode)
+			t.Errorf("Node mismatch in 2nd netmap; got: %s", j)
+		}
+	})
 }

tailcfg/tailcfg.go

@@ -40,7 +40,8 @@ import (
 //    15: 2021-04-12: client treats nil MapResponse.DNSConfig as meaning unchanged
 //    16: 2021-04-15: client understands Node.Online, MapResponse.OnlineChange
 //    17: 2021-04-18: MapResponse.Domain empty means unchanged
-const CurrentMapRequestVersion = 17
+//    18: 2021-04-19: MapResponse.Node nil means unchanged (all fields now omitempty)
+const CurrentMapRequestVersion = 18
 
 type StableID string
 
@@ -895,8 +896,14 @@ type MapResponse struct {
 	PingRequest *PingRequest `json:",omitempty"`
 
 	// Networking
-	Node    *Node
-	DERPMap *DERPMap `json:",omitempty"` // if non-empty, a change in the DERP map.
+
+	// Node describes the node making the map request.
+	// Starting with MapRequest.Version 18, nil means unchanged.
+	Node *Node `json:",omitempty"`
+
+	// DERPMap describe the set of DERP servers available.
+	// A nil value means unchanged.
+	DERPMap *DERPMap `json:",omitempty"`
 
 	// Peers, if non-empty, is the complete list of peers.
 	// It will be set in the first MapResponse for a long-polled request/response.
@@ -955,9 +962,12 @@ type MapResponse struct {
 	// previously streamed non-nil MapResponse.PacketFilter within
 	// the same HTTP response. A non-nil but empty list always means
 	// no PacketFilter (that is, to block everything).
-	PacketFilter []FilterRule
+	PacketFilter []FilterRule `json:",omitempty"`
 
-	UserProfiles []UserProfile // as of 1.1.541 (mapver 5): may be new or updated user profiles only
+	// UserProfiles are the user profiles of nodes in the network.
+	// As as of 1.1.541 (mapver 5), this contains new or updated
+	// user profiles only.
+	UserProfiles []UserProfile `json:",omitempty"`
 
 	// Debug is normally nil, except for when the control server
 	// is setting debug settings on a node.