diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index c506f1376..92d2f123f 100644 --- a/ipn/ipnlocal/local.go +++ b/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, diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index f9a967bea..5e8a3172c 100644 --- a/ipn/ipnlocal/local_test.go +++ b/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