From 00e66b5dd4a07ff8ec1ce01b1e4ab520197d3fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Mon, 7 Sep 2026 16:45:17 +0200 Subject: [PATCH] [client] Keep the updater silent until management decides the mode The update manager started in download-only mode and every engine stop reset it there again, kicking the update loop with the cached latest version. On a managed peer this published a "New version available" notification on each disconnect, session expiry or logout, and once more in the window between daemon start and the first network map. Users under enforced updates then installed the release by hand. Replace the boolean with a three-state mode. The manager starts undecided and publishes nothing, from the fetcher or from NotifyUI, until the network map picks download-only or managed. A new connection lifecycle resets to undecided; disconnects leave the last decision untouched. A missing AutoUpdateSettings now means download-only instead of a no-op, and on platforms without an installer SetVersion falls back to download-only so they keep notifying. --- client/internal/connect.go | 1 + client/internal/engine.go | 10 +-- client/internal/updater/manager.go | 54 ++++++++--- client/internal/updater/manager_linux_test.go | 16 ++-- client/internal/updater/manager_mode_test.go | 89 +++++++++++++++++++ 5 files changed, 141 insertions(+), 29 deletions(-) create mode 100644 client/internal/updater/manager_mode_test.go diff --git a/client/internal/connect.go b/client/internal/connect.go index 88d829d2f..e826335bf 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -274,6 +274,7 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan stateManager.RegisterState(&sshconfig.ShutdownState{}) if c.updateManager != nil { + c.updateManager.ResetMode() c.updateManager.CheckUpdateSuccess(c.ctx) } diff --git a/client/internal/engine.go b/client/internal/engine.go index f8b65f7d8..7374c817a 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -455,10 +455,6 @@ func (e *Engine) stopLocked() { e.sessionWatcher.Close() } - if e.updateManager != nil { - e.updateManager.SetDownloadOnly() - } - log.Info("cleaning up status recorder states") e.statusRecorder.ReplaceOfflinePeers([]peer.State{}) e.statusRecorder.UpdateDNSStates([]peer.NSGroupState{}) @@ -971,11 +967,7 @@ func (e *Engine) handleAutoUpdateVersion(autoUpdateSettings *mgmProto.AutoUpdate return } - if autoUpdateSettings == nil { - return - } - - if autoUpdateSettings.Version == disableAutoUpdate { + if autoUpdateSettings == nil || autoUpdateSettings.Version == disableAutoUpdate { log.Infof("auto-update is disabled") e.updateManager.SetDownloadOnly() return diff --git a/client/internal/updater/manager.go b/client/internal/updater/manager.go index 1b69368d0..69cce7a71 100644 --- a/client/internal/updater/manager.go +++ b/client/internal/updater/manager.go @@ -21,8 +21,16 @@ const ( latestVersion = "latest" ) +const ( + modeUndecided updateMode = iota + modeDownloadOnly + modeManaged +) + var errNoUpdateState = errors.New("no update state found") +type updateMode int + type UpdateState struct { PreUpdateVersion string TargetVersion string @@ -36,8 +44,8 @@ type Manager struct { statusRecorder *peer.Status stateManager *statemanager.Manager - downloadOnly bool // true when no enforcement from management; notifies UI to download latest - forceUpdate bool // true when management sets AlwaysUpdate; skips UI interaction and installs directly + mode updateMode + forceUpdate bool // true when management sets AlwaysUpdate; skips UI interaction and installs directly lastTrigger time.Time mgmUpdateChan chan struct{} @@ -54,7 +62,7 @@ type Manager struct { pendingVersion *v.Version // updateMutex protects update, expectedVersion, updateToLatestVersion, - // downloadOnly, forceUpdate, pendingVersion, and lastTrigger fields + // mode, forceUpdate, pendingVersion, and lastTrigger fields updateMutex sync.Mutex // installMutex and installing guard against concurrent installation attempts @@ -76,7 +84,6 @@ func NewManager(statusRecorder *peer.Status, stateManager *statemanager.Manager) updateChannel: make(chan struct{}, 1), currentVersion: version.NetbirdVersion(), update: version.NewUpdate("nb/client"), - downloadOnly: true, autoUpdateSupported: isAutoUpdateSupported, } @@ -151,7 +158,7 @@ func (m *Manager) Start(ctx context.Context) { func (m *Manager) SetDownloadOnly() { m.updateMutex.Lock() - m.downloadOnly = true + m.mode = modeDownloadOnly m.forceUpdate = false m.expectedVersion = nil m.updateToLatestVersion = false @@ -169,6 +176,7 @@ func (m *Manager) SetVersion(expectedVersion string, forceUpdate bool) { if !m.autoUpdateSupported() { log.Warnf("auto-update not supported on this platform") + m.SetDownloadOnly() return } @@ -179,7 +187,7 @@ func (m *Manager) SetVersion(expectedVersion string, forceUpdate bool) { log.Errorf("empty expected version provided") m.expectedVersion = nil m.updateToLatestVersion = false - m.downloadOnly = true + m.mode = modeDownloadOnly return } @@ -200,7 +208,7 @@ func (m *Manager) SetVersion(expectedVersion string, forceUpdate bool) { } m.lastTrigger = time.Time{} - m.downloadOnly = false + m.mode = modeManaged m.forceUpdate = forceUpdate select { @@ -209,6 +217,18 @@ func (m *Manager) SetVersion(expectedVersion string, forceUpdate bool) { } } +func (m *Manager) ResetMode() { + m.updateMutex.Lock() + defer m.updateMutex.Unlock() + + m.mode = modeUndecided + m.forceUpdate = false + m.expectedVersion = nil + m.updateToLatestVersion = false + m.pendingVersion = nil + m.lastTrigger = time.Time{} +} + // Install triggers the installation of the pending version. It is called when the user clicks the install button in the UI. func (m *Manager) Install(ctx context.Context) error { if !m.autoUpdateSupported() { @@ -255,12 +275,16 @@ func (m *Manager) NotifyUI() { m.updateMutex.Unlock() return } - downloadOnly := m.downloadOnly + mode := m.mode pendingVersion := m.pendingVersion latestVersion := m.update.LatestVersion() m.updateMutex.Unlock() - if downloadOnly { + if mode == modeUndecided { + return + } + + if mode == modeDownloadOnly { if latestVersion == nil { return } @@ -343,13 +367,17 @@ func (m *Manager) handleUpdate(ctx context.Context) { return } - downloadOnly := m.downloadOnly + mode := m.mode forceUpdate := m.forceUpdate curLatestVersion := m.update.LatestVersion() switch { + case mode == modeUndecided: + log.Tracef("auto-update mode not decided yet") + m.updateMutex.Unlock() + return // Download-only mode or resolve "latest" to actual version - case downloadOnly, m.updateToLatestVersion: + case mode == modeDownloadOnly, m.updateToLatestVersion: if curLatestVersion == nil { log.Tracef("latest version not fetched yet") m.updateMutex.Unlock() @@ -374,12 +402,12 @@ func (m *Manager) handleUpdate(ctx context.Context) { m.lastTrigger = time.Now() log.Infof("new version available: %s", updateVersion) - if !downloadOnly && !forceUpdate { + if mode == modeManaged && !forceUpdate { m.pendingVersion = updateVersion } m.updateMutex.Unlock() - if downloadOnly { + if mode == modeDownloadOnly { m.statusRecorder.PublishEvent( cProto.SystemEvent_INFO, cProto.SystemEvent_SYSTEM, diff --git a/client/internal/updater/manager_linux_test.go b/client/internal/updater/manager_linux_test.go index b05dd7e7d..c2df0d39a 100644 --- a/client/internal/updater/manager_linux_test.go +++ b/client/internal/updater/manager_linux_test.go @@ -16,7 +16,7 @@ import ( ) // On Linux, only Mode 1 (downloadOnly) is supported. -// SetVersion is a no-op because auto-update installation is not supported. +// SetVersion falls back to download-only because auto-update installation is not supported. func Test_LatestVersion_Linux(t *testing.T) { testMatrix := []struct { @@ -89,9 +89,8 @@ func Test_LatestVersion_Linux(t *testing.T) { } } -func Test_SetVersion_NoOp_Linux(t *testing.T) { - // On Linux, SetVersion should be a no-op — no events fired - tmpFile := path.Join(t.TempDir(), "update-test-noop.json") +func Test_SetVersion_FallsBackToDownloadOnly_Linux(t *testing.T) { + tmpFile := path.Join(t.TempDir(), "update-test-fallback.json") recorder := peer.NewRecorder("") sub := recorder.SubscribeToEvents() defer recorder.UnsubscribeFromEvents(sub) @@ -102,9 +101,12 @@ func Test_SetVersion_NoOp_Linux(t *testing.T) { m.Start(context.Background()) m.SetVersion("1.0.1", false) - ver, _ := waitForUpdateEvent(sub, 500*time.Millisecond) - if ver != "" { - t.Errorf("SetVersion should be a no-op on Linux, but got event with version %s", ver) + ver, enforced := waitForUpdateEvent(sub, 500*time.Millisecond) + if ver != "1.0.1" { + t.Fatalf("expected download-only event for 1.0.1, got %q", ver) + } + if enforced { + t.Error("Linux fallback must never have enforced metadata") } m.Stop() diff --git a/client/internal/updater/manager_mode_test.go b/client/internal/updater/manager_mode_test.go new file mode 100644 index 000000000..472888268 --- /dev/null +++ b/client/internal/updater/manager_mode_test.go @@ -0,0 +1,89 @@ +package updater + +import ( + "context" + "path" + "testing" + "time" + + v "github.com/hashicorp/go-version" + + "github.com/netbirdio/netbird/client/internal/peer" + "github.com/netbirdio/netbird/client/internal/statemanager" +) + +func Test_UndecidedMode_SuppressesNotification(t *testing.T) { + tmpFile := path.Join(t.TempDir(), "update-test-undecided.json") + recorder := peer.NewRecorder("") + sub := recorder.SubscribeToEvents() + defer recorder.UnsubscribeFromEvents(sub) + + mockUpdate := &versionUpdateMock{latestVersion: v.Must(v.NewSemver("1.0.1"))} + m := NewManager(recorder, statemanager.New(tmpFile)) + m.update = mockUpdate + m.currentVersion = "1.0.0" + m.Start(context.Background()) + defer m.Stop() + + mockUpdate.onUpdate() + if ver, _ := waitForUpdateEvent(sub, 300*time.Millisecond); ver != "" { + t.Fatalf("undecided mode must not publish, got %q", ver) + } + + m.NotifyUI() + if ver, _ := waitForUpdateEvent(sub, 300*time.Millisecond); ver != "" { + t.Fatalf("NotifyUI in undecided mode must not publish, got %q", ver) + } + + m.SetDownloadOnly() + ver, enforced := waitForUpdateEvent(sub, 500*time.Millisecond) + if ver != "1.0.1" { + t.Fatalf("expected download-only event for 1.0.1, got %q", ver) + } + if enforced { + t.Error("download-only event must not carry enforced metadata") + } +} + +func Test_ResetMode_ReturnsToUndecided(t *testing.T) { + tmpFile := path.Join(t.TempDir(), "update-test-reset.json") + recorder := peer.NewRecorder("") + sub := recorder.SubscribeToEvents() + defer recorder.UnsubscribeFromEvents(sub) + + mockUpdate := &versionUpdateMock{latestVersion: v.Must(v.NewSemver("1.0.1"))} + m := NewManager(recorder, statemanager.New(tmpFile)) + m.update = mockUpdate + m.currentVersion = "1.0.0" + m.autoUpdateSupported = func() bool { return true } + m.Start(context.Background()) + defer m.Stop() + + m.SetVersion("1.0.1", false) + ver, enforced := waitForUpdateEvent(sub, 500*time.Millisecond) + if ver != "1.0.1" || !enforced { + t.Fatalf("expected enforced event for 1.0.1, got %q enforced=%v", ver, enforced) + } + + m.ResetMode() + + mockUpdate.onUpdate() + if ver, _ := waitForUpdateEvent(sub, 300*time.Millisecond); ver != "" { + t.Fatalf("reset mode must not publish on fetch, got %q", ver) + } + + m.NotifyUI() + if ver, _ := waitForUpdateEvent(sub, 300*time.Millisecond); ver != "" { + t.Fatalf("NotifyUI after reset must not publish, got %q", ver) + } + + if err := m.Install(context.Background()); err == nil { + t.Fatal("Install after reset must fail without a pending version") + } + + m.SetVersion("1.0.1", false) + ver, enforced = waitForUpdateEvent(sub, 500*time.Millisecond) + if ver != "1.0.1" || !enforced { + t.Fatalf("expected enforced event again after reset, got %q enforced=%v", ver, enforced) + } +}