From cc2f4ac921106ad46691b9271008f0bf43aeb970 Mon Sep 17 00:00:00 2001 From: Nick Khyl Date: Wed, 9 Jul 2025 11:59:57 -0500 Subject: [PATCH] ipn: move ParseAutoExitNodeID from ipn/ipnlocal to ipn So it can be used from the CLI without importing ipnlocal. While there, also remove isAutoExitNodeID, a wrapper around parseAutoExitNodeID that's no longer used. Updates tailscale/corp#29969 Updates #cleanup Signed-off-by: Nick Khyl --- ipn/ipnlocal/local.go | 33 ++---------- ipn/ipnlocal/local_test.go | 104 ------------------------------------- ipn/prefs.go | 22 ++++++++ ipn/prefs_test.go | 59 +++++++++++++++++++++ 4 files changed, 84 insertions(+), 134 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index c54cb32d3..048bb1219 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1890,7 +1890,7 @@ func (b *LocalBackend) applyExitNodeSysPolicyLocked(prefs *ipn.Prefs) (anyChange // and update prefs if it differs from the current one. // This includes cases where it was previously an expression but no longer is, // or where it wasn't before but now is. - autoExitNode, useAutoExitNode := parseAutoExitNodeID(exitNodeID) + autoExitNode, useAutoExitNode := ipn.ParseAutoExitNodeString(exitNodeID) if prefs.AutoExitNode != autoExitNode { prefs.AutoExitNode = autoExitNode anyChange = true @@ -4292,7 +4292,7 @@ func (b *LocalBackend) SetUseExitNodeEnabled(actor ipnauth.Actor, v bool) (ipn.P if v { mp.ExitNodeIDSet = true mp.ExitNodeID = p0.InternalExitNodePrior() - if expr, ok := parseAutoExitNodeID(mp.ExitNodeID); ok { + if expr, ok := ipn.ParseAutoExitNodeString(mp.ExitNodeID); ok { mp.AutoExitNodeSet = true mp.AutoExitNode = expr mp.ExitNodeID = unresolvedExitNodeID @@ -4304,7 +4304,7 @@ func (b *LocalBackend) SetUseExitNodeEnabled(actor ipnauth.Actor, v bool) (ipn.P mp.AutoExitNode = "" mp.InternalExitNodePriorSet = true if p0.AutoExitNode().IsSet() { - mp.InternalExitNodePrior = tailcfg.StableNodeID(autoExitNodePrefix + p0.AutoExitNode()) + mp.InternalExitNodePrior = tailcfg.StableNodeID(ipn.AutoExitNodePrefix + p0.AutoExitNode()) } else { mp.InternalExitNodePrior = p0.ExitNodeID() } @@ -7933,10 +7933,6 @@ func longLatDistance(fromLat, fromLong, toLat, toLong float64) float64 { } const ( - // autoExitNodePrefix is the prefix used in [syspolicy.ExitNodeID] values - // to indicate that the string following the prefix is an [ipn.ExitNodeExpression]. - autoExitNodePrefix = "auto:" - // unresolvedExitNodeID is a special [tailcfg.StableNodeID] value // used as an exit node ID to install a blackhole route, preventing // accidental non-exit-node usage until the [ipn.ExitNodeExpression] @@ -7947,29 +7943,6 @@ const ( unresolvedExitNodeID tailcfg.StableNodeID = "auto:any" ) -// isAutoExitNodeID reports whether the given [tailcfg.StableNodeID] is -// actually an "auto:"-prefixed [ipn.ExitNodeExpression]. -func isAutoExitNodeID(id tailcfg.StableNodeID) bool { - _, ok := parseAutoExitNodeID(id) - return ok -} - -// parseAutoExitNodeID attempts to parse the given [tailcfg.StableNodeID] -// as an [ExitNodeExpression]. -// -// It returns the parsed expression and true on success, -// or an empty string and false if the input does not appear to be -// an [ExitNodeExpression] (i.e., it doesn't start with "auto:"). -// -// It is mainly used to parse the [syspolicy.ExitNodeID] value -// when it is set to "auto:" (e.g., auto:any). -func parseAutoExitNodeID(id tailcfg.StableNodeID) (_ ipn.ExitNodeExpression, ok bool) { - if expr, ok := strings.CutPrefix(string(id), autoExitNodePrefix); ok && expr != "" { - return ipn.ExitNodeExpression(expr), true - } - return "", false -} - func isAllowedAutoExitNodeID(exitNodeID tailcfg.StableNodeID) bool { if exitNodeID == "" { return false // an exit node is required diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 8bc84b081..73bae7ede 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -4873,110 +4873,6 @@ func TestMinLatencyDERPregion(t *testing.T) { } } -func TestShouldAutoExitNode(t *testing.T) { - tests := []struct { - name string - exitNodeIDPolicyValue string - expectedBool bool - }{ - { - name: "auto:any", - exitNodeIDPolicyValue: "auto:any", - expectedBool: true, - }, - { - name: "no auto prefix", - exitNodeIDPolicyValue: "foo", - expectedBool: false, - }, - { - name: "auto prefix but empty suffix", - exitNodeIDPolicyValue: "auto:", - expectedBool: false, - }, - { - name: "auto prefix no colon", - exitNodeIDPolicyValue: "auto", - expectedBool: false, - }, - { - name: "auto prefix unknown suffix", - exitNodeIDPolicyValue: "auto:foo", - expectedBool: true, // "auto:{unknown}" is treated as "auto:any" - }, - } - - syspolicy.RegisterWellKnownSettingsForTest(t) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := isAutoExitNodeID(tailcfg.StableNodeID(tt.exitNodeIDPolicyValue)) - if got != tt.expectedBool { - t.Fatalf("expected %v got %v for %v policy value", tt.expectedBool, got, tt.exitNodeIDPolicyValue) - } - }) - } -} - -func TestParseAutoExitNodeID(t *testing.T) { - tests := []struct { - name string - exitNodeID string - wantOk bool - wantExpr ipn.ExitNodeExpression - }{ - { - name: "empty expr", - exitNodeID: "", - wantOk: false, - wantExpr: "", - }, - { - name: "no auto prefix", - exitNodeID: "foo", - wantOk: false, - wantExpr: "", - }, - { - name: "auto:any", - exitNodeID: "auto:any", - wantOk: true, - wantExpr: ipn.AnyExitNode, - }, - { - name: "auto:foo", - exitNodeID: "auto:foo", - wantOk: true, - wantExpr: "foo", - }, - { - name: "auto prefix but empty suffix", - exitNodeID: "auto:", - wantOk: false, - wantExpr: "", - }, - { - name: "auto prefix no colon", - exitNodeID: "auto", - wantOk: false, - wantExpr: "", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotExpr, gotOk := parseAutoExitNodeID(tailcfg.StableNodeID(tt.exitNodeID)) - if gotOk != tt.wantOk || gotExpr != tt.wantExpr { - if tt.wantOk { - t.Fatalf("got %v (%q); want %v (%q)", gotOk, gotExpr, tt.wantOk, tt.wantExpr) - } else { - t.Fatalf("got %v (%q); want false", gotOk, gotExpr) - } - } - }) - } -} - func TestEnableAutoUpdates(t *testing.T) { lb := newTestLocalBackend(t) diff --git a/ipn/prefs.go b/ipn/prefs.go index 77cea0493..71a80b182 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -1088,3 +1088,25 @@ const AnyExitNode ExitNodeExpression = "any" func (e ExitNodeExpression) IsSet() bool { return e != "" } + +const ( + // AutoExitNodePrefix is the prefix used in [syspolicy.ExitNodeID] values and CLI + // to indicate that the string following the prefix is an [ipn.ExitNodeExpression]. + AutoExitNodePrefix = "auto:" +) + +// ParseAutoExitNodeString attempts to parse the given string +// as an [ExitNodeExpression]. +// +// It returns the parsed expression and true on success, +// or an empty string and false if the input does not appear to be +// an [ExitNodeExpression] (i.e., it doesn't start with "auto:"). +// +// It is mainly used to parse the [syspolicy.ExitNodeID] value +// when it is set to "auto:" (e.g., auto:any). +func ParseAutoExitNodeString[T ~string](s T) (_ ExitNodeExpression, ok bool) { + if expr, ok := strings.CutPrefix(string(s), AutoExitNodePrefix); ok && expr != "" { + return ExitNodeExpression(expr), true + } + return "", false +} diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index 268ea206c..43e360c6a 100644 --- a/ipn/prefs_test.go +++ b/ipn/prefs_test.go @@ -1129,3 +1129,62 @@ func TestPrefsDowngrade(t *testing.T) { t.Fatal("AllowSingleHosts should be true") } } + +func TestParseAutoExitNodeString(t *testing.T) { + tests := []struct { + name string + exitNodeID string + wantOk bool + wantExpr ExitNodeExpression + }{ + { + name: "empty expr", + exitNodeID: "", + wantOk: false, + wantExpr: "", + }, + { + name: "no auto prefix", + exitNodeID: "foo", + wantOk: false, + wantExpr: "", + }, + { + name: "auto:any", + exitNodeID: "auto:any", + wantOk: true, + wantExpr: AnyExitNode, + }, + { + name: "auto:foo", + exitNodeID: "auto:foo", + wantOk: true, + wantExpr: "foo", + }, + { + name: "auto prefix but empty suffix", + exitNodeID: "auto:", + wantOk: false, + wantExpr: "", + }, + { + name: "auto prefix no colon", + exitNodeID: "auto", + wantOk: false, + wantExpr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotExpr, gotOk := ParseAutoExitNodeString(tt.exitNodeID) + if gotOk != tt.wantOk || gotExpr != tt.wantExpr { + if tt.wantOk { + t.Fatalf("got %v (%q); want %v (%q)", gotOk, gotExpr, tt.wantOk, tt.wantExpr) + } else { + t.Fatalf("got %v (%q); want false", gotOk, gotExpr) + } + } + }) + } +}