From b1a62ba4222eabe92f8cf51cb7a97a51818fb69a Mon Sep 17 00:00:00 2001 From: James Sanderson Date: Wed, 7 May 2025 17:01:40 +0100 Subject: [PATCH] WIP for HealthV2 Updates tailscale/corp#27759 Signed-off-by: James Sanderson --- control/controlclient/map.go | 8 ++- control/controlclient/map_test.go | 85 +++++++++++++++++++++++++++++++ health/state.go | 21 ++++++-- tailcfg/tailcfg.go | 42 +++++++++++++-- types/netmap/nodemut.go | 1 + 5 files changed, 148 insertions(+), 9 deletions(-) diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 789042c12..8e94c125b 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -88,6 +88,7 @@ type mapSession struct { lastDomain string lastDomainAuditLogID string lastHealth []string + lastDisplayMessages map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage lastPopBrowserURL string lastTKAInfo *tailcfg.TKAInfo lastNetmapSummary string // from NetworkMap.VeryConcise @@ -410,6 +411,9 @@ func (ms *mapSession) updateStateFromResponse(resp *tailcfg.MapResponse) { if resp.Health != nil { ms.lastHealth = resp.Health } + if resp.DisplayMessages != nil { + ms.lastDisplayMessages = resp.DisplayMessages + } if resp.TKAInfo != nil { ms.lastTKAInfo = resp.TKAInfo } @@ -831,7 +835,9 @@ func (ms *mapSession) netmap() *netmap.NetworkMap { var displayMessages map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage - if len(ms.lastHealth) > 0 { + if len(ms.lastDisplayMessages) != 0 { + displayMessages = ms.lastDisplayMessages + } else if len(ms.lastHealth) > 0 { // As they all resolve to the same ID, we can only pass on one message // from ControlHealth. In practice we generally send 0 or 1, but // if there are multiple, the last one wins. diff --git a/control/controlclient/map_test.go b/control/controlclient/map_test.go index 61686d701..893061041 100644 --- a/control/controlclient/map_test.go +++ b/control/controlclient/map_test.go @@ -1168,3 +1168,88 @@ func TestNetmapHealthIntegration(t *testing.T) { t.Errorf("warning.Text = %q, want %q", got, want) } } + +func TestNetmapDisplayMessageIntegration(t *testing.T) { + ms := newTestMapSession(t, nil) + ht := health.Tracker{} + + ht.SetIPNState("NeedsLogin", true) + ht.GotStreamedMapResponse() + + nm := ms.netmapForResponse(&tailcfg.MapResponse{ + DisplayMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{ + "test-message": { + Title: "Testing", + Text: "This is a test message", + Severity: tailcfg.SeverityHigh, + ImpactsConnectivity: true, + PrimaryAction: &tailcfg.DisplayMessageAction{ + URL: "https://www.example.com", + Label: "Learn more", + }, + }}, + }) + ht.SetControlHealth(nm.DisplayMessages) + + state := ht.CurrentState() + warning, ok := state.Warnings["test-message"] + + if !ok { + t.Fatal("no warning found in current state with code 'test-message'") + } + if got, want := warning.Title, "Testing"; got != want { + t.Errorf("warning.Title = %q, want %q", got, want) + } + if got, want := warning.Severity, health.SeverityHigh; got != want { + t.Errorf("warning.Severity = %s, want %s", got, want) + } + if got, want := warning.Text, "This is a test message"; got != want { + t.Errorf("warning.Text = %q, want %q", got, want) + } + if got, want := warning.ImpactsConnectivity, true; got != want { + t.Errorf("warning.ImpactsConnectivity = %t, want %t", got, want) + } + if warning.PrimaryAction == nil { + t.Fatalf("warning.PrimaryAction = nil") + } + if got, want := warning.PrimaryAction.URL, "https://www.example.com"; got != want { + t.Errorf("warning.PrimaryAction.URL = %q, want %q", got, want) + } + if got, want := warning.PrimaryAction.Label, "Learn more"; got != want { + t.Errorf("warning.PrimaryAction.Label = %q, want %q", got, want) + } +} + +func TestNetmapDisplayMessageClears(t *testing.T) { + ms := newTestMapSession(t, nil) + ht := health.Tracker{} + + ht.SetIPNState("NeedsLogin", true) + ht.GotStreamedMapResponse() + + nm := ms.netmapForResponse(&tailcfg.MapResponse{ + DisplayMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{ + "test-message": {}, + }, + }) + ht.SetControlHealth(nm.DisplayMessages) + + state := ht.CurrentState() + _, ok := state.Warnings["test-message"] + + if !ok { + t.Fatal("no warning found in current state with code 'test-message'") + } + + nm = ms.netmapForResponse(&tailcfg.MapResponse{ + DisplayMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{}, + }) + ht.SetControlHealth(nm.DisplayMessages) + + state = ht.CurrentState() + _, ok = state.Warnings["test-message"] + + if ok { + t.Fatal("warning found with code 'test-message'; want warnings to be cleared") + } +} diff --git a/health/state.go b/health/state.go index 9d2c510e7..4802694a3 100644 --- a/health/state.go +++ b/health/state.go @@ -30,10 +30,16 @@ type UnhealthyState struct { Severity Severity Title string Text string - BrokenSince *time.Time `json:",omitempty"` - Args Args `json:",omitempty"` - DependsOn []WarnableCode `json:",omitempty"` - ImpactsConnectivity bool `json:",omitempty"` + BrokenSince *time.Time `json:",omitempty"` + Args Args `json:",omitempty"` + DependsOn []WarnableCode `json:",omitempty"` + ImpactsConnectivity bool `json:",omitempty"` + PrimaryAction *UnhealthyStateAction `json:",omitempty"` +} + +type UnhealthyStateAction struct { + URL string + Label string } // unhealthyState returns a unhealthyState of the Warnable given its current warningState. @@ -131,6 +137,13 @@ func UnhealthyStateFromDisplayMessage(id tailcfg.DisplayMessageID, message tailc ImpactsConnectivity: message.ImpactsConnectivity, } + if message.PrimaryAction != nil { + state.PrimaryAction = &UnhealthyStateAction{ + URL: message.PrimaryAction.URL, + Label: message.PrimaryAction.Label, + } + } + return state } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 4e839f75c..80ab0a070 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -162,7 +162,8 @@ type CapabilityVersion int // - 114: 2025-01-30: NodeAttrMaxKeyDuration CapMap defined, clients might use it (no tailscaled code change) (#14829) // - 115: 2025-03-07: Client understands DERPRegion.NoMeasureNoHome. // - 116: 2025-05-05: Client serves MagicDNS "AAAA" if NodeAttrMagicDNSPeerAAAA set on self node -const CurrentCapabilityVersion CapabilityVersion = 116 +// - 117: 2025-04-30: Client understands HealthV2 alpha +const CurrentCapabilityVersion CapabilityVersion = 117 // ID is an integer ID for a user, node, or login allocated by the // control plane. @@ -2031,11 +2032,26 @@ type MapResponse struct { // known problems). A non-zero length slice are the list of problems that // the control plane sees. // + // Either this will be set, or DisplayMessages will be set, but not both. + // // Note that this package's type, due its use of a slice and omitempty, is // unable to marshal a zero-length non-nil slice. The control server needs // to marshal this type using a separate type. See MapResponse docs. Health []string `json:",omitempty"` + // DisplayMessages, if non-nil, sets the health state of the node from the + // control plane's perspective. A nil value means no change from the + // previous MapResponse. A non-nil 0-length slice restores the health to + // good (no known problems). A non-zero length slice are the list of + // problems that the control plane sees. + // + // Either this will be set, or Health will be set, but not both. + // + // Note that this package's type, due its use of a slice and omitempty, is + // unable to marshal a zero-length non-nil slice. The control server needs + // to marshal this type using a separate type. See MapResponse docs. + DisplayMessages map[DisplayMessageID]DisplayMessage `json:",omitempty"` + // SSHPolicy, if non-nil, updates the SSH policy for how incoming // SSH connections should be handled. SSHPolicy *SSHPolicy `json:",omitempty"` @@ -2080,8 +2096,8 @@ type MapResponse struct { } // DisplayMessage represents a health state of the node from the control plane's -// perspective. It is deliberately similar to health.Warnable as both get -// converted into health.UnhealthyState to be sent to the GUI. +// perspective. It is deliberately similar to [health.Warnable] as both get +// converted into [health.UnhealthyState] to be sent to the GUI. type DisplayMessage struct { // Title is a string that the GUI uses as title for this message. The title // should be short and fit in a single line. @@ -2091,13 +2107,31 @@ type DisplayMessage struct { Text string // Severity is the severity of the DisplayMessage, which the GUI can use to - // determine how to display it. Maps to health.Severity. + // determine how to display it. Maps to [health.Severity]. Severity DisplayMessageSeverity // ImpactsConnectivity is whether the health problem will impact the user's // ability to connect to the Internet or other nodes on the tailnet, which // the GUI can use to determine how to display it. ImpactsConnectivity bool `json:",omitempty"` + + // Primary action, if present, represents the action to allow the user to + // take when interacting with this message. For example, if the + // DisplayMessage is shown via a notification, the action label might be a + // button on that notification and clicking the button would open the URL. + PrimaryAction *DisplayMessageAction `json:",omitempty"` +} + +// DisplayMessageAction represents an action (URL and link) to be presented to +// the user associated with a [DisplayMessage]. +type DisplayMessageAction struct { + // URL is the URL to navigate to when the user interacts with this action + URL string + + // Label is the call to action for the UI to display on the UI element that + // will open the URL (such as a button or link). For example, "Sign in" or + // "Learn more". + Label string } // DisplayMessageID is a string that uniquely identifies the kind of health diff --git a/types/netmap/nodemut.go b/types/netmap/nodemut.go index e31c731be..ccbdeae3f 100644 --- a/types/netmap/nodemut.go +++ b/types/netmap/nodemut.go @@ -163,6 +163,7 @@ func mapResponseContainsNonPatchFields(res *tailcfg.MapResponse) bool { res.PacketFilters != nil || res.UserProfiles != nil || res.Health != nil || + res.DisplayMessages != nil || res.SSHPolicy != nil || res.TKAInfo != nil || res.DomainDataPlaneAuditLogID != "" ||