Fix test for local. Notice the default value for creating a new appconnector now has a routeInfo = nil, for us to quickly know if the value needs to be loaded from store.

appc/appconnector.go

@@ -87,7 +87,7 @@ func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser, shouldSt
 		logf:              logger.WithPrefix(logf, "appc: "),
 		routeAdvertiser:   routeAdvertiser,
 		ShouldStoreRoutes: shouldStoreRoutes,
-		routeInfo:         routeinfo.NewRouteInfo(),
+		routeInfo:         nil,
 	}
 }
 

appc/appconnector_test.go

@@ -69,12 +69,15 @@ func TestUpdateRoutes(t *testing.T) {
 		routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24"), netip.MustParsePrefix("192.0.0.1/32")}
 		a.updateRoutes(routes)
 
-		wantRouteInfoControlRoutes := []netip.Prefix{}
 		if shouldStore {
-			wantRouteInfoControlRoutes = routes
-		}
-		if !slices.Equal(a.routeInfo.Control, wantRouteInfoControlRoutes) {
-			t.Fatalf("got %v, want %v (shouldStore=%t)", a.routeInfo.Control, wantRouteInfoControlRoutes, shouldStore)
+			wantRouteInfoControlRoutes := routes
+			if !slices.Equal(a.routeInfo.Control, wantRouteInfoControlRoutes) {
+				t.Fatalf("got %v, want %v (shouldStore=%t)", a.routeInfo.Control, wantRouteInfoControlRoutes, shouldStore)
+			}
+		} else {
+			if a.routeInfo != nil {
+				t.Fatalf("got %v, want %v (shouldStore=%t)", a.routeInfo.Control, nil, shouldStore)
+			}
 		}
 
 		slices.SortFunc(rc.Routes(), prefixCompare)

ipn/ipnlocal/local.go

@@ -3192,14 +3192,16 @@ func (b *LocalBackend) SetUseExitNodeEnabled(v bool) (ipn.PrefsView, error) {
 func (b *LocalBackend) PatchPrefsHandler(mp *ipn.MaskedPrefs) (ipn.PrefsView, error) {
 	// we believe that for the purpose of figuring out advertisedRoutes setPrefsLockedOnEntry is _only_ called when
 	// up or set is used on the tailscale cli _not_ when we calculate the new advertisedRoutes field.
-	routeInfo := b.appConnector.RouteInfo()
-	curRoutes := routeInfo.Routes(false, true, true)
+	if b.appConnector.ShouldStoreRoutes {
+		routeInfo := b.appConnector.RouteInfo()
+		curRoutes := routeInfo.Routes(false, true, true)
 
-	if mp.AdvertiseRoutesSet && b.appConnector.ShouldStoreRoutes {
-		routeInfo.Local = mp.AdvertiseRoutes
-		b.StoreRouteInfo(routeInfo)
-		curRoutes := append(curRoutes, mp.AdvertiseRoutes...)
-		mp.AdvertiseRoutes = curRoutes
+		if mp.AdvertiseRoutesSet && b.appConnector.ShouldStoreRoutes {
+			routeInfo.Local = mp.AdvertiseRoutes
+			b.StoreRouteInfo(routeInfo)
+			curRoutes := append(curRoutes, mp.AdvertiseRoutes...)
+			mp.AdvertiseRoutes = curRoutes
+		}
 	}
 	return b.EditPrefs(mp)
 }

ipn/ipnlocal/local_test.go

@@ -2696,6 +2696,8 @@ func TestPatchPrefsHandler(t *testing.T) {
 	mp := new(ipn.MaskedPrefs)
 	mp.AdvertiseRoutesSet = true
 	mp.AdvertiseRoutes = []netip.Prefix{prefix1}
+	mp.AppConnectorSet = true
+	mp.AppConnector.Advertise = true
 	now := time.Now()
 	ri := routeinfo.NewRouteInfo()
 	ri.Control = []netip.Prefix{prefix2}
@@ -2707,13 +2709,13 @@ func TestPatchPrefsHandler(t *testing.T) {
 		Routes:      routes,
 	}
 	ri.Discovered = discovered
-	b := newTestBackend(t)
-	b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs)
-	if b.appConnector != nil {
-		t.Fatal("unexpected app connector")
-	}
-	b.appConnector = testAppConnector
+	rc.StoreRouteInfo(ri)
 
+	testAppConnector.ShouldStoreRoutes = true
+	testAppConnector.RouteInfo()
+	b := newTestBackend(t)
+	b.appConnector = testAppConnector
+	b.ControlKnobs().AppCStoreRoutes.Store(true)
 	prefView, err := b.PatchPrefsHandler(mp)
 	if err != nil {
 		t.Fatalf(err.Error())
@@ -2731,8 +2733,40 @@ func TestPatchPrefsHandler(t *testing.T) {
 		t.Fatalf("Old prefixes are no longer advertised.")
 	}
 
-	//TODO: test if route if stored in Appc/Appc.routeAdvertiser
+	//Check if route is stored in Appc/Appc.routeAdvertiser
+	storedRouteInfo, _ := rc.ReadRouteInfo()
+	if len(storedRouteInfo.Local) != 1 {
+		t.Fatalf("wanted %d, got %d", 1, len(storedRouteInfo.Local))
+	}
+	if !slices.Contains(storedRouteInfo.Local, prefix1) {
+		t.Fatalf("New local route not stored.")
+	}
 
-	//TODO: patch again with no route, see if prefix1 is removed/ prefix2, prefix3 presists.
+	//Check if the routes in control and discovered are presisted.
+	routesShouldPresist := storedRouteInfo.Routes(false, true, true)
+	if len(routesShouldPresist) != 2 {
+		t.Fatalf("wanted %d, got %d", 1, len(routesShouldPresist))
+	}
+	if !slices.Contains(routesShouldPresist, prefix2) || !slices.Contains(routesShouldPresist, prefix3) {
+		t.Fatalf("Pre-existed routes not presisted.")
+	}
 
+	//Patch again with no route, see if prefix1 is removed/ prefix2, prefix3 presists.
+	mp.AdvertiseRoutes = []netip.Prefix{}
+	prefView2, err := b.PatchPrefsHandler(mp)
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	if prefView2.AdvertiseRoutes().Len() != 2 {
+		t.Fatalf("wanted %d, got %d", 2, prefView.AdvertiseRoutes().Len())
+	}
+
+	if slices.Contains(prefView2.AdvertiseRoutes().AsSlice(), prefix1) {
+		t.Fatalf("Local route was not removed")
+	}
+
+	if !slices.Contains(prefView2.AdvertiseRoutes().AsSlice(), prefix2) || !slices.Contains(prefView.AdvertiseRoutes().AsSlice(), prefix3) {
+		t.Fatalf("Old prefixes are no longer advertised.")
+	}
 }