ipn: remove unused Options.LegacyMigrationPrefs

I'm on a mission to simplify LocalBackend.Start and its locking
and deflake some tests.

I noticed this hasn't been used since March 2023 when it was removed
from the Windows client in corp 66be796d33c.

So, delete.

Updates #11649

Change-Id: I40f2cb75fb3f43baf23558007655f65a8ec5e1b2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

ipn/backend.go

@@ -232,18 +232,11 @@ var DebuggableComponents = []string{
 type Options struct {
 	// FrontendLogID is the public logtail id used by the frontend.
 	FrontendLogID string
-	// LegacyMigrationPrefs are used to migrate preferences from the
+	// UpdatePrefs, if provided, overrides the Prefs already stored in the
-	// frontend to the backend.
+	// backend state, *except* for the Persist member.
-	// If non-nil, they are imported as a new profile.
-	LegacyMigrationPrefs *Prefs `json:"Prefs"`
-	// UpdatePrefs, if provided, overrides Options.LegacyMigrationPrefs
-	// *and* the Prefs already stored in the backend state, *except* for
-	// the Persist member. If you just want to provide prefs, this is
-	// probably what you want.
 	//
 	// TODO(apenwarr): Rename this to Prefs, and possibly move Prefs.Persist
-	//   elsewhere entirely (as it always should have been). Or, move the
+	// elsewhere entirely (as it always should have been).
-	//   fancy state migration stuff out of Start().
 	UpdatePrefs *Prefs
 	// AuthKey is an optional node auth key used to authorize a
 	// new node key without user interaction.

ipn/ipnlocal/local.go

@@ -1623,7 +1623,6 @@ func (b *LocalBackend) startIsNoopLocked(opts ipn.Options) bool {
 	return b.state == ipn.Running &&
 		b.hostinfo != nil &&
 		b.hostinfo.FrontendLogID == opts.FrontendLogID &&
-		opts.LegacyMigrationPrefs == nil &&
 		opts.UpdatePrefs == nil &&
 		opts.AuthKey == ""
 }
@@ -1639,28 +1638,15 @@ func (b *LocalBackend) startIsNoopLocked(opts ipn.Options) bool {
 // actually a supported operation (it should be, but it's very unclear
 // from the following whether or not that is a safe transition).
 func (b *LocalBackend) Start(opts ipn.Options) error {
-	if opts.LegacyMigrationPrefs != nil {
+	b.logf("Start")
-		b.logf("Start: %v", opts.LegacyMigrationPrefs.Pretty())
-	} else {
-		b.logf("Start")
-	}
 
 	b.mu.Lock()
-	if opts.LegacyMigrationPrefs == nil && !b.pm.CurrentPrefs().Valid() {
-		b.mu.Unlock()
-		return errors.New("no prefs provided")
-	}
 
 	if opts.UpdatePrefs != nil {
 		if err := b.checkPrefsLocked(opts.UpdatePrefs); err != nil {
 			b.mu.Unlock()
 			return err
 		}
-	} else if opts.LegacyMigrationPrefs != nil {
-		if err := b.checkPrefsLocked(opts.LegacyMigrationPrefs); err != nil {
-			b.mu.Unlock()
-			return err
-		}
 	}
 	profileID := b.pm.CurrentProfile().ID
 	if b.state != ipn.Running && b.conf != nil && b.conf.Parsed.AuthKey != nil && opts.AuthKey == "" {
@@ -1720,11 +1706,6 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 	b.hostinfo = hostinfo
 	b.state = ipn.NoState
 
-	if err := b.migrateStateLocked(opts.LegacyMigrationPrefs); err != nil {
-		b.mu.Unlock()
-		return fmt.Errorf("loading requested state: %v", err)
-	}
-
 	if opts.UpdatePrefs != nil {
 		oldPrefs := b.pm.CurrentPrefs()
 		newPrefs := opts.UpdatePrefs.Clone()
@@ -1737,6 +1718,8 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 			b.logf("failed to save UpdatePrefs state: %v", err)
 		}
 		b.setAtomicValuesFromPrefsLocked(pv)
+	} else {
+		b.setAtomicValuesFromPrefsLocked(b.pm.CurrentPrefs())
 	}
 
 	prefs := b.pm.CurrentPrefs()
@@ -1799,9 +1782,9 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 	}
 
 	// TODO(apenwarr): The only way to change the ServerURL is to
-	// re-run b.Start(), because this is the only place we create a
+	// re-run b.Start, because this is the only place we create a
-	// new controlclient. SetPrefs() allows you to overwrite ServerURL,
+	// new controlclient. EditPrefs allows you to overwrite ServerURL,
-	// but it won't take effect until the next Start().
+	// but it won't take effect until the next Start.
 	cc, err := b.getNewControlClientFunc()(controlclient.Options{
 		GetMachinePrivateKey:       b.createGetMachinePrivateKeyFunc(),
 		Logf:                       logger.WithPrefix(b.logf, "control: "),
@@ -2681,30 +2664,6 @@ func (b *LocalBackend) clearMachineKeyLocked() error {
 	return nil
 }
 
-// migrateStateLocked migrates state from the frontend to the backend.
-// It is a no-op if prefs is nil
-// b.mu must be held.
-func (b *LocalBackend) migrateStateLocked(prefs *ipn.Prefs) (err error) {
-	if prefs == nil && !b.pm.CurrentPrefs().Valid() {
-		return fmt.Errorf("no prefs provided and no current profile")
-	}
-	if prefs != nil {
-		// Backend owns the state, but frontend is trying to migrate
-		// state into the backend.
-		b.logf("importing frontend prefs into backend store; frontend prefs: %s", prefs.Pretty())
-		if err := b.pm.SetPrefs(prefs.View(), ipn.NetworkProfile{
-			MagicDNSName: b.netMap.MagicDNSSuffix(),
-			DomainName:   b.netMap.DomainName(),
-		}); err != nil {
-			return fmt.Errorf("store.WriteState: %v", err)
-		}
-	}
-
-	b.setAtomicValuesFromPrefsLocked(b.pm.CurrentPrefs())
-
-	return nil
-}
-
 // setTCPPortsIntercepted populates b.shouldInterceptTCPPortAtomic with an
 // efficient func for ShouldInterceptTCPPort to use, which is called on every
 // incoming packet.

wgengine/netstack/netstack_test.go

@@ -292,7 +292,7 @@ func TestShouldProcessInbound(t *testing.T) {
 					netip.MustParsePrefix("fd7a:115c:a1e0:b1a:0:7:a01:100/120"),
 				}
 				i.lb.Start(ipn.Options{
-					LegacyMigrationPrefs: prefs,
+					UpdatePrefs: prefs,
 				})
 				i.atomicIsLocalIPFunc.Store(looksLikeATailscaleSelfAddress)
 			},
@@ -325,7 +325,7 @@ func TestShouldProcessInbound(t *testing.T) {
 					netip.MustParsePrefix("fd7a:115c:a1e0:b1a:0:7:a01:200/120"),
 				}
 				i.lb.Start(ipn.Options{
-					LegacyMigrationPrefs: prefs,
+					UpdatePrefs: prefs,
 				})
 			},
 			want: false,
@@ -343,7 +343,7 @@ func TestShouldProcessInbound(t *testing.T) {
 				prefs := ipn.NewPrefs()
 				prefs.RunSSH = true
 				i.lb.Start(ipn.Options{
-					LegacyMigrationPrefs: prefs,
+					UpdatePrefs: prefs,
 				})
 				i.atomicIsLocalIPFunc.Store(func(addr netip.Addr) bool {
 					return addr.String() == "100.101.102.104" // Dst, above
@@ -365,7 +365,7 @@ func TestShouldProcessInbound(t *testing.T) {
 				prefs := ipn.NewPrefs()
 				prefs.RunSSH = false // default, but to be explicit
 				i.lb.Start(ipn.Options{
-					LegacyMigrationPrefs: prefs,
+					UpdatePrefs: prefs,
 				})
 				i.atomicIsLocalIPFunc.Store(func(addr netip.Addr) bool {
 					return addr.String() == "100.101.102.104" // Dst, above
@@ -430,7 +430,7 @@ func TestShouldProcessInbound(t *testing.T) {
 					netip.MustParsePrefix("10.0.0.1/24"),
 				}
 				i.lb.Start(ipn.Options{
-					LegacyMigrationPrefs: prefs,
+					UpdatePrefs: prefs,
 				})
 
 				// Set the PeerAPI port to the Dst port above.
@@ -549,7 +549,7 @@ func TestTCPForwardLimits(t *testing.T) {
 		netip.MustParsePrefix("192.0.2.0/24"),
 	}
 	impl.lb.Start(ipn.Options{
-		LegacyMigrationPrefs: prefs,
+		UpdatePrefs: prefs,
 	})
 	impl.atomicIsLocalIPFunc.Store(looksLikeATailscaleSelfAddress)
 
@@ -629,7 +629,7 @@ func TestTCPForwardLimits_PerClient(t *testing.T) {
 		netip.MustParsePrefix("192.0.2.0/24"),
 	}
 	impl.lb.Start(ipn.Options{
-		LegacyMigrationPrefs: prefs,
+		UpdatePrefs: prefs,
 	})
 	impl.atomicIsLocalIPFunc.Store(looksLikeATailscaleSelfAddress)