vmalert: follow-up after cae87da (#4269)

* vmalert: follow-up after cae87da

cae87da4bb
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

@@ -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.

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

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 {

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 != ""
-}

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
+}

app/vmalert/config/url.go

@@ -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
-}

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.

docs/CHANGELOG.md

@@ -42,6 +42,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth.html): add ability to filter incoming requests by IP. See [these docs](https://docs.victoriametrics.com/vmauth.html#ip-filters) and [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3491).
 * 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.

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.