Browse Source

feature/relayserver,ipn/ipnlocal,net/udprelay: plumb DERPMap (#17881)

This commit replaces usage of local.Client in net/udprelay with DERPMap
plumbing over the eventbus. This has been a longstanding TODO. This work
was also accelerated by a memory leak in net/http when using
local.Client over long periods of time. So, this commit also addresses
said leak.

Updates #17801

Signed-off-by: Jordan Whited <[email protected]>
Jordan Whited 4 months ago
parent
commit
9e4d1fd87f

+ 109 - 111
feature/relayserver/relayserver.go

@@ -21,8 +21,10 @@ import (
 	"tailscale.com/ipn/ipnext"
 	"tailscale.com/ipn/ipnext"
 	"tailscale.com/ipn/localapi"
 	"tailscale.com/ipn/localapi"
 	"tailscale.com/net/udprelay"
 	"tailscale.com/net/udprelay"
+	"tailscale.com/net/udprelay/endpoint"
 	"tailscale.com/net/udprelay/status"
 	"tailscale.com/net/udprelay/status"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tailcfg"
+	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/ptr"
 	"tailscale.com/types/ptr"
 	"tailscale.com/util/eventbus"
 	"tailscale.com/util/eventbus"
@@ -68,25 +70,41 @@ func servePeerRelayDebugSessions(h *localapi.Handler, w http.ResponseWriter, r *
 // extension. It is registered with [ipnext.RegisterExtension] if the package is
 // extension. It is registered with [ipnext.RegisterExtension] if the package is
 // imported.
 // imported.
 func newExtension(logf logger.Logf, sb ipnext.SafeBackend) (ipnext.Extension, error) {
 func newExtension(logf logger.Logf, sb ipnext.SafeBackend) (ipnext.Extension, error) {
-	return &extension{
+	e := &extension{
+		newServerFn: func(logf logger.Logf, port int, overrideAddrs []netip.Addr) (relayServer, error) {
+			return udprelay.NewServer(logf, port, overrideAddrs)
+		},
 		logf: logger.WithPrefix(logf, featureName+": "),
 		logf: logger.WithPrefix(logf, featureName+": "),
-		bus:  sb.Sys().Bus.Get(),
-	}, nil
+	}
+	e.ec = sb.Sys().Bus.Get().Client("relayserver.extension")
+	e.respPub = eventbus.Publish[magicsock.UDPRelayAllocResp](e.ec)
+	eventbus.SubscribeFunc(e.ec, e.onDERPMapView)
+	eventbus.SubscribeFunc(e.ec, e.onAllocReq)
+	return e, nil
+}
+
+// relayServer is an interface for [udprelay.Server].
+type relayServer interface {
+	Close() error
+	AllocateEndpoint(discoA, discoB key.DiscoPublic) (endpoint.ServerEndpoint, error)
+	GetSessions() []status.ServerSession
+	SetDERPMapView(tailcfg.DERPMapView)
 }
 }
 
 
 // extension is an [ipnext.Extension] managing the relay server on platforms
 // extension is an [ipnext.Extension] managing the relay server on platforms
 // that import this package.
 // that import this package.
 type extension struct {
 type extension struct {
-	logf logger.Logf
-	bus  *eventbus.Bus
+	newServerFn func(logf logger.Logf, port int, overrideAddrs []netip.Addr) (relayServer, error) // swappable for tests
+	logf        logger.Logf
+	ec          *eventbus.Client
+	respPub     *eventbus.Publisher[magicsock.UDPRelayAllocResp]
 
 
-	mu       sync.Mutex // guards the following fields
-	shutdown bool
-
-	port                          *int                             // ipn.Prefs.RelayServerPort, nil if disabled
-	eventSubs                     *eventbus.Monitor                // nil if not connected to eventbus
-	debugSessionsCh               chan chan []status.ServerSession // non-nil if consumeEventbusTopics is running
-	hasNodeAttrDisableRelayServer bool                             // tailcfg.NodeAttrDisableRelayServer
+	mu                            sync.Mutex          // guards the following fields
+	shutdown                      bool                // true if Shutdown() has been called
+	rs                            relayServer         // nil when disabled
+	port                          *int                // ipn.Prefs.RelayServerPort, nil if disabled
+	derpMapView                   tailcfg.DERPMapView // latest seen over the eventbus
+	hasNodeAttrDisableRelayServer bool                // [tailcfg.NodeAttrDisableRelayServer]
 }
 }
 
 
 // Name implements [ipnext.Extension].
 // Name implements [ipnext.Extension].
@@ -104,26 +122,83 @@ func (e *extension) Init(host ipnext.Host) error {
 	return nil
 	return nil
 }
 }
 
 
-// handleBusLifetimeLocked handles the lifetime of consumeEventbusTopics.
-func (e *extension) handleBusLifetimeLocked() {
-	busShouldBeRunning := !e.shutdown && e.port != nil && !e.hasNodeAttrDisableRelayServer
-	if !busShouldBeRunning {
-		e.disconnectFromBusLocked()
+func (e *extension) onDERPMapView(view tailcfg.DERPMapView) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	e.derpMapView = view
+	if e.rs != nil {
+		e.rs.SetDERPMapView(view)
+	}
+}
+
+func (e *extension) onAllocReq(req magicsock.UDPRelayAllocReq) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	if e.shutdown {
+		return
+	}
+	if e.rs == nil {
+		if !e.relayServerShouldBeRunningLocked() {
+			return
+		}
+		e.tryStartRelayServerLocked()
+		if e.rs == nil {
+			return
+		}
+	}
+	se, err := e.rs.AllocateEndpoint(req.Message.ClientDisco[0], req.Message.ClientDisco[1])
+	if err != nil {
+		e.logf("error allocating endpoint: %v", err)
+		return
+	}
+	e.respPub.Publish(magicsock.UDPRelayAllocResp{
+		ReqRxFromNodeKey:  req.RxFromNodeKey,
+		ReqRxFromDiscoKey: req.RxFromDiscoKey,
+		Message: &disco.AllocateUDPRelayEndpointResponse{
+			Generation: req.Message.Generation,
+			UDPRelayEndpoint: disco.UDPRelayEndpoint{
+				ServerDisco:         se.ServerDisco,
+				ClientDisco:         se.ClientDisco,
+				LamportID:           se.LamportID,
+				VNI:                 se.VNI,
+				BindLifetime:        se.BindLifetime.Duration,
+				SteadyStateLifetime: se.SteadyStateLifetime.Duration,
+				AddrPorts:           se.AddrPorts,
+			},
+		},
+	})
+}
+
+func (e *extension) tryStartRelayServerLocked() {
+	rs, err := e.newServerFn(e.logf, *e.port, overrideAddrs())
+	if err != nil {
+		e.logf("error initializing server: %v", err)
 		return
 		return
-	} else if e.eventSubs != nil {
-		return // already running
 	}
 	}
+	e.rs = rs
+	e.rs.SetDERPMapView(e.derpMapView)
+}
 
 
-	ec := e.bus.Client("relayserver.extension")
-	e.debugSessionsCh = make(chan chan []status.ServerSession)
-	e.eventSubs = ptr.To(ec.Monitor(e.consumeEventbusTopics(ec, *e.port)))
+func (e *extension) relayServerShouldBeRunningLocked() bool {
+	return !e.shutdown && e.port != nil && !e.hasNodeAttrDisableRelayServer
+}
+
+// handleRelayServerLifetimeLocked handles the lifetime of [e.rs].
+func (e *extension) handleRelayServerLifetimeLocked() {
+	if !e.relayServerShouldBeRunningLocked() {
+		e.stopRelayServerLocked()
+		return
+	} else if e.rs != nil {
+		return // already running
+	}
+	e.tryStartRelayServerLocked()
 }
 }
 
 
 func (e *extension) selfNodeViewChanged(nodeView tailcfg.NodeView) {
 func (e *extension) selfNodeViewChanged(nodeView tailcfg.NodeView) {
 	e.mu.Lock()
 	e.mu.Lock()
 	defer e.mu.Unlock()
 	defer e.mu.Unlock()
 	e.hasNodeAttrDisableRelayServer = nodeView.HasCap(tailcfg.NodeAttrDisableRelayServer)
 	e.hasNodeAttrDisableRelayServer = nodeView.HasCap(tailcfg.NodeAttrDisableRelayServer)
-	e.handleBusLifetimeLocked()
+	e.handleRelayServerLifetimeLocked()
 }
 }
 
 
 func (e *extension) profileStateChanged(_ ipn.LoginProfileView, prefs ipn.PrefsView, sameNode bool) {
 func (e *extension) profileStateChanged(_ ipn.LoginProfileView, prefs ipn.PrefsView, sameNode bool) {
@@ -133,13 +208,13 @@ func (e *extension) profileStateChanged(_ ipn.LoginProfileView, prefs ipn.PrefsV
 	enableOrDisableServer := ok != (e.port != nil)
 	enableOrDisableServer := ok != (e.port != nil)
 	portChanged := ok && e.port != nil && newPort != *e.port
 	portChanged := ok && e.port != nil && newPort != *e.port
 	if enableOrDisableServer || portChanged || !sameNode {
 	if enableOrDisableServer || portChanged || !sameNode {
-		e.disconnectFromBusLocked()
+		e.stopRelayServerLocked()
 		e.port = nil
 		e.port = nil
 		if ok {
 		if ok {
 			e.port = ptr.To(newPort)
 			e.port = ptr.To(newPort)
 		}
 		}
 	}
 	}
-	e.handleBusLifetimeLocked()
+	e.handleRelayServerLifetimeLocked()
 }
 }
 
 
 // overrideAddrs returns TS_DEBUG_RELAY_SERVER_ADDRS as []netip.Addr, if set. It
 // overrideAddrs returns TS_DEBUG_RELAY_SERVER_ADDRS as []netip.Addr, if set. It
@@ -162,88 +237,20 @@ var overrideAddrs = sync.OnceValue(func() (ret []netip.Addr) {
 	return
 	return
 })
 })
 
 
