ipn: make Notify.Prefs be a *ipn.PrefsView

It is currently a `ipn.PrefsView` which means when we do a JSON roundtrip,
we go from an invalid Prefs to a valid one.

This makes it a pointer, which fixes the JSON roundtrip.

This was introduced in 0957bc5af2a544f7d197ea1e743696e8ba103b9f.

Signed-off-by: Maisem Ali <maisem@tailscale.com>

cmd/tailscale/cli/web.go

@@ -474,7 +474,7 @@ func tailscaleUp(ctx context.Context, prefs *ipn.Prefs, forceReauth bool) (authU
 			authURL = *url
 			cancel()
 		}
-		if !forceReauth && n.Prefs.Valid() {
+		if !forceReauth && n.Prefs != nil && n.Prefs.Valid() {
 			p1, p2 := n.Prefs.AsStruct(), *prefs
 			p1.Persist = nil
 			p2.Persist = nil

ipn/backend.go

@@ -67,7 +67,7 @@ type Notify struct {
 
 	LoginFinished *empty.Message     // non-nil when/if the login process succeeded
 	State         *State             // if non-nil, the new or current IPN state
-	Prefs         PrefsView          // if Valid, the new or current preferences
+	Prefs         *PrefsView         // if non-nil && Valid, the new or current preferences
 	NetMap        *netmap.NetworkMap // if non-nil, the new or current netmap
 	Engine        *EngineStatus      // if non-nil, the new or current wireguard stats
 	BrowseToURL   *string            // if non-nil, UI should open a browser right now

ipn/fake_test.go

@@ -22,7 +22,8 @@ func (b *FakeBackend) Start(opts Options) error {
 	}
 	nl := NeedsLogin
 	if b.notify != nil {
-		b.notify(Notify{Prefs: opts.Prefs.View()})
+		p := opts.Prefs.View()
+		b.notify(Notify{Prefs: &p})
 		b.notify(Notify{State: &nl})
 	}
 	return nil
@@ -83,7 +84,8 @@ func (b *FakeBackend) SetPrefs(new *Prefs) {
 	}
 
 	if b.notify != nil {
-		b.notify(Notify{Prefs: new.View()})
+		p := new.View()
+		b.notify(Notify{Prefs: &p})
 	}
 	if new.WantRunning && !b.live {
 		b.newState(Starting)

ipn/ipnlocal/local.go

@@ -859,7 +859,8 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 				b.logf("Failed to save new controlclient state: %v", err)
 			}
 		}
-		b.send(ipn.Notify{Prefs: prefs.View()})
+		p := prefs.View()
+		b.send(ipn.Notify{Prefs: &p})
 	}
 	if st.NetMap != nil {
 		if netMap != nil {
@@ -1089,10 +1090,11 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 		nm := b.netMap
 		state := b.state
 		b.mu.Unlock()
+		p := b.prefs
 		b.send(ipn.Notify{
 			State:         &state,
 			NetMap:        nm,
-			Prefs:         b.prefs,
+			Prefs:         &p,
 			LoginFinished: new(empty.Message),
 		})
 		return nil
@@ -1263,7 +1265,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 	blid := b.backendLogID
 	b.logf("Backend: logs: be:%v fe:%v", blid, opts.FrontendLogID)
 	b.send(ipn.Notify{BackendLogID: &blid})
-	b.send(ipn.Notify{Prefs: prefs})
+	b.send(ipn.Notify{Prefs: &prefs})
 
 	if !loggedOut && b.hasNodeKey() {
 		// Even if !WantRunning, we should verify our key, if there
@@ -2350,7 +2352,7 @@ func (b *LocalBackend) setPrefsLockedOnEntry(caller string, newp *ipn.Prefs) ipn
 		b.authReconfig()
 	}
 
-	b.send(ipn.Notify{Prefs: prefs})
+	b.send(ipn.Notify{Prefs: &prefs})
 	return prefs
 }
 

ipn/ipnlocal/state_test.go

@@ -316,7 +316,7 @@ func TestStateMachine(t *testing.T) {
 
 	b.SetNotifyCallback(func(n ipn.Notify) {
 		if n.State != nil ||
-			n.Prefs.Valid() ||
+			(n.Prefs != nil && n.Prefs.Valid()) ||
 			n.BrowseToURL != nil ||
 			n.LoginFinished != nil {
 			logf("\n%v\n\n", n)

ipn/prefs_test.go

@@ -871,3 +871,22 @@ func TestMaskedPrefsIsEmpty(t *testing.T) {
 		})
 	}
 }
+
+func TestNotifyPrefsJSONRoundtrip(t *testing.T) {
+	var n Notify
+	if n.Prefs != nil && n.Prefs.Valid() {
+		t.Fatal("Prefs should not be valid at start")
+	}
+	b, err := json.Marshal(n)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var n2 Notify
+	if err := json.Unmarshal(b, &n2); err != nil {
+		t.Fatal(err)
+	}
+	if n2.Prefs != nil && n2.Prefs.Valid() {
+		t.Fatal("Prefs should not be valid after deserialization")
+	}
+}