From 186bfa9085fe058265c05493d7761effc4781c16 Mon Sep 17 00:00:00 2001 From: Hui Wang Date: Fri, 15 Dec 2023 18:13:56 +0800 Subject: [PATCH] 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 | 24 ++++++++++++++++++------ app/vmalert/main_test.go | 19 +++++++++++++------ docs/CHANGELOG.md | 1 + 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/app/vmalert/main.go b/app/vmalert/main.go index 8d7dcdc6d..ea02ad406 100644 --- a/app/vmalert/main.go +++ b/app/vmalert/main.go @@ -126,9 +126,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) @@ -249,14 +249,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/vmalert/main_test.go b/app/vmalert/main_test.go index afa27bace..54804043e 100644 --- a/app/vmalert/main_test.go +++ b/app/vmalert/main_test.go @@ -14,22 +14,29 @@ import ( ) 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()) } } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 8baf2a68a..bcc80aa11 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -13,6 +13,7 @@ The following `tip` changes can be tested by building VictoriaMetrics components * BUGFIX: `vmstorage`: added missing `-inmemoryDataFlushInterval` command-line flag, which was missing in [VictoriaMetrics cluster](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html) after implementing [this feature](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3337) in [v1.85.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.85.0). * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly handle queries, which wrap [rollup functions](https://docs.victoriametrics.com/MetricsQL.html#rollup-functions) with multiple arguments without explicitly specified lookbehind window in square brackets into [aggregate functions](https://docs.victoriametrics.com/MetricsQL.html#aggregate-functions). For example, `sum(quantile_over_time(0.5, process_resident_memory_bytes))` was resulting to `expecting at least 2 args to ...; got 1 args' error. Thanks to @atykhyy for [the pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5414). +* 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: [vmctl](https://docs.victoriametrics.com/vmctl.html): check for Error field in response from influx client during migration. Before, only network errors were checked. Thanks to @wozz for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5446). ## [v1.93.9](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.93.9)