-// consumeEventbusTopics serves endpoint allocation requests over the eventbus.
-// It also serves [relayServer] debug information on a channel.
-// consumeEventbusTopics must never acquire [extension.mu], which can be held
-// by other goroutines while waiting to receive on [extension.eventSubs] or the
-// inner [extension.debugSessionsCh] channel.
-func (e *extension) consumeEventbusTopics(ec *eventbus.Client, port int) func(*eventbus.Client) {
-	reqSub := eventbus.Subscribe[magicsock.UDPRelayAllocReq](ec)
-	respPub := eventbus.Publish[magicsock.UDPRelayAllocResp](ec)
-	debugSessionsCh := e.debugSessionsCh
-
-	return func(ec *eventbus.Client) {
-		rs, err := udprelay.NewServer(e.logf, port, overrideAddrs())
-		if err != nil {
-			e.logf("error initializing server: %v", err)
-		}
-
-		defer func() {
-			if rs != nil {
-				rs.Close()
-			}
-		}()
-		for {
-			select {
-			case <-ec.Done():
-				return
-			case respCh := <-debugSessionsCh:
-				if rs == nil {
-					respCh <- nil
-					continue
-				}
-				sessions := rs.GetSessions()
-				respCh <- sessions
-			case req := <-reqSub.Events():
-				if rs == nil {
-					// The server may have previously failed to initialize if
-					// the configured port was in use, try again.
-					rs, err = udprelay.NewServer(e.logf, port, overrideAddrs())
-					if err != nil {
-						e.logf("error initializing server: %v", err)
-						continue
-					}
-				}
-				se, err := rs.AllocateEndpoint(req.Message.ClientDisco[0], req.Message.ClientDisco[1])
-				if err != nil {
-					e.logf("error allocating endpoint: %v", err)
-					continue
-				}
-				respPub.Publish(magicsock.UDPRelayAllocResp{
-					ReqRxFromNodeKey:  req.RxFromNodeKey,
-					ReqRxFromDiscoKey: req.RxFromDiscoKey,
-					Message: &disco.AllocateUDPRelayEndpointResponse{
-						Generation: req.Message.Generation,
-						UDPRelayEndpoint: disco.UDPRelayEndpoint{
-							ServerDisco:         se.ServerDisco,
-							ClientDisco:         se.ClientDisco,
-							LamportID:           se.LamportID,
-							VNI:                 se.VNI,
-							BindLifetime:        se.BindLifetime.Duration,
-							SteadyStateLifetime: se.SteadyStateLifetime.Duration,
-							AddrPorts:           se.AddrPorts,
-						},
-					},
-				})
-			}
-		}
-	}
-}
-
-func (e *extension) disconnectFromBusLocked() {
-	if e.eventSubs != nil {
-		e.eventSubs.Close()
-		e.eventSubs = nil
-		e.debugSessionsCh = nil
+func (e *extension) stopRelayServerLocked() {
+	if e.rs != nil {
+		e.rs.Close()
 	}
 	}
+	e.rs = nil
 }
 }
 
 
 // Shutdown implements [ipnlocal.Extension].
 // Shutdown implements [ipnlocal.Extension].
 func (e *extension) Shutdown() error {
 func (e *extension) Shutdown() error {
 	e.mu.Lock()
 	e.mu.Lock()
 	defer e.mu.Unlock()
 	defer e.mu.Unlock()
-	e.disconnectFromBusLocked()
 	e.shutdown = true
 	e.shutdown = true
+	e.ec.Close()
+	e.stopRelayServerLocked()
 	return nil
 	return nil
 }
 }
 
 
