control/controlclient, types/netmap: remove unused LocalPort field

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

control/controlclient/auto.go

@@ -668,11 +668,8 @@ func (c *Auto) SetExpirySooner(ctx context.Context, expiry time.Time) error {
 // them to the control server if they've changed.
 //
 // It does not retain the provided slice.
-//
+func (c *Auto) UpdateEndpoints(endpoints []tailcfg.Endpoint) {
-// The localPort field is unused except for integration tests in
+	changed := c.direct.SetEndpoints(endpoints)
-// another repo.
-func (c *Auto) UpdateEndpoints(localPort uint16, endpoints []tailcfg.Endpoint) {
-	changed := c.direct.SetEndpoints(localPort, endpoints)
 	if changed {
 		c.sendNewMapRequest()
 	}

control/controlclient/client.go

@@ -75,11 +75,10 @@ type Client interface {
 	SetNetInfo(*tailcfg.NetInfo)
 	// UpdateEndpoints changes the Endpoint structure that will be sent
 	// in subsequent node registration requests.
-	// The localPort field is unused except for integration tests in another repo.
 	// TODO: a server-side change would let us simply upload this
 	// in a separate http request. It has nothing to do with the rest of
 	// the state machine.
-	UpdateEndpoints(localPort uint16, endpoints []tailcfg.Endpoint)
+	UpdateEndpoints(endpoints []tailcfg.Endpoint)
 	// SetDNS sends the SetDNSRequest request to the control plane server,
 	// requesting a DNS record be created or updated.
 	SetDNS(context.Context, *tailcfg.SetDNSRequest) error

control/controlclient/direct.go

@@ -88,7 +88,6 @@ type Direct struct {
 	netinfo       *tailcfg.NetInfo
 	endpoints     []tailcfg.Endpoint
 	everEndpoints bool   // whether we've ever had non-empty endpoints
-	localPort     uint16 // or zero to mean auto
 	lastPingURL   string // last PingRequest.URL received, for dup suppression
 }
 
@@ -586,20 +585,19 @@ func sameEndpoints(a, b []tailcfg.Endpoint) bool {
 // whether they've changed.
 //
 // It does not retain the provided slice.
-func (c *Direct) newEndpoints(localPort uint16, endpoints []tailcfg.Endpoint) (changed bool) {
+func (c *Direct) newEndpoints(endpoints []tailcfg.Endpoint) (changed bool) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
 	// Nothing new?
-	if c.localPort == localPort && sameEndpoints(c.endpoints, endpoints) {
+	if sameEndpoints(c.endpoints, endpoints) {
 		return false // unchanged
 	}
 	var epStrs []string
 	for _, ep := range endpoints {
 		epStrs = append(epStrs, ep.Addr.String())
 	}
-	c.logf("[v2] client.newEndpoints(%v, %v)", localPort, epStrs)
+	c.logf("[v2] client.newEndpoints(%v)", epStrs)
-	c.localPort = localPort
 	c.endpoints = append(c.endpoints[:0], endpoints...)
 	if len(endpoints) > 0 {
 		c.everEndpoints = true
@@ -610,10 +608,10 @@ func (c *Direct) newEndpoints(localPort uint16, endpoints []tailcfg.Endpoint) (c
 // SetEndpoints updates the list of locally advertised endpoints.
 // It won't be replicated to the server until a *fresh* call to PollNetMap().
 // You don't need to restart PollNetMap if we return changed==false.
-func (c *Direct) SetEndpoints(localPort uint16, endpoints []tailcfg.Endpoint) (changed bool) {
+func (c *Direct) SetEndpoints(endpoints []tailcfg.Endpoint) (changed bool) {
 	// (no log message on function entry, because it clutters the logs
 	//  if endpoints haven't changed. newEndpoints() will log it.)
-	return c.newEndpoints(localPort, endpoints)
+	return c.newEndpoints(endpoints)
 }
 
 func inTest() bool { return flag.Lookup("test.v") != nil }
@@ -666,7 +664,6 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, readOnly bool
 	serverNoiseKey := c.serverNoiseKey
 	hi := c.hostInfoLocked()
 	backendLogID := hi.BackendLogID
-	localPort := c.localPort
 	var epStrs []string
 	var epTypes []tailcfg.EndpointType
 	for _, ep := range c.endpoints {
@@ -692,7 +689,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, readOnly bool
 	}
 
 	allowStream := maxPolls != 1
-	c.logf("[v1] PollNetMap: stream=%v :%v ep=%v", allowStream, localPort, epStrs)
+	c.logf("[v1] PollNetMap: stream=%v ep=%v", allowStream, epStrs)
 
 	vlogf := logger.Discard
 	if Debug.NetMap {
@@ -948,14 +945,6 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, readOnly bool
 			nm.SelfNode.Capabilities = nil
 		}
 
-		// Get latest localPort. This might've changed if
-		// a lite map update occurred meanwhile. This only affects
-		// the end-to-end test.
-		// TODO(bradfitz): remove the NetworkMap.LocalPort field entirely.
-		c.mu.Lock()
-		nm.LocalPort = c.localPort
-		c.mu.Unlock()
-
 		// Occasionally print the netmap header.
 		// This is handy for debugging, and our logs processing
 		// pipeline depends on it. (TODO: Remove this dependency.)

control/controlclient/direct_test.go

@@ -68,22 +68,18 @@ func TestNewDirect(t *testing.T) {
 	}
 
 	endpoints := fakeEndpoints(1, 2, 3)
-	changed = c.newEndpoints(12, endpoints)
+	changed = c.newEndpoints(endpoints)
 	if !changed {
-		t.Errorf("c.newEndpoints(12) want true got %v", changed)
+		t.Errorf("c.newEndpoints want true got %v", changed)
 	}
-	changed = c.newEndpoints(12, endpoints)
+	changed = c.newEndpoints(endpoints)
 	if changed {
-		t.Errorf("c.newEndpoints(12) want false got %v", changed)
+		t.Errorf("c.newEndpoints want false got %v", changed)
-	}
-	changed = c.newEndpoints(13, endpoints)
-	if !changed {
-		t.Errorf("c.newEndpoints(13) want true got %v", changed)
 	}
 	endpoints = fakeEndpoints(4, 5, 6)
-	changed = c.newEndpoints(13, endpoints)
+	changed = c.newEndpoints(endpoints)
 	if !changed {
-		t.Errorf("c.newEndpoints(13) want true got %v", changed)
+		t.Errorf("c.newEndpoints want true got %v", changed)
 	}
 }
 

ipn/ipnlocal/local.go

@@ -796,7 +796,7 @@ func (b *LocalBackend) setWgengineStatus(s *wgengine.Status, err error) {
 
 	if cc != nil {
 		if needUpdateEndpoints {
-			cc.UpdateEndpoints(0, s.LocalAddrs)
+			cc.UpdateEndpoints(s.LocalAddrs)
 		}
 		b.stateMachine()
 	}
@@ -1070,7 +1070,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 	b.mu.Unlock()
 
 	if endpoints != nil {
-		cc.UpdateEndpoints(0, endpoints)
+		cc.UpdateEndpoints(endpoints)
 	}
 
 	cc.SetStatusFunc(b.setClientStatus)

ipn/ipnlocal/state_test.go

@@ -258,9 +258,9 @@ func (cc *mockControl) SetNetInfo(ni *tailcfg.NetInfo) {
 	cc.called("SetNetInfo")
 }
 
-func (cc *mockControl) UpdateEndpoints(localPort uint16, endpoints []tailcfg.Endpoint) {
+func (cc *mockControl) UpdateEndpoints(endpoints []tailcfg.Endpoint) {
 	// validate endpoint information here?
-	cc.logf("UpdateEndpoints: lp=%v ep=%v", localPort, endpoints)
+	cc.logf("UpdateEndpoints:  ep=%v", endpoints)
 	cc.called("UpdateEndpoints")
 }
 

types/netmap/netmap.go

@@ -32,7 +32,6 @@ type NetworkMap struct {
 	// Name is the DNS name assigned to this node.
 	Name          string
 	Addresses     []netaddr.IPPrefix // same as tailcfg.Node.Addresses (IP addresses of this Node directly)
-	LocalPort     uint16             // used for debugging
 	MachineStatus tailcfg.MachineStatus
 	MachineKey    key.MachinePublic
 	Peers         []*tailcfg.Node // sorted by Node.ID
@@ -148,9 +147,6 @@ func (nm *NetworkMap) printConciseHeader(buf *strings.Builder) {
 		}
 	}
 	fmt.Fprintf(buf, " u=%s", login)
-	if nm.LocalPort != 0 {
-		fmt.Fprintf(buf, " port=%v", nm.LocalPort)
-	}
 	if nm.Debug != nil {
 		j, _ := json.Marshal(nm.Debug)
 		fmt.Fprintf(buf, " debug=%s", j)
@@ -164,7 +160,6 @@ func (nm *NetworkMap) printConciseHeader(buf *strings.Builder) {
 func (a *NetworkMap) equalConciseHeader(b *NetworkMap) bool {
 	if a.NodeKey != b.NodeKey ||
 		a.MachineStatus != b.MachineStatus ||
-		a.LocalPort != b.LocalPort ||
 		a.User != b.User ||
 		len(a.Addresses) != len(b.Addresses) {
 		return false