ipn: apply tailnet-wide default for auto-updates (#10508)

When auto-update setting in local Prefs is unset, apply the tailnet
default value from control. This only happens once, when we apply the
default (or when the user manually overrides it), tailnet default no
longer affects the node.

Updates #16244

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

cmd/tailscale/cli/cli_test.go

@@ -558,7 +558,6 @@ func TestPrefsFromUpArgs(t *testing.T) {
 				AllowSingleHosts: true,
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: false,
 				},
 			},
 		},
@@ -575,7 +574,6 @@ func TestPrefsFromUpArgs(t *testing.T) {
 				NetfilterMode:    preftype.NetfilterOn,
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: false,
 				},
 			},
 		},
@@ -594,7 +592,6 @@ func TestPrefsFromUpArgs(t *testing.T) {
 				NetfilterMode: preftype.NetfilterOn,
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: false,
 				},
 			},
 		},
@@ -684,7 +681,6 @@ func TestPrefsFromUpArgs(t *testing.T) {
 				NoSNAT:        true,
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: false,
 				},
 			},
 		},
@@ -701,7 +697,6 @@ func TestPrefsFromUpArgs(t *testing.T) {
 				NoSNAT:        true,
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: false,
 				},
 			},
 		},
@@ -720,7 +715,6 @@ func TestPrefsFromUpArgs(t *testing.T) {
 				},
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: false,
 				},
 			},
 		},

cmd/tailscale/cli/set.go

@@ -17,6 +17,7 @@
 	"tailscale.com/net/netutil"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/safesocket"
+	"tailscale.com/types/opt"
 	"tailscale.com/types/views"
 	"tailscale.com/version"
 )
