vmalert-521: allow to disable rules expression validation. (#536)

This feature may be useful for using `vmalert` with PromQL
compatible datasources like Loki.

app/vmalert/README.md

@@ -106,8 +106,10 @@ Usage of vmalert:
          -rule /path/to/file. Path to a single file with alerting rules
          -rule dir/*.yaml -rule /*.yaml. Relative path to all .yaml files in "dir" folder, 
         absolute path to all .yaml files in root.
+  -rule.validateExpressions
+        Whether to validate rules expressions via MetricsQL engine (default true)
   -rule.validateTemplates
-        Indicates to validate annotation and label templates (default true)
+        Whether to validate annotation and label templates (default true)
 ```
 
 Pass `-help` to `vmalert` in order to see the full list of supported 

app/vmalert/config/config.go

@@ -25,7 +25,7 @@ type Group struct {
 }
 
 // Validate check for internal Group or Rule configuration errors
-func (g *Group) Validate(validateAnnotations bool) error {
+func (g *Group) Validate(validateAnnotations, validateExpressions bool) error {
 	if g.Name == "" {
 		return fmt.Errorf("group name must be set")
 	}
@@ -45,9 +45,12 @@ func (g *Group) Validate(validateAnnotations bool) error {
 		if err := r.Validate(); err != nil {
 			return fmt.Errorf("invalid rule %q.%q: %s", g.Name, ruleName, err)
 		}
-		if !validateAnnotations {
-			continue
+		if validateExpressions {
+			if _, err := metricsql.Parse(r.Expr); err != nil {
+				return fmt.Errorf("invalid expression for rule %q.%q: %s", g.Name, ruleName, err)
 			}
+		}
+		if validateAnnotations {
 			if err := notifier.ValidateTemplates(r.Annotations); err != nil {
 				return fmt.Errorf("invalid annotations for rule %q.%q: %s", g.Name, ruleName, err)
 			}
@@ -55,6 +58,7 @@ func (g *Group) Validate(validateAnnotations bool) error {
 				return fmt.Errorf("invalid labels for rule %q.%q: %s", g.Name, ruleName, err)
 			}
 		}
+	}
 	return checkOverflow(g.XXX, fmt.Sprintf("group %q", g.Name))
 }
 
@@ -77,14 +81,11 @@ func (r *Rule) Validate() error {
 	if r.Expr == "" {
 		return fmt.Errorf("expression can't be empty")
 	}
-	if _, err := metricsql.Parse(r.Expr); err != nil {
-		return fmt.Errorf("invalid expression: %w", err)
-	}
 	return nil
 }
 
 // Parse parses rule configs from given file patterns
-func Parse(pathPatterns []string, validateAnnotations bool) ([]Group, error) {
+func Parse(pathPatterns []string, validateAnnotations, validateExpressions bool) ([]Group, error) {
 	var fp []string
 	for _, pattern := range pathPatterns {
 		matches, err := filepath.Glob(pattern)
@@ -101,7 +102,7 @@ func Parse(pathPatterns []string, validateAnnotations bool) ([]Group, error) {
 			return nil, fmt.Errorf("failed to parse file %q: %w", file, err)
 		}
 		for _, g := range gr {
-			if err := g.Validate(validateAnnotations); err != nil {
+			if err := g.Validate(validateAnnotations, validateExpressions); err != nil {
 				return nil, fmt.Errorf("invalid group %q in file %q: %s", g.Name, file, err)
 			}
 			if _, ok := uniqueGroups[g.Name]; ok {

app/vmalert/config/config_test.go

@@ -16,7 +16,7 @@ func TestMain(m *testing.M) {
 }
 
 func TestParseGood(t *testing.T) {
-	if _, err := Parse([]string{"testdata/*good.rules", "testdata/dir/*good.*"}, true); err != nil {
+	if _, err := Parse([]string{"testdata/*good.rules", "testdata/dir/*good.*"}, true, true); err != nil {
 		t.Errorf("error parsing files %s", err)
 	}
 }
@@ -56,7 +56,7 @@ func TestParseBad(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		_, err := Parse(tc.path, true)
+		_, err := Parse(tc.path, true, true)
 		if err == nil {
 			t.Errorf("expected to get error")
 			return
@@ -74,10 +74,90 @@ func TestRule_Validate(t *testing.T) {
 	if err := (&Rule{Alert: "alert"}).Validate(); err == nil {
 		t.Errorf("exptected empty expr error")
 	}
-	if err := (&Rule{Alert: "alert", Expr: "test{"}).Validate(); err == nil {
-		t.Errorf("exptected invalid expr error")
-	}
 	if err := (&Rule{Alert: "alert", Expr: "test>0"}).Validate(); err != nil {
-		t.Errorf("exptected valid rule got %s", err)
+		t.Errorf("exptected valid rule; got %s", err)
+	}
+}
+
+func TestGroup_Validate(t *testing.T) {
+	testCases := []struct {
+		group               *Group
+		rules               []Rule
+		validateAnnotations bool
+		validateExpressions bool
+		expErr              string
+	}{
+		{
+			group:  &Group{},
+			expErr: "group name must be set",
+		},
+		{
+			group:  &Group{Name: "test"},
+			expErr: "contain no rules",
+		},
+		{
+			group: &Group{Name: "test",
+				Rules: []Rule{
+					{
+						Record: "record",
+						Expr:   "up | 0",
+					},
+				},
+			},
+			expErr: "",
+		},
+		{
+			group: &Group{Name: "test",
+				Rules: []Rule{
+					{
+						Record: "record",
+						Expr:   "up | 0",
+					},
+				},
+			},
+			expErr:              "invalid expression",
+			validateExpressions: true,
+		},
+		{
+			group: &Group{Name: "test",
+				Rules: []Rule{
+					{
+						Alert: "alert",
+						Expr:  "up == 1",
+						Labels: map[string]string{
+							"summary": "{{ value|query }}",
+						},
+					},
+				},
+			},
+			expErr: "",
+		},
+		{
+			group: &Group{Name: "test",
+				Rules: []Rule{
+					{
+						Alert: "alert",
+						Expr:  "up == 1",
+						Labels: map[string]string{
+							"summary": "{{ value|query }}",
+						},
+					},
+				},
+			},
+			expErr:              "error parsing annotation",
+			validateAnnotations: true,
+		},
+	}
+	for _, tc := range testCases {
+		err := tc.group.Validate(tc.validateAnnotations, tc.validateExpressions)
+		if err == nil {
+			if tc.expErr != "" {
+				t.Errorf("expected to get err %q; got nil insted", tc.expErr)
+			}
+			continue
+		}
+		if !strings.Contains(err.Error(), tc.expErr) {
+			t.Errorf("expected err to contain %q; got %q instead", tc.expErr, err)
+		}
 	}
 }

app/vmalert/group_test.go

@@ -133,7 +133,7 @@ func TestUpdateWith(t *testing.T) {
 
 func TestGroupStart(t *testing.T) {
 	// TODO: make parsing from string instead of file
-	groups, err := config.Parse([]string{"config/testdata/rules1-good.rules"}, true)
+	groups, err := config.Parse([]string{"config/testdata/rules1-good.rules"}, true, true)
 	if err != nil {
 		t.Fatalf("failed to parse rules: %s", err)
 	}

app/vmalert/main.go

@@ -30,7 +30,10 @@ Examples:
  -rule /path/to/file. Path to a single file with alerting rules
  -rule dir/*.yaml -rule /*.yaml. Relative path to all .yaml files in "dir" folder, 
 absolute path to all .yaml files in root.`)
-	validateTemplates = flag.Bool("rule.validateTemplates", true, "Indicates to validate annotation and label templates")
+
+	validateTemplates   = flag.Bool("rule.validateTemplates", true, "Whether to validate annotation and label templates")
+	validateExpressions = flag.Bool("rule.validateExpressions", true, "Whether to validate rules expressions via MetricsQL engine")
+
 	httpListenAddr = flag.String("httpListenAddr", ":8880", "Address to listen for http connections")
 
 	datasourceURL = flag.String("datasource.url", "", "Victoria Metrics or VMSelect url. Required parameter."+
@@ -100,7 +103,7 @@ func main() {
 		manager.rr = datasource.NewVMStorage(*remoteReadURL, *remoteReadUsername, *remoteReadPassword, &http.Client{})
 	}
 
-	if err := manager.start(ctx, *rulePath, *validateTemplates); err != nil {
+	if err := manager.start(ctx, *rulePath, *validateTemplates, *validateExpressions); err != nil {
 		logger.Fatalf("failed to start: %s", err)
 	}
 
@@ -113,7 +116,7 @@ func main() {
 			<-sigHup
 			configReloads.Inc()
 			logger.Infof("SIGHUP received. Going to reload rules %q ...", *rulePath)
-			if err := manager.update(ctx, *rulePath, *validateTemplates, false); err != nil {
+			if err := manager.update(ctx, *rulePath, *validateTemplates, *validateExpressions, false); err != nil {
 				configReloadErrors.Inc()
 				configSuccess.Set(0)
 				logger.Errorf("error while reloading rules: %s", err)

app/vmalert/manager.go

@@ -48,8 +48,8 @@ func (m *manager) AlertAPI(gID, aID uint64) (*APIAlert, error) {
 	return nil, fmt.Errorf("can't find alert with id %q in group %q", aID, g.Name)
 }
 
-func (m *manager) start(ctx context.Context, path []string, validate bool) error {
-	return m.update(ctx, path, validate, true)
+func (m *manager) start(ctx context.Context, path []string, validateTpl, validateExpr bool) error {
+	return m.update(ctx, path, validateTpl, validateExpr, true)
 }
 
 func (m *manager) close() {
@@ -79,9 +79,9 @@ func (m *manager) startGroup(ctx context.Context, group *Group, restore bool) {
 	m.groups[id] = group
 }
 
-func (m *manager) update(ctx context.Context, path []string, validate, restore bool) error {
+func (m *manager) update(ctx context.Context, path []string, validateTpl, validateExpr, restore bool) error {
 	logger.Infof("reading rules configuration file from %q", strings.Join(path, ";"))
-	groupsCfg, err := config.Parse(path, validate)
+	groupsCfg, err := config.Parse(path, validateTpl, validateExpr)
 	if err != nil {
 		return fmt.Errorf("cannot parse configuration file: %s", err)
 	}

app/vmalert/manager_test.go

@@ -22,7 +22,7 @@ func TestMain(m *testing.M) {
 func TestManagerUpdateError(t *testing.T) {
 	m := &manager{groups: make(map[uint64]*Group)}
 	path := []string{"foo/bar"}
-	err := m.update(context.Background(), path, true, false)
+	err := m.update(context.Background(), path, true, true, false)
 	if err == nil {
 		t.Fatalf("expected to have err; got nil instead")
 	}
@@ -51,7 +51,7 @@ func TestManagerUpdateConcurrent(t *testing.T) {
 		"config/testdata/rules2-good.rules",
 	}
 	*evaluationInterval = time.Millisecond
-	if err := m.start(context.Background(), []string{paths[0]}, true); err != nil {
+	if err := m.start(context.Background(), []string{paths[0]}, true, true); err != nil {
 		t.Fatalf("failed to start: %s", err)
 	}
 
@@ -65,7 +65,7 @@ func TestManagerUpdateConcurrent(t *testing.T) {
 			for i := 0; i < iterations; i++ {
 				rnd := rand.Intn(len(paths))
 				path := []string{paths[rnd]}
-				_ = m.update(context.Background(), path, true, false)
+				_ = m.update(context.Background(), path, true, true, false)
 			}
 		}()
 	}
@@ -186,12 +186,12 @@ func TestManagerUpdate(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.TODO())
 			m := &manager{groups: make(map[uint64]*Group), storage: &fakeQuerier{}}
 			path := []string{tc.initPath}
-			if err := m.update(ctx, path, true, false); err != nil {
+			if err := m.update(ctx, path, true, true, false); err != nil {
 				t.Fatalf("failed to complete initial rules update: %s", err)
 			}
 
 			path = []string{tc.updatePath}
-			_ = m.update(ctx, path, true, false)
+			_ = m.update(ctx, path, true, true, false)
 			if len(tc.want) != len(m.groups) {
 				t.Fatalf("\nwant number of groups: %d;\ngot: %d ", len(tc.want), len(m.groups))
 			}