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

This reverts commit 1b303ee5baef3ddab40be4d1c2 (#14609).

It caused a deadlock; see tailscale/corp#25965

Updates tailscale/corp#25965
Updates #13680
Updates #14606
This commit is contained in:
Brad Fitzpatrick 2025-01-21 08:02:24 -08:00 committed by Brad Fitzpatrick
parent bcc262269f
commit 51adaec35a
2 changed files with 3 additions and 79 deletions

View File

@ -4356,33 +4356,6 @@ 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.
@ -4461,7 +4434,6 @@ func (b *LocalBackend) authReconfig() {
}
b.initPeerAPIListener()
b.readvertiseAppConnectorRoutes()
}
// shouldUseOneCGNATRoute reports whether we should prefer to make one big
@ -7204,7 +7176,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()
var newRoutes []netip.Prefix
newRoutes := false
for _, ipp := range ipps {
if !allowedAutoRoute(ipp) {
@ -7220,14 +7192,13 @@ func (b *LocalBackend) AdvertiseRoute(ipps ...netip.Prefix) error {
}
finalRoutes = append(finalRoutes, ipp)
newRoutes = append(newRoutes, ipp)
newRoutes = true
}
if len(newRoutes) == 0 {
if !newRoutes {
return nil
}
b.logf("advertising new app connector routes: %v", newRoutes)
_, err := b.EditPrefs(&ipn.MaskedPrefs{
Prefs: ipn.Prefs{
AdvertiseRoutes: finalRoutes,

View File

@ -1501,53 +1501,6 @@ 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