From 2c630e126b84b537053947b579f0b44623deb496 Mon Sep 17 00:00:00 2001 From: Nick Khyl Date: Mon, 7 Jul 2025 19:05:41 -0500 Subject: [PATCH] ipn/ipnlocal: make applySysPolicy a method on LocalBackend Now that applySysPolicy is only called by (*LocalBackend).reconcilePrefsLocked, we can make it a method to avoid passing state via parameters and to support future extensibility. Also factor out exit node-specific logic into applyExitNodeSysPolicyLocked. Updates tailscale/corp#29969 Signed-off-by: Nick Khyl --- ipn/ipnlocal/local.go | 52 ++++++++++++++++++++++++-------------- ipn/ipnlocal/local_test.go | 6 +++-- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 221edad92..9ed9522ab 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1800,9 +1800,11 @@ var preferencePolicies = []preferencePolicyInfo{ }, } -// applySysPolicy overwrites configured preferences with policies that may be +// applySysPolicyLocked overwrites configured preferences with policies that may be // configured by the system administrator in an OS-specific way. -func applySysPolicy(prefs *ipn.Prefs, overrideAlwaysOn bool) (anyChange bool) { +// +// b.mu must be held. +func (b *LocalBackend) applySysPolicyLocked(prefs *ipn.Prefs) (anyChange bool) { if controlURL, err := syspolicy.GetString(syspolicy.ControlURL, prefs.ControlURL); err == nil && prefs.ControlURL != controlURL { prefs.ControlURL = controlURL anyChange = true @@ -1839,6 +1841,34 @@ func applySysPolicy(prefs *ipn.Prefs, overrideAlwaysOn bool) (anyChange bool) { } } + if b.applyExitNodeSysPolicyLocked(prefs) { + anyChange = true + } + + if alwaysOn, _ := syspolicy.GetBoolean(syspolicy.AlwaysOn, false); alwaysOn && !b.overrideAlwaysOn && !prefs.WantRunning { + prefs.WantRunning = true + anyChange = true + } + + for _, opt := range preferencePolicies { + if po, err := syspolicy.GetPreferenceOption(opt.key); err == nil { + curVal := opt.get(prefs.View()) + newVal := po.ShouldEnable(curVal) + if curVal != newVal { + opt.set(prefs, newVal) + anyChange = true + } + } + } + + return anyChange +} + +// applyExitNodeSysPolicyLocked applies the exit node policy settings to prefs +// and reports whether any change was made. +// +// b.mu must be held. +func (b *LocalBackend) applyExitNodeSysPolicyLocked(prefs *ipn.Prefs) (anyChange bool) { if exitNodeIDStr, _ := syspolicy.GetString(syspolicy.ExitNodeID, ""); exitNodeIDStr != "" { exitNodeID := tailcfg.StableNodeID(exitNodeIDStr) @@ -1894,22 +1924,6 @@ func applySysPolicy(prefs *ipn.Prefs, overrideAlwaysOn bool) (anyChange bool) { } } - if alwaysOn, _ := syspolicy.GetBoolean(syspolicy.AlwaysOn, false); alwaysOn && !overrideAlwaysOn && !prefs.WantRunning { - prefs.WantRunning = true - anyChange = true - } - - for _, opt := range preferencePolicies { - if po, err := syspolicy.GetPreferenceOption(opt.key); err == nil { - curVal := opt.get(prefs.View()) - newVal := po.ShouldEnable(curVal) - if curVal != newVal { - opt.set(prefs, newVal) - anyChange = true - } - } - } - return anyChange } @@ -6024,7 +6038,7 @@ func (b *LocalBackend) resolveExitNode() (changed bool) { // // b.mu must be held. func (b *LocalBackend) reconcilePrefsLocked(prefs *ipn.Prefs) (changed bool) { - if applySysPolicy(prefs, b.overrideAlwaysOn) { + if b.applySysPolicyLocked(prefs) { changed = true } if b.resolveExitNodeInPrefsLocked(prefs) { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 1e1b7663a..b8526a4fc 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -2968,7 +2968,8 @@ func TestApplySysPolicy(t *testing.T) { t.Run("unit", func(t *testing.T) { prefs := tt.prefs.Clone() - gotAnyChange := applySysPolicy(prefs, false) + lb := newTestLocalBackend(t) + gotAnyChange := lb.applySysPolicyLocked(prefs) if gotAnyChange && prefs.Equals(&tt.prefs) { t.Errorf("anyChange but prefs is unchanged: %v", prefs.Pretty()) @@ -3116,7 +3117,8 @@ func TestPreferencePolicyInfo(t *testing.T) { prefs := defaultPrefs.AsStruct() pp.set(prefs, tt.initialValue) - gotAnyChange := applySysPolicy(prefs, false) + lb := newTestLocalBackend(t) + gotAnyChange := lb.applySysPolicyLocked(prefs) if gotAnyChange != tt.wantChange { t.Errorf("anyChange=%v, want %v", gotAnyChange, tt.wantChange)