From 1ef8fbf4705637ee73c46300566e3df56c4885e4 Mon Sep 17 00:00:00 2001 From: Simon Law Date: Wed, 23 Jul 2025 11:50:42 -0700 Subject: [PATCH] ipn/ipnlocal: send Hostinfo after resolveExitNode for "auto:any" (#16632) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In #16625, I introduced a mechanism for sending the selected exit node to Control via tailcfg.Hostinfo.ExitNodeID as part of the MapRequest. @nickkhyl pointed out that LocalBackend.doSetHostinfoFilterServices needs to be triggered in order to actually send this update. This patch adds that command. It also prevents the client from sending "auto:any" in that field, because that’s not a real exit node ID. This patch also fills in some missing checks in TestConfigureExitNode. Updates tailscale/corp#30536 Signed-off-by: Simon Law --- ipn/ipnlocal/local.go | 40 +++++++++++++++++----- ipn/ipnlocal/local_test.go | 69 +++++++++++++++++++------------------- 2 files changed, 66 insertions(+), 43 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index ce0f4f687..7154b942c 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -5614,9 +5614,22 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip hi.AppConnector.Set(prefs.AppConnector().Advertise) // The [tailcfg.Hostinfo.ExitNodeID] field tells control which exit node - // was selected, if any. Since [LocalBackend.resolveExitNodeIPLocked] - // has already run, there is no need to consult [ipn.Prefs.ExitNodeIP]. - hi.ExitNodeID = prefs.ExitNodeID() + // was selected, if any. + // + // If auto exit node is enabled (via [ipn.Prefs.AutoExitNode] or + // [syspolicy.ExitNodeID]), or an exit node is specified by ExitNodeIP + // instead of ExitNodeID , and we don't yet have enough info to resolve + // it (usually due to missing netmap or net report), then ExitNodeID in + // the prefs may be invalid (typically, [unresolvedExitNodeID]) until + // the netmap is available. + // + // In this case, we shouldn't update the Hostinfo with the bogus + // ExitNodeID here; [LocalBackend.ResolveExitNode] will be called once + // the netmap and/or net report have been received to both pick the exit + // node and notify control of the change. + if sid := prefs.ExitNodeID(); sid != unresolvedExitNodeID { + hi.ExitNodeID = prefs.ExitNodeID() + } } // enterState transitions the backend into newState, updating internal @@ -6117,9 +6130,10 @@ func (b *LocalBackend) RefreshExitNode() { } } -// resolveExitNode determines which exit node to use based on the current -// prefs and netmap. It updates the exit node ID in the prefs if needed, -// sends a notification to clients, and returns true if the exit node has changed. +// resolveExitNode determines which exit node to use based on the current prefs +// and netmap. It updates the exit node ID in the prefs if needed, updates the +// exit node ID in the hostinfo if needed, sends a notification to clients, and +// returns true if the exit node has changed. // // It is the caller's responsibility to reconfigure routes and actually // start using the selected exit node, if needed. @@ -6142,8 +6156,18 @@ func (b *LocalBackend) resolveExitNode() (changed bool) { b.logf("failed to save exit node changes: %v", err) } - // Send the resolved exit node to Control via Hostinfo. - b.hostinfo.ExitNodeID = prefs.ExitNodeID + // Send the resolved exit node to control via [tailcfg.Hostinfo]. + // [LocalBackend.applyPrefsToHostinfoLocked] usually sets the Hostinfo, + // but it deferred until this point because there was a bogus ExitNodeID + // in the prefs. + // + // TODO(sfllaw): Mutating b.hostinfo here is undesirable, mutating + // in-place doubly so. + sid := prefs.ExitNodeID + if sid != unresolvedExitNodeID && b.hostinfo.ExitNodeID != sid { + b.hostinfo.ExitNodeID = sid + b.goTracker.Go(b.doSetHostinfoFilterServices) + } b.sendToLocked(ipn.Notify{Prefs: ptr.To(prefs.View())}, allClients) return true diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index da6fc8b4a..dd2837022 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -627,7 +627,7 @@ func TestConfigureExitNode(t *testing.T) { wantChangePrefsErr error // if non-nil, the error we expect from [LocalBackend.EditPrefsAs] wantPrefs ipn.Prefs wantExitNodeToggleErr error // if non-nil, the error we expect from [LocalBackend.SetUseExitNodeEnabled] - wantHostinfoExitNodeID *tailcfg.StableNodeID + wantHostinfoExitNodeID tailcfg.StableNodeID }{ { name: "exit-node-id-via-prefs", // set exit node ID via prefs @@ -644,7 +644,7 @@ func TestConfigureExitNode(t *testing.T) { ControlURL: controlURL, ExitNodeID: exitNode1.StableID(), }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "exit-node-ip-via-prefs", // set exit node IP via prefs (should be resolved to an ID) @@ -661,7 +661,7 @@ func TestConfigureExitNode(t *testing.T) { ControlURL: controlURL, ExitNodeID: exitNode1.StableID(), }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-exit-node-via-prefs/any", // set auto exit node via prefs @@ -679,7 +679,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode1.StableID(), AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-exit-node-via-prefs/set-exit-node-id-via-prefs", // setting exit node ID explicitly should disable auto exit node @@ -699,7 +699,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode2.StableID(), AutoExitNode: "", // should be unset }, - wantHostinfoExitNodeID: ptr.To(exitNode2.StableID()), + wantHostinfoExitNodeID: exitNode2.StableID(), }, { name: "auto-exit-node-via-prefs/any/no-report", // set auto exit node via prefs, but no report means we can't resolve the exit node ID @@ -716,7 +716,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: unresolvedExitNodeID, // cannot resolve; traffic will be dropped AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(unresolvedExitNodeID), + wantHostinfoExitNodeID: "", }, { name: "auto-exit-node-via-prefs/any/no-netmap", // similarly, but without a netmap (no exit node should be selected) @@ -733,7 +733,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: unresolvedExitNodeID, // cannot resolve; traffic will be dropped AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(unresolvedExitNodeID), + wantHostinfoExitNodeID: "", }, { name: "auto-exit-node-via-prefs/foo", // set auto exit node via prefs with an unknown/unsupported expression @@ -751,7 +751,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode1.StableID(), // unknown exit node expressions should work as "any" AutoExitNode: "foo", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-exit-node-via-prefs/off", // toggle the exit node off after it was set to "any" @@ -771,7 +771,7 @@ func TestConfigureExitNode(t *testing.T) { AutoExitNode: "", InternalExitNodePrior: "auto:any", }, - wantHostinfoExitNodeID: ptr.To(tailcfg.StableNodeID("")), + wantHostinfoExitNodeID: "", }, { name: "auto-exit-node-via-prefs/on", // toggle the exit node on @@ -788,7 +788,7 @@ func TestConfigureExitNode(t *testing.T) { AutoExitNode: "any", InternalExitNodePrior: "auto:any", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "id-via-policy", // set exit node ID via syspolicy @@ -801,7 +801,7 @@ func TestConfigureExitNode(t *testing.T) { ControlURL: controlURL, ExitNodeID: exitNode1.StableID(), }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "id-via-policy/cannot-override-via-prefs/by-id", // syspolicy should take precedence over prefs @@ -820,7 +820,7 @@ func TestConfigureExitNode(t *testing.T) { ControlURL: controlURL, ExitNodeID: exitNode1.StableID(), }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), wantChangePrefsErr: errManagedByPolicy, }, { @@ -840,7 +840,7 @@ func TestConfigureExitNode(t *testing.T) { ControlURL: controlURL, ExitNodeID: exitNode1.StableID(), }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), wantChangePrefsErr: errManagedByPolicy, }, { @@ -860,7 +860,8 @@ func TestConfigureExitNode(t *testing.T) { ControlURL: controlURL, ExitNodeID: exitNode1.StableID(), }, - wantChangePrefsErr: errManagedByPolicy, + wantHostinfoExitNodeID: exitNode1.StableID(), + wantChangePrefsErr: errManagedByPolicy, }, { name: "ip-via-policy", // set exit node IP via syspolicy (should be resolved to an ID) @@ -873,7 +874,7 @@ func TestConfigureExitNode(t *testing.T) { ControlURL: controlURL, ExitNodeID: exitNode2.StableID(), }, - wantHostinfoExitNodeID: ptr.To(exitNode2.StableID()), + wantHostinfoExitNodeID: exitNode2.StableID(), }, { name: "auto-any-via-policy", // set auto exit node via syspolicy (an exit node should be selected) @@ -888,7 +889,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode1.StableID(), AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-any-via-policy/no-report", // set auto exit node via syspolicy without a netcheck report (no exit node should be selected) @@ -903,7 +904,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: unresolvedExitNodeID, AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(unresolvedExitNodeID), + wantHostinfoExitNodeID: "", }, { name: "auto-any-via-policy/no-netmap", // similarly, but without a netmap (no exit node should be selected) @@ -918,7 +919,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: unresolvedExitNodeID, AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(unresolvedExitNodeID), + wantHostinfoExitNodeID: "", }, { name: "auto-any-via-policy/no-netmap/with-existing", // set auto exit node via syspolicy without a netmap, but with a previously set exit node ID @@ -935,7 +936,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode2.StableID(), AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(exitNode2.StableID()), + wantHostinfoExitNodeID: exitNode2.StableID(), }, { name: "auto-any-via-policy/no-netmap/with-allowed-existing", // same, but now with a syspolicy setting that explicitly allows the existing exit node ID @@ -954,7 +955,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode2.StableID(), AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(exitNode2.StableID()), + wantHostinfoExitNodeID: exitNode2.StableID(), }, { name: "auto-any-via-policy/no-netmap/with-disallowed-existing", // same, but now with a syspolicy setting that does not allow the existing exit node ID @@ -973,7 +974,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: unresolvedExitNodeID, // we don't have a netmap yet, and the current exit node ID is not allowed; block traffic AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(unresolvedExitNodeID), + wantHostinfoExitNodeID: "", }, { name: "auto-any-via-policy/with-netmap/with-allowed-existing", // same, but now with a syspolicy setting that does not allow the existing exit node ID @@ -992,7 +993,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode2.StableID(), // we have a netmap; switch to the best allowed exit node AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(exitNode2.StableID()), + wantHostinfoExitNodeID: exitNode2.StableID(), }, { name: "auto-any-via-policy/with-netmap/switch-to-better", // if all exit nodes are allowed, switch to the best one once we have a netmap @@ -1008,7 +1009,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode1.StableID(), // switch to the best exit node AutoExitNode: "any", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-foo-via-policy", // set auto exit node via syspolicy with an unknown/unsupported expression @@ -1023,7 +1024,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode1.StableID(), // unknown exit node expressions should work as "any" AutoExitNode: "foo", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-foo-via-edit-prefs", // set auto exit node via EditPrefs with an unknown/unsupported expression @@ -1041,7 +1042,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode1.StableID(), // unknown exit node expressions should work as "any" AutoExitNode: "foo", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-any-via-policy/toggle-off", // cannot toggle off the exit node if it was set via syspolicy @@ -1059,7 +1060,7 @@ func TestConfigureExitNode(t *testing.T) { AutoExitNode: "any", InternalExitNodePrior: "", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-any-via-policy/allow-override/change", // changing the exit node is allowed by [syspolicy.AllowExitNodeOverride] @@ -1081,7 +1082,7 @@ func TestConfigureExitNode(t *testing.T) { ExitNodeID: exitNode2.StableID(), // overridden by user AutoExitNode: "", // cleared, as we are setting the exit node ID explicitly }, - wantHostinfoExitNodeID: ptr.To(exitNode2.StableID()), + wantHostinfoExitNodeID: exitNode2.StableID(), }, { name: "auto-any-via-policy/allow-override/clear", // clearing the exit node ID is not allowed by [syspolicy.AllowExitNodeOverride] @@ -1105,7 +1106,7 @@ func TestConfigureExitNode(t *testing.T) { AutoExitNode: "any", InternalExitNodePrior: "", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-any-via-policy/allow-override/toggle-off", // similarly, toggling off the exit node is not allowed even with [syspolicy.AllowExitNodeOverride] @@ -1124,7 +1125,7 @@ func TestConfigureExitNode(t *testing.T) { AutoExitNode: "any", InternalExitNodePrior: "", }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, { name: "auto-any-via-initial-prefs/no-netmap/clear-auto-exit-node", @@ -1145,7 +1146,7 @@ func TestConfigureExitNode(t *testing.T) { AutoExitNode: "", // cleared ExitNodeID: "", // has never been resolved, so it should be cleared as well }, - wantHostinfoExitNodeID: ptr.To(tailcfg.StableNodeID("")), + wantHostinfoExitNodeID: "", }, { name: "auto-any-via-initial-prefs/with-netmap/clear-auto-exit-node", @@ -1166,7 +1167,7 @@ func TestConfigureExitNode(t *testing.T) { AutoExitNode: "", // cleared ExitNodeID: exitNode1.StableID(), // a resolved exit node ID should be retained }, - wantHostinfoExitNodeID: ptr.To(exitNode1.StableID()), + wantHostinfoExitNodeID: exitNode1.StableID(), }, } syspolicy.RegisterWellKnownSettingsForTest(t) @@ -1229,10 +1230,8 @@ func TestConfigureExitNode(t *testing.T) { } // And check Hostinfo. - if tt.wantHostinfoExitNodeID != nil { - if got := lb.hostinfo.ExitNodeID; got != *tt.wantHostinfoExitNodeID { - t.Errorf("Hostinfo.ExitNodeID got %v, want %v", got, *tt.wantHostinfoExitNodeID) - } + if got := lb.hostinfo.ExitNodeID; got != tt.wantHostinfoExitNodeID { + t.Errorf("Hostinfo.ExitNodeID got %s, want %s", got, tt.wantHostinfoExitNodeID) } }) }