From 824985afe1767c49a1337974cfeceaa07ad8fdbd Mon Sep 17 00:00:00 2001 From: Nick Khyl Date: Wed, 14 May 2025 11:57:01 -0500 Subject: [PATCH] feature/taildrop,ipn/ipn{ext,local}: initialize taildrop for initial profile Currently, LocalBackend/ExtensionHost doesn't invoke the profile change callback for the initial profile. Since the initial profile may vary depending on loaded extensions and applied policy settings, it can't be reliably determined until all extensions are initialized. Additionally, some extensions may asynchronously trigger a switch to the "best" profile (based on system state and policy settings) during initialization. We intended to address these issues as part of the ongoing profileManager/LocalBackend refactoring, but the changes didn't land in time for the v1.84 release and the Taildrop refactoring. In this PR, we update the Taildrop extension to retrieve the current profile at initialization time and handle it as a profile change. We also defer extension initialization until LocalBackend has started, since the Taildrop extension already relies on this behavior (e.g., it requires clients to call SetDirectFileRoot before Init). Fixes #15970 Updates #15812 Updates tailscale/corp#28449 Signed-off-by: Nick Khyl --- feature/taildrop/ext.go | 4 ++++ feature/taildrop/integration_test.go | 1 - ipn/ipnext/ipnext.go | 6 +++++- ipn/ipnlocal/local.go | 11 +++++++++-- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/feature/taildrop/ext.go b/feature/taildrop/ext.go index aee825ee7..ed26996fe 100644 --- a/feature/taildrop/ext.go +++ b/feature/taildrop/ext.go @@ -100,6 +100,10 @@ func (e *Extension) Init(h ipnext.Host) error { h.Hooks().SetPeerStatus.Add(e.setPeerStatus) h.Hooks().BackendStateChange.Add(e.onBackendStateChange) + // TODO(nickkhyl): remove this after the profileManager refactoring. + // See tailscale/tailscale#15974. + profile, prefs := h.Profiles().CurrentProfileState() + e.onChangeProfile(profile, prefs, false) return nil } diff --git a/feature/taildrop/integration_test.go b/feature/taildrop/integration_test.go index 6e60b7cba..75896a95b 100644 --- a/feature/taildrop/integration_test.go +++ b/feature/taildrop/integration_test.go @@ -26,7 +26,6 @@ import ( // TODO(bradfitz): add test between different users with the peercap to permit that? func TestTaildropIntegration(t *testing.T) { - t.Skip("known failing test; see https://github.com/tailscale/tailscale/issues/15970") testTaildropIntegration(t, false) } diff --git a/ipn/ipnext/ipnext.go b/ipn/ipnext/ipnext.go index 895fadc1c..7a9c39dbb 100644 --- a/ipn/ipnext/ipnext.go +++ b/ipn/ipnext/ipnext.go @@ -37,7 +37,10 @@ type Extension interface { // It must be the same as the name used to register the extension. Name() string - // Init is called to initialize the extension when LocalBackend is initialized. + // Init is called to initialize the extension when LocalBackend's + // Start method is called. Extensions are created but not initialized + // unless LocalBackend is started. + // // If the extension cannot be initialized, it must return an error, // and its Shutdown method will not be called on the host's shutdown. // Returned errors are not fatal; they are used for logging. @@ -333,6 +336,7 @@ type Hooks struct { // BackendStateChange is called when the backend state changes. BackendStateChange feature.Hooks[func(ipn.State)] + // ProfileStateChange contains callbacks that are invoked when the current login profile // or its [ipn.Prefs] change, after those changes have been made. The current login profile // may be changed either because of a profile switch, or because the profile information // was updated by [LocalBackend.SetControlClientStatus], including when the profile diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 79383aa37..468fd72eb 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -237,6 +237,8 @@ type LocalBackend struct { // for testing and graceful shutdown purposes. goTracker goroutines.Tracker + startOnce sync.Once // protects the one‑time initialization in [LocalBackend.Start] + // extHost is the bridge between [LocalBackend] and the registered [ipnext.Extension]s. // It may be nil in tests that use direct composite literal initialization of [LocalBackend] // instead of calling [NewLocalBackend]. A nil pointer is a valid, no-op host. @@ -568,8 +570,6 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo } } } - - b.extHost.Init() return b, nil } @@ -2162,6 +2162,11 @@ func (b *LocalBackend) getNewControlClientFuncLocked() clientGen { return b.ccGen } +// initOnce is called on the first call to [LocalBackend.Start]. +func (b *LocalBackend) initOnce() { + b.extHost.Init() +} + // Start applies the configuration specified in opts, and starts the // state machine. // @@ -2175,6 +2180,8 @@ func (b *LocalBackend) getNewControlClientFuncLocked() clientGen { func (b *LocalBackend) Start(opts ipn.Options) error { b.logf("Start") + b.startOnce.Do(b.initOnce) + var clientToShutdown controlclient.Client defer func() { if clientToShutdown != nil {