diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index bb8bba2518..0afc28d582 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -1229,16 +1229,22 @@ receives `resolve` for the alert's dedup key. Only the wording changes. agent for the current reading.") in place of " on is now healthy" for every templated webhook `.Message`. Grouped lists append the short outcome ("moved to pve1 (Host Agent)") to the moved entry. ntfy titles a -single moved alert `MOVED: ` and swaps the green check tag for -`arrow_right`. Email and Apprise title it `Pulse alert moved: ` and -add the summary after the breach message. The resolution rides the Alert JSON, -so it survives the delivery queue round trip and appears in the default JSON -payload's `alerts` array. Built-in template titles still say Resolved, which is -accurate for the closed alert, and a custom template that hard-codes recovery -text keeps it. Issue #1682 is the precedent: a recipient called "is now -healthy" a false factual claim when an alert closed without recovering. +single moved alert `MOVED: `. Email and Apprise title it `Pulse alert +moved: ` and add the summary after the breach message. Recovery +visuals follow the same rule: when any close in a resolved delivery was not a +recovery, `WebhookPayloadData.NotRecovered` is set and the built-in templates +drop them (Discord's green embed becomes the neutral blue, both Teams cards +lose recovery green, Mattermost's check emoji becomes `:arrow_right:`), and +ntfy swaps its green check tag for `arrow_right`, for a single alert or a batch. +Built-in titles still say Resolved, which is accurate for the closed alert; +custom templates can read `.NotRecovered`, and one that hard-codes recovery +text keeps it. The resolution rides the Alert JSON, so it survives the delivery +queue round trip and appears in the default JSON payload's `alerts` array. +Issue #1682 is the precedent: a recipient called "is now healthy" a false +factual claim when an alert closed without recovering. Verification: `resolved_handover_test.go` drives the firing, its receipt and the queued close through the normal entry points for ntfy, and renders Discord, -PagerDuty, a grouped Slack list and email/Apprise content, plus the unchanged -ordinary recovery wording. +both Teams cards, Mattermost, PagerDuty, a grouped Slack list, a grouped ntfy +batch and email/Apprise content, plus the unchanged ordinary recovery wording +and visuals. diff --git a/internal/notifications/notifications.go b/internal/notifications/notifications.go index 93ae16e0f3..71f467b967 100644 --- a/internal/notifications/notifications.go +++ b/internal/notifications/notifications.go @@ -453,6 +453,17 @@ func resolvedAlertNotRecoveredSummary(alert *alerts.Alert) string { return alert.Resolution.Describe(label) } +// resolvedAlertsIncludeNotRecovered reports whether any close in a resolved +// notification was not a recovery, so recovery visuals must not be used. +func resolvedAlertsIncludeNotRecovered(alertList []*alerts.Alert) bool { + for _, alert := range alertList { + if alert != nil && alert.Resolution.Outcome() != "" { + return true + } + } + return false +} + // resolvedAlertListEntry names one alert in a grouped recovery list. func resolvedAlertListEntry(alert *alerts.Alert) string { entry := fmt.Sprintf("%s on %s", alert.ResourceName, alert.Node) @@ -3018,6 +3029,7 @@ func (n *NotificationManager) sendResolvedWebhook(webhook WebhookConfig, alertLi data.ResolvedAtISO = resolvedAt.Format(time.RFC3339) data.Duration = formatWebhookDuration(resolvedAt.Sub(alert.StartTime)) data.Message = resolvedAlertMessage(alert) + data.NotRecovered = resolvedAlertsIncludeNotRecovered(alertList) data.AlertCount = len(alertList) data.Alerts = alertList if len(alertList) > 1 { @@ -3074,16 +3086,19 @@ func (n *NotificationManager) sendResolvedWebhookNtfy(webhook WebhookConfig, ale } // Build plain-text body and title. A single alert that moved rather than - // recovered says so in both, and drops the green check. + // recovered says so in both, and no batch holding such a close carries + // the green check. var body strings.Builder title := "RESOLVED" tags := "white_check_mark,pulse,resolved" + if resolvedAlertsIncludeNotRecovered(alertList) { + tags = "arrow_right,pulse,resolved" + } if len(alertList) == 1 && alertList[0] != nil { a := alertList[0] title = fmt.Sprintf("RESOLVED: %s", a.ResourceName) if summary := resolvedAlertNotRecoveredSummary(a); summary != "" { title = fmt.Sprintf("MOVED: %s", a.ResourceName) - tags = "arrow_right,pulse,resolved" body.WriteString(summary) } else { fmt.Fprintf(&body, "Resolved: %s", resolvedAlertMessage(a)) diff --git a/internal/notifications/resolved_handover_test.go b/internal/notifications/resolved_handover_test.go index b980c6fb56..f144a178be 100644 --- a/internal/notifications/resolved_handover_test.go +++ b/internal/notifications/resolved_handover_test.go @@ -99,64 +99,108 @@ func TestMovedAlertCloseIsDeliveredAsMovedNotRecovered(t *testing.T) { } } -func TestMovedAlertCloseRendersHonestlyOnEveryChannel(t *testing.T) { - capture := func(t *testing.T, service string) map[string]any { - t.Helper() - var gotBody []byte - server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - gotBody, _ = io.ReadAll(r.Body) - w.WriteHeader(http.StatusOK) - })) - defer server.Close() - nm := NewNotificationManager("https://pulse.local") - _ = nm.UpdateAllowedPrivateCIDRs("127.0.0.1") - webhook := WebhookConfig{Name: service, URL: server.URL + "/hook", Enabled: true, Service: service, - CustomFields: map[string]string{"routing_key": "rk"}} - if err := nm.sendResolvedWebhook(webhook, []*alerts.Alert{movedNodeMemoryAlert()}, time.Now()); err != nil { - t.Fatalf("sendResolvedWebhook %s: %v", service, err) - } - var payload map[string]any - if err := json.Unmarshal(gotBody, &payload); err != nil { - t.Fatalf("unmarshal %s payload: %v", service, err) - } - return payload +// captureResolvedWebhook renders one resolved delivery through an isolated +// manager and returns what the destination received. +func captureResolvedWebhook(t *testing.T, service string, alertList []*alerts.Alert) ([]byte, http.Header) { + t.Helper() + type request struct { + body []byte + header http.Header } + received := make(chan request, 1) + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + received <- request{body: body, header: r.Header.Clone()} + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + nm := NewNotificationManagerWithDataDir("https://pulse.local", t.TempDir()) + t.Cleanup(nm.Stop) + if err := nm.UpdateAllowedPrivateCIDRs("127.0.0.1"); err != nil { + t.Fatal(err) + } + webhook := WebhookConfig{Name: service, URL: server.URL + "/hook", Enabled: true, Service: service, + CustomFields: map[string]string{"routing_key": "rk"}} + if err := nm.sendResolvedWebhook(webhook, alertList, time.Now()); err != nil { + t.Fatalf("sendResolvedWebhook %s: %v", service, err) + } + select { + case got := <-received: + return got.body, got.header + case <-time.After(5 * time.Second): + t.Fatalf("%s destination received nothing", service) + return nil, nil + } +} - t.Run("discord says moved", func(t *testing.T) { - embed := capture(t, "discord")["embeds"].([]any)[0].(map[string]any) +func decodeResolvedPayload(t *testing.T, service string, body []byte) map[string]any { + t.Helper() + var payload map[string]any + if err := json.Unmarshal(body, &payload); err != nil { + t.Fatalf("unmarshal %s payload: %v\n%s", service, err, body) + } + return payload +} + +func TestMovedAlertCloseRendersHonestlyOnEveryChannel(t *testing.T) { + recovered := &alerts.Alert{ID: "vm-cpu", Type: "cpu", ResourceName: "web-vm-01", Node: "pve1", StartTime: time.Now().Add(-time.Minute)} + + t.Run("discord says moved without the recovery colour", func(t *testing.T) { + body, _ := captureResolvedWebhook(t, "discord", []*alerts.Alert{movedNodeMemoryAlert()}) + embed := decodeResolvedPayload(t, "discord", body)["embeds"].([]any)[0].(map[string]any) description, _ := embed["description"].(string) if !strings.Contains(description, movedSummary) || strings.Contains(description, "healthy") { t.Fatalf("description = %q, want the handover summary", description) } + if embed["color"] != float64(3447003) { + t.Fatalf("embed color = %v, want the neutral 3447003, not recovery green", embed["color"]) + } + }) + + t.Run("teams cards drop recovery green", func(t *testing.T) { + body, _ := captureResolvedWebhook(t, "teams", []*alerts.Alert{movedNodeMemoryAlert()}) + if got := decodeResolvedPayload(t, "teams", body)["themeColor"]; got != "0076D7" { + t.Fatalf("teams themeColor = %v, want 0076D7", got) + } + body, _ = captureResolvedWebhook(t, "teams-adaptive", []*alerts.Alert{movedNodeMemoryAlert()}) + if !strings.Contains(string(body), `"color": "Accent"`) || strings.Contains(string(body), `"color": "Good"`) { + t.Fatalf("teams adaptive card = %s, want the Accent title colour", body) + } + }) + + t.Run("mattermost drops the check mark", func(t *testing.T) { + body, _ := captureResolvedWebhook(t, "mattermost", []*alerts.Alert{movedNodeMemoryAlert()}) + text, _ := decodeResolvedPayload(t, "mattermost", body)["text"].(string) + if strings.Contains(text, ":white_check_mark:") || !strings.Contains(text, ":arrow_right: **RESOLVED**") || !strings.Contains(text, movedSummary) { + t.Fatalf("mattermost text = %q", text) + } }) t.Run("pagerduty still closes the incident", func(t *testing.T) { - payload := capture(t, "pagerduty") + body, _ := captureResolvedWebhook(t, "pagerduty", []*alerts.Alert{movedNodeMemoryAlert()}) + payload := decodeResolvedPayload(t, "pagerduty", body) if payload["event_action"] != "resolve" || payload["dedup_key"] != "pve1-memory" { t.Fatalf("payload = %v, want a resolve for the node alert's incident", payload) } }) t.Run("grouped list names the move", func(t *testing.T) { - var gotBody []byte - server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - gotBody, _ = io.ReadAll(r.Body) - w.WriteHeader(http.StatusOK) - })) - defer server.Close() - nm := NewNotificationManager("https://pulse.local") - _ = nm.UpdateAllowedPrivateCIDRs("127.0.0.1") - recovered := &alerts.Alert{ID: "vm-cpu", Type: "cpu", ResourceName: "web-vm-01", Node: "pve1", StartTime: time.Now().Add(-time.Minute)} - webhook := WebhookConfig{Name: "slack", URL: server.URL + "/hook", Enabled: true, Service: "slack"} - if err := nm.sendResolvedWebhook(webhook, []*alerts.Alert{movedNodeMemoryAlert(), recovered}, time.Now()); err != nil { - t.Fatalf("sendResolvedWebhook: %v", err) - } - body := string(gotBody) - if !strings.Contains(body, "pve1 on pve1, moved to pve1 (Host Agent)") || !strings.Contains(body, "web-vm-01 on pve1") { + body, _ := captureResolvedWebhook(t, "slack", []*alerts.Alert{movedNodeMemoryAlert(), recovered}) + if !strings.Contains(string(body), "pve1 on pve1, moved to pve1 (Host Agent)") || !strings.Contains(string(body), "web-vm-01 on pve1") { t.Fatalf("grouped body = %s, want the moved alert marked and the recovery listed", body) } }) + t.Run("grouped ntfy batch drops the check mark", func(t *testing.T) { + body, header := captureResolvedWebhook(t, "ntfy", []*alerts.Alert{movedNodeMemoryAlert(), recovered}) + if tags := header.Get("Tags"); strings.Contains(tags, "white_check_mark") || !strings.Contains(tags, "arrow_right") { + t.Fatalf("ntfy tags = %q, want no check mark on a batch holding a move", tags) + } + if !strings.Contains(string(body), "- pve1 on pve1, moved to pve1 (Host Agent)\n") { + t.Fatalf("ntfy body = %q", body) + } + }) + t.Run("email and apprise say moved", func(t *testing.T) { title, htmlBody, textBody := buildResolvedNotificationContent([]*alerts.Alert{movedNodeMemoryAlert()}, time.Now(), "") if title != "Pulse alert moved: pve1" { @@ -170,14 +214,26 @@ func TestMovedAlertCloseRendersHonestlyOnEveryChannel(t *testing.T) { } }) - t.Run("ordinary recovery is unchanged", func(t *testing.T) { - recovered := movedNodeMemoryAlert() - recovered.Resolution = nil - if got := resolvedAlertMessage(recovered); got != "pve1 on pve1 is now healthy" { + t.Run("ordinary recovery keeps its wording and visuals", func(t *testing.T) { + plain := movedNodeMemoryAlert() + plain.Resolution = nil + if got := resolvedAlertMessage(plain); got != "pve1 on pve1 is now healthy" { t.Fatalf("resolvedAlertMessage() = %q", got) } - if title, _, _ := buildResolvedNotificationContent([]*alerts.Alert{recovered}, time.Now(), ""); title != "Pulse alert resolved: pve1" { + if title, _, _ := buildResolvedNotificationContent([]*alerts.Alert{plain}, time.Now(), ""); title != "Pulse alert resolved: pve1" { t.Fatalf("title = %q", title) } + body, _ := captureResolvedWebhook(t, "discord", []*alerts.Alert{plain}) + if embed := decodeResolvedPayload(t, "discord", body)["embeds"].([]any)[0].(map[string]any); embed["color"] != float64(3066993) { + t.Fatalf("ordinary recovery embed color = %v, want green 3066993", embed["color"]) + } + body, _ = captureResolvedWebhook(t, "mattermost", []*alerts.Alert{plain}) + if text, _ := decodeResolvedPayload(t, "mattermost", body)["text"].(string); !strings.Contains(text, ":white_check_mark: **RESOLVED**") { + t.Fatalf("ordinary mattermost text = %q", text) + } + _, header := captureResolvedWebhook(t, "ntfy", []*alerts.Alert{plain}) + if tags := header.Get("Tags"); tags != "white_check_mark,pulse,resolved" { + t.Fatalf("ordinary ntfy tags = %q", tags) + } }) } diff --git a/internal/notifications/webhook_enhanced.go b/internal/notifications/webhook_enhanced.go index 5c390fd1c6..c586c2e9fb 100644 --- a/internal/notifications/webhook_enhanced.go +++ b/internal/notifications/webhook_enhanced.go @@ -65,6 +65,10 @@ type WebhookPayloadData struct { Event string // "alert" or "resolved" ResolvedAt string // RFC3339 timestamp when alert was resolved ResolvedAtISO string // Same as ResolvedAt (alias for template consistency) + // NotRecovered marks a resolved notification in which at least one close + // was not a recovery (an alert that moved to a linked Pulse agent), so + // templates can drop recovery colours and check marks. + NotRecovered bool // Tenant identity (multi-tenant orgs and MSP client runtimes). // Empty on plain single-tenant installs. diff --git a/internal/notifications/webhook_templates.go b/internal/notifications/webhook_templates.go index 8c9f8db4f0..5e4b2bf152 100644 --- a/internal/notifications/webhook_templates.go +++ b/internal/notifications/webhook_templates.go @@ -56,7 +56,7 @@ func GetWebhookTemplates() []WebhookTemplate { "embeds": [{ "title": "Resolved: {{.ResourceName | jsonString}}", "description": "{{.Message | jsonString}}", - "color": 3066993, + "color": {{if .NotRecovered}}3447003{{else}}3066993{{end}}, "fields": [ {"name": "Resource", "value": "{{.ResourceName | jsonString}}", "inline": true}, {"name": "Node", "value": "{{.Node | jsonString}}", "inline": true}, @@ -241,7 +241,7 @@ func GetWebhookTemplates() []WebhookTemplate { ResolvedPayloadTemplate: `{ "@type": "MessageCard", "@context": "http://schema.org/extensions", - "themeColor": "2DC72D", + "themeColor": "{{if .NotRecovered}}0076D7{{else}}2DC72D{{end}}", "summary": "Resolved: {{.ResourceName | jsonString}}", {{if .Mention}}"text": "{{.Mention | jsonString}}",{{end}} "sections": [{ @@ -372,7 +372,7 @@ func GetWebhookTemplates() []WebhookTemplate { "text": "Resolved: {{.ResourceName | jsonString}}", "weight": "Bolder", "size": "Large", - "color": "Good" + "color": "{{if .NotRecovered}}Accent{{else}}Good{{end}}" }, { "type": "TextBlock", @@ -521,7 +521,7 @@ View in Pulse: {{.Instance}}`, ResolvedPayloadTemplate: `{ "username": "Pulse Monitoring", "icon_url": "https://raw.githubusercontent.com/rcourtman/Pulse/main/frontend-modern/public/android-chrome-192x192.png", - "text": "{{if .Mention}}{{.Mention | jsonString}}\n\n{{end}}:white_check_mark: **RESOLVED**\n\n**{{.ResourceName | jsonString}}** on **{{.Node | jsonString}}**\n\n{{.Message | jsonString}}\n\n| Detail | Value |\n|:-------|:------|\n| Resource | {{.ResourceName | jsonString}} |\n| Node | {{.Node | jsonString}} |\n| Type | {{.Type | title | jsonString}} |\n| Duration | {{.Duration | jsonString}} |\n| Resolved At | {{.ResolvedAt | jsonString}} |\n| Alert Identifier | {{.ID | jsonString}} |" + "text": "{{if .Mention}}{{.Mention | jsonString}}\n\n{{end}}{{if .NotRecovered}}:arrow_right:{{else}}:white_check_mark:{{end}} **RESOLVED**\n\n**{{.ResourceName | jsonString}}** on **{{.Node | jsonString}}**\n\n{{.Message | jsonString}}\n\n| Detail | Value |\n|:-------|:------|\n| Resource | {{.ResourceName | jsonString}} |\n| Node | {{.Node | jsonString}} |\n| Type | {{.Type | title | jsonString}} |\n| Duration | {{.Duration | jsonString}} |\n| Resolved At | {{.ResolvedAt | jsonString}} |\n| Alert Identifier | {{.ID | jsonString}} |" }`, Instructions: "1. In Mattermost, go to Integrations > Incoming Webhooks\n2. Create a new webhook and select the channel\n3. Copy the webhook URL and paste it here\n\nNote: This template uses Markdown formatting which is fully supported by Mattermost.", },