tsnet: reset serve config only once

Prior to this change, we were resetting the tsnet's serve config every
time tsnet.Server.Up was run. This is important to do on startup, to
prevent messy interactions with stale configuration when the code has
changed.

However, Up is frequently run as a just-in-case step (for example, by
Server.ListenTLS/ListenFunnel and possibly by consumers of tsnet). When
the serve config is reset on each of these calls to Up, this creates
situations in which the serve config disappears unexpectedly. The
solution is to reset the serve config only on the first call to Up.

Fixes #8800
Updates tailscale/corp#27200
Signed-off-by: Harry Harpham <harry@tailscale.com>

tsnet/tsnet.go

@@ -160,25 +160,26 @@ type Server struct {
 
 	getCertForTesting func(*tls.ClientHelloInfo) (*tls.Certificate, error)
 
-	initOnce         sync.Once
-	initErr          error
-	lb               *ipnlocal.LocalBackend
-	sys              *tsd.System
-	netstack         *netstack.Impl
-	netMon           *netmon.Monitor
-	rootPath         string // the state directory
-	hostname         string
-	shutdownCtx      context.Context
-	shutdownCancel   context.CancelFunc
-	proxyCred        string        // SOCKS5 proxy auth for loopbackListener
-	localAPICred     string        // basic auth password for loopbackListener
-	loopbackListener net.Listener  // optional loopback for localapi and proxies
-	localAPIListener net.Listener  // in-memory, used by localClient
-	localClient      *local.Client // in-memory
-	localAPIServer   *http.Server
-	logbuffer        *filch.Filch
-	logtail          *logtail.Logger
-	logid            logid.PublicID
+	initOnce             sync.Once
+	initErr              error
+	lb                   *ipnlocal.LocalBackend
+	sys                  *tsd.System
+	netstack             *netstack.Impl
+	netMon               *netmon.Monitor
+	rootPath             string // the state directory
+	hostname             string
+	shutdownCtx          context.Context
+	shutdownCancel       context.CancelFunc
+	proxyCred            string        // SOCKS5 proxy auth for loopbackListener
+	localAPICred         string        // basic auth password for loopbackListener
+	loopbackListener     net.Listener  // optional loopback for localapi and proxies
+	localAPIListener     net.Listener  // in-memory, used by localClient
+	localClient          *local.Client // in-memory
+	localAPIServer       *http.Server
+	resetServeConfigOnce sync.Once
+	logbuffer            *filch.Filch
+	logtail              *logtail.Logger
+	logid                logid.PublicID
 
 	mu                  sync.Mutex
 	listeners           map[listenKey]*listener
@@ -388,8 +389,8 @@ func (s *Server) Up(ctx context.Context) (*ipnstate.Status, error) {
 		if n.ErrMessage != nil {
 			return nil, fmt.Errorf("tsnet.Up: backend: %s", *n.ErrMessage)
 		}
-		if s := n.State; s != nil {
-			if *s == ipn.Running {
+		if st := n.State; st != nil {
+			if *st == ipn.Running {
 				status, err := lc.Status(ctx)
 				if err != nil {
 					return nil, fmt.Errorf("tsnet.Up: %w", err)
@@ -398,11 +399,15 @@ func (s *Server) Up(ctx context.Context) (*ipnstate.Status, error) {
 					return nil, errors.New("tsnet.Up: running, but no ip")
 				}
 
-				// Clear the persisted serve config state to prevent stale configuration
-				// from code changes. This is a temporary workaround until we have a better
-				// way to handle this. (2023-03-11)
-				if err := lc.SetServeConfig(ctx, new(ipn.ServeConfig)); err != nil {
-					return nil, fmt.Errorf("tsnet.Up: %w", err)
+				// The first time Up is run, clear the persisted serve config.
+				// We do this to prevent messy interactions with stale config in
+				// the face of code changes.
+				var srvResetErr error
+				s.resetServeConfigOnce.Do(func() {
+					srvResetErr = lc.SetServeConfig(ctx, new(ipn.ServeConfig))
+				})
+				if srvResetErr != nil {
+					return nil, fmt.Errorf("tsnet.Up: clearing serve config: %w", err)
 				}
 
 				return status, nil