@@ -116,7 +117,7 @@ func runSet(ctx context.Context, args []string) (retErr error) {
 			ForceDaemon:            setArgs.forceDaemon,
 			AutoUpdate: ipn.AutoUpdatePrefs{
 				Check: setArgs.updateCheck,
-				Apply: setArgs.updateApply,
+				Apply: opt.NewBool(setArgs.updateApply),
 			},
 			AppConnector: ipn.AppConnectorPrefs{
 				Advertise: setArgs.advertiseConnector,
@@ -172,7 +173,7 @@ func runSet(ctx context.Context, args []string) (retErr error) {
 		// does not use clientupdate.
 		if version.IsMacSysExt() {
 			apply := "0"
-			if maskedPrefs.AutoUpdate.Apply {
+			if maskedPrefs.AutoUpdate.Apply.EqualBool(true) {
 				apply = "1"
 			}
 			out, err := exec.Command("defaults", "write", "io.tailscale.ipn.macsys", "SUAutomaticallyUpdate", apply).CombinedOutput()

control/controlclient/direct.go

@@ -78,6 +78,7 @@ type Direct struct {
 	c2nHandler                 http.Handler                 // or nil
 	onClientVersion            func(*tailcfg.ClientVersion) // or nil
 	onControlTime              func(time.Time)              // or nil
+	onTailnetDefaultAutoUpdate func(bool)                   // or nil
 
 	dialPlan ControlDialPlanner // can be nil
 
@@ -125,6 +126,7 @@ type Options struct {
 	PopBrowserURL              func(url string)             // optional func to open browser
 	OnClientVersion            func(*tailcfg.ClientVersion) // optional func to inform GUI of client version status
 	OnControlTime              func(time.Time)              // optional func to notify callers of new time from control
+	OnTailnetDefaultAutoUpdate func(bool)                   // optional func to inform GUI of default auto-update setting for the tailnet
 	Dialer                     *tsdial.Dialer               // non-nil
 	C2NHandler                 http.Handler                 // or nil
 	ControlKnobs               *controlknobs.Knobs          // or nil to ignore
@@ -278,6 +280,7 @@ func NewDirect(opts Options) (*Direct, error) {
 		pinger:                     opts.Pinger,
 		popBrowser:                 opts.PopBrowserURL,
 		onClientVersion:            opts.OnClientVersion,
+		onTailnetDefaultAutoUpdate: opts.OnTailnetDefaultAutoUpdate,
 		onControlTime:              opts.OnControlTime,
 		c2nHandler:                 opts.C2NHandler,
 		dialer:                     opts.Dialer,
@@ -1091,6 +1094,11 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap
 			metricMapResponseKeepAlives.Add(1)
 			continue
 		}
+		if au, ok := resp.DefaultAutoUpdate.Get(); ok {
+			if c.onTailnetDefaultAutoUpdate != nil {
+				c.onTailnetDefaultAutoUpdate(au)
+			}
+		}
 
 		metricMapResponseMap.Add(1)
 		if gotNonKeepAliveMessage {

ipn/ipnlocal/c2n.go

@@ -378,7 +378,7 @@ func (b *LocalBackend) newC2NUpdateResponse() tailcfg.C2NUpdateResponse {
 	// invoke it here. For this purpose, it is ok to pass it a zero Arguments.
 	prefs := b.Prefs().AutoUpdate()
 	return tailcfg.C2NUpdateResponse{
-		Enabled:   envknob.AllowsRemoteUpdate() || prefs.Apply,
+		Enabled:   envknob.AllowsRemoteUpdate() || prefs.Apply.EqualBool(true),
 		Supported: clientupdate.CanAutoUpdate(),
 	}
 }

ipn/ipnlocal/local.go

@@ -76,6 +76,7 @@
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logid"
 	"tailscale.com/types/netmap"
+	"tailscale.com/types/opt"
 	"tailscale.com/types/persist"
 	"tailscale.com/types/preftype"
 	"tailscale.com/types/ptr"
@@ -1271,8 +1272,8 @@ type preferencePolicyInfo struct {
 	},
 	{
 		key: syspolicy.ApplyUpdates,
-		get: func(p ipn.PrefsView) bool { return p.AutoUpdate().Apply },
-		set: func(p *ipn.Prefs, v bool) { p.AutoUpdate.Apply = v },
+		get: func(p ipn.PrefsView) bool { v, _ := p.AutoUpdate().Apply.Get(); return v },
+		set: func(p *ipn.Prefs, v bool) { p.AutoUpdate.Apply.Set(v) },
 	},
 	{
 		key: syspolicy.EnableRunExitNode,
@@ -1780,6 +1781,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 		Pinger:                     b,
 		PopBrowserURL:              b.tellClientToBrowseToURL,
 		OnClientVersion:            b.onClientVersion,
+		OnTailnetDefaultAutoUpdate: b.onTailnetDefaultAutoUpdate,
 		OnControlTime:              b.em.onControlTime,
 		Dialer:                     b.Dialer(),
 		Observer:                   b,
@@ -2500,6 +2502,32 @@ func (b *LocalBackend) onClientVersion(v *tailcfg.ClientVersion) {
 	b.send(ipn.Notify{ClientVersion: v})
 }
 
+func (b *LocalBackend) onTailnetDefaultAutoUpdate(au bool) {
+	prefs := b.pm.CurrentPrefs()
+	if !prefs.Valid() {
+		b.logf("[unexpected]: received tailnet default auto-update callback but current prefs are nil")
+		return
+	}
+	if _, ok := prefs.AutoUpdate().Apply.Get(); ok {
+		// Apply was already set from a previous default or manually by the
+		// user. Tailnet default should not affect us, even if it changes.
+		return
+	}
+	b.logf("using tailnet default auto-update setting: %v", au)
+	prefsClone := prefs.AsStruct()
+	prefsClone.AutoUpdate.Apply = opt.NewBool(au)
+	_, err := b.EditPrefs(&ipn.MaskedPrefs{
+		Prefs: *prefsClone,
+		AutoUpdateSet: ipn.AutoUpdatePrefsMask{
+			ApplySet: true,
+		},
+	})
+	if err != nil {
+		b.logf("failed to apply tailnet-wide default for auto-updates (%v): %v", au, err)
+		return
+	}
+}
+
 // For testing lazy machine key generation.
 var panicOnMachineKeyGeneration = envknob.RegisterBool("TS_DEBUG_PANIC_MACHINE_KEY")
 
@@ -4079,7 +4107,7 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip
 	hi.RoutableIPs = prefs.AdvertiseRoutes().AsSlice()
 	hi.RequestTags = prefs.AdvertiseTags().AsSlice()
 	hi.ShieldsUp = prefs.ShieldsUp()
-	hi.AllowsUpdate = envknob.AllowsRemoteUpdate() || prefs.AutoUpdate().Apply
+	hi.AllowsUpdate = envknob.AllowsRemoteUpdate() || prefs.AutoUpdate().Apply.EqualBool(true)
 
 	var sshHostKeys []string
 	if prefs.RunSSH() && envknob.CanSSHD() {

ipn/ipnlocal/local_test.go

@@ -31,6 +31,7 @@
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logid"
 	"tailscale.com/types/netmap"
+	"tailscale.com/types/opt"
 	"tailscale.com/types/ptr"
 	"tailscale.com/util/dnsname"
 	"tailscale.com/util/mak"
@@ -1780,13 +1781,13 @@ func TestApplySysPolicy(t *testing.T) {
 			prefs: ipn.Prefs{
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: false,
+					Apply: opt.NewBool(false),
 				},
 			},
 			wantPrefs: ipn.Prefs{
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: true,
+					Apply: opt.NewBool(true),
 				},
 			},
 			wantAnyChange: true,
@@ -1799,13 +1800,13 @@ func TestApplySysPolicy(t *testing.T) {
 			prefs: ipn.Prefs{
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: true,
+					Apply: opt.NewBool(true),
 				},
 			},
 			wantPrefs: ipn.Prefs{
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: false,
+					Apply: opt.NewBool(false),
 				},
 			},
 			wantAnyChange: true,
@@ -1818,13 +1819,13 @@ func TestApplySysPolicy(t *testing.T) {
 			prefs: ipn.Prefs{
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: false,
-					Apply: true,
+					Apply: opt.NewBool(true),
 				},
 			},
 			wantPrefs: ipn.Prefs{
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: true,
+					Apply: opt.NewBool(true),
 				},
 			},
 			wantAnyChange: true,
@@ -1837,13 +1838,13 @@ func TestApplySysPolicy(t *testing.T) {
 			prefs: ipn.Prefs{
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: true,
-					Apply: true,
+					Apply: opt.NewBool(true),
 				},
 			},
 			wantPrefs: ipn.Prefs{
 				AutoUpdate: ipn.AutoUpdatePrefs{
 					Check: false,
-					Apply: true,
+					Apply: opt.NewBool(true),
 				},
 			},
 			wantAnyChange: true,
@@ -2055,3 +2056,56 @@ func TestPreferencePolicyInfo(t *testing.T) {
 		})
 	}
 }
+
+func TestOnTailnetDefaultAutoUpdate(t *testing.T) {
+	tests := []struct {
+		desc           string
+		before, after  opt.Bool
+		tailnetDefault bool
+	}{
+		{
+			before:         opt.Bool(""),
+			tailnetDefault: true,
+			after:          opt.NewBool(true),
+		},
+		{
+			before:         opt.Bool(""),
+			tailnetDefault: false,
+			after:          opt.NewBool(false),
+		},
+		{
+			before:         opt.Bool("unset"),
+			tailnetDefault: true,
+			after:          opt.NewBool(true),
+		},
+		{
+			before:         opt.Bool("unset"),
+			tailnetDefault: false,
+			after:          opt.NewBool(false),
+		},
+		{
+			before:         opt.NewBool(false),
+			tailnetDefault: true,
+			after:          opt.NewBool(false),
+		},
+		{
+			before:         opt.NewBool(true),
+			tailnetDefault: false,
+			after:          opt.NewBool(true),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("before=%s after=%s", tt.before, tt.after), func(t *testing.T) {
+			b := newTestBackend(t)
+			p := ipn.NewPrefs()
+			p.AutoUpdate.Apply = tt.before
+			if err := b.pm.setPrefsLocked(p.View()); err != nil {
+				t.Fatal(err)
+			}
+			b.onTailnetDefaultAutoUpdate(tt.tailnetDefault)
+			if want, got := tt.after, b.pm.CurrentPrefs().AutoUpdate().Apply; got != want {
+				t.Errorf("got: %q, want %q", got, want)
+			}
+		})
+	}
+}

ipn/prefs.go

@@ -21,6 +21,7 @@
 	"tailscale.com/net/netaddr"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/tailcfg"
+	"tailscale.com/types/opt"
 	"tailscale.com/types/persist"
 	"tailscale.com/types/preftype"
 	"tailscale.com/types/views"
@@ -237,7 +238,17 @@ type AutoUpdatePrefs struct {
 	// Apply specifies whether background auto-updates are enabled. When
 	// enabled, tailscaled will apply available updates in the background.
 	// Check must also be set when Apply is set.
-	Apply bool
+	Apply opt.Bool
+}
+
+func (au1 AutoUpdatePrefs) Equals(au2 AutoUpdatePrefs) bool {
+	// This could almost be as easy as `au1.Apply == au2.Apply`, except that
+	// opt.Bool("") and opt.Bool("unset") should be treated as equal.
+	apply1, ok1 := au1.Apply.Get()
+	apply2, ok2 := au2.Apply.Get()
+	return au1.Check == au2.Check &&
+		apply1 == apply2 &&
+		ok1 == ok2
 }
 
 // AppConnectorPrefs are the app connector settings for the node agent.
@@ -533,14 +544,14 @@ func (p *Prefs) Equals(p2 *Prefs) bool {
 		compareStrings(p.AdvertiseTags, p2.AdvertiseTags) &&
 		p.Persist.Equals(p2.Persist) &&
 		p.ProfileName == p2.ProfileName &&
-		p.AutoUpdate == p2.AutoUpdate &&
+		p.AutoUpdate.Equals(p2.AutoUpdate) &&
 		p.AppConnector == p2.AppConnector &&
 		p.PostureChecking == p2.PostureChecking &&
 		p.NetfilterKind == p2.NetfilterKind
 }
 
 func (au AutoUpdatePrefs) Pretty() string {
-	if au.Apply {
+	if au.Apply.EqualBool(true) {
 		return "update=on "
 	}
 	if au.Check {
@@ -600,7 +611,7 @@ func NewPrefs() *Prefs {
 		NetfilterMode:    preftype.NetfilterOn,
 		AutoUpdate: AutoUpdatePrefs{
 			Check: true,
-			Apply: false,
+			Apply: opt.Bool("unset"),
 		},
 	}
 }

ipn/prefs_test.go

@@ -20,6 +20,7 @@
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
 	"tailscale.com/types/key"
+	"tailscale.com/types/opt"
 	"tailscale.com/types/persist"
 	"tailscale.com/types/preftype"
 )
@@ -294,18 +295,18 @@ func TestPrefsEqual(t *testing.T) {
 			false,
 		},
 		{
-			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: false}},
-			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: false, Apply: false}},
+			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: opt.NewBool(false)}},
+			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: false, Apply: opt.NewBool(false)}},
 			false,
 		},
 		{
-			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: true}},
-			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: false}},
+			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: opt.NewBool(true)}},
+			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: opt.NewBool(false)}},
 			false,
 		},
 		{
-			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: false}},
-			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: false}},
+			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: opt.NewBool(false)}},
+			&Prefs{AutoUpdate: AutoUpdatePrefs{Check: true, Apply: opt.NewBool(false)}},
 			true,
 		},
 		{
@@ -522,7 +523,7 @@ func TestPrefsPretty(t *testing.T) {
 			Prefs{
 				AutoUpdate: AutoUpdatePrefs{
 					Check: true,
-					Apply: false,
+					Apply: opt.NewBool(false),
 				},
 			},
 			"linux",
@@ -532,7 +533,7 @@ func TestPrefsPretty(t *testing.T) {
 			Prefs{
 				AutoUpdate: AutoUpdatePrefs{
 					Check: true,
-					Apply: true,
+					Apply: opt.NewBool(true),
 				},
 			},
 			"linux",
@@ -764,7 +765,7 @@ func TestMaskedPrefsPretty(t *testing.T) {
 		{
 			m: &MaskedPrefs{
 				Prefs: Prefs{
-					AutoUpdate: AutoUpdatePrefs{Check: true, Apply: false},
+					AutoUpdate: AutoUpdatePrefs{Check: true, Apply: opt.NewBool(false)},
 				},
 				AutoUpdateSet: AutoUpdatePrefsMask{CheckSet: true, ApplySet: false},
 			},
@@ -773,7 +774,7 @@ func TestMaskedPrefsPretty(t *testing.T) {
 		{
 			m: &MaskedPrefs{
 				Prefs: Prefs{
-					AutoUpdate: AutoUpdatePrefs{Check: true, Apply: true},
+					AutoUpdate: AutoUpdatePrefs{Check: true, Apply: opt.NewBool(true)},
 				},
 				AutoUpdateSet: AutoUpdatePrefsMask{CheckSet: true, ApplySet: true},
 			},
@@ -782,7 +783,7 @@ func TestMaskedPrefsPretty(t *testing.T) {
 		{
 			m: &MaskedPrefs{
 				Prefs: Prefs{
-					AutoUpdate: AutoUpdatePrefs{Check: true, Apply: false},
+					AutoUpdate: AutoUpdatePrefs{Check: true, Apply: opt.NewBool(false)},
 				},
 				AutoUpdateSet: AutoUpdatePrefsMask{CheckSet: false, ApplySet: true},
 			},
@@ -791,7 +792,7 @@ func TestMaskedPrefsPretty(t *testing.T) {
 		{
 			m: &MaskedPrefs{
 				Prefs: Prefs{
-					AutoUpdate: AutoUpdatePrefs{Check: true, Apply: true},
+					AutoUpdate: AutoUpdatePrefs{Check: true, Apply: opt.NewBool(true)},
 				},
 				AutoUpdateSet: AutoUpdatePrefsMask{CheckSet: false, ApplySet: false},
 			},

tailcfg/tailcfg.go

@@ -123,7 +123,8 @@
 //   - 80: 2023-11-16: can handle c2n GET /tls-cert-status
 //   - 81: 2023-11-17: MapResponse.PacketFilters (incremental packet filter updates)
 //   - 82: 2023-12-01: Client understands NodeAttrLinuxMustUseIPTables, NodeAttrLinuxMustUseNfTables, c2n /netfilter-kind
-const CurrentCapabilityVersion CapabilityVersion = 82
+//   - 83: 2023-12-18: Client understands DefaultAutoUpdate
+const CurrentCapabilityVersion CapabilityVersion = 83
 
 type StableID string
 
@@ -1877,6 +1878,13 @@ type MapResponse struct {
 	// download and whether the client is using it. A nil value means no change
 	// or nothing to report.
 	ClientVersion *ClientVersion `json:",omitempty"`
+
+	// DefaultAutoUpdate is the default node auto-update setting for this
+	// tailnet. The node is free to opt-in or out locally regardless of this
+	// value. This value is only used on first MapResponse from control, the
+	// auto-update setting doesn't change if the tailnet admin flips the
+	// default after the node registered.
+	DefaultAutoUpdate opt.Bool `json:",omitempty"`
 }
 
 // ClientVersion is information about the latest client version that's available

types/netmap/nodemut.go

@@ -176,5 +176,6 @@ func mapResponseContainsNonPatchFields(res *tailcfg.MapResponse) bool {
 		// PeersChanged to PeersChangedPatch in patchifyPeersChanged before this
 		// function is called, so it should never be set anyway. But for
 		// completedness, and for tests, check it too:
-		res.PeersChanged != nil
+		res.PeersChanged != nil ||
+		res.DefaultAutoUpdate != ""
 }

types/netmap/nodemut_test.go

@@ -13,6 +13,7 @@
 	"github.com/google/go-cmp/cmp"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/logger"
+	"tailscale.com/types/opt"
 	"tailscale.com/types/ptr"
 )
 
@@ -26,6 +27,9 @@ func TestMapResponseContainsNonPatchFields(t *testing.T) {
 		case reflect.Bool:
 			return reflect.ValueOf(true)
 		case reflect.String:
+			if reflect.TypeOf(opt.Bool("")) == t {
+				return reflect.ValueOf("true").Convert(t)
+			}
 			return reflect.ValueOf("foo").Convert(t)
 		case reflect.Int64:
 			return reflect.ValueOf(int64(1))

types/opt/bool.go

@@ -18,6 +18,12 @@
 // field without it being dropped.
 type Bool string
 
+// NewBool constructs a new Bool value equal to b. The returned Bool is set,
+// unless Set("") or Clear() methods are called.
+func NewBool(b bool) Bool {
+	return Bool(strconv.FormatBool(b))
+}
+
 func (b *Bool) Set(v bool) {
 	*b = Bool(strconv.FormatBool(v))
 }