vmalert: escape query params if external alert source defined (#3267)

vmalert: escape query args if external alert source defined

app/vmalert/README.md

@@ -816,7 +816,7 @@ The shortlist of configuration flags is the following:
      How often to evaluate the rules (default 1m0s)
   -external.alert.source string
      External Alert Source allows to override the Source link for alerts sent to AlertManager for cases where you want to build a custom link to Grafana, Prometheus or any other service. Supports templating - see https://docs.victoriametrics.com/vmalert.html#templating . For example, link to Grafana: -external.alert.source='explore?orgId=1&left=[\"now-1h\",\"now\",\"VictoriaMetrics\",{\"expr\": \"{{$expr|quotesEscape|crlfEscape|queryEscape}}\"},{\"mode\":\"Metrics\"},{\"ui\":[true,true,true,\"none\"]}]' . If empty 'vmalert/alert?group_id={{.GroupID}}&alert_id={{.AlertID}}' is used
-     If empty 'vmalert/alert?group_id={{.GroupID}}&alert_id={{.AlertID}}' is used.
+     If empty 'vmalert/alert?group_id={{.GroupID}}&alert_id={{.AlertID}}' is used. All query params will be automatically escaped.
   -external.label array
      Optional label in the form 'Name=value' to add to all generated recording rules and alerts. Pass multiple -label flags in order to add multiple label sets.
      Supports an array of values separated by comma or specified via multiple flags.

app/vmalert/main.go

@@ -63,7 +63,7 @@ absolute path to all .tpl files in root.`)
 		`for cases where you want to build a custom link to Grafana, Prometheus or any other service. `+
 		`Supports templating - see https://docs.victoriametrics.com/vmalert.html#templating . `+
 		`For example, link to Grafana: -external.alert.source='explore?orgId=1&left=[\"now-1h\",\"now\",\"VictoriaMetrics\",{\"expr\": \"{{$expr|quotesEscape|crlfEscape|queryEscape}}\"},{\"mode\":\"Metrics\"},{\"ui\":[true,true,true,\"none\"]}]' . `+
-		`If empty 'vmalert/alert?group_id={{.GroupID}}&alert_id={{.AlertID}}' is used`)
+		`If empty 'vmalert/alert?group_id={{.GroupID}}&alert_id={{.AlertID}}' is used. All query params will be automatically escaped`)
 	externalLabels = flagutil.NewArrayString("external.label", "Optional label in the form 'Name=value' to add to all generated recording rules and alerts. "+
 		"Pass multiple -label flags in order to add multiple label sets.")
 
@@ -270,7 +270,7 @@ func getAlertURLGenerator(externalURL *url.URL, externalAlertSource string, vali
 		if err != nil {
 			logger.Errorf("can not exec source template %s", err)
 		}
-		return fmt.Sprintf("%s/%s", externalURL, templated["tpl"])
+		return fmt.Sprintf("%s/%s", externalURL, url.QueryEscape(templated["tpl"]))
 	}, nil
 }
 

app/vmalert/main_test.go

@@ -52,7 +52,7 @@ func TestGetAlertURLGenerator(t *testing.T) {
 	if err != nil {
 		t.Errorf("unexpected error %s", err)
 	}
-	if exp := "https://victoriametrics.com/path/foo?query=4&ds=baz"; exp != fn(testAlert) {
+	if exp := "https://victoriametrics.com/path/foo%3Fquery%3D4%26ds%3Dbaz"; exp != fn(testAlert) {
 		t.Errorf("unexpected url want %s, got %s", exp, fn(testAlert))
 	}
 }

docs/CHANGELOG.md

@@ -56,6 +56,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): fix panic if `vmagent` runs with `-clusterMode` command-line flag in [multitenant mode](https://docs.victoriametrics.com/vmalert.html#multitenancy). The issue has been introduced in [v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820).
 * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): do not show invalid error message in Kubernetes service discovery: `cannot parse WatchEvent json response: EOF`. The invalid error message has been appeared in [v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820).
 * BUGFIX: `vmselect`: expose missing metric `vm_cache_size_max_bytes{type="promql/rollupResult"}` . This metric is used for monitoring rollup cache usage with the query `vm_cache_size_bytes{type="promql/rollupResult"} / vm_cache_size_max_bytes{type="promql/rollupResult"}` in the same way as this is done for other cache types.
+* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): fix escaping of query params when using `external.alert.source` flag.  See this [issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3139).
 
 ## [v1.82.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.82.1)
 

docs/vmalert.md

@@ -820,7 +820,7 @@ The shortlist of configuration flags is the following:
      How often to evaluate the rules (default 1m0s)
   -external.alert.source string
      External Alert Source allows to override the Source link for alerts sent to AlertManager for cases where you want to build a custom link to Grafana, Prometheus or any other service. Supports templating - see https://docs.victoriametrics.com/vmalert.html#templating . For example, link to Grafana: -external.alert.source='explore?orgId=1&left=[\"now-1h\",\"now\",\"VictoriaMetrics\",{\"expr\": \"{{$expr|quotesEscape|crlfEscape|queryEscape}}\"},{\"mode\":\"Metrics\"},{\"ui\":[true,true,true,\"none\"]}]' . If empty 'vmalert/alert?group_id={{.GroupID}}&alert_id={{.AlertID}}' is used
-     If empty 'vmalert/alert?group_id={{.GroupID}}&alert_id={{.AlertID}}' is used.
+     If empty 'vmalert/alert?group_id={{.GroupID}}&alert_id={{.AlertID}}' is used. All query params will be automatically escaped.
   -external.label array
      Optional label in the form 'Name=value' to add to all generated recording rules and alerts. Pass multiple -label flags in order to add multiple label sets.
      Supports an array of values separated by comma or specified via multiple flags.