From 6f7ed94ccf00de729f25a1bbc9bddce14524da29 Mon Sep 17 00:00:00 2001 From: Jonathan Nobels Date: Fri, 1 Aug 2025 08:29:51 -0400 Subject: [PATCH] ipn/local: add the suggested exit node to the ipn bus fixes tailscale/corp#26369 The suggested exit node is currently only calculated during a localAPI request. For older UIs, this wasn't a bad choice - we could just fetch it on-demand when a menu presented itself. For newer incarnations however, this is an always-visible field that needs to react to changes in the suggested exit node's value. This change recalculates the suggested exit node ID on netmap updates and broadcasts it on the IPN bus. The localAPI version of this remains intact for the time being. Signed-off-by: Jonathan Nobels --- ipn/backend.go | 24 +++++++++++++++++------- ipn/ipnlocal/bus.go | 3 ++- ipn/ipnlocal/local.go | 19 ++++++++++++++++++- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/ipn/backend.go b/ipn/backend.go index ab01d2fde..fd4442f71 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -83,6 +83,8 @@ const ( NotifyRateLimit NotifyWatchOpt = 1 << 8 // if set, rate limit spammy netmap updates to every few seconds NotifyHealthActions NotifyWatchOpt = 1 << 9 // if set, include PrimaryActions in health.State. Otherwise append the action URL to the text + + NotifyInitialSuggestedExitNode NotifyWatchOpt = 1 << 10 // if set, the first Notify message (sent immediately) will contain the current SuggestedExitNode if available ) // Notify is a communication from a backend (e.g. tailscaled) to a frontend @@ -98,7 +100,7 @@ type Notify struct { // This field is only set in the first message when requesting // NotifyInitialState. Clients must store it on their side as // following notifications will not include this field. - SessionID string `json:",omitempty"` + SessionID string `json:",omitzero"` // ErrMessage, if non-nil, contains a critical error message. // For State InUseOtherUser, ErrMessage is not critical and just contains the details. @@ -116,7 +118,7 @@ type Notify struct { // user's preferred storage location. // // Deprecated: use LocalClient.AwaitWaitingFiles instead. - FilesWaiting *empty.Message `json:",omitempty"` + FilesWaiting *empty.Message `json:",omitzero"` // IncomingFiles, if non-nil, specifies which files are in the // process of being received. A nil IncomingFiles means this @@ -125,22 +127,22 @@ type Notify struct { // of being transferred. // // Deprecated: use LocalClient.AwaitWaitingFiles instead. - IncomingFiles []PartialFile `json:",omitempty"` + IncomingFiles []PartialFile `json:",omitzero"` // OutgoingFiles, if non-nil, tracks which files are in the process of // being sent via TailDrop, including files that finished, whether // successful or failed. This slice is sorted by Started time, then Name. - OutgoingFiles []*OutgoingFile `json:",omitempty"` + OutgoingFiles []*OutgoingFile `json:",omitzero"` // LocalTCPPort, if non-nil, informs the UI frontend which // (non-zero) localhost TCP port it's listening on. // This is currently only used by Tailscale when run in the // macOS Network Extension. - LocalTCPPort *uint16 `json:",omitempty"` + LocalTCPPort *uint16 `json:",omitzero"` // ClientVersion, if non-nil, describes whether a client version update // is available. - ClientVersion *tailcfg.ClientVersion `json:",omitempty"` + ClientVersion *tailcfg.ClientVersion `json:",omitzero"` // DriveShares tracks the full set of current DriveShares that we're // publishing. Some client applications, like the MacOS and Windows clients, @@ -153,7 +155,11 @@ type Notify struct { // Health is the last-known health state of the backend. When this field is // non-nil, a change in health verified, and the API client should surface // any changes to the user in the UI. - Health *health.State `json:",omitempty"` + Health *health.State `json:",omitzero"` + + // SuggestedExitNode, if non-nil, is the node that the backend has determined to + // be the best exit node for the current network conditions. + SuggestedExitNode *tailcfg.StableNodeID `json:",omitzero"` // type is mirrored in xcode/IPN/Core/LocalAPI/Model/LocalAPIModel.swift } @@ -194,6 +200,10 @@ func (n Notify) String() string { if n.Health != nil { sb.WriteString("Health{...} ") } + if n.SuggestedExitNode != nil { + fmt.Fprintf(&sb, "SuggestedExitNode=%v ", *n.SuggestedExitNode) + } + s := sb.String() return s[0:len(s)-1] + "}" } diff --git a/ipn/ipnlocal/bus.go b/ipn/ipnlocal/bus.go index 111a877d8..910e4e774 100644 --- a/ipn/ipnlocal/bus.go +++ b/ipn/ipnlocal/bus.go @@ -156,5 +156,6 @@ func isNotableNotify(n *ipn.Notify) bool { n.Health != nil || len(n.IncomingFiles) > 0 || len(n.OutgoingFiles) > 0 || - n.FilesWaiting != nil + n.FilesWaiting != nil || + n.SuggestedExitNode != nil } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 5fb3d5771..bd7fdf8a4 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1741,6 +1741,10 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control b.send(ipn.Notify{NetMap: st.NetMap}) + // The error here is unimportant as is the result. This will recalculate the suggested exit node + // cache the value and push any changes to the IPN bus. + b.SuggestExitNode() + // Check and update the exit node if needed, now that we have a new netmap. // // This must happen after the netmap change is sent via [ipn.Notify], @@ -2038,6 +2042,9 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo } if cn.NetMap() != nil && mutationsAreWorthyOfTellingIPNBus(muts) { + // Recompute the suggested exit node + b.suggestExitNodeLocked() + nm := cn.netMapWithPeers() notify = &ipn.Notify{NetMap: nm} } else if testenv.InTest() { @@ -3068,7 +3075,7 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A b.mu.Lock() - const initialBits = ipn.NotifyInitialState | ipn.NotifyInitialPrefs | ipn.NotifyInitialNetMap | ipn.NotifyInitialDriveShares + const initialBits = ipn.NotifyInitialState | ipn.NotifyInitialPrefs | ipn.NotifyInitialNetMap | ipn.NotifyInitialDriveShares | ipn.NotifyInitialSuggestedExitNode if mask&initialBits != 0 { cn := b.currentNode() ini = &ipn.Notify{Version: version.Long()} @@ -3091,6 +3098,11 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A if mask&ipn.NotifyInitialHealthState != 0 { ini.Health = b.HealthTracker().CurrentState() } + if mask&ipn.NotifyInitialSuggestedExitNode != 0 { + if en, err := b.SuggestExitNode(); err != nil { + ini.SuggestedExitNode = &en.ID + } + } } ctx, cancel := context.WithCancel(ctx) @@ -7706,7 +7718,12 @@ func (b *LocalBackend) suggestExitNodeLocked() (response apitype.ExitNodeSuggest if err != nil { return res, err } + if prevSuggestion != res.ID { + // Notify the clients via the IPN bus if the exit node suggestion has changed. + b.sendToLocked(ipn.Notify{SuggestedExitNode: &res.ID}, allClients) + } b.lastSuggestedExitNode = res.ID + return res, err }