From aeff360f775f106595f3011a67243a886eeef681 Mon Sep 17 00:00:00 2001
From: Kevin Liang <kevinliang@tailscale.com>
Date: Tue, 9 Apr 2024 20:16:21 +0000
Subject: [PATCH] Unadvertise routes when turning an appConnector down, and
 also only unadvertise routes for a domain when the domain actually have a
 datedRoutes structure

---
 appc/appconnector.go      | 17 ++++++++++++++---
 appc/appconnector_test.go |  2 ++
 ipn/ipnlocal/local.go     |  2 +-
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/appc/appconnector.go b/appc/appconnector.go
index 26705b6ff..65cd7a643 100644
--- a/appc/appconnector.go
+++ b/appc/appconnector.go
@@ -144,6 +144,15 @@ func (e *AppConnector) UpdateRouteInfo(ri *routeinfo.RouteInfo) {
 	e.routeInfo = ri
 }
 
+func (e *AppConnector) UnadvertiseRemoteRoutes() {
+	e.queue.Add(func() {
+		toRemove := e.RouteInfo().Routes(false, true, true)
+		if err := e.routeAdvertiser.UnadvertiseRoute(toRemove...); err != nil {
+			e.logf("failed to unadvertise routes %v: %v", toRemove, err)
+		}
+	})
+}
+
 // UpdateDomains asynchronously replaces the current set of configured domains
 // with the supplied set of domains. Domains must not contain a trailing dot,
 // and should be lower case. If the domain contains a leading '*' label it
@@ -216,9 +225,11 @@ func (e *AppConnector) updateDomains(domains []string) {
 		e.UpdateRouteInfo(routeInfo)
 		// everything left in oldDiscovered a) won't be in e.domains and b) can be unadvertised if it's not in local or control
 		for domainName, domainsRoutes := range oldDiscovered {
-			toRemove := domainsRoutes.RoutesSlice()
-			e.logf("unadvertising %d routes for domain: %s", len(toRemove), domainName)
-			e.scheduleUnadvertisement(domainName, toRemove...)
+			if domainsRoutes != nil {
+				toRemove := domainsRoutes.RoutesSlice()
+				e.logf("unadvertising %d routes for domain: %s", len(toRemove), domainName)
+				e.scheduleUnadvertisement(domainName, toRemove...)
+			}
 		}
 	}
 	e.logf("handling domains: %v and wildcards: %v", xmaps.Keys(e.domains), e.wildcards)
diff --git a/appc/appconnector_test.go b/appc/appconnector_test.go
index b5d051cf8..aa57f6232 100644
--- a/appc/appconnector_test.go
+++ b/appc/appconnector_test.go
@@ -22,6 +22,8 @@ func TestUpdateDomains(t *testing.T) {
 	for _, shouldStore := range []bool{true, false} {
 		ctx := context.Background()
 		a := NewAppConnector(t.Logf, nil, shouldStore)
+		a.routeAdvertiser = &appctest.RouteCollector{}
+		a.UpdateRouteInfo(routeinfo.NewRouteInfo())
 		a.UpdateDomains([]string{"example.com"})
 
 		a.Wait(ctx)
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 73bb55b6a..ccd888502 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -3585,7 +3585,7 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
 
 	if !prefs.AppConnector().Advertise {
 		if b.appConnector != nil && shouldAppCStoreRoutes {
-			b.appConnector.UpdateDomainsAndRoutes([]string{}, []netip.Prefix{})
+			b.appConnector.UnadvertiseRemoteRoutes()
 		}
 		b.appConnector = nil
 		return