diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 8c8e5ea76..f0eec1281 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -151,6 +151,12 @@ type watchSession struct { sessionID string } +// lastSuggestedExitNode stores the last suggested exit node ID and name in local backend. +type lastSuggestedExitNode struct { + id tailcfg.StableNodeID + name string +} + // LocalBackend is the glue between the major pieces of the Tailscale // network software: the cloud control plane (via controlclient), the // network data plane (via wgengine), and the user-facing UIs and CLIs @@ -324,6 +330,10 @@ type LocalBackend struct { // outgoingFiles keeps track of Taildrop outgoing files keyed to their OutgoingFile.ID outgoingFiles map[string]*ipn.OutgoingFile + + // lastSuggestedExitNode stores the last suggested exit node ID and name. + // lastSuggestedExitNode updates whenever the suggestion changes. + lastSuggestedExitNode lastSuggestedExitNode } // HealthTracker returns the health tracker for the backend. @@ -5958,7 +5968,8 @@ func (b *LocalBackend) resetForProfileChangeLockedOnEntry(unlock unlockOnce) err } b.lastServeConfJSON = mem.B(nil) b.serveConfig = ipn.ServeConfigView{} - b.enterStateLockedOnEntry(ipn.NoState, unlock) // Reset state; releases b.mu + b.lastSuggestedExitNode = lastSuggestedExitNode{} // Reset last suggested exit node. + b.enterStateLockedOnEntry(ipn.NoState, unlock) // Reset state; releases b.mu b.health.SetLocalLogConfigHealth(nil) return b.Start(ipn.Options{}) } @@ -6335,6 +6346,7 @@ func mayDeref[T any](p *T) (v T) { var ErrNoPreferredDERP = errors.New("no preferred DERP, try again later") var ErrCannotSuggestExitNode = errors.New("unable to suggest an exit node, try again later") +var ErrUnableToSuggestLastExitNode = errors.New("unable to suggest last exit node") // SuggestExitNode computes a suggestion based on the current netmap and last netcheck report. If // there are multiple equally good options, one is selected at random, so the result is not stable. To be @@ -6347,13 +6359,41 @@ func (b *LocalBackend) SuggestExitNode() (response apitype.ExitNodeSuggestionRes b.mu.Lock() lastReport := b.MagicConn().GetLastNetcheckReport(b.ctx) netMap := b.netMap + lastSuggestedExitNode := b.lastSuggestedExitNode b.mu.Unlock() if lastReport == nil || netMap == nil { - return response, ErrCannotSuggestExitNode + last, err := suggestLastExitNode(lastSuggestedExitNode) + if err != nil { + return response, ErrCannotSuggestExitNode + } + return last, err } seed := time.Now().UnixNano() r := rand.New(rand.NewSource(seed)) - return suggestExitNode(lastReport, netMap, r) + res, err := suggestExitNode(lastReport, netMap, r) + if err != nil { + last, err := suggestLastExitNode(lastSuggestedExitNode) + if err != nil { + return response, ErrCannotSuggestExitNode + } + return last, err + } + b.mu.Lock() + b.lastSuggestedExitNode.id = res.ID + b.lastSuggestedExitNode.name = res.Name + b.mu.Unlock() + return res, err +} + +// suggestLastExitNode formats a response with the last suggested exit node's ID and name. +// Used as a fallback before returning a nil response and error. +func suggestLastExitNode(lastSuggestedExitNode lastSuggestedExitNode) (res apitype.ExitNodeSuggestionResponse, err error) { + if lastSuggestedExitNode.id != "" && lastSuggestedExitNode.name != "" { + res.ID = lastSuggestedExitNode.id + res.Name = lastSuggestedExitNode.name + return res, nil + } + return res, ErrUnableToSuggestLastExitNode } func suggestExitNode(report *netcheck.Report, netMap *netmap.NetworkMap, r *rand.Rand) (res apitype.ExitNodeSuggestionResponse, err error) { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 88373106a..d8e05237e 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -25,6 +25,7 @@ import ( "golang.org/x/net/dns/dnsmessage" "tailscale.com/appc" "tailscale.com/appc/appctest" + "tailscale.com/client/tailscale/apitype" "tailscale.com/clientupdate" "tailscale.com/control/controlclient" "tailscale.com/drive" @@ -3436,6 +3437,357 @@ func TestMinLatencyDERPregion(t *testing.T) { } } +func TestSuggestLastExitNode(t *testing.T) { + tests := []struct { + name string + lastSuggestedExitNode lastSuggestedExitNode + wantRes apitype.ExitNodeSuggestionResponse + wantLastSuggestedExitNode lastSuggestedExitNode + wantErr error + }{ + { + name: "last suggested exit node is populated", + lastSuggestedExitNode: lastSuggestedExitNode{id: "test", name: "test"}, + wantRes: apitype.ExitNodeSuggestionResponse{ID: "test", Name: "test"}, + wantLastSuggestedExitNode: lastSuggestedExitNode{id: "test", name: "test"}, + }, + { + name: "last suggested exit node is not populated", + wantErr: ErrUnableToSuggestLastExitNode, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := suggestLastExitNode(tt.lastSuggestedExitNode) + if got != tt.wantRes || err != tt.wantErr { + t.Errorf("got %v error %v, want %v error %v", got, err, tt.wantRes, tt.wantErr) + } + }) + } +} + +func TestLocalBackendSuggestExitNode(t *testing.T) { + tests := []struct { + name string + lastSuggestedExitNode lastSuggestedExitNode + report netcheck.Report + netMap netmap.NetworkMap + wantID tailcfg.StableNodeID + wantName string + wantErr error + wantLastSuggestedExitNode lastSuggestedExitNode + }{ + { + name: "nil netmap, returns last suggested exit node", + lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, + report: netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 0, + 2: -1, + 3: 0, + }, + }, + wantID: "test", + wantName: "test", + wantLastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, + }, + { + name: "nil report, returns last suggested exit node", + lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, + netMap: netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{ + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.64.1.1/32"), + netip.MustParsePrefix("fe70::1/128"), + }, + }).View(), + DERPMap: &tailcfg.DERPMap{ + Regions: map[int]*tailcfg.DERPRegion{ + 1: {}, + 2: {}, + 3: {}, + }, + }, + }, + wantID: "test", + wantName: "test", + wantLastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, + }, + { + name: "found better derp node, last suggested exit node updates", + lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, + report: netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 10, + 2: 10, + 3: 5, + }, + PreferredDERP: 1, + }, + netMap: netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{ + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.64.1.1/32"), + netip.MustParsePrefix("fe70::1/128"), + }, + }).View(), + DERPMap: &tailcfg.DERPMap{ + Regions: map[int]*tailcfg.DERPRegion{ + 1: {}, + 2: {}, + 3: {}, + }, + }, + Peers: []tailcfg.NodeView{ + (&tailcfg.Node{ + ID: 2, + StableID: "test", + Name: "test", + DERP: "127.3.3.40:1", + AllowedIPs: []netip.Prefix{ + netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), + }, + CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ + tailcfg.NodeAttrSuggestExitNode: {}, + }), + }).View(), + (&tailcfg.Node{ + ID: 3, + StableID: "foo", + Name: "foo", + DERP: "127.3.3.40:3", + AllowedIPs: []netip.Prefix{ + netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), + }, + CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ + tailcfg.NodeAttrSuggestExitNode: {}, + }), + }).View(), + }, + }, + wantID: "foo", + wantName: "foo", + wantLastSuggestedExitNode: lastSuggestedExitNode{name: "foo", id: "foo"}, + }, + { + name: "found better mullvad node, last suggested exit node updates", + lastSuggestedExitNode: lastSuggestedExitNode{name: "San Jose", id: "3"}, + report: netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 0, + 2: 0, + 3: 0, + }, + PreferredDERP: 1, + }, + netMap: netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{ + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.64.1.1/32"), + netip.MustParsePrefix("fe70::1/128"), + }, + }).View(), + DERPMap: &tailcfg.DERPMap{ + Regions: map[int]*tailcfg.DERPRegion{ + 1: { + Latitude: 40.73061, + Longitude: -73.935242, + }, + 2: {}, + 3: {}, + }, + }, + Peers: []tailcfg.NodeView{ + (&tailcfg.Node{ + ID: 2, + StableID: "2", + AllowedIPs: []netip.Prefix{ + netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), + }, + Name: "Dallas", + Hostinfo: (&tailcfg.Hostinfo{ + Location: &tailcfg.Location{ + Latitude: 32.89748, + Longitude: -97.040443, + Priority: 100, + }, + }).View(), + CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ + tailcfg.NodeAttrSuggestExitNode: {}, + }), + }).View(), + (&tailcfg.Node{ + ID: 3, + StableID: "3", + AllowedIPs: []netip.Prefix{ + netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), + }, + Name: "San Jose", + Hostinfo: (&tailcfg.Hostinfo{ + Location: &tailcfg.Location{ + Latitude: 37.3382082, + Longitude: -121.8863286, + Priority: 20, + }, + }).View(), + CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ + tailcfg.NodeAttrSuggestExitNode: {}, + }), + }).View(), + }, + }, + wantID: "2", + wantName: "Dallas", + wantLastSuggestedExitNode: lastSuggestedExitNode{name: "Dallas", id: "2"}, + }, + { + name: "ErrNoPreferredDERP, use last suggested exit node", + lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, + report: netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 10, + 2: 10, + 3: 5, + }, + PreferredDERP: 0, + }, + netMap: netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{ + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.64.1.1/32"), + netip.MustParsePrefix("fe70::1/128"), + }, + }).View(), + DERPMap: &tailcfg.DERPMap{ + Regions: map[int]*tailcfg.DERPRegion{ + 1: {}, + 2: {}, + 3: {}, + }, + }, + Peers: []tailcfg.NodeView{ + (&tailcfg.Node{ + ID: 2, + StableID: "test", + Name: "test", + DERP: "127.3.3.40:1", + AllowedIPs: []netip.Prefix{ + netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), + }, + CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ + tailcfg.NodeAttrSuggestExitNode: {}, + }), + }).View(), + (&tailcfg.Node{ + ID: 3, + StableID: "foo", + Name: "foo", + DERP: "127.3.3.40:3", + AllowedIPs: []netip.Prefix{ + netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), + }, + CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ + tailcfg.NodeAttrSuggestExitNode: {}, + }), + }).View(), + }, + }, + wantID: "test", + wantName: "test", + wantLastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, + }, + { + name: "ErrNoPreferredDERP, use last suggested exit node", + lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, + report: netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 10, + 2: 10, + 3: 5, + }, + PreferredDERP: 0, + }, + netMap: netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{ + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.64.1.1/32"), + netip.MustParsePrefix("fe70::1/128"), + }, + }).View(), + DERPMap: &tailcfg.DERPMap{ + Regions: map[int]*tailcfg.DERPRegion{ + 1: {}, + 2: {}, + 3: {}, + }, + }, + Peers: []tailcfg.NodeView{ + (&tailcfg.Node{ + ID: 2, + StableID: "test", + Name: "test", + DERP: "127.3.3.40:1", + AllowedIPs: []netip.Prefix{ + netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), + }, + CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ + tailcfg.NodeAttrSuggestExitNode: {}, + }), + }).View(), + (&tailcfg.Node{ + ID: 3, + StableID: "foo", + Name: "foo", + DERP: "127.3.3.40:3", + AllowedIPs: []netip.Prefix{ + netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), + }, + CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ + tailcfg.NodeAttrSuggestExitNode: {}, + }), + }).View(), + }, + }, + wantID: "test", + wantName: "test", + wantLastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, + }, + { + name: "unable to use last suggested exit node", + report: netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 10, + 2: 10, + 3: 5, + }, + PreferredDERP: 0, + }, + wantErr: ErrCannotSuggestExitNode, + }, + } + + for _, tt := range tests { + lb := newTestLocalBackend(t) + lb.lastSuggestedExitNode = tt.lastSuggestedExitNode + lb.netMap = &tt.netMap + lb.sys.MagicSock.Get().SetLastNetcheckReport(context.Background(), tt.report) + got, err := lb.SuggestExitNode() + if got.ID != tt.wantID { + t.Errorf("ID=%v, want=%v", got.ID, tt.wantID) + } + if got.Name != tt.wantName { + t.Errorf("Name=%v, want=%v", got.Name, tt.wantName) + } + if lb.lastSuggestedExitNode != tt.wantLastSuggestedExitNode { + t.Errorf("lastSuggestedExitNode=%v, want=%v", lb.lastSuggestedExitNode, tt.wantLastSuggestedExitNode) + } + if err != tt.wantErr { + t.Errorf("Error=%v, want=%v", err, tt.wantErr) + } + } +} + func TestEnableAutoUpdates(t *testing.T) { lb := newTestLocalBackend(t) diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index f30682f13..8505aff44 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -3088,3 +3088,9 @@ func (c *Conn) GetLastNetcheckReport(ctx context.Context) *netcheck.Report { } return lastReport } + +// SetLastNetcheckReport sets local backend's last netcheck report. +// Used for testing purposes. +func (c *Conn) SetLastNetcheckReport(ctx context.Context, report netcheck.Report) { + c.lastNetCheckReport.Store(&report) +}