@@ -253,23 +260,14 @@ func (e *extension) Shutdown() error {
 func (e *extension) serverStatus() status.ServerStatus {
 func (e *extension) serverStatus() status.ServerStatus {
 	e.mu.Lock()
 	e.mu.Lock()
 	defer e.mu.Unlock()
 	defer e.mu.Unlock()
-
 	st := status.ServerStatus{
 	st := status.ServerStatus{
 		UDPPort:  nil,
 		UDPPort:  nil,
 		Sessions: nil,
 		Sessions: nil,
 	}
 	}
-	if e.port == nil || e.eventSubs == nil {
+	if e.rs == nil {
 		return st
 		return st
 	}
 	}
 	st.UDPPort = ptr.To(*e.port)
 	st.UDPPort = ptr.To(*e.port)
-
-	ch := make(chan []status.ServerSession)
-	select {
-	case e.debugSessionsCh <- ch:
-		resp := <-ch
-		st.Sessions = resp
-		return st
-	case <-e.eventSubs.Done():
-		return st
-	}
+	st.Sessions = e.rs.GetSessions()
+	return st
 }
 }

+ 179 - 43
feature/relayserver/relayserver_test.go

@@ -4,13 +4,20 @@
 package relayserver
 package relayserver
 
 
 import (
 import (
+	"errors"
+	"net/netip"
+	"reflect"
 	"testing"
 	"testing"
 
 
 	"tailscale.com/ipn"
 	"tailscale.com/ipn"
+	"tailscale.com/net/udprelay/endpoint"
+	"tailscale.com/net/udprelay/status"
+	"tailscale.com/tailcfg"
 	"tailscale.com/tsd"
 	"tailscale.com/tsd"
+	"tailscale.com/tstime"
+	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/ptr"
 	"tailscale.com/types/ptr"
-	"tailscale.com/util/eventbus"
 )
 )
 
 
 func Test_extension_profileStateChanged(t *testing.T) {
 func Test_extension_profileStateChanged(t *testing.T) {
@@ -19,29 +26,33 @@ func Test_extension_profileStateChanged(t *testing.T) {
 
 
 	type fields struct {
 	type fields struct {
 		port *int
 		port *int
+		rs   relayServer
 	}
 	}
 	type args struct {
 	type args struct {
 		prefs    ipn.PrefsView
 		prefs    ipn.PrefsView
 		sameNode bool
 		sameNode bool
 	}
 	}
 	tests := []struct {
 	tests := []struct {
-		name           string
-		fields         fields
-		args           args
-		wantPort       *int
-		wantBusRunning bool
+		name                        string
+		fields                      fields
+		args                        args
+		wantPort                    *int
+		wantRelayServerFieldNonNil  bool
+		wantRelayServerFieldMutated bool
 	}{
 	}{
 		{
 		{
-			name: "no changes non-nil port",
+			name: "no changes non-nil port previously running",
 			fields: fields{
 			fields: fields{
 				port: ptr.To(1),
 				port: ptr.To(1),
+				rs:   mockRelayServerNotZeroVal(),
 			},
 			},
 			args: args{
 			args: args{
 				prefs:    prefsWithPortOne.View(),
 				prefs:    prefsWithPortOne.View(),
 				sameNode: true,
 				sameNode: true,
 			},
 			},
-			wantPort:       ptr.To(1),
-			wantBusRunning: true,
+			wantPort:                    ptr.To(1),
+			wantRelayServerFieldNonNil:  true,
+			wantRelayServerFieldMutated: false,
 		},
 		},
 		{
 		{
 			name: "prefs port nil",
 			name: "prefs port nil",
@@ -52,8 +63,23 @@ func Test_extension_profileStateChanged(t *testing.T) {
 				prefs:    prefsWithNilPort.View(),
 				prefs:    prefsWithNilPort.View(),
 				sameNode: true,
 				sameNode: true,
 			},
 			},
-			wantPort:       nil,
-			wantBusRunning: false,
+			wantPort:                    nil,
+			wantRelayServerFieldNonNil:  false,
+			wantRelayServerFieldMutated: false,
+		},
+		{
+			name: "prefs port nil previously running",
+			fields: fields{
+				port: ptr.To(1),
+				rs:   mockRelayServerNotZeroVal(),
+			},
+			args: args{
+				prefs:    prefsWithNilPort.View(),
+				sameNode: true,
+			},
+			wantPort:                    nil,
+			wantRelayServerFieldNonNil:  false,
+			wantRelayServerFieldMutated: true,
 		},
 		},
 		{
 		{
 			name: "prefs port changed",
 			name: "prefs port changed",
@@ -64,8 +90,23 @@ func Test_extension_profileStateChanged(t *testing.T) {
 				prefs:    prefsWithPortOne.View(),
 				prefs:    prefsWithPortOne.View(),
 				sameNode: true,
 				sameNode: true,
 			},
 			},
-			wantPort:       ptr.To(1),
-			wantBusRunning: true,
+			wantPort:                    ptr.To(1),
+			wantRelayServerFieldNonNil:  true,
+			wantRelayServerFieldMutated: true,
+		},
+		{
+			name: "prefs port changed previously running",
+			fields: fields{
+				port: ptr.To(2),
+				rs:   mockRelayServerNotZeroVal(),
+			},
+			args: args{
+				prefs:    prefsWithPortOne.View(),
+				sameNode: true,
+			},
+			wantPort:                    ptr.To(1),
+			wantRelayServerFieldNonNil:  true,
+			wantRelayServerFieldMutated: true,
 		},
 		},
 		{
 		{
 			name: "sameNode false",
 			name: "sameNode false",
@@ -76,8 +117,23 @@ func Test_extension_profileStateChanged(t *testing.T) {
 				prefs:    prefsWithPortOne.View(),
 				prefs:    prefsWithPortOne.View(),
 				sameNode: false,
 				sameNode: false,
 			},
 			},
-			wantPort:       ptr.To(1),
-			wantBusRunning: true,
+			wantPort:                    ptr.To(1),
+			wantRelayServerFieldNonNil:  true,
+			wantRelayServerFieldMutated: true,
+		},
+		{
+			name: "sameNode false previously running",
+			fields: fields{
+				port: ptr.To(1),
+				rs:   mockRelayServerNotZeroVal(),
+			},
+			args: args{
+				prefs:    prefsWithPortOne.View(),
+				sameNode: false,
+			},
+			wantPort:                    ptr.To(1),
+			wantRelayServerFieldNonNil:  true,
+			wantRelayServerFieldMutated: true,
 		},
 		},
 		{
 		{
 			name: "prefs port non-nil extension port nil",
 			name: "prefs port non-nil extension port nil",
@@ -88,85 +144,165 @@ func Test_extension_profileStateChanged(t *testing.T) {
 				prefs:    prefsWithPortOne.View(),
 				prefs:    prefsWithPortOne.View(),
 				sameNode: false,
 				sameNode: false,
 			},
 			},
-			wantPort:       ptr.To(1),
-			wantBusRunning: true,
+			wantPort:                    ptr.To(1),
+			wantRelayServerFieldNonNil:  true,
+			wantRelayServerFieldMutated: true,
 		},
 		},
 	}
 	}
 	for _, tt := range tests {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			sys := tsd.NewSystem()
 			sys := tsd.NewSystem()
-			bus := sys.Bus.Get()
-			e := &extension{
-				logf: logger.Discard,
-				port: tt.fields.port,
-				bus:  bus,
+			ipne, err := newExtension(logger.Discard, mockSafeBackend{sys})
+			if err != nil {
+				t.Fatal(err)
+			}
+			e := ipne.(*extension)
+			e.newServerFn = func(logf logger.Logf, port int, overrideAddrs []netip.Addr) (relayServer, error) {
+				return &mockRelayServer{}, nil
 			}
 			}
-			defer e.disconnectFromBusLocked()
+			e.port = tt.fields.port
+			e.rs = tt.fields.rs
+			defer e.Shutdown()
 			e.profileStateChanged(ipn.LoginProfileView{}, tt.args.prefs, tt.args.sameNode)
 			e.profileStateChanged(ipn.LoginProfileView{}, tt.args.prefs, tt.args.sameNode)
-			if tt.wantBusRunning != (e.eventSubs != nil) {
-				t.Errorf("wantBusRunning: %v != (e.eventSubs != nil): %v", tt.wantBusRunning, e.eventSubs != nil)
+			if tt.wantRelayServerFieldNonNil != (e.rs != nil) {
+				t.Errorf("wantRelayServerFieldNonNil: %v != (e.rs != nil): %v", tt.wantRelayServerFieldNonNil, e.rs != nil)
 			}
 			}
 			if (tt.wantPort == nil) != (e.port == nil) {
 			if (tt.wantPort == nil) != (e.port == nil) {
 				t.Errorf("(tt.wantPort == nil): %v != (e.port == nil): %v", tt.wantPort == nil, e.port == nil)
 				t.Errorf("(tt.wantPort == nil): %v != (e.port == nil): %v", tt.wantPort == nil, e.port == nil)
 			} else if tt.wantPort != nil && *tt.wantPort != *e.port {
 			} else if tt.wantPort != nil && *tt.wantPort != *e.port {
 				t.Errorf("wantPort: %d != *e.port: %d", *tt.wantPort, *e.port)
 				t.Errorf("wantPort: %d != *e.port: %d", *tt.wantPort, *e.port)
 			}
 			}
+			if tt.wantRelayServerFieldMutated != !reflect.DeepEqual(tt.fields.rs, e.rs) {
+				t.Errorf("wantRelayServerFieldMutated: %v != !reflect.DeepEqual(tt.fields.rs, e.rs): %v", tt.wantRelayServerFieldMutated, !reflect.DeepEqual(tt.fields.rs, e.rs))
+			}
 		})
 		})
 	}
 	}
 }
 }
 
 
