Skip to content
This repository was archived by the owner on Jul 13, 2025. It is now read-only.

Commit 51adaec

Browse files
committed
Revert "ipn/ipnlocal: re-advertise appc routes on startup (tailscale#14609)"
This reverts commit 1b303ee (tailscale#14609). It caused a deadlock; see tailscale/corp#25965 Updates tailscale/corp#25965 Updates tailscale#13680 Updates tailscale#14606
1 parent bcc2622 commit 51adaec

File tree

2 files changed

+3
-79
lines changed

2 files changed

+3
-79
lines changed

ipn/ipnlocal/local.go

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4356,33 +4356,6 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
43564356
b.appConnector.UpdateDomainsAndRoutes(domains, routes)
43574357
}
43584358

4359-
func (b *LocalBackend) readvertiseAppConnectorRoutes() {
4360-
var domainRoutes map[string][]netip.Addr
4361-
b.mu.Lock()
4362-
if b.appConnector != nil {
4363-
domainRoutes = b.appConnector.DomainRoutes()
4364-
}
4365-
b.mu.Unlock()
4366-
if domainRoutes == nil {
4367-
return
4368-
}
4369-
4370-
// Re-advertise the stored routes, in case stored state got out of
4371-
// sync with previously advertised routes in prefs.
4372-
var prefixes []netip.Prefix
4373-
for _, ips := range domainRoutes {
4374-
for _, ip := range ips {
4375-
prefixes = append(prefixes, netip.PrefixFrom(ip, ip.BitLen()))
4376-
}
4377-
}
4378-
// Note: AdvertiseRoute will trim routes that are already
4379-
// advertised, so if everything is already being advertised this is
4380-
// a noop.
4381-
if err := b.AdvertiseRoute(prefixes...); err != nil {
4382-
b.logf("error advertising stored app connector routes: %v", err)
4383-
}
4384-
}
4385-
43864359
// authReconfig pushes a new configuration into wgengine, if engine
43874360
// updates are not currently blocked, based on the cached netmap and
43884361
// user prefs.
@@ -4461,7 +4434,6 @@ func (b *LocalBackend) authReconfig() {
44614434
}
44624435

44634436
b.initPeerAPIListener()
4464-
b.readvertiseAppConnectorRoutes()
44654437
}
44664438

44674439
// shouldUseOneCGNATRoute reports whether we should prefer to make one big
@@ -7204,7 +7176,7 @@ var ErrDisallowedAutoRoute = errors.New("route is not allowed")
72047176
// If the route is disallowed, ErrDisallowedAutoRoute is returned.
72057177
func (b *LocalBackend) AdvertiseRoute(ipps ...netip.Prefix) error {
72067178
finalRoutes := b.Prefs().AdvertiseRoutes().AsSlice()
7207-
var newRoutes []netip.Prefix
7179+
newRoutes := false
72087180

72097181
for _, ipp := range ipps {
72107182
if !allowedAutoRoute(ipp) {
@@ -7220,14 +7192,13 @@ func (b *LocalBackend) AdvertiseRoute(ipps ...netip.Prefix) error {
72207192
}
72217193

72227194
finalRoutes = append(finalRoutes, ipp)
7223-
newRoutes = append(newRoutes, ipp)
7195+
newRoutes = true
72247196
}
72257197

7226-
if len(newRoutes) == 0 {
7198+
if !newRoutes {
72277199
return nil
72287200
}
72297201

7230-
b.logf("advertising new app connector routes: %v", newRoutes)
72317202
_, err := b.EditPrefs(&ipn.MaskedPrefs{
72327203
Prefs: ipn.Prefs{
72337204
AdvertiseRoutes: finalRoutes,

ipn/ipnlocal/local_test.go

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,53 +1501,6 @@ func TestReconfigureAppConnector(t *testing.T) {
15011501
}
15021502
}
15031503

1504-
func TestBackfillAppConnectorRoutes(t *testing.T) {
1505-
// Create backend with an empty app connector.
1506-
b := newTestBackend(t)
1507-
if err := b.Start(ipn.Options{}); err != nil {
1508-
t.Fatal(err)
1509-
}
1510-
if _, err := b.EditPrefs(&ipn.MaskedPrefs{
1511-
Prefs: ipn.Prefs{
1512-
AppConnector: ipn.AppConnectorPrefs{Advertise: true},
1513-
},
1514-
AppConnectorSet: true,
1515-
}); err != nil {
1516-
t.Fatal(err)
1517-
}
1518-
b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs)
1519-
1520-
// Smoke check that AdvertiseRoutes doesn't have the test IP.
1521-
ip := netip.MustParseAddr("1.2.3.4")
1522-
routes := b.Prefs().AdvertiseRoutes().AsSlice()
1523-
if slices.Contains(routes, netip.PrefixFrom(ip, ip.BitLen())) {
1524-
t.Fatalf("AdvertiseRoutes %v on a fresh backend already contains advertised route for %v", routes, ip)
1525-
}
1526-
1527-
// Store the test IP in profile data, but not in Prefs.AdvertiseRoutes.
1528-
b.ControlKnobs().AppCStoreRoutes.Store(true)
1529-
if err := b.storeRouteInfo(&appc.RouteInfo{
1530-
Domains: map[string][]netip.Addr{
1531-
"example.com": {ip},
1532-
},
1533-
}); err != nil {
1534-
t.Fatal(err)
1535-
}
1536-
1537-
// Mimic b.authReconfigure for the app connector bits.
1538-
b.mu.Lock()
1539-
b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs)
1540-
b.mu.Unlock()
1541-
b.readvertiseAppConnectorRoutes()
1542-
1543-
// Check that Prefs.AdvertiseRoutes got backfilled with routes stored in
1544-
// profile data.
1545-
routes = b.Prefs().AdvertiseRoutes().AsSlice()
1546-
if !slices.Contains(routes, netip.PrefixFrom(ip, ip.BitLen())) {
1547-
t.Fatalf("AdvertiseRoutes %v was not backfilled from stored app connector routes with %v", routes, ip)
1548-
}
1549-
}
1550-
15511504
func resolversEqual(t *testing.T, a, b []*dnstype.Resolver) bool {
15521505
if a == nil && b == nil {
15531506
return true

0 commit comments

Comments
 (0)