ipnlocal, magicsock: add more description to storing last suggested exit (#11998)

node related functions
Updates tailscale/corp#19681

Signed-off-by: Claire Wang <claire@tailscale.com>

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)
 }