-func Test_extension_handleBusLifetimeLocked(t *testing.T) {
+func mockRelayServerNotZeroVal() *mockRelayServer {
+	return &mockRelayServer{true}
+}
+
+type mockRelayServer struct {
+	set bool
+}
+
+func (mockRelayServer) Close() error { return nil }
+func (mockRelayServer) AllocateEndpoint(_, _ key.DiscoPublic) (endpoint.ServerEndpoint, error) {
+	return endpoint.ServerEndpoint{}, errors.New("not implemented")
+}
+func (mockRelayServer) GetSessions() []status.ServerSession { return nil }
+func (mockRelayServer) SetDERPMapView(tailcfg.DERPMapView)  { return }
+
+type mockSafeBackend struct {
+	sys *tsd.System
+}
+
+func (m mockSafeBackend) Sys() *tsd.System       { return m.sys }
+func (mockSafeBackend) Clock() tstime.Clock      { return nil }
+func (mockSafeBackend) TailscaleVarRoot() string { return "" }
+
+func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) {
 	tests := []struct {
 	tests := []struct {
 		name                          string
 		name                          string
 		shutdown                      bool
 		shutdown                      bool
 		port                          *int
 		port                          *int
-		eventSubs                     *eventbus.Monitor
+		rs                            relayServer
 		hasNodeAttrDisableRelayServer bool
 		hasNodeAttrDisableRelayServer bool
-		wantBusRunning                bool
+		wantRelayServerFieldNonNil    bool
+		wantRelayServerFieldMutated   bool
 	}{
 	}{
 		{
 		{
 			name:                          "want running",
 			name:                          "want running",
 			shutdown:                      false,
 			shutdown:                      false,
 			port:                          ptr.To(1),
 			port:                          ptr.To(1),
 			hasNodeAttrDisableRelayServer: false,
 			hasNodeAttrDisableRelayServer: false,
-			wantBusRunning:                true,
+			wantRelayServerFieldNonNil:    true,
+			wantRelayServerFieldMutated:   true,
+		},
+		{
+			name:                          "want running previously running",
+			shutdown:                      false,
+			port:                          ptr.To(1),
+			rs:                            mockRelayServerNotZeroVal(),
+			hasNodeAttrDisableRelayServer: false,
+			wantRelayServerFieldNonNil:    true,
+			wantRelayServerFieldMutated:   false,
 		},
 		},
 		{
 		{
 			name:                          "shutdown true",
 			name:                          "shutdown true",
 			shutdown:                      true,
 			shutdown:                      true,
 			port:                          ptr.To(1),
 			port:                          ptr.To(1),
 			hasNodeAttrDisableRelayServer: false,
 			hasNodeAttrDisableRelayServer: false,
-			wantBusRunning:                false,
+			wantRelayServerFieldNonNil:    false,
+			wantRelayServerFieldMutated:   false,
+		},
+		{
+			name:                          "shutdown true previously running",
+			shutdown:                      true,
+			port:                          ptr.To(1),
+			rs:                            mockRelayServerNotZeroVal(),
+			hasNodeAttrDisableRelayServer: false,
+			wantRelayServerFieldNonNil:    false,
+			wantRelayServerFieldMutated:   true,
 		},
 		},
 		{
 		{
 			name:                          "port nil",
 			name:                          "port nil",
 			shutdown:                      false,
 			shutdown:                      false,
 			port:                          nil,
 			port:                          nil,
 			hasNodeAttrDisableRelayServer: false,
 			hasNodeAttrDisableRelayServer: false,
-			wantBusRunning:                false,
+			wantRelayServerFieldNonNil:    false,
+			wantRelayServerFieldMutated:   false,
+		},
+		{
+			name:                          "port nil previously running",
+			shutdown:                      false,
+			port:                          nil,
+			rs:                            mockRelayServerNotZeroVal(),
+			hasNodeAttrDisableRelayServer: false,
+			wantRelayServerFieldNonNil:    false,
+			wantRelayServerFieldMutated:   true,
 		},
 		},
 		{
 		{
 			name:                          "hasNodeAttrDisableRelayServer true",
 			name:                          "hasNodeAttrDisableRelayServer true",
 			shutdown:                      false,
 			shutdown:                      false,
 			port:                          nil,
 			port:                          nil,
 			hasNodeAttrDisableRelayServer: true,
 			hasNodeAttrDisableRelayServer: true,
-			wantBusRunning:                false,
+			wantRelayServerFieldNonNil:    false,
+			wantRelayServerFieldMutated:   false,
+		},
+		{
+			name:                          "hasNodeAttrDisableRelayServer true previously running",
+			shutdown:                      false,
+			port:                          nil,
+			rs:                            mockRelayServerNotZeroVal(),
+			hasNodeAttrDisableRelayServer: true,
+			wantRelayServerFieldNonNil:    false,
+			wantRelayServerFieldMutated:   true,
 		},
 		},
 	}
 	}
 	for _, tt := range tests {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
-			e := &extension{
-				logf:                          logger.Discard,
-				bus:                           eventbus.New(),
-				shutdown:                      tt.shutdown,
-				port:                          tt.port,
-				eventSubs:                     tt.eventSubs,
-				hasNodeAttrDisableRelayServer: tt.hasNodeAttrDisableRelayServer,
+			sys := tsd.NewSystem()
+			ipne, err := newExtension(logger.Discard, mockSafeBackend{sys})
+			if err != nil {
+				t.Fatal(err)
+			}
+			e := ipne.(*extension)
+			e.newServerFn = func(logf logger.Logf, port int, overrideAddrs []netip.Addr) (relayServer, error) {
+				return &mockRelayServer{}, nil
+			}
+			e.shutdown = tt.shutdown
+			e.port = tt.port
+			e.rs = tt.rs
+			e.hasNodeAttrDisableRelayServer = tt.hasNodeAttrDisableRelayServer
+			e.handleRelayServerLifetimeLocked()
+			defer e.Shutdown()
+			if tt.wantRelayServerFieldNonNil != (e.rs != nil) {
+				t.Errorf("wantRelayServerFieldNonNil: %v != (e.rs != nil): %v", tt.wantRelayServerFieldNonNil, e.rs != nil)
 			}
 			}
-			e.handleBusLifetimeLocked()
-			defer e.disconnectFromBusLocked()
-			if tt.wantBusRunning != (e.eventSubs != nil) {
-				t.Errorf("wantBusRunning: %v != (e.eventSubs != nil): %v", tt.wantBusRunning, e.eventSubs != nil)
+			if tt.wantRelayServerFieldMutated != !reflect.DeepEqual(tt.rs, e.rs) {
+				t.Errorf("wantRelayServerFieldMutated: %v != !reflect.DeepEqual(tt.rs, e.rs): %v", tt.wantRelayServerFieldMutated, !reflect.DeepEqual(tt.rs, e.rs))
 			}
 			}
 		})
 		})
 	}
 	}

