From b370274b29529b308c55de108b0c6156586fc632 Mon Sep 17 00:00:00 2001 From: Sonia Appasamy Date: Thu, 2 Nov 2023 12:55:01 -0400 Subject: [PATCH] ipn/ipnlocal: pull CapabilityPreviewWebClient into webClientAtomicBool Now uses webClientAtomicBool as the source of truth for whether the web client should be running in tailscaled, with it updated when either the RunWebClient pref or CapabilityPreviewWebClient node capability changes. This avoids requiring holding the LocalBackend lock on each call to ShouldRunWebClient to check for the CapabilityPreviewWebClient value. Updates tailscale/corp#14335 Signed-off-by: Sonia Appasamy --- ipn/ipnlocal/local.go | 18 +++++++++++------- ipn/ipnlocal/web_client.go | 3 +-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 886f6ead0..4102c1e5f 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -170,7 +170,7 @@ type LocalBackend struct { logFlushFunc func() // or nil if SetLogFlusher wasn't called em *expiryManager // non-nil sshAtomicBool atomic.Bool - webclientAtomicBool atomic.Bool + webClientAtomicBool atomic.Bool shutdownCalled bool // if Shutdown has been called debugSink *capture.Sink sockstatLogger *sockstatlog.Logger @@ -1113,6 +1113,7 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control // Perform all reconfiguration based on the netmap here. if st.NetMap != nil { b.capTailnetLock = hasCapability(st.NetMap, tailcfg.CapabilityTailnetLock) + b.setWebClientAtomicBoolLocked(st.NetMap, prefs.View()) b.mu.Unlock() // respect locking rules for tkaSyncIfNeeded if err := b.tkaSyncIfNeeded(st.NetMap, prefs.View()); err != nil { @@ -2504,7 +2505,7 @@ func (b *LocalBackend) setTCPPortsIntercepted(ports []uint16) { // and shouldInterceptTCPPortAtomic from the prefs p, which may be !Valid(). func (b *LocalBackend) setAtomicValuesFromPrefsLocked(p ipn.PrefsView) { b.sshAtomicBool.Store(p.Valid() && p.RunSSH() && envknob.CanSSHD()) - b.webclientAtomicBool.Store(p.Valid() && p.RunWebClient()) + b.setWebClientAtomicBoolLocked(b.netMap, p) if !p.Valid() { b.containsViaIPFuncAtomic.Store(tsaddr.FalseContainsIPFunc()) @@ -3018,9 +3019,6 @@ func (b *LocalBackend) setPrefsLockedOnEntry(caller string, newp *ipn.Prefs) ipn b.sshServer = nil } } - if oldp.ShouldWebClientBeRunning() && !newp.ShouldWebClientBeRunning() { - go b.WebClientShutdown() - } if netMap != nil { newProfile := netMap.UserProfiles[netMap.User()] if newLoginName := newProfile.LoginName; newLoginName != "" { @@ -4212,8 +4210,14 @@ func (b *LocalBackend) ResetForClientDisconnect() { func (b *LocalBackend) ShouldRunSSH() bool { return b.sshAtomicBool.Load() && envknob.CanSSHD() } -func (b *LocalBackend) ShouldRunWebClient() bool { - return b.webclientAtomicBool.Load() && hasCapability(b.netMap, tailcfg.CapabilityPreviewWebClient) +func (b *LocalBackend) ShouldRunWebClient() bool { return b.webClientAtomicBool.Load() } + +func (b *LocalBackend) setWebClientAtomicBoolLocked(nm *netmap.NetworkMap, prefs ipn.PrefsView) { + shouldRun := prefs.Valid() && prefs.RunWebClient() && hasCapability(nm, tailcfg.CapabilityPreviewWebClient) + wasRunning := b.webClientAtomicBool.Swap(shouldRun) + if wasRunning && !shouldRun { + go b.WebClientShutdown() // stop web client + } } // ShouldHandleViaIP reports whether ip is an IPv6 address in the diff --git a/ipn/ipnlocal/web_client.go b/ipn/ipnlocal/web_client.go index 738666257..bf48648fb 100644 --- a/ipn/ipnlocal/web_client.go +++ b/ipn/ipnlocal/web_client.go @@ -14,7 +14,6 @@ import ( "tailscale.com/client/tailscale" "tailscale.com/client/web" "tailscale.com/net/netutil" - "tailscale.com/tailcfg" ) // webClient holds state for the web interface for managing @@ -41,7 +40,7 @@ func (b *LocalBackend) SetWebLocalClient(lc *tailscale.LocalClient) { // tailscaled instance. // If the web interface is already running, WebClientInit is a no-op. func (b *LocalBackend) WebClientInit() (err error) { - if !hasCapability(b.netMap, tailcfg.CapabilityPreviewWebClient) { + if !b.ShouldRunWebClient() { return errors.New("web client not enabled for this device") }