From 7a467a37e1b9232ca1b8715d9568e06d0ce91b68 Mon Sep 17 00:00:00 2001 From: Claire Wang Date: Fri, 3 May 2024 14:58:48 -0400 Subject: [PATCH] ipnlocal, magicsock: add more description to storing last suggested exit node related functions Updates tailscale/corp#19681 Signed-off-by: Claire Wang --- ipn/ipnlocal/local.go | 15 ++++++++------- ipn/ipnlocal/local_test.go | 20 ++++++++++---------- wgengine/magicsock/magicsock.go | 6 +++--- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index e2326e306..414d49b27 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -6424,7 +6424,7 @@ func (b *LocalBackend) SuggestExitNode() (response apitype.ExitNodeSuggestionRes lastSuggestedExitNode := b.lastSuggestedExitNode b.mu.Unlock() if lastReport == nil || netMap == nil { - last, err := suggestLastExitNode(lastSuggestedExitNode) + last, err := lastSuggestedExitNode.asAPIType() if err != nil { return response, ErrCannotSuggestExitNode } @@ -6434,7 +6434,7 @@ func (b *LocalBackend) SuggestExitNode() (response apitype.ExitNodeSuggestionRes r := rand.New(rand.NewSource(seed)) res, err := suggestExitNode(lastReport, netMap, r) if err != nil { - last, err := suggestLastExitNode(lastSuggestedExitNode) + last, err := lastSuggestedExitNode.asAPIType() if err != nil { return response, ErrCannotSuggestExitNode } @@ -6447,12 +6447,13 @@ func (b *LocalBackend) SuggestExitNode() (response apitype.ExitNodeSuggestionRes return res, err } -// suggestLastExitNode formats a response with the last suggested exit node's ID and name. +// asAPIType formats a response with the last suggested exit node's ID and name. +// Returns error if there is no id or 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 +func (n lastSuggestedExitNode) asAPIType() (res apitype.ExitNodeSuggestionResponse, _ error) { + if n.id != "" && n.name != "" { + res.ID = n.id + res.Name = n.name return res, nil } return res, ErrUnableToSuggestLastExitNode diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 2bf6ade30..cf706e928 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -3438,7 +3438,7 @@ func TestMinLatencyDERPregion(t *testing.T) { } } -func TestSuggestLastExitNode(t *testing.T) { +func TestLastSuggestedExitNodeAsAPIType(t *testing.T) { tests := []struct { name string lastSuggestedExitNode lastSuggestedExitNode @@ -3460,7 +3460,7 @@ func TestSuggestLastExitNode(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := suggestLastExitNode(tt.lastSuggestedExitNode) + got, err := tt.lastSuggestedExitNode.asAPIType() if got != tt.wantRes || err != tt.wantErr { t.Errorf("got %v error %v, want %v error %v", got, err, tt.wantRes, tt.wantErr) } @@ -3472,7 +3472,7 @@ func TestLocalBackendSuggestExitNode(t *testing.T) { tests := []struct { name string lastSuggestedExitNode lastSuggestedExitNode - report netcheck.Report + report *netcheck.Report netMap netmap.NetworkMap wantID tailcfg.StableNodeID wantName string @@ -3482,7 +3482,7 @@ func TestLocalBackendSuggestExitNode(t *testing.T) { { name: "nil netmap, returns last suggested exit node", lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, - report: netcheck.Report{ + report: &netcheck.Report{ RegionLatency: map[int]time.Duration{ 1: 0, 2: -1, @@ -3518,7 +3518,7 @@ func TestLocalBackendSuggestExitNode(t *testing.T) { { name: "found better derp node, last suggested exit node updates", lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, - report: netcheck.Report{ + report: &netcheck.Report{ RegionLatency: map[int]time.Duration{ 1: 10, 2: 10, @@ -3574,7 +3574,7 @@ func TestLocalBackendSuggestExitNode(t *testing.T) { { name: "found better mullvad node, last suggested exit node updates", lastSuggestedExitNode: lastSuggestedExitNode{name: "San Jose", id: "3"}, - report: netcheck.Report{ + report: &netcheck.Report{ RegionLatency: map[int]time.Duration{ 1: 0, 2: 0, @@ -3645,7 +3645,7 @@ func TestLocalBackendSuggestExitNode(t *testing.T) { { name: "ErrNoPreferredDERP, use last suggested exit node", lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, - report: netcheck.Report{ + report: &netcheck.Report{ RegionLatency: map[int]time.Duration{ 1: 10, 2: 10, @@ -3701,7 +3701,7 @@ func TestLocalBackendSuggestExitNode(t *testing.T) { { name: "ErrNoPreferredDERP, use last suggested exit node", lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"}, - report: netcheck.Report{ + report: &netcheck.Report{ RegionLatency: map[int]time.Duration{ 1: 10, 2: 10, @@ -3756,7 +3756,7 @@ func TestLocalBackendSuggestExitNode(t *testing.T) { }, { name: "unable to use last suggested exit node", - report: netcheck.Report{ + report: &netcheck.Report{ RegionLatency: map[int]time.Duration{ 1: 10, 2: 10, @@ -3772,7 +3772,7 @@ func TestLocalBackendSuggestExitNode(t *testing.T) { lb := newTestLocalBackend(t) lb.lastSuggestedExitNode = tt.lastSuggestedExitNode lb.netMap = &tt.netMap - lb.sys.MagicSock.Get().SetLastNetcheckReport(context.Background(), tt.report) + lb.sys.MagicSock.Get().SetLastNetcheckReportForTest(context.Background(), tt.report) got, err := lb.SuggestExitNode() if got.ID != tt.wantID { t.Errorf("ID=%v, want=%v", got.ID, tt.wantID) diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 8505aff44..397796030 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -3089,8 +3089,8 @@ func (c *Conn) GetLastNetcheckReport(ctx context.Context) *netcheck.Report { return lastReport } -// SetLastNetcheckReport sets local backend's last netcheck report. +// SetLastNetcheckReportForTest sets the magicsock conn's last netcheck report. // Used for testing purposes. -func (c *Conn) SetLastNetcheckReport(ctx context.Context, report netcheck.Report) { - c.lastNetCheckReport.Store(&report) +func (c *Conn) SetLastNetcheckReportForTest(ctx context.Context, report *netcheck.Report) { + c.lastNetCheckReport.Store(report) }