+ 9 - 4
ipn/ipnlocal/node_backend.go

@@ -75,10 +75,11 @@ type nodeBackend struct {
 	filterAtomic atomic.Pointer[filter.Filter]
 	filterAtomic atomic.Pointer[filter.Filter]
 
 
 	// initialized once and immutable
 	// initialized once and immutable
-	eventClient  *eventbus.Client
-	filterPub    *eventbus.Publisher[magicsock.FilterUpdate]
-	nodeViewsPub *eventbus.Publisher[magicsock.NodeViewsUpdate]
-	nodeMutsPub  *eventbus.Publisher[magicsock.NodeMutationsUpdate]
+	eventClient    *eventbus.Client
+	filterPub      *eventbus.Publisher[magicsock.FilterUpdate]
+	nodeViewsPub   *eventbus.Publisher[magicsock.NodeViewsUpdate]
+	nodeMutsPub    *eventbus.Publisher[magicsock.NodeMutationsUpdate]
+	derpMapViewPub *eventbus.Publisher[tailcfg.DERPMapView]
 
 
 	// TODO(nickkhyl): maybe use sync.RWMutex?
 	// TODO(nickkhyl): maybe use sync.RWMutex?
 	mu sync.Mutex // protects the following fields
 	mu sync.Mutex // protects the following fields
@@ -121,6 +122,7 @@ func newNodeBackend(ctx context.Context, logf logger.Logf, bus *eventbus.Bus) *n
 	nb.filterPub = eventbus.Publish[magicsock.FilterUpdate](nb.eventClient)
 	nb.filterPub = eventbus.Publish[magicsock.FilterUpdate](nb.eventClient)
 	nb.nodeViewsPub = eventbus.Publish[magicsock.NodeViewsUpdate](nb.eventClient)
 	nb.nodeViewsPub = eventbus.Publish[magicsock.NodeViewsUpdate](nb.eventClient)
 	nb.nodeMutsPub = eventbus.Publish[magicsock.NodeMutationsUpdate](nb.eventClient)
 	nb.nodeMutsPub = eventbus.Publish[magicsock.NodeMutationsUpdate](nb.eventClient)
+	nb.derpMapViewPub = eventbus.Publish[tailcfg.DERPMapView](nb.eventClient)
 	nb.filterPub.Publish(magicsock.FilterUpdate{Filter: nb.filterAtomic.Load()})
 	nb.filterPub.Publish(magicsock.FilterUpdate{Filter: nb.filterAtomic.Load()})
 	return nb
 	return nb
 }
 }
