From 6596c9d37df26f1e0f952bc5543c0f9170d13975 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Thu, 31 Mar 2022 12:21:17 -0700 Subject: [PATCH] Fix empty contact point URLs when template parsing fails (#47029) (#47162) * fix empty URLs * leave URL templating, use fallback * better fix, new tests cases * fix linting errors (cherry picked from commit 4b1af6fb06de132eb9648d69e9942351f0537aa7) Co-authored-by: Santiago --- .../ngalert/notifier/channels/dingding.go | 8 +- .../notifier/channels/dingding_test.go | 60 +++++++++++++ .../ngalert/notifier/channels/googlechat.go | 8 +- .../notifier/channels/googlechat_test.go | 53 +++++++++++- .../ngalert/notifier/channels/teams.go | 8 +- .../ngalert/notifier/channels/teams_test.go | 84 +++++++++++++++++++ .../ngalert/notifier/channels/victorops.go | 8 +- .../notifier/channels/victorops_test.go | 77 +++++++++++++++++ pkg/services/notifications/webhook.go | 4 +- 9 files changed, 303 insertions(+), 7 deletions(-) diff --git a/pkg/services/ngalert/notifier/channels/dingding.go b/pkg/services/ngalert/notifier/channels/dingding.go index bca261e756c..90399d72102 100644 --- a/pkg/services/ngalert/notifier/channels/dingding.go +++ b/pkg/services/ngalert/notifier/channels/dingding.go @@ -102,9 +102,15 @@ func (dd *DingDingNotifier) Notify(ctx context.Context, as ...*types.Alert) (boo } } - u := tmpl(dd.URL) if tmplErr != nil { dd.log.Warn("failed to template DingDing message", "err", tmplErr.Error()) + tmplErr = nil + } + + u := tmpl(dd.URL) + if tmplErr != nil { + dd.log.Warn("failed to template DingDing URL", "err", tmplErr.Error(), "fallback", dd.URL) + u = dd.URL } body, err := json.Marshal(bodyMsg) diff --git a/pkg/services/ngalert/notifier/channels/dingding_test.go b/pkg/services/ngalert/notifier/channels/dingding_test.go index 64df838fdcf..b411c70f763 100644 --- a/pkg/services/ngalert/notifier/channels/dingding_test.go +++ b/pkg/services/ngalert/notifier/channels/dingding_test.go @@ -79,6 +79,64 @@ func TestDingdingNotifier(t *testing.T) { "msgtype": "actionCard", }, expMsgError: nil, + }, { + name: "Missing field in template", + settings: `{ + "url": "http://localhost", + "message": "I'm a custom template {{ .NotAField }} bad template", + "msgType": "actionCard" + }`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1"}, + }, + }, { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val2"}, + Annotations: model.LabelSet{"ann1": "annv2"}, + }, + }, + }, + expMsg: map[string]interface{}{ + "link": map[string]interface{}{ + "messageUrl": "dingtalk://dingtalkclient/page/link?pc_slide=false&url=http%3A%2F%2Flocalhost%2Falerting%2Flist", + "text": "I'm a custom template ", + "title": "", + }, + "msgtype": "link", + }, + expMsgError: nil, + }, { + name: "Invalid template", + settings: `{ + "url": "http://localhost", + "message": "I'm a custom template {{ {.NotAField }} bad template", + "msgType": "actionCard" + }`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1"}, + }, + }, { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val2"}, + Annotations: model.LabelSet{"ann1": "annv2"}, + }, + }, + }, + expMsg: map[string]interface{}{ + "link": map[string]interface{}{ + "messageUrl": "dingtalk://dingtalkclient/page/link?pc_slide=false&url=http%3A%2F%2Flocalhost%2Falerting%2Flist", + "text": "", + "title": "", + }, + "msgtype": "link", + }, + expMsgError: nil, }, { name: "Error in initing", settings: `{}`, @@ -117,6 +175,8 @@ func TestDingdingNotifier(t *testing.T) { require.NoError(t, err) require.True(t, ok) + require.NotEmpty(t, webhookSender.Webhook.Url) + expBody, err := json.Marshal(c.expMsg) require.NoError(t, err) diff --git a/pkg/services/ngalert/notifier/channels/googlechat.go b/pkg/services/ngalert/notifier/channels/googlechat.go index f14c1f373e2..88d465b62ba 100644 --- a/pkg/services/ngalert/notifier/channels/googlechat.go +++ b/pkg/services/ngalert/notifier/channels/googlechat.go @@ -120,9 +120,15 @@ func (gcn *GoogleChatNotifier) Notify(ctx context.Context, as ...*types.Alert) ( }, } - u := tmpl(gcn.URL) if tmplErr != nil { gcn.log.Warn("failed to template GoogleChat message", "err", tmplErr.Error()) + tmplErr = nil + } + + u := tmpl(gcn.URL) + if tmplErr != nil { + gcn.log.Warn("failed to template GoogleChat URL", "err", tmplErr.Error(), "fallback", gcn.URL) + u = gcn.URL } body, err := json.Marshal(res) diff --git a/pkg/services/ngalert/notifier/channels/googlechat_test.go b/pkg/services/ngalert/notifier/channels/googlechat_test.go index 527d77be16f..2dab4a91fed 100644 --- a/pkg/services/ngalert/notifier/channels/googlechat_test.go +++ b/pkg/services/ngalert/notifier/channels/googlechat_test.go @@ -205,7 +205,7 @@ func TestGoogleChatNotifier(t *testing.T) { }, expMsgError: nil, }, { - name: "Invalid template", + name: "Missing field in template", settings: `{"url": "http://localhost", "message": "I'm a custom template {{ .NotAField }} bad template"}`, alerts: []*types.Alert{ { @@ -258,6 +258,55 @@ func TestGoogleChatNotifier(t *testing.T) { }, }, expMsgError: nil, + }, { + name: "Invalid template", + settings: `{"url": "http://localhost", "message": "I'm a custom template {{ {.NotAField }} bad template"}`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1", "__dashboardUid__": "abcd", "__panelId__": "efgh"}, + }, + }, + }, + expMsg: &outerStruct{ + PreviewText: "[FIRING:1] (val1)", + FallbackText: "[FIRING:1] (val1)", + Cards: []card{ + { + Header: header{ + Title: "[FIRING:1] (val1)", + }, + Sections: []section{ + { + Widgets: []widget{ + buttonWidget{ + Buttons: []button{ + { + TextButton: textButton{ + Text: "OPEN IN GRAFANA", + OnClick: onClick{ + OpenLink: openLink{ + URL: "http://localhost/alerting/list", + }, + }, + }, + }, + }, + }, + textParagraphWidget{ + Text: text{ + // RFC822 only has the minute, hence it works in most cases. + Text: "Grafana v" + setting.BuildVersion + " | " + constNow.Format(time.RFC822), + }, + }, + }, + }, + }, + }, + }, + }, + expMsgError: nil, }, } @@ -293,6 +342,8 @@ func TestGoogleChatNotifier(t *testing.T) { require.NoError(t, err) require.True(t, ok) + require.NotEmpty(t, webhookSender.Webhook.Url) + expBody, err := json.Marshal(c.expMsg) require.NoError(t, err) diff --git a/pkg/services/ngalert/notifier/channels/teams.go b/pkg/services/ngalert/notifier/channels/teams.go index 509b6e10844..028f89fcc2e 100644 --- a/pkg/services/ngalert/notifier/channels/teams.go +++ b/pkg/services/ngalert/notifier/channels/teams.go @@ -88,9 +88,15 @@ func (tn *TeamsNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, }, } - u := tmpl(tn.URL) if tmplErr != nil { tn.log.Warn("failed to template Teams message", "err", tmplErr.Error()) + tmplErr = nil + } + + u := tmpl(tn.URL) + if tmplErr != nil { + tn.log.Warn("failed to template Teams URL", "err", tmplErr.Error(), "fallback", tn.URL) + u = tn.URL } b, err := json.Marshal(&body) diff --git a/pkg/services/ngalert/notifier/channels/teams_test.go b/pkg/services/ngalert/notifier/channels/teams_test.go index 7f743f6fe91..fbe525d5ef4 100644 --- a/pkg/services/ngalert/notifier/channels/teams_test.go +++ b/pkg/services/ngalert/notifier/channels/teams_test.go @@ -103,6 +103,88 @@ func TestTeamsNotifier(t *testing.T) { }, }, expMsgError: nil, + }, { + name: "Missing field in template", + settings: `{ + "url": "http://localhost", + "message": "I'm a custom template {{ .NotAField }} bad template" + }`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1"}, + }, + }, { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val2"}, + Annotations: model.LabelSet{"ann1": "annv2"}, + }, + }, + }, + expMsg: map[string]interface{}{ + "@type": "MessageCard", + "@context": "http://schema.org/extensions", + "summary": "[FIRING:2] ", + "title": "[FIRING:2] ", + "themeColor": "#D63232", + "sections": []map[string]interface{}{ + { + "title": "Details", + "text": "I'm a custom template ", + }, + }, + "potentialAction": []map[string]interface{}{ + { + "@context": "http://schema.org", + "@type": "OpenUri", + "name": "View Rule", + "targets": []map[string]interface{}{{"os": "default", "uri": "http://localhost/alerting/list"}}, + }, + }, + }, + expMsgError: nil, + }, { + name: "Invalid template", + settings: `{ + "url": "http://localhost", + "message": "I'm a custom template {{ {.NotAField }} bad template" + }`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1"}, + }, + }, { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val2"}, + Annotations: model.LabelSet{"ann1": "annv2"}, + }, + }, + }, + expMsg: map[string]interface{}{ + "@type": "MessageCard", + "@context": "http://schema.org/extensions", + "summary": "[FIRING:2] ", + "title": "[FIRING:2] ", + "themeColor": "#D63232", + "sections": []map[string]interface{}{ + { + "title": "Details", + "text": "", + }, + }, + "potentialAction": []map[string]interface{}{ + { + "@context": "http://schema.org", + "@type": "OpenUri", + "name": "View Rule", + "targets": []map[string]interface{}{{"os": "default", "uri": "http://localhost/alerting/list"}}, + }, + }, + }, + expMsgError: nil, }, { name: "Error in initing", settings: `{}`, @@ -142,6 +224,8 @@ func TestTeamsNotifier(t *testing.T) { require.True(t, ok) require.NoError(t, err) + require.NotEmpty(t, webhookSender.Webhook.Url) + expBody, err := json.Marshal(c.expMsg) require.NoError(t, err) diff --git a/pkg/services/ngalert/notifier/channels/victorops.go b/pkg/services/ngalert/notifier/channels/victorops.go index aba0167965f..1cc037f1610 100644 --- a/pkg/services/ngalert/notifier/channels/victorops.go +++ b/pkg/services/ngalert/notifier/channels/victorops.go @@ -97,9 +97,15 @@ func (vn *VictoropsNotifier) Notify(ctx context.Context, as ...*types.Alert) (bo ruleURL := joinUrlPath(vn.tmpl.ExternalURL.String(), "/alerting/list", vn.log) bodyJSON.Set("alert_url", ruleURL) - u := tmpl(vn.URL) if tmplErr != nil { vn.log.Warn("failed to template VictorOps message", "err", tmplErr.Error()) + tmplErr = nil + } + + u := tmpl(vn.URL) + if tmplErr != nil { + vn.log.Info("failed to template VictorOps URL", "err", tmplErr.Error(), "fallback", vn.URL) + u = vn.URL } b, err := bodyJSON.MarshalJSON() diff --git a/pkg/services/ngalert/notifier/channels/victorops_test.go b/pkg/services/ngalert/notifier/channels/victorops_test.go index 3f11e25429b..d2900fe129e 100644 --- a/pkg/services/ngalert/notifier/channels/victorops_test.go +++ b/pkg/services/ngalert/notifier/channels/victorops_test.go @@ -75,6 +75,81 @@ func TestVictoropsNotifier(t *testing.T) { "state_message": "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val2\nAnnotations:\n - ann1 = annv2\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval2\n", }, expMsgError: nil, + }, { + name: "Custom message", + settings: `{"url": "http://localhost", "messageType": "Alerts firing: {{ len .Alerts.Firing }}"}`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1"}, + }, + }, { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val2"}, + Annotations: model.LabelSet{"ann1": "annv2"}, + }, + }, + }, + expMsg: map[string]interface{}{ + "alert_url": "http://localhost/alerting/list", + "entity_display_name": "[FIRING:2] ", + "entity_id": "6e3538104c14b583da237e9693b76debbc17f0f8058ef20492e5853096cf8733", + "message_type": "ALERTS FIRING: 2", + "monitoring_tool": "Grafana v" + setting.BuildVersion, + "state_message": "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val2\nAnnotations:\n - ann1 = annv2\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval2\n", + }, + expMsgError: nil, + }, { + name: "Missing field in template", + settings: `{"url": "http://localhost", "messageType": "custom template {{ .NotAField }} bad template"}`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1"}, + }, + }, { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val2"}, + Annotations: model.LabelSet{"ann1": "annv2"}, + }, + }, + }, + expMsg: map[string]interface{}{ + "alert_url": "http://localhost/alerting/list", + "entity_display_name": "", + "entity_id": "6e3538104c14b583da237e9693b76debbc17f0f8058ef20492e5853096cf8733", + "message_type": "CUSTOM TEMPLATE ", + "monitoring_tool": "Grafana v" + setting.BuildVersion, + "state_message": "", + }, + expMsgError: nil, + }, { + name: "Invalid template", + settings: `{"url": "http://localhost", "messageType": "custom template {{ {.NotAField }} bad template"}`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1"}, + }, + }, { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val2"}, + Annotations: model.LabelSet{"ann1": "annv2"}, + }, + }, + }, + expMsg: map[string]interface{}{ + "alert_url": "http://localhost/alerting/list", + "entity_display_name": "", + "entity_id": "6e3538104c14b583da237e9693b76debbc17f0f8058ef20492e5853096cf8733", + "message_type": "CRITICAL", + "monitoring_tool": "Grafana v" + setting.BuildVersion, + "state_message": "", + }, + expMsgError: nil, }, { name: "Error in initing, no URL", settings: `{}`, @@ -114,6 +189,8 @@ func TestVictoropsNotifier(t *testing.T) { require.NoError(t, err) require.True(t, ok) + require.NotEmpty(t, webhookSender.Webhook.Url) + // Remove the non-constant timestamp j, err := simplejson.NewJson([]byte(webhookSender.Webhook.Body)) require.NoError(t, err) diff --git a/pkg/services/notifications/webhook.go b/pkg/services/notifications/webhook.go index 2ae7f5f4fcc..2ea0ea19048 100644 --- a/pkg/services/notifications/webhook.go +++ b/pkg/services/notifications/webhook.go @@ -42,12 +42,12 @@ var netClient = &http.Client{ } func (ns *NotificationService) sendWebRequestSync(ctx context.Context, webhook *Webhook) error { - ns.log.Debug("Sending webhook", "url", webhook.Url, "http method", webhook.HttpMethod) - if webhook.HttpMethod == "" { webhook.HttpMethod = http.MethodPost } + ns.log.Debug("Sending webhook", "url", webhook.Url, "http method", webhook.HttpMethod) + if webhook.HttpMethod != http.MethodPost && webhook.HttpMethod != http.MethodPut { return fmt.Errorf("webhook only supports HTTP methods PUT or POST") }