diff --git a/feature/feature.go b/feature/feature.go index 6415cfc4a..6c8cd7eae 100644 --- a/feature/feature.go +++ b/feature/feature.go @@ -52,3 +52,21 @@ func (h *Hook[Func]) Get() Func { } return h.f } + +// Hooks is a slice of funcs. +// +// As opposed to a single Hook, this is meant to be used when +// multiple parties are able to install the same hook. +type Hooks[Func any] []Func + +// Add adds a hook to the list of hooks. +// +// Add should only be called during early program +// startup before Tailscale has started. +// It is not safe for concurrent use. +func (h *Hooks[Func]) Add(f Func) { + if reflect.ValueOf(f).IsZero() { + panic("Add with zero value") + } + *h = append(*h, f) +} diff --git a/feature/relayserver/relayserver.go b/feature/relayserver/relayserver.go index f73689245..e5c2afc17 100644 --- a/feature/relayserver/relayserver.go +++ b/feature/relayserver/relayserver.go @@ -71,7 +71,7 @@ func (e *extension) Name() string { func (e *extension) Init(host ipnext.Host) error { profile, prefs := host.Profiles().CurrentProfileState() e.profileStateChanged(profile, prefs, false) - host.Profiles().RegisterProfileStateChangeCallback(e.profileStateChanged) + host.Hooks().ProfileStateChange.Add(e.profileStateChanged) // TODO(jwhited): callback for netmap/nodeattr changes (e.hasNodeAttrRelayServer) return nil } diff --git a/ipn/auditlog/extension.go b/ipn/auditlog/extension.go index 509ab61a8..f73681db0 100644 --- a/ipn/auditlog/extension.go +++ b/ipn/auditlog/extension.go @@ -63,9 +63,9 @@ func (e *extension) Name() string { // Init implements [ipnext.Extension] by registering callbacks and providers // for the duration of the extension's lifetime. func (e *extension) Init(h ipnext.Host) error { - h.RegisterControlClientCallback(e.controlClientChanged) - h.Profiles().RegisterProfileStateChangeCallback(e.profileChanged) - h.RegisterAuditLogProvider(e.getCurrentLogger) + h.Hooks().NewControlClient.Add(e.controlClientChanged) + h.Hooks().ProfileStateChange.Add(e.profileChanged) + h.Hooks().AuditLoggers.Add(e.getCurrentLogger) return nil } diff --git a/ipn/desktop/extension.go b/ipn/desktop/extension.go index 6c59b1e5a..f204a90de 100644 --- a/ipn/desktop/extension.go +++ b/ipn/desktop/extension.go @@ -77,7 +77,7 @@ func (e *desktopSessionsExt) Init(host ipnext.Host) (err error) { if err != nil { return fmt.Errorf("session callback registration failed: %w", err) } - host.Profiles().RegisterBackgroundProfileResolver(e.getBackgroundProfile) + host.Hooks().BackgroundProfileResolvers.Add(e.getBackgroundProfile) e.cleanup = []func(){unregisterSessionCb} return nil } diff --git a/ipn/ipnext/ipnext.go b/ipn/ipnext/ipnext.go index b926ee23a..a671874d1 100644 --- a/ipn/ipnext/ipnext.go +++ b/ipn/ipnext/ipnext.go @@ -10,6 +10,7 @@ import ( "fmt" "tailscale.com/control/controlclient" + "tailscale.com/feature" "tailscale.com/ipn" "tailscale.com/ipn/ipnauth" "tailscale.com/tsd" @@ -182,14 +183,6 @@ type Host interface { // Profiles returns the host's [ProfileServices]. Profiles() ProfileServices - // RegisterAuditLogProvider registers an audit log provider, - // which returns a function to be called when an auditable action - // is about to be performed. - // - // It is a runtime error to register a nil provider or call after the host - // has been initialized. - RegisterAuditLogProvider(AuditLogProvider) - // AuditLogger returns a function that calls all currently registered audit loggers. // The function fails if any logger returns an error, indicating that the action // cannot be logged and must not be performed. @@ -198,12 +191,9 @@ type Host interface { // the time of the call and must not be persisted. AuditLogger() ipnauth.AuditLogFunc - // RegisterControlClientCallback registers a function to be called every time a new - // control client is created. - // - // It is a runtime error to register a nil provider or call after the host - // has been initialized. - RegisterControlClientCallback(NewControlClientCallback) + // Hooks returns a non-nil pointer to a [Hooks] struct. + // Hooks must not be modified concurrently or after Tailscale has started. + Hooks() *Hooks // SendNotifyAsync sends a notification to the IPN bus, // typically to the GUI client. @@ -269,28 +259,6 @@ type ProfileServices interface { // to a client connecting or disconnecting or a change in the desktop // session state. It is used for logging. SwitchToBestProfileAsync(reason string) - - // RegisterBackgroundProfileResolver registers a function to be used when - // resolving the background profile. - // - // It is a runtime error to register a nil provider or call after the host - // has been initialized. - // - // TODO(nickkhyl): allow specifying some kind of priority/altitude for the resolver. - // TODO(nickkhyl): make it a "profile resolver" instead of a "background profile resolver". - // The concepts of the "current user", "foreground profile" and "background profile" - // only exist on Windows, and we're moving away from them anyway. - RegisterBackgroundProfileResolver(ProfileResolver) - - // RegisterProfileStateChangeCallback registers a function to be called when the current - // [ipn.LoginProfile] or its [ipn.Prefs] change. - // - // To get the initial profile or prefs, use [ProfileServices.CurrentProfileState] - // or [ProfileServices.CurrentPrefs] from the extension's [Extension.Init]. - // - // It is a runtime error to register a nil provider or call after the host - // has been initialized. - RegisterProfileStateChangeCallback(ProfileStateChangeCallback) } // ProfileStore provides read-only access to available login profiles and their preferences. @@ -354,3 +322,36 @@ type ProfileStateChangeCallback func(_ ipn.LoginProfileView, _ ipn.PrefsView, sa // It returns a function to be called when the cc is being shut down, // or nil if no cleanup is needed. type NewControlClientCallback func(controlclient.Client, ipn.LoginProfileView) (cleanup func()) + +// Hooks is a collection of hooks that extensions can add to (non-concurrently) +// during program initialization and can be called by LocalBackend and others at +// runtime. +// +// Each hook has its own rules about when it's called and what environment it +// has access to and what it's allowed to do. +type Hooks struct { + // ProfileStateChange are 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 + // is first populated and persisted. + ProfileStateChange feature.Hooks[ProfileStateChangeCallback] + + // BackgroundProfileResolvers are registered background profile resolvers. + // They're used to determine the profile to use when no GUI/CLI client is connected. + // + // TODO(nickkhyl): allow specifying some kind of priority/altitude for the resolver. + // TODO(nickkhyl): make it a "profile resolver" instead of a "background profile resolver". + // The concepts of the "current user", "foreground profile" and "background profile" + // only exist on Windows, and we're moving away from them anyway. + BackgroundProfileResolvers feature.Hooks[ProfileResolver] + + // AuditLoggers are registered [AuditLogProvider]s. + // Each provider is called to get an [ipnauth.AuditLogFunc] when an auditable action + // is about to be performed. If an audit logger returns an error, the action is denied. + AuditLoggers feature.Hooks[AuditLogProvider] + + // NewControlClient are the functions to be called when a new control client + // is created. It is called with the LocalBackend locked. + NewControlClient feature.Hooks[NewControlClientCallback] +} diff --git a/ipn/ipnlocal/extension_host.go b/ipn/ipnlocal/extension_host.go index 85da27ab0..6aa42ba12 100644 --- a/ipn/ipnlocal/extension_host.go +++ b/ipn/ipnlocal/extension_host.go @@ -64,8 +64,9 @@ import ( // and to further reduce the risk of accessing unexported methods or fields of [LocalBackend], the host interacts // with it via the [Backend] interface. type ExtensionHost struct { - b Backend - logf logger.Logf // prefixed with "ipnext:" + b Backend + hooks ipnext.Hooks + logf logger.Logf // prefixed with "ipnext:" // allExtensions holds the extensions in the order they were registered, // including those that have not yet attempted initialization or have failed to initialize. @@ -84,22 +85,6 @@ type ExtensionHost struct { // doEnqueueBackendOperation adds an asynchronous [LocalBackend] operation to the workQueue. doEnqueueBackendOperation func(func(Backend)) - // profileStateChangeCbs are 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 - // is first populated and persisted. - profileStateChangeCbs []ipnext.ProfileStateChangeCallback - // backgroundProfileResolvers are registered background profile resolvers. - // They're used to determine the profile to use when no GUI/CLI client is connected. - backgroundProfileResolvers []ipnext.ProfileResolver - // auditLoggers are registered [AuditLogProvider]s. - // Each provider is called to get an [ipnauth.AuditLogFunc] when an auditable action - // is about to be performed. If an audit logger returns an error, the action is denied. - auditLoggers []ipnext.AuditLogProvider - // newControlClientCbs are the functions to be called when a new control client is created. - newControlClientCbs []ipnext.NewControlClientCallback - shuttingDown atomic.Bool // mu protects the following fields. @@ -208,6 +193,15 @@ func (h *ExtensionHost) Init() { } } +var zeroHooks ipnext.Hooks + +func (h *ExtensionHost) Hooks() *ipnext.Hooks { + if h == nil { + return &zeroHooks + } + return &h.hooks +} + func (h *ExtensionHost) init() { defer h.initDone.Store(true) @@ -360,24 +354,6 @@ func (h *ExtensionHost) SendNotifyAsync(n ipn.Notify) { }) } -// addFuncHook appends non-nil fn to hooks. -func addFuncHook[F any](h *ExtensionHost, hooks *[]F, fn F) { - if h.initDone.Load() { - panic("invalid callback register after init") - } - if reflect.ValueOf(fn).IsZero() { - panic("nil function hook") - } - *hooks = append(*hooks, fn) -} - -// RegisterProfileStateChangeCallback implements [ipnext.ProfileServices]. -func (h *ExtensionHost) RegisterProfileStateChangeCallback(cb ipnext.ProfileStateChangeCallback) { - if h != nil { - addFuncHook(h, &h.profileStateChangeCbs, cb) - } -} - // NotifyProfileChange invokes registered profile state change callbacks // and updates the current profile and prefs in the host. // It strips private keys from the [ipn.Prefs] before preserving @@ -397,7 +373,7 @@ func (h *ExtensionHost) NotifyProfileChange(profile ipn.LoginProfileView, prefs h.currentProfile = profile h.mu.Unlock() - for _, cb := range h.profileStateChangeCbs { + for _, cb := range h.hooks.ProfileStateChange { cb(profile, prefs, sameNode) } } @@ -421,18 +397,11 @@ func (h *ExtensionHost) NotifyProfilePrefsChanged(profile ipn.LoginProfileView, // Get the callbacks to be invoked. h.mu.Unlock() - for _, cb := range h.profileStateChangeCbs { + for _, cb := range h.hooks.ProfileStateChange { cb(profile, newPrefs, true) } } -// RegisterBackgroundProfileResolver implements [ipnext.ProfileServices]. -func (h *ExtensionHost) RegisterBackgroundProfileResolver(resolver ipnext.ProfileResolver) { - if h != nil { - addFuncHook(h, &h.backgroundProfileResolvers, resolver) - } -} - func (h *ExtensionHost) active() bool { return h != nil && !h.shuttingDown.Load() } @@ -455,7 +424,7 @@ func (h *ExtensionHost) DetermineBackgroundProfile(profiles ipnext.ProfileStore) // Attempt to resolve the background profile using the registered // background profile resolvers (e.g., [ipn/desktop.desktopSessionsExt] on Windows). - for _, resolver := range h.backgroundProfileResolvers { + for _, resolver := range h.hooks.BackgroundProfileResolvers { if profile := resolver(profiles); profile.Valid() { return profile } @@ -466,37 +435,20 @@ func (h *ExtensionHost) DetermineBackgroundProfile(profiles ipnext.ProfileStore) return ipn.LoginProfileView{} } -// RegisterControlClientCallback implements [ipnext.Host]. -func (h *ExtensionHost) RegisterControlClientCallback(cb ipnext.NewControlClientCallback) { - if h != nil { - addFuncHook(h, &h.newControlClientCbs, cb) - } -} - // NotifyNewControlClient invokes all registered control client callbacks. // It returns callbacks to be executed when the control client shuts down. func (h *ExtensionHost) NotifyNewControlClient(cc controlclient.Client, profile ipn.LoginProfileView) (ccShutdownCbs []func()) { if !h.active() { return nil } - if len(h.newControlClientCbs) > 0 { - ccShutdownCbs = make([]func(), 0, len(h.newControlClientCbs)) - for _, cb := range h.newControlClientCbs { - if shutdown := cb(cc, profile); shutdown != nil { - ccShutdownCbs = append(ccShutdownCbs, shutdown) - } + for _, cb := range h.hooks.NewControlClient { + if shutdown := cb(cc, profile); shutdown != nil { + ccShutdownCbs = append(ccShutdownCbs, shutdown) } } return ccShutdownCbs } -// RegisterAuditLogProvider implements [ipnext.Host]. -func (h *ExtensionHost) RegisterAuditLogProvider(provider ipnext.AuditLogProvider) { - if h != nil { - addFuncHook(h, &h.auditLoggers, provider) - } -} - // AuditLogger returns a function that reports an auditable action // to all registered audit loggers. It fails if any of them returns an error, // indicating that the action cannot be logged and must not be performed. @@ -510,8 +462,8 @@ func (h *ExtensionHost) AuditLogger() ipnauth.AuditLogFunc { if !h.active() { return func(tailcfg.ClientAuditAction, string) error { return nil } } - loggers := make([]ipnauth.AuditLogFunc, 0, len(h.auditLoggers)) - for _, provider := range h.auditLoggers { + loggers := make([]ipnauth.AuditLogFunc, 0, len(h.hooks.AuditLoggers)) + for _, provider := range h.hooks.AuditLoggers { loggers = append(loggers, provider()) } return func(action tailcfg.ClientAuditAction, details string) error { diff --git a/ipn/ipnlocal/extension_host_test.go b/ipn/ipnlocal/extension_host_test.go index 31b38196a..aa4a27d45 100644 --- a/ipn/ipnlocal/extension_host_test.go +++ b/ipn/ipnlocal/extension_host_test.go @@ -748,7 +748,7 @@ func TestExtensionHostProfileStateChangeCallback(t *testing.T) { tt.ext.InitHook = func(e *testExtension) error { // Create and register the callback on init. handler := makeStateChangeAppender(e) - e.host.Profiles().RegisterProfileStateChangeCallback(handler) + e.host.Hooks().ProfileStateChange.Add(handler) return nil } } @@ -875,7 +875,7 @@ func TestBackgroundProfileResolver(t *testing.T) { // This is typically done by the extensions themselves, // but we do it here for testing purposes. for _, r := range tt.resolvers { - h.Profiles().RegisterBackgroundProfileResolver(r) + h.Hooks().BackgroundProfileResolvers.Add(r) } h.Init() @@ -968,7 +968,7 @@ func TestAuditLogProviders(t *testing.T) { } } ext.InitHook = func(e *testExtension) error { - e.host.RegisterAuditLogProvider(provider) + e.host.Hooks().AuditLoggers.Add(provider) return nil } exts = append(exts, ext)