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) + } +}