ipn/ipnlocal: re-advertise appc routes on startup (#14609)

There's at least one example of stored routes and advertised routes
getting out of sync. I don't know how they got there yet, but this would
backfill missing advertised routes on startup from stored routes.

Also add logging in LocalBackend.AdvertiseRoute to record when new
routes actually get put into prefs.

Updates #14606

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>

ipn/ipnlocal/local.go

@@ -4319,6 +4319,33 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
 	b.appConnector.UpdateDomainsAndRoutes(domains, routes)
 }
 
+func (b *LocalBackend) readvertiseAppConnectorRoutes() {
+	var domainRoutes map[string][]netip.Addr
+	b.mu.Lock()
+	if b.appConnector != nil {
+		domainRoutes = b.appConnector.DomainRoutes()
+	}
+	b.mu.Unlock()
+	if domainRoutes == nil {
+		return
+	}
+
+	// Re-advertise the stored routes, in case stored state got out of
+	// sync with previously advertised routes in prefs.
+	var prefixes []netip.Prefix
+	for _, ips := range domainRoutes {
+		for _, ip := range ips {
+			prefixes = append(prefixes, netip.PrefixFrom(ip, ip.BitLen()))
+		}
+	}
+	// Note: AdvertiseRoute will trim routes that are already
+	// advertised, so if everything is already being advertised this is
+	// a noop.
+	if err := b.AdvertiseRoute(prefixes...); err != nil {
+		b.logf("error advertising stored app connector routes: %v", err)
+	}
+}
+
 // authReconfig pushes a new configuration into wgengine, if engine
 // updates are not currently blocked, based on the cached netmap and
 // user prefs.
@@ -4397,6 +4424,7 @@ func (b *LocalBackend) authReconfig() {
 	}
 
 	b.initPeerAPIListener()
+	b.readvertiseAppConnectorRoutes()
 }
 
 // shouldUseOneCGNATRoute reports whether we should prefer to make one big
@@ -7111,7 +7139,7 @@ var ErrDisallowedAutoRoute = errors.New("route is not allowed")
 // If the route is disallowed, ErrDisallowedAutoRoute is returned.
 func (b *LocalBackend) AdvertiseRoute(ipps ...netip.Prefix) error {
 	finalRoutes := b.Prefs().AdvertiseRoutes().AsSlice()
-	newRoutes := false
+	var newRoutes []netip.Prefix
 
 	for _, ipp := range ipps {
 		if !allowedAutoRoute(ipp) {
@@ -7127,13 +7155,14 @@ func (b *LocalBackend) AdvertiseRoute(ipps ...netip.Prefix) error {
 		}
 
 		finalRoutes = append(finalRoutes, ipp)
-		newRoutes = true
+		newRoutes = append(newRoutes, ipp)
 	}
 
-	if !newRoutes {
+	if len(newRoutes) == 0 {
 		return nil
 	}
 
+	b.logf("advertising new app connector routes: %v", newRoutes)
 	_, err := b.EditPrefs(&ipn.MaskedPrefs{
 		Prefs: ipn.Prefs{
 			AdvertiseRoutes: finalRoutes,

ipn/ipnlocal/local_test.go

@@ -1501,6 +1501,53 @@ func TestReconfigureAppConnector(t *testing.T) {
 	}
 }
 
+func TestBackfillAppConnectorRoutes(t *testing.T) {
+	// Create backend with an empty app connector.
+	b := newTestBackend(t)
+	if err := b.Start(ipn.Options{}); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := b.EditPrefs(&ipn.MaskedPrefs{
+		Prefs: ipn.Prefs{
+			AppConnector: ipn.AppConnectorPrefs{Advertise: true},
+		},
+		AppConnectorSet: true,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs)
+
+	// Smoke check that AdvertiseRoutes doesn't have the test IP.
+	ip := netip.MustParseAddr("1.2.3.4")
+	routes := b.Prefs().AdvertiseRoutes().AsSlice()
+	if slices.Contains(routes, netip.PrefixFrom(ip, ip.BitLen())) {
+		t.Fatalf("AdvertiseRoutes %v on a fresh backend already contains advertised route for %v", routes, ip)
+	}
+
+	// Store the test IP in profile data, but not in Prefs.AdvertiseRoutes.
+	b.ControlKnobs().AppCStoreRoutes.Store(true)
+	if err := b.storeRouteInfo(&appc.RouteInfo{
+		Domains: map[string][]netip.Addr{
+			"example.com": {ip},
+		},
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	// Mimic b.authReconfigure for the app connector bits.
+	b.mu.Lock()
+	b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs)
+	b.mu.Unlock()
+	b.readvertiseAppConnectorRoutes()
+
+	// Check that Prefs.AdvertiseRoutes got backfilled with routes stored in
+	// profile data.
+	routes = b.Prefs().AdvertiseRoutes().AsSlice()
+	if !slices.Contains(routes, netip.PrefixFrom(ip, ip.BitLen())) {
+		t.Fatalf("AdvertiseRoutes %v was not backfilled from stored app connector routes with %v", routes, ip)
+	}
+}
+
 func resolversEqual(t *testing.T, a, b []*dnstype.Resolver) bool {
 	if a == nil && b == nil {
 		return true