From fa3a17938e43949ba821c322b929874762798267 Mon Sep 17 00:00:00 2001 From: Roman Khavronenko <roman@victoriametrics.com> Date: Mon, 8 May 2023 13:31:54 +0200 Subject: [PATCH] vmalert: follow-up after cae87da (#4269) * vmalert: follow-up after cae87da https://github.com/VictoriaMetrics/VictoriaMetrics/commit/cae87da4bbb54e901bfc5b355958b413488a6840 Signed-off-by: hagen1778 <roman@victoriametrics.com> * vmalert: update struct comments Signed-off-by: hagen1778 <roman@victoriametrics.com> * vmalert: rm typo Signed-off-by: hagen1778 <roman@victoriametrics.com> --------- Signed-off-by: hagen1778 <roman@victoriametrics.com> --- app/vmalert/README.md | 5 +-- app/vmalert/config/config.go | 33 ++---------------- app/vmalert/config/config_test.go | 38 ++++++++++++++++++++- app/vmalert/config/fs.go | 13 +++---- app/vmalert/config/fsurl/url.go | 57 +++++++++++++++++++++++++++++++ app/vmalert/config/url.go | 34 ------------------ app/vmalert/main.go | 4 +-- docs/CHANGELOG.md | 1 + docs/vmalert.md | 5 +-- 9 files changed, 111 insertions(+), 79 deletions(-) create mode 100644 app/vmalert/config/fsurl/url.go delete mode 100644 app/vmalert/config/url.go diff --git a/app/vmalert/README.md b/app/vmalert/README.md index f3ff1488ff..61dca921f3 100644 --- a/app/vmalert/README.md +++ b/app/vmalert/README.md @@ -29,7 +29,7 @@ Use this feature for the following cases: * Recording and Alerting rules backfilling (aka `replay`). See [these docs](#rules-backfilling); * Lightweight and without extra dependencies. * Supports [reusable templates](#reusable-templates) for annotations; -* Load of recording and alerting rules from local filesystem, GCS and S3; +* Load of recording and alerting rules from local filesystem, URL, GCS and S3; * Detect alerting rules which [don't match any series](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4039). ## Limitations @@ -1179,7 +1179,8 @@ The shortlist of configuration flags is the following: Path to the files with alerting and/or recording rules. Supports hierarchical patterns and regexpes. Examples: - -rule="/path/to/file". Path to a single file with alerting rules + -rule="/path/to/file". Path to a single file with alerting rules. + -rule="http://<some-server-addr>/path/to/rules". HTTP URL to a page with alerting rules. -rule="dir/*.yaml" -rule="/*.yaml" -rule="gcs://vmalert-rules/tenant_%{TENANT_ID}/prod". -rule="dir/**/*.yaml". Includes to all .yaml files in "dir" folder and it's subfolders recursively. Rule files may contain %{ENV_VAR} placeholders, which are substituted by the corresponding env vars. diff --git a/app/vmalert/config/config.go b/app/vmalert/config/config.go index 438847d2e2..0fc201ef90 100644 --- a/app/vmalert/config/config.go +++ b/app/vmalert/config/config.go @@ -209,7 +209,8 @@ var cLogger = &log.Logger{} func ParseSilent(pathPatterns []string, validateTplFn ValidateTplFn, validateExpressions bool) ([]Group, error) { cLogger.Suppress(true) defer cLogger.Suppress(false) - files, err := readFromFSOrHTTP(pathPatterns) + + files, err := readFromFS(pathPatterns) if err != nil { return nil, fmt.Errorf("failed to read from the config: %s", err) } @@ -218,7 +219,7 @@ func ParseSilent(pathPatterns []string, validateTplFn ValidateTplFn, validateExp // Parse parses rule configs from given file patterns func Parse(pathPatterns []string, validateTplFn ValidateTplFn, validateExpressions bool) ([]Group, error) { - files, err := readFromFSOrHTTP(pathPatterns) + files, err := readFromFS(pathPatterns) if err != nil { return nil, fmt.Errorf("failed to read from the config: %s", err) } @@ -232,34 +233,6 @@ func Parse(pathPatterns []string, validateTplFn ValidateTplFn, validateExpressio return groups, nil } -// readFromFSOrHTTP reads path either from filesystem or from http if path starts with http or https. -func readFromFSOrHTTP(paths []string) (map[string][]byte, error) { - var httpPaths []string - var fsPaths []string - for _, path := range paths { - if isHTTPURL(path) { - httpPaths = append(httpPaths, path) - continue - } - fsPaths = append(fsPaths, path) - } - result, err := readFromFS(fsPaths) - if err != nil { - return nil, err - } - httpResult, err := readFromHTTP(httpPaths) - if err != nil { - return nil, err - } - for k, v := range httpResult { - if _, ok := result[k]; ok { - return nil, fmt.Errorf("duplicate found for config name %q: config names must be unique", k) - } - result[k] = v - } - return result, nil -} - func parse(files map[string][]byte, validateTplFn ValidateTplFn, validateExpressions bool) ([]Group, error) { errGroup := new(utils.ErrGroup) var groups []Group diff --git a/app/vmalert/config/config_test.go b/app/vmalert/config/config_test.go index 4d7ed3ec45..726cb11c07 100644 --- a/app/vmalert/config/config_test.go +++ b/app/vmalert/config/config_test.go @@ -1,6 +1,8 @@ package config import ( + "net/http" + "net/http/httptest" "net/url" "os" "strings" @@ -27,6 +29,40 @@ func TestParseGood(t *testing.T) { } } +func TestParseFromURL(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/bad", func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte("foo bar")) + }) + mux.HandleFunc("/good-alert", func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte(` +groups: + - name: TestGroup + rules: + - alert: Conns + expr: vm_tcplistener_conns > 0`)) + }) + mux.HandleFunc("/good-rr", func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte(` +groups: + - name: TestGroup + rules: + - record: conns + expr: max(vm_tcplistener_conns)`)) + }) + + srv := httptest.NewServer(mux) + defer srv.Close() + + if _, err := Parse([]string{srv.URL + "/good-alert", srv.URL + "/good-rr"}, notifier.ValidateTemplates, true); err != nil { + t.Errorf("error parsing URLs %s", err) + } + + if _, err := Parse([]string{srv.URL + "/bad"}, notifier.ValidateTemplates, true); err == nil { + t.Errorf("expected parsing error: %s", err) + } +} + func TestParseBad(t *testing.T) { testCases := []struct { path []string @@ -66,7 +102,7 @@ func TestParseBad(t *testing.T) { }, { []string{"http://unreachable-url"}, - "failed to read from the config: cannot fetch \"http://unreachable-url\"", + "no such host", }, } for _, tc := range testCases { diff --git a/app/vmalert/config/fs.go b/app/vmalert/config/fs.go index 609d6b8a62..2fc068370d 100644 --- a/app/vmalert/config/fs.go +++ b/app/vmalert/config/fs.go @@ -2,12 +2,12 @@ package config import ( "fmt" - "net/url" "strings" "sync" "time" "github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/config/fslocal" + "github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/config/fsurl" ) // FS represent a file system abstract for reading files. @@ -89,8 +89,9 @@ func readFromFS(paths []string) (map[string][]byte, error) { // newFS creates FS based on the give path. // Supported file systems are: fs -func newFS(path string) (FS, error) { +func newFS(originPath string) (FS, error) { scheme := "fs" + path := originPath n := strings.Index(path, "://") if n >= 0 { scheme = path[:n] @@ -102,13 +103,9 @@ func newFS(path string) (FS, error) { switch scheme { case "fs": return &fslocal.FS{Pattern: path}, nil + case "http", "https": + return &fsurl.FS{Path: originPath}, nil default: return nil, fmt.Errorf("unsupported scheme %q", scheme) } } - -// isHTTPURL checks if a given targetURL is valid and contains a valid http scheme -func isHTTPURL(targetURL string) bool { - parsed, err := url.Parse(targetURL) - return err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") && parsed.Host != "" -} diff --git a/app/vmalert/config/fsurl/url.go b/app/vmalert/config/fsurl/url.go new file mode 100644 index 0000000000..6cb99ee730 --- /dev/null +++ b/app/vmalert/config/fsurl/url.go @@ -0,0 +1,57 @@ +package fsurl + +import ( + "fmt" + "io" + "net/http" + "net/url" +) + +// FS represents a struct which can read content from URL Path +type FS struct { + // Path defines the URL to read the data from + Path string +} + +// Init verifies that configured Path is correct +func (fs *FS) Init() error { + _, err := url.Parse(fs.Path) + return err +} + +// String implements Stringer interface +func (fs *FS) String() string { + return fmt.Sprintf("URL {Path: %q}", fs.Path) +} + +// List returns the list of file names which will be read via Read fn +// List isn't supported by FS and reads from Path only +func (fs *FS) List() ([]string, error) { + return []string{fs.Path}, nil +} + +// Read returns a map of read files where +// key is the file name and value is file's content. +func (fs *FS) Read(files []string) (map[string][]byte, error) { + result := make(map[string][]byte) + for _, path := range files { + resp, err := http.Get(path) + if err != nil { + return nil, fmt.Errorf("failed to read from %q: %w", path, err) + } + data, err := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + if len(data) > 4*1024 { + data = data[:4*1024] + } + return nil, fmt.Errorf("unexpected status code when fetching %q: %d, expecting %d; response: %q", + path, resp.StatusCode, http.StatusOK, data) + } + if err != nil { + return nil, fmt.Errorf("cannot read %q: %s", path, err) + } + result[path] = data + } + return result, nil +} diff --git a/app/vmalert/config/url.go b/app/vmalert/config/url.go deleted file mode 100644 index a89fdf3131..0000000000 --- a/app/vmalert/config/url.go +++ /dev/null @@ -1,34 +0,0 @@ -package config - -import ( - "fmt" - "io" - "net/http" -) - -// readFromHTTP reads config from http path. -func readFromHTTP(paths []string) (map[string][]byte, error) { - result := make(map[string][]byte) - for _, path := range paths { - if _, ok := result[path]; ok { - return nil, fmt.Errorf("duplicate found for url path %q: url path must be unique", path) - } - resp, err := http.Get(path) - if err != nil { - return nil, fmt.Errorf("cannot fetch %q: %w", path, err) - } - data, err := io.ReadAll(resp.Body) - _ = resp.Body.Close() - if resp.StatusCode != http.StatusOK { - if len(data) > 4*1024 { - data = data[:4*1024] - } - return nil, fmt.Errorf("unexpected status code when fetching %q: %d, expecting %d; response: %q", path, resp.StatusCode, http.StatusOK, data) - } - if err != nil { - return nil, fmt.Errorf("cannot read %q: %s", path, err) - } - result[path] = data - } - return result, nil -} diff --git a/app/vmalert/main.go b/app/vmalert/main.go index cea9be80fa..eab42d7a6b 100644 --- a/app/vmalert/main.go +++ b/app/vmalert/main.go @@ -32,8 +32,8 @@ var ( rulePath = flagutil.NewArrayString("rule", `Path to the files or http url with alerting and/or recording rules. Supports hierarchical patterns and regexpes. Examples: - -rule="/path/to/file". Path to a single file with alerting rules - -rule="http://<some-server-addr>:path/to/rules". HTTP URL to alerting rules. + -rule="/path/to/file". Path to a single file with alerting rules. + -rule="http://<some-server-addr>/path/to/rules". HTTP URL to a page with alerting rules. -rule="dir/*.yaml" -rule="/*.yaml" -rule="gcs://vmalert-rules/tenant_%{TENANT_ID}/prod". -rule="dir/**/*.yaml". Includes to all .yaml files in "dir" folder and it's subfolders recursively. Rule files may contain %{ENV_VAR} placeholders, which are substituted by the corresponding env vars. diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 068b7b1a9b..29898b52a6 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -41,6 +41,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth.html): add ability to specify default route (`default_url`) for processing non-matched requests. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4084). * FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): support configuring of custom HTTP headers sent to notifiers on the Group level. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3260). * FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): detect alerting rules which don't match any series. See this [issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4039) for details. +* FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): support rules loading via HTTP URL. See this [issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3352). Thanks to @Haleygo for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4212). * FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): add `-s3StorageClass` command-line flag for setting the storage class for AWS S3 backups. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4164). Thanks to @justcompile for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4166). * BUGFIX: reduce the probability of sudden increase in the number of small parts on systems with small number of CPU cores. diff --git a/docs/vmalert.md b/docs/vmalert.md index c8ac6ef10d..f416524af2 100644 --- a/docs/vmalert.md +++ b/docs/vmalert.md @@ -33,7 +33,7 @@ Use this feature for the following cases: * Recording and Alerting rules backfilling (aka `replay`). See [these docs](#rules-backfilling); * Lightweight and without extra dependencies. * Supports [reusable templates](#reusable-templates) for annotations; -* Load of recording and alerting rules from local filesystem, GCS and S3; +* Load of recording and alerting rules from local filesystem, URL, GCS and S3; * Detect alerting rules which [don't match any series](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4039). ## Limitations @@ -1183,7 +1183,8 @@ The shortlist of configuration flags is the following: Path to the files with alerting and/or recording rules. Supports hierarchical patterns and regexpes. Examples: - -rule="/path/to/file". Path to a single file with alerting rules + -rule="/path/to/file". Path to a single file with alerting rules. + -rule="http://<some-server-addr>/path/to/rules". HTTP URL to a page with alerting rules. -rule="dir/*.yaml" -rule="/*.yaml" -rule="gcs://vmalert-rules/tenant_%{TENANT_ID}/prod". -rule="dir/**/*.yaml". Includes to all .yaml files in "dir" folder and it's subfolders recursively. Rule files may contain %{ENV_VAR} placeholders, which are substituted by the corresponding env vars.