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

This feature may be useful for using `vmalert` with PromQL
compatible datasources like Loki.
This commit is contained in:
Roman Khavronenko 2020-06-06 21:27:09 +01:00 committed by Aliaksandr Valialkin
parent 60b8ce47ad
commit 5c049bf4dd
7 changed files with 120 additions and 34 deletions

View File

@ -106,8 +106,10 @@ Usage of vmalert:
-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 dir/*.yaml -rule /*.yaml. Relative path to all .yaml files in "dir" folder, -rule dir/*.yaml -rule /*.yaml. Relative path to all .yaml files in "dir" folder,
absolute path to all .yaml files in root. absolute path to all .yaml files in root.
-rule.validateExpressions
Whether to validate rules expressions via MetricsQL engine (default true)
-rule.validateTemplates -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 Pass `-help` to `vmalert` in order to see the full list of supported

View File

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

View File

@ -16,7 +16,7 @@ func TestMain(m *testing.M) {
} }
func TestParseGood(t *testing.T) { 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) t.Errorf("error parsing files %s", err)
} }
} }
@ -56,7 +56,7 @@ func TestParseBad(t *testing.T) {
}, },
} }
for _, tc := range testCases { for _, tc := range testCases {
_, err := Parse(tc.path, true) _, err := Parse(tc.path, true, true)
if err == nil { if err == nil {
t.Errorf("expected to get error") t.Errorf("expected to get error")
return return
@ -74,10 +74,90 @@ func TestRule_Validate(t *testing.T) {
if err := (&Rule{Alert: "alert"}).Validate(); err == nil { if err := (&Rule{Alert: "alert"}).Validate(); err == nil {
t.Errorf("exptected empty expr error") 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 { 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)
}
} }
} }

View File

@ -133,7 +133,7 @@ func TestUpdateWith(t *testing.T) {
func TestGroupStart(t *testing.T) { func TestGroupStart(t *testing.T) {
// TODO: make parsing from string instead of file // 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 { if err != nil {
t.Fatalf("failed to parse rules: %s", err) t.Fatalf("failed to parse rules: %s", err)
} }

View File

@ -30,8 +30,11 @@ 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 dir/*.yaml -rule /*.yaml. Relative path to all .yaml files in "dir" folder, -rule dir/*.yaml -rule /*.yaml. Relative path to all .yaml files in "dir" folder,
absolute path to all .yaml files in root.`) absolute path to all .yaml files in root.`)
validateTemplates = flag.Bool("rule.validateTemplates", true, "Indicates to validate annotation and label templates")
httpListenAddr = flag.String("httpListenAddr", ":8880", "Address to listen for http connections") 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."+ datasourceURL = flag.String("datasource.url", "", "Victoria Metrics or VMSelect url. Required parameter."+
" E.g. http://127.0.0.1:8428") " E.g. http://127.0.0.1:8428")
@ -100,7 +103,7 @@ func main() {
manager.rr = datasource.NewVMStorage(*remoteReadURL, *remoteReadUsername, *remoteReadPassword, &http.Client{}) 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) logger.Fatalf("failed to start: %s", err)
} }
@ -113,7 +116,7 @@ func main() {
<-sigHup <-sigHup
configReloads.Inc() configReloads.Inc()
logger.Infof("SIGHUP received. Going to reload rules %q ...", *rulePath) 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() configReloadErrors.Inc()
configSuccess.Set(0) configSuccess.Set(0)
logger.Errorf("error while reloading rules: %s", err) logger.Errorf("error while reloading rules: %s", err)

View File

@ -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) 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 { func (m *manager) start(ctx context.Context, path []string, validateTpl, validateExpr bool) error {
return m.update(ctx, path, validate, true) return m.update(ctx, path, validateTpl, validateExpr, true)
} }
func (m *manager) close() { func (m *manager) close() {
@ -79,9 +79,9 @@ func (m *manager) startGroup(ctx context.Context, group *Group, restore bool) {
m.groups[id] = group 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, ";")) 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 { if err != nil {
return fmt.Errorf("cannot parse configuration file: %s", err) return fmt.Errorf("cannot parse configuration file: %s", err)
} }

View File

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