diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 7ff2633f1..fab8a76c1 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -151,12 +151,6 @@ 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 @@ -330,10 +324,6 @@ 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. @@ -5967,8 +5957,7 @@ func (b *LocalBackend) resetForProfileChangeLockedOnEntry(unlock unlockOnce) err } b.lastServeConfJSON = mem.B(nil) b.serveConfig = ipn.ServeConfigView{} - b.lastSuggestedExitNode = lastSuggestedExitNode{} // Reset last suggested exit node. - b.enterStateLockedOnEntry(ipn.NoState, unlock) // Reset state; releases b.mu + b.enterStateLockedOnEntry(ipn.NoState, unlock) // Reset state; releases b.mu b.health.SetLocalLogConfigHealth(nil) return b.Start(ipn.Options{}) } @@ -6345,7 +6334,6 @@ 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 @@ -6358,41 +6346,13 @@ 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 { - last, err := suggestLastExitNode(lastSuggestedExitNode) - if err != nil { - return response, ErrCannotSuggestExitNode - } - return last, err + return response, ErrCannotSuggestExitNode } seed := time.Now().UnixNano() r := rand.New(rand.NewSource(seed)) - 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 + return suggestExitNode(lastReport, netMap, r) } 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 b05f2950c..25ea71636 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -25,7 +25,6 @@ "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" @@ -3438,357 +3437,6 @@ 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 8505aff44..f30682f13 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -3088,9 +3088,3 @@ 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) -}