From 4b1af6fb06de132eb9648d69e9942351f0537aa7 Mon Sep 17 00:00:00 2001 From: Santiago Date: Thu, 31 Mar 2022 15:57:48 -0300 Subject: [PATCH] Fix empty contact point URLs when template parsing fails (#47029) * fix empty URLs * leave URL templating, use fallback * better fix, new tests cases * fix linting errors --- .../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 9e92a4c6d83..17b06f8c648 100644 --- a/pkg/services/ngalert/notifier/channels/dingding.go +++ b/pkg/services/ngalert/notifier/channels/dingding.go @@ -122,9 +122,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 a4b9fd8a286..db1a927aea4 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: `{}`, @@ -118,6 +176,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 20003464722..be7f7537ec9 100644 --- a/pkg/services/ngalert/notifier/channels/googlechat.go +++ b/pkg/services/ngalert/notifier/channels/googlechat.go @@ -139,9 +139,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 93455ae8a61..7b4629cec57 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, }, } @@ -294,6 +343,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 e03a48bf8d7..f358084fd1e 100644 --- a/pkg/services/ngalert/notifier/channels/teams.go +++ b/pkg/services/ngalert/notifier/channels/teams.go @@ -108,9 +108,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 1d09c78661c..f91322c3905 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: `{}`, @@ -143,6 +225,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 12f9fb0c7fc..1035a149f16 100644 --- a/pkg/services/ngalert/notifier/channels/victorops.go +++ b/pkg/services/ngalert/notifier/channels/victorops.go @@ -118,9 +118,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 f0c8740f9c1..04f1f6e167e 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: `{}`, @@ -115,6 +190,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") }