Repository: VictoriaMetrics/VictoriaMetrics
Pull request #5450 (2023-12-11 13:00:32)

vmalert: add schema check for `-external.url`

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5450?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) Report
Attention: `14 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`72dbd24`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/72dbd24b22c0cb1824bfc2833a958151797cc809?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 60.15% compared to head [(`168d353`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5450?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 60.18%.
> Report is 3 commits behind head on master.

| [Files](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5450?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) | Patch % | Lines |
|---|---|---|
| [app/vmalert/main.go](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5450?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-YXBwL3ZtYWxlcnQvbWFpbi5nbw==) | 12.50% | [13 Missing and 1 partial :warning: ](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5450?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #5450      +/-   ##
==========================================
+ Coverage   60.15%   60.18%   +0.02%     
==========================================
  Files         400      400              
  Lines       75475    75485      +10     
==========================================
+ Hits        45403    45429      +26     
+ Misses      27546    27536      -10     
+ Partials     2526     2520       -6     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5450?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).


Patch:
diff --git a/app/vmalert/main.go b/app/vmalert/main.go
index 40ffe1eb9..1c7a78408 100644
--- a/app/vmalert/main.go
+++ b/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]
diff --git a/app/vmui/packages/vmui/src/hooks/useFetchQuery.ts b/app/vmui/packages/vmui/src/hooks/useFetchQuery.ts
index a9e3cc74c..980527713 100644
--- a/app/vmui/packages/vmui/src/hooks/useFetchQuery.ts
+++ b/app/vmui/packages/vmui/src/hooks/useFetchQuery.ts
@@ -136,7 +136,11 @@ export const useFetchQuery = ({
           totalLength += resp.data.result.length;
         } else {
           tempData.push({ metric: {}, values: [], group: counter } as MetricBase);
-          setQueryErrors(prev => [...prev, `${resp.errorType}\r\n${resp?.error}`]);
+          const errorType = resp.errorType || ErrorTypes.unknownType;
+          const errorMessage = resp?.error || resp?.message || "see console for more details";
+          const error = [errorType, errorMessage].join(",\r\n");
+          setQueryErrors(prev => [...prev, `${error}`]);
+          console.error(`Fetch query error: ${errorType}`, resp);
         }
         counter++;
       }
diff --git a/app/vmui/packages/vmui/src/types/index.ts b/app/vmui/packages/vmui/src/types/index.ts
index c04aea5bd..f4bd75873 100644
--- a/app/vmui/packages/vmui/src/types/index.ts
+++ b/app/vmui/packages/vmui/src/types/index.ts
@@ -47,7 +47,8 @@ export enum ErrorTypes {
   traceNotFound = "Not found the tracing information",
   emptyTitle = "Please enter title",
   positiveNumber = "Please enter positive number",
-  validStep = "Please enter a valid step"
+  validStep = "Please enter a valid step",
+  unknownType = "Unknown server response format: must have 'errorType'",
 }
 
 export interface PanelSettings {
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index e1f211f3a..1bc153e59 100644
--- a/docs/CHANGELOG.md
+++ b/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).


Test patch:
diff --git a/app/vmalert/main_test.go b/app/vmalert/main_test.go
index 5a0377eef..12531c66e 100644
--- a/app/vmalert/main_test.go
+++ b/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())
 	}
 }