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

@@ -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.

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

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

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 {