From f5bfdefa00cd0cc7357a6b7199d4bb5ca593e42f Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Wed, 30 Aug 2023 11:35:43 -0700
Subject: [PATCH] control/controlclient: de-pointer Status.PersistView,
 document more

Updates #cleanup
Updates #1909

Change-Id: I31d91e120e3b299508de2136021eab3b34131a44
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 control/controlclient/auto.go   |  5 ++---
 control/controlclient/status.go | 22 +++++++++++++++++-----
 ipn/ipnlocal/local.go           |  5 ++---
 ipn/ipnlocal/state_test.go      |  3 +--
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go
index 6f354bdd3..9c7139459 100644
--- a/control/controlclient/auto.go
+++ b/control/controlclient/auto.go
@@ -21,7 +21,6 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
 	"tailscale.com/types/persist"
-	"tailscale.com/types/ptr"
 	"tailscale.com/types/structs"
 )
 
@@ -644,9 +643,9 @@ func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkM
 
 	c.logf("[v1] sendStatus: %s: %v", who, state)
 
-	var p *persist.PersistView
+	var p persist.PersistView
 	if nm != nil && loggedIn && synced {
-		p = ptr.To(c.direct.GetPersist())
+		p = c.direct.GetPersist()
 	} else {
 		// don't send netmap status, as it's misleading when we're
 		// not logged in.
diff --git a/control/controlclient/status.go b/control/controlclient/status.go
index 7f425b546..5d086d344 100644
--- a/control/controlclient/status.go
+++ b/control/controlclient/status.go
@@ -61,12 +61,24 @@ func (s State) String() string {
 }
 
 type Status struct {
-	_      structs.Incomparable
-	Err    error
-	URL    string             // interactive URL to visit to finish logging in
-	NetMap *netmap.NetworkMap // server-pushed configuration
+	_ structs.Incomparable
 
-	Persist *persist.PersistView // locally persisted configuration
+	// Err, if non-nil, is an error that occurred while logging in.
+	//
+	// If it's of type UserVisibleError then it's meant to be shown to users in
+	// their Tailscale client. Otherwise it's just logged to tailscaled's logs.
+	Err error
+
+	// URL, if non-empty, is the interactive URL to visit to finish logging in.
+	URL string
+
+	// NetMap is the latest server-pushed state of the tailnet network.
+	NetMap *netmap.NetworkMap
+
+	// Persist, when Valid, is the locally persisted configuration.
+	//
+	// TODO(bradfitz,maisem): clarify this.
+	Persist persist.PersistView
 
 	// state is the internal state. It should not be exposed outside this
 	// package, but we have some automated tests elsewhere that need to
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 3cc60a035..0342affd0 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -893,7 +893,6 @@ func (b *LocalBackend) SetDecompressor(fn func() (controlclient.Decompressor, er
 func (b *LocalBackend) SetControlClientStatus(st controlclient.Status) {
 	// The following do not depend on any data for which we need to lock b.
 	if st.Err != nil {
-		// TODO(crawshaw): display in the UI.
 		if errors.Is(st.Err, io.EOF) {
 			b.logf("[v1] Received error: EOF")
 			return
@@ -1007,8 +1006,8 @@ func (b *LocalBackend) SetControlClientStatus(st controlclient.Status) {
 		prefs.ControlURL = prefs.ControlURLOrDefault()
 		prefsChanged = true
 	}
-	if st.Persist != nil && st.Persist.Valid() {
-		if !prefs.Persist.View().Equals(*st.Persist) {
+	if st.Persist.Valid() {
+		if !prefs.Persist.View().Equals(st.Persist) {
 			prefsChanged = true
 			prefs.Persist = st.Persist.AsStruct()
 		}
diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go
index b047e1fff..4ce904eba 100644
--- a/ipn/ipnlocal/state_test.go
+++ b/ipn/ipnlocal/state_test.go
@@ -162,11 +162,10 @@ func (cc *mockControl) send(err error, url string, loginFinished bool, nm *netma
 		cc.mu.Unlock()
 	}
 	if cc.opts.Observer != nil {
-		pv := cc.persist.View()
 		s := controlclient.Status{
 			URL:     url,
 			NetMap:  nm,
-			Persist: &pv,
+			Persist: cc.persist.View(),
 			Err:     err,
 		}
 		if loginFinished {