From 45be37cb01f71072ba322097d37ce0b6907fb937 Mon Sep 17 00:00:00 2001
From: James Tucker <james@tailscale.com>
Date: Thu, 9 Nov 2023 17:21:10 -0800
Subject: [PATCH] ipn/ipnlocal: ensure that hostinfo is updated on app
 connector preference changes

Some conditional paths may otherwise skip the hostinfo update, so kick
it off asynchronously as other code paths do.

Updates tailscale/corp#15437
Signed-off-by: James Tucker <james@tailscale.com>
---
 ipn/ipnlocal/local.go      | 12 +++++++++-
 ipn/ipnlocal/local_test.go | 49 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 376063acd..d877c8d03 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -3272,7 +3272,13 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
 	const appConnectorCapName = "tailscale.com/app-connectors"
 
 	if !prefs.AppConnector().Advertise {
-		b.appConnector = nil
+		var old *appc.AppConnector
+		old, b.appConnector = b.appConnector, nil
+		if old != nil {
+			// Ensure that the app connector service will not be advertised now
+			// that b.appConnector is no longer set.
+			go b.doSetHostinfoFilterServices()
+		}
 		return
 	}
 
@@ -3306,6 +3312,10 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
 	slices.Sort(domains)
 	slices.Compact(domains)
 	b.appConnector.UpdateDomains(domains)
+
+	// Ensure that the app connector service will be advertised now that
+	// b.appConnector is set.
+	go b.doSetHostinfoFilterServices()
 }
 
 // authReconfig pushes a new configuration into wgengine, if engine
diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go
index 9568bd2fe..22f3b89f3 100644
--- a/ipn/ipnlocal/local_test.go
+++ b/ipn/ipnlocal/local_test.go
@@ -1250,6 +1250,55 @@ func TestReconfigureAppConnector(t *testing.T) {
 		t.Fatalf("got domains %v, want %v", b.appConnector.Domains(), want)
 	}
 
+	// Check that hostinfo has been updated to include the AppConnector service
+	// This is spawned in a goroutine, so may take some attempts to observe
+	var foundAppConnectorService bool
+	for i := 0; i < 10; i++ {
+		foundAppConnectorService = hasAppConnectorService(b)
+		if foundAppConnectorService {
+			break
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	if !foundAppConnectorService {
+		t.Fatalf("expected app connector service")
+	}
+
+	// disable the connector in order to assert that the service is removed
+	b.EditPrefs(&ipn.MaskedPrefs{
+		Prefs: ipn.Prefs{
+			AppConnector: ipn.AppConnectorPrefs{
+				Advertise: false,
+			},
+		},
+		AppConnectorSet: true,
+	})
+	b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs)
+	if b.appConnector != nil {
+		t.Fatal("expected no app connector")
+	}
+	// expect the connector service to be removed
+	for i := 0; i < 10; i++ {
+		foundAppConnectorService = hasAppConnectorService(b)
+		if !foundAppConnectorService {
+			break
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	if foundAppConnectorService {
+		t.Fatalf("expected no app connector service")
+	}
+}
+
+func hasAppConnectorService(b *LocalBackend) bool {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	for _, s := range b.peerAPIServicesLocked() {
+		if s.Proto == tailcfg.AppConnector && s.Port == 1 {
+			return true
+		}
+	}
+	return false
 }
 
 func resolversEqual(t *testing.T, a, b []*dnstype.Resolver) bool {