Alerting: Do not include button in googlechat notification if URL invalid (#47317)

* Alerting: Do not include button in googlechat notification if URL invalid

* Apply suggestions from code review

Co-authored-by: Alexander Weaver <weaver.alex.d@gmail.com>

* Alerting: Add test case for invalid external URL in googlechat notifier

Co-authored-by: Alexander Weaver <weaver.alex.d@gmail.com>

pkg/services/alerting/notifiers/googlechat.go

@@ -3,6 +3,7 @@ package notifiers
 import (
 	"encoding/json"
 	"fmt"
+	"net/url"
 	"time"
 
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -171,21 +172,25 @@ func (gcn *GoogleChatNotifier) Notify(evalContext *alerting.EvalContext) error {
 		}
 	}
 
-	// add a button widget (link to Grafana)
+	if gcn.isUrlAbsolute(ruleURL) {
-	widgets = append(widgets, buttonWidget{
+		// add a button widget (link to Grafana)
-		Buttons: []button{
+		widgets = append(widgets, buttonWidget{
-			{
+			Buttons: []button{
-				TextButton: textButton{
+				{
-					Text: "OPEN IN GRAFANA",
+					TextButton: textButton{
-					OnClick: onClick{
+						Text: "OPEN IN GRAFANA",
-						OpenLink: openLink{
+						OnClick: onClick{
-							URL: ruleURL,
+							OpenLink: openLink{
+								URL: ruleURL,
+							},
 						},
 					},
 				},
 			},
-		},
+		})
-	})
+	} else {
+		gcn.log.Warn("Grafana External URL setting is missing or invalid. Skipping 'open in grafana' button to prevent google from displaying empty alerts.", "ruleURL", ruleURL)
+	}
 
 	// add text paragraph widget for the build version and timestamp
 	widgets = append(widgets, textParagraphWidget{
@@ -227,3 +232,13 @@ func (gcn *GoogleChatNotifier) Notify(evalContext *alerting.EvalContext) error {
 
 	return nil
 }
+
+func (gcn *GoogleChatNotifier) isUrlAbsolute(urlToCheck string) bool {
+	parsed, err := url.Parse(urlToCheck)
+	if err != nil {
+		gcn.log.Warn("Could not parse URL", "urlToCheck", urlToCheck)
+		return false
+	}
+
+	return parsed.IsAbs()
+}

pkg/services/ngalert/notifier/channels/googlechat.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"net/url"
 	"time"
 
 	"github.com/prometheus/alertmanager/template"
@@ -101,21 +102,25 @@ func (gcn *GoogleChatNotifier) Notify(ctx context.Context, as ...*types.Alert) (
 	}
 
 	ruleURL := joinUrlPath(gcn.tmpl.ExternalURL.String(), "/alerting/list", gcn.log)
-	// Add a button widget (link to Grafana).
+	if gcn.isUrlAbsolute(ruleURL) {
-	widgets = append(widgets, buttonWidget{
+		// Add a button widget (link to Grafana).
-		Buttons: []button{
+		widgets = append(widgets, buttonWidget{
-			{
+			Buttons: []button{
-				TextButton: textButton{
+				{
-					Text: "OPEN IN GRAFANA",
+					TextButton: textButton{
-					OnClick: onClick{
+						Text: "OPEN IN GRAFANA",
-						OpenLink: openLink{
+						OnClick: onClick{
-							URL: ruleURL,
+							OpenLink: openLink{
+								URL: ruleURL,
+							},
 						},
 					},
 				},
 			},
-		},
+		})
-	})
+	} else {
+		gcn.log.Warn("Grafana External URL setting is missing or invalid. Skipping 'open in grafana' button to prevent google from displaying empty alerts.", "ruleURL", ruleURL)
+	}
 
 	// Add text paragraph widget for the build version and timestamp.
 	widgets = append(widgets, textParagraphWidget{
@@ -182,6 +187,16 @@ func (gcn *GoogleChatNotifier) SendResolved() bool {
 	return !gcn.GetDisableResolveMessage()
 }
 
+func (gcn *GoogleChatNotifier) isUrlAbsolute(urlToCheck string) bool {
+	parsed, err := url.Parse(urlToCheck)
+	if err != nil {
+		gcn.log.Warn("Could not parse URL", "urlToCheck", urlToCheck)
+		return false
+	}
+
+	return parsed.IsAbs()
+}
+
 func (gcn *GoogleChatNotifier) buildScreenshotCard(ctx context.Context, alerts []*types.Alert) *card {
 	card := card{
 		Header: header{

pkg/services/ngalert/notifier/channels/googlechat_test.go

@@ -20,12 +20,6 @@ func TestGoogleChatNotifier(t *testing.T) {
 	constNow := time.Now()
 	defer mockTimeNow(constNow)()
 
-	tmpl := templateForTests(t)
-
-	externalURL, err := url.Parse("http://localhost")
-	require.NoError(t, err)
-	tmpl.ExternalURL = externalURL
-
 	cases := []struct {
 		name         string
 		settings     string
@@ -33,10 +27,12 @@ func TestGoogleChatNotifier(t *testing.T) {
 		expMsg       *outerStruct
 		expInitError string
 		expMsgError  error
+		externalURL  string
 	}{
 		{
-			name:     "One alert",
+			name:        "One alert",
-			settings: `{"url": "http://localhost"}`,
+			settings:    `{"url": "http://localhost"}`,
+			externalURL: "http://localhost",
 			alerts: []*types.Alert{
 				{
 					Alert: model.Alert{
@@ -89,8 +85,9 @@ func TestGoogleChatNotifier(t *testing.T) {
 			},
 			expMsgError: nil,
 		}, {
-			name:     "Multiple alerts",
+			name:        "Multiple alerts",
-			settings: `{"url": "http://localhost"}`,
+			settings:    `{"url": "http://localhost"}`,
+			externalURL: "http://localhost",
 			alerts: []*types.Alert{
 				{
 					Alert: model.Alert{
@@ -149,10 +146,12 @@ func TestGoogleChatNotifier(t *testing.T) {
 		}, {
 			name:         "Error in initing",
 			settings:     `{}`,
+			externalURL:  "http://localhost",
 			expInitError: `could not find url property in settings`,
 		}, {
-			name:     "Customized message",
+			name:        "Customized message",
-			settings: `{"url": "http://localhost", "message": "I'm a custom template and you have {{ len .Alerts.Firing }} firing alert."}`,
+			settings:    `{"url": "http://localhost", "message": "I'm a custom template and you have {{ len .Alerts.Firing }} firing alert."}`,
+			externalURL: "http://localhost",
 			alerts: []*types.Alert{
 				{
 					Alert: model.Alert{
@@ -205,8 +204,9 @@ func TestGoogleChatNotifier(t *testing.T) {
 			},
 			expMsgError: nil,
 		}, {
-			name:     "Missing field in template",
+			name:        "Missing field in template",
-			settings: `{"url": "http://localhost", "message": "I'm a custom template {{ .NotAField }} bad template"}`,
+			settings:    `{"url": "http://localhost", "message": "I'm a custom template {{ .NotAField }} bad template"}`,
+			externalURL: "http://localhost",
 			alerts: []*types.Alert{
 				{
 					Alert: model.Alert{
@@ -259,8 +259,9 @@ func TestGoogleChatNotifier(t *testing.T) {
 			},
 			expMsgError: nil,
 		}, {
-			name:     "Invalid template",
+			name:        "Invalid template",
-			settings: `{"url": "http://localhost", "message": "I'm a custom template {{ {.NotAField }} bad template"}`,
+			settings:    `{"url": "http://localhost", "message": "I'm a custom template {{ {.NotAField }} bad template"}`,
+			externalURL: "http://localhost",
 			alerts: []*types.Alert{
 				{
 					Alert: model.Alert{
@@ -308,10 +309,104 @@ func TestGoogleChatNotifier(t *testing.T) {
 			},
 			expMsgError: nil,
 		},
+		{
+			name:        "Empty external URL",
+			settings:    `{ "url": "http://localhost" }`, // URL in settings = googlechat url
+			externalURL: "",                              // external URL = URL of grafana from configuration
+			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{
+									textParagraphWidget{
+										Text: text{
+											Text: "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\n",
+										},
+									},
+
+									// No button widget here since the external URL is not absolute
+
+									textParagraphWidget{
+										Text: text{
+											// RFC822 only has the minute, hence it works in most cases.
+											Text: "Grafana v" + setting.BuildVersion + " | " + constNow.Format(time.RFC822),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:        "Relative external URL",
+			settings:    `{ "url": "http://localhost" }`, // URL in settings = googlechat url
+			externalURL: "/grafana",                      // external URL = URL of grafana from configuration
+			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{
+									textParagraphWidget{
+										Text: text{
+											Text: "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: /grafana/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\nDashboard: /grafana/d/abcd\nPanel: /grafana/d/abcd?viewPanel=efgh\n",
+										},
+									},
+
+									// No button widget here since the external URL is not absolute
+
+									textParagraphWidget{
+										Text: text{
+											// RFC822 only has the minute, hence it works in most cases.
+											Text: "Grafana v" + setting.BuildVersion + " | " + constNow.Format(time.RFC822),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
+			tmpl := templateForTests(t)
+
+			externalURL, err := url.Parse(c.externalURL)
+			require.NoError(t, err)
+			tmpl.ExternalURL = externalURL
+
 			settingsJSON, err := simplejson.NewJson([]byte(c.settings))
 			require.NoError(t, err)