@@ -435,6 +437,9 @@ func (nb *nodeBackend) SetNetMap(nm *netmap.NetworkMap) {
 	if nm != nil {
 	if nm != nil {
 		nv.SelfNode = nm.SelfNode
 		nv.SelfNode = nm.SelfNode
 		nv.Peers = nm.Peers
 		nv.Peers = nm.Peers
+		nb.derpMapViewPub.Publish(nm.DERPMap.View())
+	} else {
+		nb.derpMapViewPub.Publish(tailcfg.DERPMapView{})
 	}
 	}
 	nb.nodeViewsPub.Publish(nv)
 	nb.nodeViewsPub.Publish(nv)
 }
 }

+ 27 - 11
net/udprelay/server.go

@@ -21,7 +21,6 @@ import (
 
 
 	"go4.org/mem"
 	"go4.org/mem"
 	"golang.org/x/net/ipv6"
 	"golang.org/x/net/ipv6"
-	"tailscale.com/client/local"
 	"tailscale.com/disco"
 	"tailscale.com/disco"
 	"tailscale.com/net/batching"
 	"tailscale.com/net/batching"
 	"tailscale.com/net/netaddr"
 	"tailscale.com/net/netaddr"
@@ -32,6 +31,7 @@ import (
 	"tailscale.com/net/stun"
 	"tailscale.com/net/stun"
 	"tailscale.com/net/udprelay/endpoint"
 	"tailscale.com/net/udprelay/endpoint"
 	"tailscale.com/net/udprelay/status"
 	"tailscale.com/net/udprelay/status"
+	"tailscale.com/tailcfg"
 	"tailscale.com/tstime"
 	"tailscale.com/tstime"
 	"tailscale.com/types/key"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logger"
@@ -72,7 +72,8 @@ type Server struct {
 	closeCh             chan struct{}
 	closeCh             chan struct{}
 	netChecker          *netcheck.Client
 	netChecker          *netcheck.Client
 
 
-	mu                sync.Mutex       // guards the following fields
+	mu                sync.Mutex // guards the following fields
+	derpMap           *tailcfg.DERPMap
 	addrDiscoveryOnce bool             // addrDiscovery completed once (successfully or unsuccessfully)
 	addrDiscoveryOnce bool             // addrDiscovery completed once (successfully or unsuccessfully)
 	addrPorts         []netip.AddrPort // the ip:port pairs returned as candidate endpoints
 	addrPorts         []netip.AddrPort // the ip:port pairs returned as candidate endpoints
 	closed            bool
 	closed            bool
@@ -374,15 +375,12 @@ func (s *Server) addrDiscoveryLoop() {
 			}
 			}
 		}
 		}
 
 
