ipn/ipnlocal: move vipServiceHash etc to serve.go, out of local.go

Updates #12614

Change-Id: I3c16b94fcb997088ff18d5a21355e0279845ed7e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

ipn/ipnlocal/local.go

@@ -10,7 +10,6 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/binary"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -5487,20 +5486,9 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip
 	}
 	hi.SSH_HostKeys = sshHostKeys
 
-	hi.ServicesHash = b.vipServiceHash(b.vipServicesFromPrefsLocked(prefs))
+	for _, f := range hookMaybeMutateHostinfoLocked {
-
+		f(b, hi, prefs)
-	// The Hostinfo.IngressEnabled field is used to communicate to control whether
+	}
-	// the node has funnel enabled.
-	hi.IngressEnabled = b.hasIngressEnabledLocked()
-	// The Hostinfo.WantIngress field tells control whether the user intends
-	// to use funnel with this node even though it is not currently enabled.
-	// This is an optimization to control- Funnel requires creation of DNS
-	// records and because DNS propagation can take time, we want to ensure
-	// that the records exist for any node that intends to use funnel even
-	// if it's not enabled. If hi.IngressEnabled is true, control knows that
-	// DNS records are needed, so we can save bandwidth and not send
-	// WireIngress.
-	hi.WireIngress = b.shouldWireInactiveIngressLocked()
 
 	if buildfeatures.HasAppConnectors {
 		hi.AppConnector.Set(prefs.AppConnector().Advertise)
@@ -6284,36 +6272,34 @@ func (b *LocalBackend) setTCPPortsInterceptedFromNetmapAndPrefsLocked(prefs ipn.
 	}
 
 	// Update funnel and service hash info in hostinfo and kick off control update if needed.
-	b.updateIngressAndServiceHashLocked(prefs)
+	b.maybeSentHostinfoIfChangedLocked(prefs)
 	b.setTCPPortsIntercepted(handlePorts)
 }
 
