From db6447ce63f5dd7374c3df1efdd25b62a12f702e Mon Sep 17 00:00:00 2001 From: Adrian Dewhurst Date: Fri, 31 May 2024 23:21:55 -0400 Subject: [PATCH] ipn/ipnlocal: simplify suggest exit node tests This mostly removes a lot of repetition by predefining some nodes and other data structures, plus adds some helpers for creating Peer entries in the netmap. Several existing test cases were reworked to ensure better coverage of edge cases, and several new test cases were added to handle some additional responsibility that is in (or will be shortly moving in) suggestExitNode(). Updates tailscale/corp#19681 Change-Id: Ie14c2988d7fd482f7d6a877f78525f7788669b85 Signed-off-by: Adrian Dewhurst --- ipn/ipnlocal/local_test.go | 957 +++++++++++++++++-------------------- 1 file changed, 428 insertions(+), 529 deletions(-) diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index e496a266c..a1de50ec5 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -2716,517 +2716,464 @@ func (b *LocalBackend) SetPrefsForTest(newp *ipn.Prefs) { b.setPrefsLockedOnEntry(newp, unlock) } +type peerOptFunc func(*tailcfg.Node) + +func makePeer(id tailcfg.NodeID, opts ...peerOptFunc) tailcfg.NodeView { + node := &tailcfg.Node{ + ID: id, + StableID: tailcfg.StableNodeID(fmt.Sprintf("stable%d", id)), + Name: fmt.Sprintf("peer%d", id), + DERP: fmt.Sprintf("127.3.3.40:%d", id), + } + for _, opt := range opts { + opt(node) + } + return node.View() +} + +func withName(name string) peerOptFunc { + return func(n *tailcfg.Node) { + n.Name = name + } +} + +func withDERP(region int) peerOptFunc { + return func(n *tailcfg.Node) { + n.DERP = fmt.Sprintf("127.3.3.40:%d", region) + } +} + +func withoutDERP() peerOptFunc { + return func(n *tailcfg.Node) { + n.DERP = "" + } +} + +func withLocation(loc tailcfg.LocationView) peerOptFunc { + return func(n *tailcfg.Node) { + var hi *tailcfg.Hostinfo + if n.Hostinfo.Valid() { + hi = n.Hostinfo.AsStruct() + } else { + hi = new(tailcfg.Hostinfo) + } + hi.Location = loc.AsStruct() + + n.Hostinfo = hi.View() + } +} + +func withExitRoutes() peerOptFunc { + return func(n *tailcfg.Node) { + n.AllowedIPs = append(n.AllowedIPs, tsaddr.ExitRoutes()...) + } +} + +func withSuggest() peerOptFunc { + return func(n *tailcfg.Node) { + mak.Set(&n.CapMap, tailcfg.NodeAttrSuggestExitNode, []tailcfg.RawMessage{}) + } +} + func TestSuggestExitNode(t *testing.T) { + defaultDERPMap := &tailcfg.DERPMap{ + Regions: map[int]*tailcfg.DERPRegion{ + 1: { + Latitude: 32, + Longitude: -97, + }, + 2: {}, + 3: {}, + }, + } + + preferred1Report := &netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 10 * time.Millisecond, + 2: 20 * time.Millisecond, + 3: 30 * time.Millisecond, + }, + PreferredDERP: 1, + } + noLatency1Report := &netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 0, + 2: 0, + 3: 0, + }, + PreferredDERP: 1, + } + preferredNoneReport := &netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 10 * time.Millisecond, + 2: 20 * time.Millisecond, + 3: 30 * time.Millisecond, + }, + PreferredDERP: 0, + } + + dallas := tailcfg.Location{ + Latitude: 32.779167, + Longitude: -96.808889, + Priority: 100, + } + sanJose := tailcfg.Location{ + Latitude: 37.3382082, + Longitude: -121.8863286, + Priority: 20, + } + fortWorth := tailcfg.Location{ + Latitude: 32.756389, + Longitude: -97.3325, + Priority: 150, + } + fortWorthLowPriority := tailcfg.Location{ + Latitude: 32.756389, + Longitude: -97.3325, + Priority: 100, + } + + peer1 := makePeer(1, + withExitRoutes(), + withSuggest()) + peer2DERP1 := makePeer(2, + withDERP(1), + withExitRoutes(), + withSuggest()) + peer3 := makePeer(3, + withExitRoutes(), + withSuggest()) + peer4DERP3 := makePeer(4, + withDERP(3), + withExitRoutes(), + withSuggest()) + dallasPeer5 := makePeer(5, + withName("Dallas"), + withoutDERP(), + withExitRoutes(), + withSuggest(), + withLocation(dallas.View())) + sanJosePeer6 := makePeer(6, + withName("San Jose"), + withoutDERP(), + withExitRoutes(), + withSuggest(), + withLocation(sanJose.View())) + fortWorthPeer7 := makePeer(7, + withName("Fort Worth"), + withoutDERP(), + withExitRoutes(), + withSuggest(), + withLocation(fortWorth.View())) + fortWorthPeer8LowPriority := makePeer(8, + withName("Fort Worth Low"), + withoutDERP(), + withExitRoutes(), + withSuggest(), + withLocation(fortWorthLowPriority.View())) + + selfNode := tailcfg.Node{ + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.64.1.1/32"), + netip.MustParsePrefix("fe70::1/128"), + }, + } + + defaultNetmap := &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, + Peers: []tailcfg.NodeView{ + peer2DERP1, + peer3, + }, + } + locationNetmap := &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, + Peers: []tailcfg.NodeView{ + dallasPeer5, + sanJosePeer6, + }, + } + largeNetmap := &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, + Peers: []tailcfg.NodeView{ + peer1, + peer2DERP1, + peer3, + peer4DERP3, + dallasPeer5, + sanJosePeer6, + fortWorthPeer7, + }, + } + tests := []struct { - name string - lastReport netcheck.Report - netMap netmap.NetworkMap + name string + seed int64 + + lastReport *netcheck.Report + netMap *netmap.NetworkMap + lastSuggestion tailcfg.StableNodeID + + allowPolicy []string + wantID tailcfg.StableNodeID wantName string wantLocation tailcfg.LocationView - wantError error + + wantError error }{ { - name: "2 exit nodes in same region", - lastReport: netcheck.Report{ - RegionLatency: map[int]time.Duration{ - 1: 10 * time.Millisecond, - 2: 20 * time.Millisecond, - 3: 30 * time.Millisecond, - }, - 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: {}, - }, - }, + name: "2 exit nodes in same region", + lastReport: preferred1Report, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, Peers: []tailcfg.NodeView{ - (&tailcfg.Node{ - ID: 2, - Name: "2", - StableID: "2", - 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, - Name: "3", - StableID: "3", - 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(), + peer1, + peer2DERP1, }, }, - wantName: "3", - wantID: tailcfg.StableNodeID("3"), + wantName: "peer1", + wantID: "stable1", }, { - name: "2 derp based exit nodes, different regions, no latency measurements", - lastReport: 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: {}, - 2: {}, - 3: {}, - }, - }, - Peers: []tailcfg.NodeView{ - (&tailcfg.Node{ - ID: 2, - StableID: "2", - Name: "2", - DERP: "127.3.3.40:2", - 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: "3", - Name: "3", - 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(), - }, - }, - wantName: "3", - wantID: tailcfg.StableNodeID("3"), + name: "2 exit nodes different regions unknown latency", + lastReport: noLatency1Report, + netMap: defaultNetmap, + wantName: "peer2", + wantID: "stable2", }, { - name: "2 derp based exit nodes, different regions, same latency", - lastReport: netcheck.Report{ + name: "2 derp based exit nodes, different regions, equal latency", + lastReport: &netcheck.Report{ RegionLatency: map[int]time.Duration{ 1: 10, - 2: 10, - 3: 0, + 2: 20, + 3: 10, }, 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: {}, - }, - }, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, Peers: []tailcfg.NodeView{ - (&tailcfg.Node{ - ID: 2, - StableID: "2", - Name: "2", - 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: "3", - Name: "3", - DERP: "127.3.3.40:2", - AllowedIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), - }, - CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ - tailcfg.NodeAttrSuggestExitNode: {}, - }), - }).View(), + peer1, + peer3, }, }, - wantName: "2", - wantID: tailcfg.StableNodeID("2"), + wantName: "peer1", + wantID: "stable1", }, { - name: "mullvad nodes, no derp based exit nodes", - lastReport: 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: {}, - }, - }, + name: "mullvad nodes, no derp based exit nodes", + lastReport: noLatency1Report, + netMap: locationNetmap, + wantID: "stable5", + wantLocation: dallas.View(), + wantName: "Dallas", + }, + { + name: "nearby mullvad nodes with different priorities", + lastReport: noLatency1Report, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, 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(), + dallasPeer5, + sanJosePeer6, + fortWorthPeer7, }, }, - wantID: tailcfg.StableNodeID("2"), - wantLocation: (&tailcfg.Location{ - Latitude: 32.89748, - Longitude: -97.040443, - Priority: 100, - }).View(), - wantName: "Dallas", + wantID: "stable7", + wantLocation: fortWorth.View(), + wantName: "Fort Worth", }, { - name: "mullvad nodes close to each other, different priorities", - lastReport: 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: {}, - }, - }, + name: "nearby mullvad nodes with same priorities", + lastReport: noLatency1Report, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, 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: 10, - }, - }).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: "Fort Worth", - Hostinfo: (&tailcfg.Hostinfo{ - Location: &tailcfg.Location{ - Latitude: 37.768799, - Longitude: -97.309341, - Priority: 50, - }, - }).View(), - CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ - tailcfg.NodeAttrSuggestExitNode: {}, - }), - }).View(), + dallasPeer5, + sanJosePeer6, + fortWorthPeer8LowPriority, }, }, - wantID: tailcfg.StableNodeID("3"), - wantLocation: (&tailcfg.Location{ - Latitude: 37.768799, - Longitude: -97.309341, - Priority: 50, - }).View(), - wantName: "Fort Worth", + wantID: "stable5", + wantLocation: dallas.View(), + wantName: "Dallas", }, { - name: "mullvad nodes, no preferred derp region exit nodes", - lastReport: 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: {}, - }, - }, + name: "mullvad nodes, remaining node is not in preferred derp", + lastReport: noLatency1Report, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, 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: 20, - }, - }).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: 30, - }, - }).View(), - CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ - tailcfg.NodeAttrSuggestExitNode: {}, - }), - }).View(), - (&tailcfg.Node{ - ID: 3, - StableID: "3", - Name: "3", - DERP: "127.3.3.40:2", - AllowedIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), - }, - CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{ - tailcfg.NodeAttrSuggestExitNode: {}, - }), - }).View(), + dallasPeer5, + sanJosePeer6, + peer4DERP3, }, }, - wantID: tailcfg.StableNodeID("3"), - wantName: "3", + wantID: "stable4", + wantName: "peer4", }, { - name: "no mullvad nodes; no derp nodes", - lastReport: netcheck.Report{ - RegionLatency: map[int]time.Duration{ - 1: 0, - 2: 0, - 3: 0, - }, - PreferredDERP: 1, + name: "no peers", + lastReport: noLatency1Report, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, }, - 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: {}, - }, - }, + }, + { + name: "no preferred derp region", + lastReport: preferredNoneReport, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, }, + wantError: ErrNoPreferredDERP, }, { - name: "no preferred derp region", - lastReport: netcheck.Report{ - RegionLatency: map[int]time.Duration{ - 1: 0, - 2: -1, - 3: 0, + name: "missing suggestion capability", + lastReport: noLatency1Report, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, + Peers: []tailcfg.NodeView{ + makePeer(1, withExitRoutes()), + makePeer(2, withLocation(dallas.View()), withExitRoutes()), }, }, - 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: {}, - }, + }, + { + name: "prefer last node", + seed: 1, + lastReport: preferred1Report, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, + Peers: []tailcfg.NodeView{ + peer1, + peer2DERP1, }, }, - wantError: ErrNoPreferredDERP, + lastSuggestion: "stable2", + wantName: "peer2", + wantID: "stable2", }, { - name: "derp exit node and mullvad exit node both with no suggest exit node attribute", - lastReport: netcheck.Report{ - RegionLatency: map[int]time.Duration{ - 1: 0, - 2: 0, - 3: 0, + name: "found better derp node", + lastSuggestion: "stable3", + lastReport: preferred1Report, + netMap: defaultNetmap, + wantID: "stable2", + wantName: "peer2", + }, + { + name: "prefer last mullvad node", + lastSuggestion: "stable2", + lastReport: preferred1Report, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, + Peers: []tailcfg.NodeView{ + dallasPeer5, + sanJosePeer6, + fortWorthPeer8LowPriority, }, - 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: {}, - }, - }, + wantID: "stable5", + wantName: "Dallas", + wantLocation: dallas.View(), + }, + { + name: "prefer better mullvad node", + lastSuggestion: "stable2", + lastReport: preferred1Report, + netMap: &netmap.NetworkMap{ + SelfNode: selfNode.View(), + DERPMap: defaultDERPMap, Peers: []tailcfg.NodeView{ - (&tailcfg.Node{ - ID: 2, - StableID: "2", - Name: "2", - DERP: "127.3.3.40:1", - AllowedIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), - }, - }).View(), - (&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: 30, - }, - }).View(), - }).View(), + dallasPeer5, + sanJosePeer6, + fortWorthPeer7, }, }, + wantID: "stable7", + wantName: "Fort Worth", + wantLocation: fortWorth.View(), + }, + { + name: "large netmap", + seed: 1, + lastReport: preferred1Report, + netMap: largeNetmap, + wantID: "stable2", + wantName: "peer2", + }, + { + name: "no allowed suggestions", + lastReport: preferred1Report, + netMap: largeNetmap, + allowPolicy: []string{}, + }, + { + name: "only derp suggestions", + seed: 1, + lastReport: preferred1Report, + netMap: largeNetmap, + allowPolicy: []string{"stable1", "stable2", "stable3"}, + wantID: "stable2", + wantName: "peer2", + }, + { + name: "only mullvad suggestions", + lastReport: preferred1Report, + netMap: largeNetmap, + allowPolicy: []string{"stable5", "stable6", "stable7"}, + wantID: "stable7", + wantName: "Fort Worth", + wantLocation: fortWorth.View(), + }, + { + name: "only worst derp", + lastReport: preferred1Report, + netMap: largeNetmap, + allowPolicy: []string{"stable3"}, + wantID: "stable3", + wantName: "peer3", + }, + { + name: "only worst mullvad", + lastReport: preferred1Report, + netMap: largeNetmap, + allowPolicy: []string{"stable6"}, + wantID: "stable6", + wantName: "San Jose", + wantLocation: sanJose.View(), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := rand.New(rand.NewSource(100)) - got, err := suggestExitNode(&tt.lastReport, &tt.netMap, r) + mh := mockSyspolicyHandler{ + t: t, + } + if tt.allowPolicy != nil { + mh.stringArrayPolicies = map[syspolicy.Key][]string{ + syspolicy.AllowedSuggestedExitNodes: tt.allowPolicy, + } + } + syspolicy.SetHandlerForTest(t, &mh) + + r := rand.New(rand.NewSource(tt.seed)) + got, err := suggestExitNode(tt.lastReport, tt.netMap, r) if got.Name != tt.wantName { t.Errorf("name=%v, want %v", got.Name, tt.wantName) } @@ -3247,104 +3194,57 @@ func TestSuggestExitNode(t *testing.T) { } func TestSuggestExitNodePickWeighted(t *testing.T) { + location10 := tailcfg.Location{ + Priority: 10, + } + location20 := tailcfg.Location{ + Priority: 20, + } + tests := []struct { name string candidates []tailcfg.NodeView - wantValue tailcfg.NodeView - wantValid bool + wantID tailcfg.StableNodeID }{ { - name: ">1 candidates", + name: "different priorities", candidates: []tailcfg.NodeView{ - (&tailcfg.Node{ - ID: 2, - StableID: "2", - AllowedIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), - }, - Hostinfo: (&tailcfg.Hostinfo{ - Location: &tailcfg.Location{ - Priority: 20, - }, - }).View(), - }).View(), - (&tailcfg.Node{ - ID: 3, - StableID: "3", - AllowedIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), - }, - Hostinfo: (&tailcfg.Hostinfo{ - Location: &tailcfg.Location{ - Priority: 10, - }, - }).View(), - }).View(), + makePeer(2, withExitRoutes(), withLocation(location20.View())), + makePeer(3, withExitRoutes(), withLocation(location10.View())), }, - wantValue: (&tailcfg.Node{ - ID: 2, - StableID: "2", - AllowedIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), - }, - Hostinfo: (&tailcfg.Hostinfo{ - Location: &tailcfg.Location{ - Priority: 20, - }, - }).View(), - }).View(), - wantValid: true, + wantID: "stable2", + }, + { + name: "same priorities", + candidates: []tailcfg.NodeView{ + makePeer(2, withExitRoutes(), withLocation(location10.View())), + makePeer(3, withExitRoutes(), withLocation(location10.View())), + }, + wantID: "stable2", }, { name: "<1 candidates", candidates: []tailcfg.NodeView{}, - wantValid: false, + wantID: "", }, { name: "1 candidate", candidates: []tailcfg.NodeView{ - (&tailcfg.Node{ - ID: 2, - StableID: "2", - AllowedIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), - }, - Hostinfo: (&tailcfg.Hostinfo{ - Location: &tailcfg.Location{ - Priority: 20, - }, - }).View(), - }).View(), + makePeer(2, withExitRoutes(), withLocation(location20.View())), }, - wantValue: (&tailcfg.Node{ - ID: 2, - StableID: "2", - AllowedIPs: []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), - }, - Hostinfo: (&tailcfg.Hostinfo{ - Location: &tailcfg.Location{ - Priority: 20, - }, - }).View(), - }).View(), - wantValid: true, + wantID: "stable2", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := pickWeighted(tt.candidates) - if !reflect.DeepEqual(got, tt.wantValue) { - t.Errorf("got value %v want %v", got, tt.wantValue) - if tt.wantValid != got.Valid() { - t.Errorf("got invalid candidate expected valid") - } - if tt.wantValid { - if !reflect.DeepEqual(got, tt.wantValue) { - t.Errorf("got value %v want %v", got, tt.wantValue) - } - } + gotID := tailcfg.StableNodeID("") + if got.Valid() { + gotID = got.StableID() + } + if gotID != tt.wantID { + t.Errorf("node IDs = %v, want %v", gotID, tt.wantID) } }) } @@ -3926,7 +3826,6 @@ func TestLocalBackendSuggestExitNode(t *testing.T) { } } } - func TestEnableAutoUpdates(t *testing.T) { lb := newTestLocalBackend(t)