From 07b98db2bc01d64ad311259a41f57929c74d8cc2 Mon Sep 17 00:00:00 2001
From: James Sanderson <jsanderson@tailscale.com>
Date: Tue, 15 Apr 2025 13:56:38 +0100
Subject: [PATCH] attempting to make healthv2 warnables work better

---
 control/controlclient/direct.go |  4 ++++
 health/health.go                | 21 ++++++++-------------
 health/state.go                 | 11 +++++++++++
 ipn/ipnlocal/local.go           |  3 +++
 4 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go
index 68ab9ca17..da8b58672 100644
--- a/control/controlclient/direct.go
+++ b/control/controlclient/direct.go
@@ -1614,6 +1614,10 @@ func postPingResult(start time.Time, logf logger.Logf, c *http.Client, pr *tailc
 // ReportHealthChange reports to the control plane a change to this node's
 // health. w must be non-nil. us can be nil to indicate a healthy state for w.
 func (c *Direct) ReportHealthChange(w *health.Warnable, us *health.UnhealthyState) {
+	// TODO: Don't tell control about health messages that came from control?
+	if w == nil {
+		return
+	}
 	if w == health.NetworkStatusWarnable || w == health.IPNStateWarnable || w == health.LoginStateWarnable {
 		// We don't report these. These include things like the network is down
 		// (in which case we can't report anyway) or the user wanted things
diff --git a/health/health.go b/health/health.go
index 62ea194a8..714c5859c 100644
--- a/health/health.go
+++ b/health/health.go
@@ -1151,20 +1151,15 @@ func (t *Tracker) updateBuiltinWarnablesLocked() {
 		t.setHealthyLocked(derpRegionErrorWarnable)
 	}
 
-	if len(t.controlHealth) > 0 {
-		for _, s := range t.controlHealth {
-			t.setUnhealthyLocked(&Warnable{
-				Code:  WarnableCode(s.ID),
-				Title: s.Title,
-				Text: func(args Args) string {
-					return s.Text
-				},
-				Severity:            SeverityHigh,
-				ImpactsConnectivity: s.ImpactsConnectivity,
-			}, nil)
+	if t.controlHealth != nil {
+		// for _, cb := range t.watchers {
+		// 	go cb(nil, nil)
+		// }
+		if len(t.controlHealth) > 0 {
+			t.setUnhealthyLocked(controlHealthWarnable, nil)
+		} else {
+			t.setHealthyLocked(controlHealthWarnable)
 		}
-	} else {
-		t.setHealthyLocked(controlHealthWarnable)
 	}
 
 	if err := envknob.ApplyDiskConfigError(); err != nil {
diff --git a/health/state.go b/health/state.go
index 8fe87f0a9..8d764bbec 100644
--- a/health/state.go
+++ b/health/state.go
@@ -100,6 +100,17 @@ func (t *Tracker) CurrentState() *State {
 		wm[w.Code] = *w.unhealthyState(ws)
 	}
 
+	for _, h := range t.controlHealth {
+		wm[WarnableCode(h.ID)] = UnhealthyState{
+			WarnableCode:        WarnableCode(h.ID),
+			Severity:            SeverityHigh,
+			Title:               h.Title,
+			Text:                h.Text,
+			ImpactsConnectivity: h.ImpactsConnectivity,
+			URL:                 h.URL,
+		}
+	}
+
 	return &State{
 		Warnings: wm,
 	}
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 10a02d3cd..c967cdab1 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -968,6 +968,9 @@ func (b *LocalBackend) linkChange(delta *netmon.ChangeDelta) {
 }
 
 func (b *LocalBackend) onHealthChange(w *health.Warnable, us *health.UnhealthyState) {
+	if w == nil {
+		return
+	}
 	if us == nil {
 		b.logf("health(warnable=%s): ok", w.Code)
 	} else {