-		// fetch DERPMap to feed to netcheck
-		derpMapCtx, derpMapCancel := context.WithTimeout(context.Background(), time.Second)
-		defer derpMapCancel()
-		localClient := &local.Client{}
-		// TODO(jwhited): We are in-process so use eventbus or similar.
-		//  local.Client gets us going.
-		dm, err := localClient.CurrentDERPMap(derpMapCtx)
-		if err != nil {
-			return nil, err
+		dm := s.getDERPMap()
+		if dm == nil {
+			// We don't have a DERPMap which is required to dynamically
+			// discover external addresses, but we can return the endpoints we
+			// do have.
+			return addrPorts.Slice(), nil
 		}
 		}
 
 
 		// get addrPorts as visible from DERP
 		// get addrPorts as visible from DERP
@@ -864,3 +862,21 @@ func (s *Server) GetSessions() []status.ServerSession {
 	}
 	}
 	return sessions
 	return sessions
 }
 }
+
+// SetDERPMapView sets the [tailcfg.DERPMapView] to use for future netcheck
+// reports.
+func (s *Server) SetDERPMapView(view tailcfg.DERPMapView) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if !view.Valid() {
+		s.derpMap = nil
+		return
+	}
+	s.derpMap = view.AsStruct()
+}
+
+func (s *Server) getDERPMap() *tailcfg.DERPMap {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.derpMap
+}