From 32d109548f0ba46f00a0d79bd99f78f6e44bae4f Mon Sep 17 00:00:00 2001
From: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>
Date: Fri, 30 May 2025 17:30:05 -0400
Subject: [PATCH] ipn/ipnlocal: Update hostinfo to control on service config
 change

This commit fixes the bug that c2n requests are skiped when updating vipServices in serveConfig. This then resulted
netmap update being skipped which caused inaccuracy of Capmap info on client side. After this fix, client always
inform control about it's vipServices config changes.

Fixes tailscale/corp#29219

Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com>
---
 ipn/ipnlocal/local.go      | 16 +++++++----
 ipn/ipnlocal/local_test.go | 56 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 05f026631..666876b05 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -6146,17 +6146,17 @@ func (b *LocalBackend) setTCPPortsInterceptedFromNetmapAndPrefsLocked(prefs ipn.
 		}
 	}
 
-	// Update funnel info in hostinfo and kick off control update if needed.
-	b.updateIngressLocked()
+	// Update funnel and service hash info in hostinfo and kick off control update if needed.
+	b.updateIngressAndServiceHashLocked(prefs)
 	b.setTCPPortsIntercepted(handlePorts)
 	b.setVIPServicesTCPPortsInterceptedLocked(vipServicesPorts)
 }
 
-// updateIngressLocked updates the hostinfo.WireIngress and hostinfo.IngressEnabled fields and kicks off a Hostinfo
-// update if the values have changed.
+// updateIngressAndServiceHashLocked updates the hostinfo.ServicesHash, hostinfo.WireIngress and
+// hostinfo.IngressEnabled fields and kicks off a Hostinfo update if the values have changed.
 //
 // b.mu must be held.
-func (b *LocalBackend) updateIngressLocked() {
+func (b *LocalBackend) updateIngressAndServiceHashLocked(prefs ipn.PrefsView) {
 	if b.hostinfo == nil {
 		return
 	}
@@ -6171,6 +6171,12 @@ func (b *LocalBackend) updateIngressLocked() {
 		b.hostinfo.WireIngress = wire
 		hostInfoChanged = true
 	}
+	latestHash := b.vipServiceHash(b.vipServicesFromPrefsLocked(prefs))
+	if b.hostinfo.ServicesHash != latestHash {
+		b.logf("Hostinfo.ServicesHash changed to %s", latestHash)
+		b.hostinfo.ServicesHash = latestHash
+		hostInfoChanged = true
+	}
 	// Kick off a Hostinfo update to control if ingress status has changed.
 	if hostInfoChanged {
 		b.goTracker.Go(b.doSetHostinfoFilterServices)
diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go
index 1ad3225a5..2e4e8d48c 100644
--- a/ipn/ipnlocal/local_test.go
+++ b/ipn/ipnlocal/local_test.go
@@ -5134,7 +5134,8 @@ func TestUpdatePrefsOnSysPolicyChange(t *testing.T) {
 	}
 }
 
-func TestUpdateIngressLocked(t *testing.T) {
+func TestUpdateIngressAndServiceHashLocked(t *testing.T) {
+	prefs := ipn.NewPrefs().View()
 	tests := []struct {
 		name              string
 		hi                *tailcfg.Hostinfo
@@ -5163,6 +5164,16 @@ func TestUpdateIngressLocked(t *testing.T) {
 			wantWireIngress:   false, // implied by wantIngress
 			wantControlUpdate: true,
 		},
+		{
+			name: "empty_hostinfo_service_configured",
+			hi:   &tailcfg.Hostinfo{},
+			sc: &ipn.ServeConfig{
+				Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{
+					"svc:abc": {Tun: true},
+				},
+			},
+			wantControlUpdate: true,
+		},
 		{
 			name: "empty_hostinfo_funnel_disabled",
 			hi:   &tailcfg.Hostinfo{},
@@ -5175,7 +5186,7 @@ func TestUpdateIngressLocked(t *testing.T) {
 			wantControlUpdate: true,
 		},
 		{
-			name: "empty_hostinfo_no_funnel",
+			name: "empty_hostinfo_no_funnel_no_service",
 			hi:   &tailcfg.Hostinfo{},
 			sc: &ipn.ServeConfig{
 				TCP: map[uint16]*ipn.TCPPortHandler{
@@ -5196,6 +5207,17 @@ func TestUpdateIngressLocked(t *testing.T) {
 			wantIngress:     true,
 			wantWireIngress: false, // implied by wantIngress
 		},
+		{
+			name: "service_hash_no_change",
+			hi: &tailcfg.Hostinfo{
+				ServicesHash: "5c5fd5093eb8764e1b70110dbfe5bc4370a214fdc5bc5a0fcdf32689fb5cb571",
+			},
+			sc: &ipn.ServeConfig{
+				Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{
+					"svc:abc": {Tun: true},
+				},
+			},
+		},
 		{
 			name: "funnel_disabled_no_change",
 			hi: &tailcfg.Hostinfo{
@@ -5208,6 +5230,14 @@ func TestUpdateIngressLocked(t *testing.T) {
 			},
 			wantWireIngress: true, // true if there is any AllowFunnel block
 		},
+		{
+			name: "service_got_removed",
+			hi: &tailcfg.Hostinfo{
+				ServicesHash: "5c5fd5093eb8764e1b70110dbfe5bc4370a214fdc5bc5a0fcdf32689fb5cb571",
+			},
+			sc:                &ipn.ServeConfig{},
+			wantControlUpdate: true,
+		},
 		{
 			name: "funnel_changes_to_disabled",
 			hi: &tailcfg.Hostinfo{
@@ -5235,6 +5265,22 @@ func TestUpdateIngressLocked(t *testing.T) {
 			wantWireIngress:   false, // implied by wantIngress
 			wantControlUpdate: true,
 		},
+		{
+			name: "both_funnel_and_service_changes",
+			hi: &tailcfg.Hostinfo{
+				IngressEnabled: true,
+			},
+			sc: &ipn.ServeConfig{
+				AllowFunnel: map[ipn.HostPort]bool{
+					"tailnet.xyz:443": false,
+				},
+				Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{
+					"svc:abc": {Tun: true},
+				},
+			},
+			wantWireIngress:   true, // true if there is any AllowFunnel block
+			wantControlUpdate: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -5256,7 +5302,7 @@ func TestUpdateIngressLocked(t *testing.T) {
 			})()
 
 			was := b.goTracker.StartedGoroutines()
-			b.updateIngressLocked()
+			b.updateIngressAndServiceHashLocked(prefs)
 
 			if tt.hi != nil {
 				if tt.hi.IngressEnabled != tt.wantIngress {
@@ -5265,6 +5311,10 @@ func TestUpdateIngressLocked(t *testing.T) {
 				if tt.hi.WireIngress != tt.wantWireIngress {
 					t.Errorf("WireIngress = %v, want %v", tt.hi.WireIngress, tt.wantWireIngress)
 				}
+				svcHash := b.vipServiceHash(b.vipServicesFromPrefsLocked(prefs))
+				if tt.hi.ServicesHash != svcHash {
+					t.Errorf("ServicesHash = %v, want %v", tt.hi.ServicesHash, svcHash)
+				}
 			}
 
 			startedGoroutine := b.goTracker.StartedGoroutines() != was