health,ipn/ipnlocal: hide update warning when auto-updates are enabled (#12631)

When auto-udpates are enabled, we don't need to nag users to update
after a new release, before we release auto-updates.

Updates https://github.com/tailscale/corp/issues/20081

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>

health/health.go

@@ -78,6 +78,7 @@ type Tracker struct {
 
 	latestVersion   *tailcfg.ClientVersion // or nil
 	checkForUpdates bool
+	applyUpdates    opt.Bool
 
 	inMapPoll               bool
 	inMapPollSince          time.Time
@@ -782,17 +783,20 @@ func (t *Tracker) SetLatestVersion(v *tailcfg.ClientVersion) {
 	t.selfCheckLocked()
 }
 
-// SetCheckForUpdates sets whether the client wants to check for updates.
-func (t *Tracker) SetCheckForUpdates(v bool) {
+// SetAutoUpdatePrefs sets the client auto-update preferences. The arguments
+// match the fields of ipn.AutoUpdatePrefs, but we cannot pass that struct
+// directly due to a circular import.
+func (t *Tracker) SetAutoUpdatePrefs(check bool, apply opt.Bool) {
 	if t.nil() {
 		return
 	}
 	t.mu.Lock()
 	defer t.mu.Unlock()
-	if t.checkForUpdates == v {
+	if t.checkForUpdates == check && t.applyUpdates == apply {
 		return
 	}
-	t.checkForUpdates = v
+	t.checkForUpdates = check
+	t.applyUpdates = apply
 	t.selfCheckLocked()
 }
 
@@ -883,20 +887,14 @@ var fakeErrForTesting = envknob.RegisterString("TS_DEBUG_FAKE_HEALTH_ERROR")
 func (t *Tracker) updateBuiltinWarnablesLocked() {
 	t.updateWarmingUpWarnableLocked()
 
-	if t.checkForUpdates {
-		if cv := t.latestVersion; cv != nil && !cv.RunningLatest && cv.LatestVersion != "" {
-			if cv.UrgentSecurityUpdate {
-				t.setUnhealthyLocked(securityUpdateAvailableWarnable, Args{
+	if w, show := t.showUpdateWarnable(); show {
+		t.setUnhealthyLocked(w, Args{
 			ArgCurrentVersion:   version.Short(),
-					ArgAvailableVersion: cv.LatestVersion,
+			ArgAvailableVersion: t.latestVersion.LatestVersion,
 		})
 	} else {
-				t.setUnhealthyLocked(updateAvailableWarnable, Args{
-					ArgCurrentVersion:   version.Short(),
-					ArgAvailableVersion: cv.LatestVersion,
-				})
-			}
-		}
+		t.setHealthyLocked(updateAvailableWarnable)
+		t.setHealthyLocked(securityUpdateAvailableWarnable)
 	}
 
 	if version.IsUnstableBuild() {
@@ -1070,6 +1068,24 @@ func (t *Tracker) updateWarmingUpWarnableLocked() {
 	}
 }
 
+func (t *Tracker) showUpdateWarnable() (*Warnable, bool) {
+	if !t.checkForUpdates {
+		return nil, false
+	}
+	cv := t.latestVersion
+	if cv == nil || cv.RunningLatest || cv.LatestVersion == "" {
+		return nil, false
+	}
+	if cv.UrgentSecurityUpdate {
+		return securityUpdateAvailableWarnable, true
+	}
+	// Only show update warning when auto-updates are off
+	if !t.applyUpdates.EqualBool(true) {
+		return updateAvailableWarnable, true
+	}
+	return nil, false
+}
+
 // ReceiveFuncStats tracks the calls made to a wireguard-go receive func.
 type ReceiveFuncStats struct {
 	// name is the name of the receive func.

health/health_test.go

@@ -9,6 +9,9 @@ import (
 	"slices"
 	"testing"
 	"time"
+
+	"tailscale.com/tailcfg"
+	"tailscale.com/types/opt"
 )
 
 func TestAppendWarnableDebugFlags(t *testing.T) {
@@ -214,3 +217,89 @@ func TestCheckDependsOnAppearsInUnhealthyState(t *testing.T) {
 		t.Fatalf("Expected DependsOn = %v in the unhealthy state, got: %v", wantDependsOn, us2.DependsOn)
 	}
 }
+
+func TestShowUpdateWarnable(t *testing.T) {
+	tests := []struct {
+		desc         string
+		check        bool
+		apply        opt.Bool
+		cv           *tailcfg.ClientVersion
+		wantWarnable *Warnable
+		wantShow     bool
+	}{
+		{
+			desc:         "nil CientVersion",
+			check:        true,
+			cv:           nil,
+			wantWarnable: nil,
+			wantShow:     false,
+		},
+		{
+			desc:         "RunningLatest",
+			check:        true,
+			cv:           &tailcfg.ClientVersion{RunningLatest: true},
+			wantWarnable: nil,
+			wantShow:     false,
+		},
+		{
+			desc:         "no LatestVersion",
+			check:        true,
+			cv:           &tailcfg.ClientVersion{RunningLatest: false, LatestVersion: ""},
+			wantWarnable: nil,
+			wantShow:     false,
+		},
+		{
+			desc:         "show regular update",
+			check:        true,
+			cv:           &tailcfg.ClientVersion{RunningLatest: false, LatestVersion: "1.2.3"},
+			wantWarnable: updateAvailableWarnable,
+			wantShow:     true,
+		},
+		{
+			desc:         "show security update",
+			check:        true,
+			cv:           &tailcfg.ClientVersion{RunningLatest: false, LatestVersion: "1.2.3", UrgentSecurityUpdate: true},
+			wantWarnable: securityUpdateAvailableWarnable,
+			wantShow:     true,
+		},
+		{
+			desc:         "update check disabled",
+			check:        false,
+			cv:           &tailcfg.ClientVersion{RunningLatest: false, LatestVersion: "1.2.3"},
+			wantWarnable: nil,
+			wantShow:     false,
+		},
+		{
+			desc:         "hide update with auto-updates",
+			check:        true,
+			apply:        opt.NewBool(true),
+			cv:           &tailcfg.ClientVersion{RunningLatest: false, LatestVersion: "1.2.3"},
+			wantWarnable: nil,
+			wantShow:     false,
+		},
+		{
+			desc:         "show security update with auto-updates",
+			check:        true,
+			apply:        opt.NewBool(true),
+			cv:           &tailcfg.ClientVersion{RunningLatest: false, LatestVersion: "1.2.3", UrgentSecurityUpdate: true},
+			wantWarnable: securityUpdateAvailableWarnable,
+			wantShow:     true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			tr := &Tracker{
+				checkForUpdates: tt.check,
+				applyUpdates:    tt.apply,
+				latestVersion:   tt.cv,
+			}
+			gotWarnable, gotShow := tr.showUpdateWarnable()
+			if gotWarnable != tt.wantWarnable {
+				t.Errorf("got warnable: %v, want: %v", gotWarnable, tt.wantWarnable)
+			}
+			if gotShow != tt.wantShow {
+				t.Errorf("got show: %v, want: %v", gotShow, tt.wantShow)
+			}
+		})
+	}
+}

ipn/ipnlocal/profiles.go

@@ -448,7 +448,7 @@ func (pm *profileManager) updateHealth() {
 	if !pm.prefs.Valid() {
 		return
 	}
-	pm.health.SetCheckForUpdates(pm.prefs.AutoUpdate().Check)
+	pm.health.SetAutoUpdatePrefs(pm.prefs.AutoUpdate().Check, pm.prefs.AutoUpdate().Apply)
 }
 
 // NewProfile creates and switches to a new unnamed profile. The new profile is