vmalert: validate schema for -external.url (#5450)

Requests with wrong or no schema in  `-external.url` could be rejected by alertmanager.
So we validate schema on start up.

app/vmalert/main.go

@@ -118,9 +118,9 @@ func main() {
 		return
 	}
 
-	eu, err := getExternalURL(*externalURL, *httpListenAddr, httpserver.IsTLS())
+	eu, err := getExternalURL(*externalURL)
 	if err != nil {
-		logger.Fatalf("failed to init `external.url`: %s", err)
+		logger.Fatalf("failed to init `-external.url`: %s", err)
 	}
 
 	alertURLGeneratorFn, err = getAlertURLGenerator(eu, *externalAlertSource, *validateTemplates)
@@ -243,14 +243,26 @@ func newManager(ctx context.Context) (*manager, error) {
 	return manager, nil
 }
 
-func getExternalURL(externalURL, httpListenAddr string, isSecure bool) (*url.URL, error) {
-	if externalURL != "" {
-		return url.Parse(externalURL)
+func getExternalURL(customURL string) (*url.URL, error) {
+	if customURL == "" {
+		// use local hostname as external URL
+		return getHostnameAsExternalURL(*httpListenAddr, httpserver.IsTLS())
 	}
-	hname, err := os.Hostname()
+	u, err := url.Parse(customURL)
 	if err != nil {
 		return nil, err
 	}
+	if u.Scheme != "http" && u.Scheme != "https" {
+		return nil, fmt.Errorf("invalid scheme %q in url %q, only 'http' and 'https' are supported", u.Scheme, u.String())
+	}
+	return u, nil
+}
+
+func getHostnameAsExternalURL(httpListenAddr string, isSecure bool) (*url.URL, error) {
+	hname, err := os.Hostname()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get hostname: %w", err)
+	}
 	port := ""
 	if ipport := strings.Split(httpListenAddr, ":"); len(ipport) > 1 {
 		port = ":" + ipport[1]

app/vmalert/main_test.go

@@ -22,22 +22,29 @@ func init() {
 }
 
 func TestGetExternalURL(t *testing.T) {
-	expURL := "https://vicotriametrics.com/path"
-	u, err := getExternalURL(expURL, "", false)
+	invalidURL := "victoriametrics.com/path"
+	_, err := getExternalURL(invalidURL)
+	if err == nil {
+		t.Errorf("expected error, got nil")
+	}
+
+	expURL := "https://victoriametrics.com/path"
+	u, err := getExternalURL(expURL)
 	if err != nil {
 		t.Errorf("unexpected error %s", err)
 	}
 	if u.String() != expURL {
-		t.Errorf("unexpected url want %s, got %s", expURL, u.String())
+		t.Errorf("unexpected url: want %q, got %s", expURL, u.String())
 	}
+
 	h, _ := os.Hostname()
-	expURL = fmt.Sprintf("https://%s:4242", h)
-	u, err = getExternalURL("", "0.0.0.0:4242", true)
+	expURL = fmt.Sprintf("http://%s:8880", h)
+	u, err = getExternalURL("")
 	if err != nil {
 		t.Errorf("unexpected error %s", err)
 	}
 	if u.String() != expURL {
-		t.Errorf("unexpected url want %s, got %s", expURL, u.String())
+		t.Errorf("unexpected url: want %s, got %s", expURL, u.String())
 	}
 }
 

docs/CHANGELOG.md

@@ -76,6 +76,7 @@ Released at 2023-12-13
 * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly add new labels at `output_relabel_configs` during [stream aggregation](https://docs.victoriametrics.com/stream-aggregation.html). Previously this could lead to corrupted labels in output samples. Thanks to @ChengChung for providing [detailed report for this bug](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5402).
 * BUGFIX: [vmalert-tool](https://docs.victoriametrics.com/#vmalert-tool): allow using arbitrary `eval_time`  in [alert_rule_test](https://docs.victoriametrics.com/vmalert-tool.html#alert_test_case) case. Previously, test cases with `eval_time`  not being a multiple of `evaluation_interval` would fail.
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): sanitize label names before sending the alert notification to Alertmanager. Before, vmalert would send notifications with labels containing characters not supported by Alertmanager validator, resulting into validation errors like `msg="Failed to validate alerts" err="invalid label set: invalid name "foo.bar"`.
+* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): check `-external.url` schema when starting vmalert, must be `http` or `https`. Before, alertmanager could reject alert notifications if `-external.url` contained no or wrong schema.
 * BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/vmbackupmanager.html): fix `vmbackupmanager` not deleting previous object versions from S3 when applying retention policy with `-deleteAllObjectVersions` command-line flag.
 * BUGFIX: [vminsert](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): fix panic when ingesting data via [NewRelic protocol](https://docs.victoriametrics.com/#how-to-send-data-from-newrelic-agent) into VictoriaMetrics cluster. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5416).
 * BUGFIX: properly escape `<` character in responses returned via [`/federate`](https://docs.victoriametrics.com/#federation) endpoint. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5431).