-// updateIngressAndServiceHashLocked updates the hostinfo.ServicesHash, hostinfo.WireIngress and
+// hookMaybeMutateHostinfoLocked is a hook that allows conditional features
+// to mutate the provided hostinfo before it is sent to control.
+//
+// The hook function should return true if it mutated the hostinfo.
+//
+// The LocalBackend's mutex is held while calling.
+var hookMaybeMutateHostinfoLocked feature.Hooks[func(*LocalBackend, *tailcfg.Hostinfo, ipn.PrefsView) bool]
+
+// maybeSentHostinfoIfChangedLocked 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) updateIngressAndServiceHashLocked(prefs ipn.PrefsView) {
+func (b *LocalBackend) maybeSentHostinfoIfChangedLocked(prefs ipn.PrefsView) {
 	if b.hostinfo == nil {
 		return
 	}
-	hostInfoChanged := false
+	changed := false
-	if ie := b.hasIngressEnabledLocked(); b.hostinfo.IngressEnabled != ie {
+	for _, f := range hookMaybeMutateHostinfoLocked {
-		b.logf("Hostinfo.IngressEnabled changed to %v", ie)
+		if f(b, b.hostinfo, prefs) {
-		b.hostinfo.IngressEnabled = ie
+			changed = true
-		hostInfoChanged = true
 		}
-	if wire := b.shouldWireInactiveIngressLocked(); b.hostinfo.WireIngress != wire {
-		b.logf("Hostinfo.WireIngress changed to %v", wire)
-		b.hostinfo.WireIngress = wire
-		hostInfoChanged = true
-	}
-	latestHash := b.vipServiceHash(b.vipServicesFromPrefsLocked(prefs))
-	if b.hostinfo.ServicesHash != latestHash {
-		b.hostinfo.ServicesHash = latestHash
-		hostInfoChanged = true
 	}
 	// Kick off a Hostinfo update to control if ingress status has changed.
-	if hostInfoChanged {
+	if changed {
 		b.goTracker.Go(b.doSetHostinfoFilterServices)
 	}
 }
@@ -7707,19 +7693,6 @@ func maybeUsernameOf(actor ipnauth.Actor) string {
 	return username
 }
 
-func (b *LocalBackend) vipServiceHash(services []*tailcfg.VIPService) string {
-	if len(services) == 0 {
-		return ""
-	}
-	buf, err := json.Marshal(services)
-	if err != nil {
-		b.logf("vipServiceHashLocked: %v", err)
-		return ""
-	}
-	hash := sha256.Sum256(buf)
-	return hex.EncodeToString(hash[:])
-}
-
 var (
 	metricCurrentWatchIPNBus = clientmetric.NewGauge("localbackend_current_watch_ipn_bus")
 )

ipn/ipnlocal/local_test.go

@@ -6745,7 +6745,7 @@ func TestUpdateIngressAndServiceHashLocked(t *testing.T) {
 			if tt.hasPreviousSC {
 				b.mu.Lock()
 				b.serveConfig = previousSC.View()
-				b.hostinfo.ServicesHash = b.vipServiceHash(b.vipServicesFromPrefsLocked(prefs))
+				b.hostinfo.ServicesHash = vipServiceHash(b.logf, b.vipServicesFromPrefsLocked(prefs))
 				b.mu.Unlock()
 			}
 			b.serveConfig = tt.sc.View()
@@ -6763,7 +6763,7 @@ func TestUpdateIngressAndServiceHashLocked(t *testing.T) {
 			})()
 
 			was := b.goTracker.StartedGoroutines()
-			b.updateIngressAndServiceHashLocked(prefs)
+			b.maybeSentHostinfoIfChangedLocked(prefs)
 
 			if tt.hi != nil {
 				if tt.hi.IngressEnabled != tt.wantIngress {
@@ -6773,7 +6773,7 @@ func TestUpdateIngressAndServiceHashLocked(t *testing.T) {
 					t.Errorf("WireIngress = %v, want %v", tt.hi.WireIngress, tt.wantWireIngress)
 				}
 				b.mu.Lock()
-				svcHash := b.vipServiceHash(b.vipServicesFromPrefsLocked(prefs))
+				svcHash := vipServiceHash(b.logf, b.vipServicesFromPrefsLocked(prefs))
 				b.mu.Unlock()
 				if tt.hi.ServicesHash != svcHash {
 					t.Errorf("ServicesHash = %v, want %v", tt.hi.ServicesHash, svcHash)

ipn/ipnlocal/serve.go

@@ -59,6 +59,9 @@ func init() {
 		b.setVIPServicesTCPPortsInterceptedLocked(nil)
 	})
 
+	hookMaybeMutateHostinfoLocked.Add(maybeUpdateHostinfoServicesHashLocked)
+	hookMaybeMutateHostinfoLocked.Add(maybeUpdateHostinfoFunnelLocked)
+
 	RegisterC2N("GET /vip-services", handleC2NVIPServicesGet)
 }
 
@@ -1227,7 +1230,7 @@ func handleC2NVIPServicesGet(b *LocalBackend, w http.ResponseWriter, r *http.Req
 	b.logf("c2n: GET /vip-services received")
 	var res tailcfg.C2NVIPServicesResponse
 	res.VIPServices = b.VIPServices()
-	res.ServicesHash = b.vipServiceHash(res.VIPServices)
+	res.ServicesHash = vipServiceHash(b.logf, res.VIPServices)
 
 	w.Header().Set("Content-Type", "application/json")
 	json.NewEncoder(w).Encode(res)
@@ -1443,3 +1446,51 @@ func (b *LocalBackend) setVIPServicesTCPPortsInterceptedLocked(svcPorts map[tail
 
 	b.shouldInterceptVIPServicesTCPPortAtomic.Store(generateInterceptVIPServicesTCPPortFunc(svcAddrPorts))
 }
+
+func maybeUpdateHostinfoServicesHashLocked(b *LocalBackend, hi *tailcfg.Hostinfo, prefs ipn.PrefsView) bool {
+	latestHash := vipServiceHash(b.logf, b.vipServicesFromPrefsLocked(prefs))
+	if hi.ServicesHash != latestHash {
+		hi.ServicesHash = latestHash
+		return true
+	}
+	return false
+}
+
+func maybeUpdateHostinfoFunnelLocked(b *LocalBackend, hi *tailcfg.Hostinfo, prefs ipn.PrefsView) (changed bool) {
+	// The Hostinfo.IngressEnabled field is used to communicate to control whether
+	// the node has funnel enabled.
+	if ie := b.hasIngressEnabledLocked(); hi.IngressEnabled != ie {
+		b.logf("Hostinfo.IngressEnabled changed to %v", ie)
+		hi.IngressEnabled = ie
+		changed = true
+	}
+	// The Hostinfo.WireIngress field tells control whether the user intends
+	// to use funnel with this node even though it is not currently enabled.
+	// This is an optimization to control- Funnel requires creation of DNS
+	// records and because DNS propagation can take time, we want to ensure
+	// that the records exist for any node that intends to use funnel even
+	// if it's not enabled. If hi.IngressEnabled is true, control knows that
+	// DNS records are needed, so we can save bandwidth and not send
+	// WireIngress.
+	if wire := b.shouldWireInactiveIngressLocked(); hi.WireIngress != wire {
+		b.logf("Hostinfo.WireIngress changed to %v", wire)
+		hi.WireIngress = wire
+		changed = true
+	}
+	return changed
+}
+
+func vipServiceHash(logf logger.Logf, services []*tailcfg.VIPService) string {
+	if len(services) == 0 {
+		return ""
+	}
+	h := sha256.New()
+	jh := json.NewEncoder(h)
+	if err := jh.Encode(services); err != nil {
+		logf("vipServiceHashLocked: %v", err)
+		return ""
+	}
+	var buf [sha256.Size]byte
+	h.Sum(buf[:0])
+	return hex.EncodeToString(buf[:])
+}