diff --git a/CHANGELOG.md b/CHANGELOG.md index c296b34e..fa136611 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## v0.3.0 + +### Added + +- `rule` block can now specify `ignore` conditions that have the same syntax as `match` + but will disable `rule` for matching alerting and recording rules #48. +- `match` and `ignore` blocks can now filter alerting and recording rules by name. + `record` will be used as name for recording rules and `alert` for alerting rules. + ## v0.2.0 ### Added diff --git a/cmd/pint/tests/0018_match_alerting.txt b/cmd/pint/tests/0018_match_alerting.txt new file mode 100644 index 00000000..42a2423d --- /dev/null +++ b/cmd/pint/tests/0018_match_alerting.txt @@ -0,0 +1,30 @@ +pint.ok -l debug lint rules +! stdout . +cmp stderr stderr.txt + +-- stderr.txt -- +level=info msg="Loading configuration file" path=.pint.hcl +level=info msg="File parsed" path=rules/0001.yml rules=2 +level=debug msg="Found recording rule" lines=1-2 path=rules/0001.yml record=colo:recording +level=debug msg="Configured checks for rule" enabled=["promql/syntax"] path=rules/0001.yml rule=colo:recording +level=debug msg="Found alerting rule" alert=colo:alerting lines=4-5 path=rules/0001.yml +level=debug msg="Configured checks for rule" enabled=["promql/syntax","promql/without(job:true)","promql/by(job:true)"] path=rules/0001.yml rule=colo:alerting +rules/0001.yml:5: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/without) + expr: sum(bar) without(job) + +-- rules/0001.yml -- +- record: "colo:recording" + expr: sum(foo) without(job) + +- alert: "colo:alerting" + expr: sum(bar) without(job) + +-- .pint.hcl -- +rule { + match { + kind = "alerting" + } + aggregate ".+" { + keep = [ "job" ] + } +} diff --git a/cmd/pint/tests/0019_match_recording.txt b/cmd/pint/tests/0019_match_recording.txt new file mode 100644 index 00000000..c05a9e7c --- /dev/null +++ b/cmd/pint/tests/0019_match_recording.txt @@ -0,0 +1,30 @@ +pint.ok -l debug lint rules +! stdout . +cmp stderr stderr.txt + +-- stderr.txt -- +level=info msg="Loading configuration file" path=.pint.hcl +level=info msg="File parsed" path=rules/0001.yml rules=2 +level=debug msg="Found recording rule" lines=1-2 path=rules/0001.yml record=colo:recording +level=debug msg="Configured checks for rule" enabled=["promql/syntax","promql/without(job:true)","promql/by(job:true)"] path=rules/0001.yml rule=colo:recording +level=debug msg="Found alerting rule" alert=colo:alerting lines=4-5 path=rules/0001.yml +level=debug msg="Configured checks for rule" enabled=["promql/syntax"] path=rules/0001.yml rule=colo:alerting +rules/0001.yml:2: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/without) + expr: sum(foo) without(job) + +-- rules/0001.yml -- +- record: "colo:recording" + expr: sum(foo) without(job) + +- alert: "colo:alerting" + expr: sum(bar) without(job) + +-- .pint.hcl -- +rule { + match { + kind = "recording" + } + aggregate ".+" { + keep = [ "job" ] + } +} diff --git a/cmd/pint/tests/0020_ignore_kind.txt b/cmd/pint/tests/0020_ignore_kind.txt new file mode 100644 index 00000000..4e9c6681 --- /dev/null +++ b/cmd/pint/tests/0020_ignore_kind.txt @@ -0,0 +1,30 @@ +pint.ok -l debug lint rules +! stdout . +cmp stderr stderr.txt + +-- stderr.txt -- +level=info msg="Loading configuration file" path=.pint.hcl +level=info msg="File parsed" path=rules/0001.yml rules=2 +level=debug msg="Found recording rule" lines=1-2 path=rules/0001.yml record=colo:recording +level=debug msg="Configured checks for rule" enabled=["promql/syntax","promql/without(job:true)","promql/by(job:true)"] path=rules/0001.yml rule=colo:recording +level=debug msg="Found alerting rule" alert=colo:alerting lines=4-5 path=rules/0001.yml +level=debug msg="Configured checks for rule" enabled=["promql/syntax"] path=rules/0001.yml rule=colo:alerting +rules/0001.yml:2: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/without) + expr: sum(foo) without(job) + +-- rules/0001.yml -- +- record: "colo:recording" + expr: sum(foo) without(job) + +- alert: "colo:alerting" + expr: sum(bar) without(job) + +-- .pint.hcl -- +rule { + ignore { + kind = "alerting" + } + aggregate ".+" { + keep = [ "job" ] + } +} diff --git a/cmd/pint/tests/0021_ignore_all.txt b/cmd/pint/tests/0021_ignore_all.txt new file mode 100644 index 00000000..7c3a5ddb --- /dev/null +++ b/cmd/pint/tests/0021_ignore_all.txt @@ -0,0 +1,21 @@ +pint.error -l debug lint rules +! stdout . +cmp stderr stderr.txt + +-- stderr.txt -- +level=info msg="Loading configuration file" path=.pint.hcl +level=fatal msg="Fatal error" error="failed to load config file \".pint.hcl\": ignore block must have at least one condition" +-- rules/0001.yml -- +- record: "colo:recording" + expr: sum(foo) without(job) + +- alert: "colo:alerting" + expr: sum(bar) without(job) + +-- .pint.hcl -- +rule { + ignore {} + aggregate ".+" { + keep = [ "job" ] + } +} diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index a3c652bb..339d98e5 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -127,7 +127,19 @@ Syntax: ```JS rule { match { - path = "..." + path = "(.+)" + name = "(.+)" + kind = "alerting|recording" + annotation "(.*)" { + value = "(.*)" + } + label "(.*)" { + value = "(.*)" + } + } + ignore { + path = "(.+)" + name = "(.+)" kind = "alerting|recording" annotation "(.*)" { value = "(.*)" @@ -144,12 +156,16 @@ rule { ``` - `match:path` - only files matching this pattern will be checked by this rule +- `match:name` - only rules with names (`record` for recording rules and `alert` for alerting + rules) matching this pattern will be checked rule - `match:kind` - optional rule type filter, only rule of this type will be checked - `match:annotation` - optional annotation filter, only alert rules with at least one annotation matching this pattern will be checked by this rule. - `match:label` - optional annotation filter, only rules with at least one label matching this pattern will be checked by this rule. For recording rules only static labels set on the recording rule are considered. +- `ignore` - works exactly like `match` but does the opposite - any alerting or recording rule + matching all conditions defined on `ignore` will not be checked by this `rule` block. Example: diff --git a/go.mod b/go.mod index 4a072e30..c6848e8e 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/prometheus/prometheus v1.8.2-0.20211105201321-411021ada9ab github.com/rogpeppe/go-internal v1.8.0 github.com/rs/zerolog v1.26.0 + github.com/stretchr/testify v1.7.0 github.com/urfave/cli/v2 v2.3.0 golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8 gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b @@ -54,7 +55,6 @@ require ( github.com/prometheus/procfs v0.7.3 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/sergi/go-diff v1.2.0 // indirect - github.com/stretchr/testify v1.7.0 // indirect github.com/uber/jaeger-client-go v2.29.1+incompatible // indirect github.com/uber/jaeger-lib v2.4.1+incompatible // indirect github.com/zclconf/go-cty v1.10.0 // indirect diff --git a/internal/config/config.go b/internal/config/config.go index a6ba76ea..1c6f70c5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -79,7 +79,6 @@ func (cfg Config) GetChecksForRule(path string, r parser.Rule) []checks.RuleChec continue } enabled = append(enabled, c) - } } @@ -151,7 +150,12 @@ func Load(path string, failOnMissing bool) (cfg Config, err error) { for _, rule := range cfg.Rules { if rule.Match != nil { - if err = rule.Match.validate(); err != nil { + if err = rule.Match.validate(true); err != nil { + return cfg, err + } + } + if rule.Ignore != nil { + if err = rule.Ignore.validate(false); err != nil { return cfg, err } } diff --git a/internal/config/rule.go b/internal/config/rule.go index 1ffabb54..6bd90ff2 100644 --- a/internal/config/rule.go +++ b/internal/config/rule.go @@ -71,16 +71,21 @@ func (ml MatchLabel) isMatching(rule parser.Rule) bool { type Match struct { Path string `hcl:"path,optional"` + Name string `hcl:"name,optional"` Kind string `hcl:"kind,optional"` Label *MatchLabel `hcl:"label,block"` Annotation *MatchLabel `hcl:"annotation,block"` } -func (m Match) validate() error { +func (m Match) validate(allowEmpty bool) error { if _, err := regexp.Compile(m.Path); err != nil { return err } + if _, err := regexp.Compile(m.Name); err != nil { + return err + } + switch m.Kind { case "": // not set @@ -96,11 +101,52 @@ func (m Match) validate() error { } } + if !allowEmpty && m.Path == "" && m.Name == "" && m.Kind == "" && m.Label == nil && m.Annotation == nil { + return fmt.Errorf("ignore block must have at least one condition") + } + return nil } +func (m Match) IsMatch(path string, r parser.Rule) bool { + if m.Kind != "" { + if r.AlertingRule != nil && m.Kind != alertingRuleType { + return false + } + if r.RecordingRule != nil && m.Kind != recordingRuleType { + return false + } + } + + if m.Path != "" { + re := strictRegex(m.Path) + if !re.MatchString(path) { + return false + } + } + + if m.Name != "" { + re := strictRegex(m.Name) + if r.AlertingRule != nil && !re.MatchString(r.AlertingRule.Alert.Value.Value) { + return false + } + if r.RecordingRule != nil && !re.MatchString(r.RecordingRule.Record.Value.Value) { + return false + } + } + + if m.Label != nil { + if !m.Label.isMatching(r) { + return false + } + } + + return true +} + type Rule struct { Match *Match `hcl:"match,block"` + Ignore *Match `hcl:"ignore,block"` Aggregate []AggregateSettings `hcl:"aggregate,block"` Rate *RateSettings `hcl:"rate,block"` Annotation []AnnotationSettings `hcl:"annotation,block"` @@ -117,34 +163,12 @@ type Rule struct { func (rule Rule) resolveChecks(path string, r parser.Rule, enabledChecks, disabledChecks []string, proms []PrometheusConfig) []checks.RuleChecker { enabled := []checks.RuleChecker{} - if rule.Match != nil && rule.Match.Kind != "" { - var isAllowed bool - recordingEnabled := rule.Match.Kind == recordingRuleType - alertingEnabled := rule.Match.Kind == alertingRuleType - if r.AlertingRule != nil && alertingEnabled { - isAllowed = true - } - if r.RecordingRule != nil && recordingEnabled { - isAllowed = true - } - if !isAllowed { - return enabled - } + if rule.Ignore != nil && rule.Ignore.IsMatch(path, r) { + return enabled } - if rule.Match != nil { - if rule.Match.Path != "" { - re := strictRegex(rule.Match.Path) - if !re.MatchString(path) { - return enabled - } - } - - if rule.Match.Label != nil { - if !rule.Match.Label.isMatching(r) { - return enabled - } - } + if rule.Match != nil && !rule.Match.IsMatch(path, r) { + return enabled } if len(rule.Aggregate) > 0 { diff --git a/internal/config/rule_test.go b/internal/config/rule_test.go new file mode 100644 index 00000000..c57c42f0 --- /dev/null +++ b/internal/config/rule_test.go @@ -0,0 +1,178 @@ +package config_test + +import ( + "strconv" + "testing" + + "github.com/cloudflare/pint/internal/config" + "github.com/cloudflare/pint/internal/parser" + + "github.com/stretchr/testify/assert" +) + +func TestMatch(t *testing.T) { + type testCaseT struct { + path string + rule parser.Rule + match config.Match + isMatch bool + } + + testCases := []testCaseT{ + { + path: "foo.yaml", + rule: parser.Rule{}, + match: config.Match{}, + isMatch: true, + }, + { + path: "foo.yaml", + rule: parser.Rule{}, + match: config.Match{ + Path: "bar.yaml", + }, + isMatch: false, + }, + { + path: "foo.yaml", + rule: parser.Rule{}, + match: config.Match{ + Path: "foo.yaml", + }, + isMatch: true, + }, + { + path: "foo.yaml", + rule: parser.Rule{}, + match: config.Match{ + Path: ".+.yaml", + }, + isMatch: true, + }, + { + path: "foo.yaml", + rule: parser.Rule{}, + match: config.Match{ + Path: "bar.+.yaml", + }, + isMatch: false, + }, + { + path: "foo.yaml", + rule: parser.Rule{ + AlertingRule: &parser.AlertingRule{ + Alert: parser.YamlKeyValue{ + Key: &parser.YamlNode{Value: "alert"}, + Value: &parser.YamlNode{Value: "Foo"}, + }, + }, + }, + match: config.Match{ + Name: "Foo", + }, + isMatch: true, + }, + { + path: "foo.yaml", + rule: parser.Rule{ + AlertingRule: &parser.AlertingRule{ + Alert: parser.YamlKeyValue{ + Key: &parser.YamlNode{Value: "alert"}, + Value: &parser.YamlNode{Value: "Foo"}, + }, + }, + }, + match: config.Match{ + Name: "Foo", + Path: "bar.yml", + }, + isMatch: false, + }, + { + path: "foo.yaml", + rule: parser.Rule{ + AlertingRule: &parser.AlertingRule{ + Alert: parser.YamlKeyValue{ + Key: &parser.YamlNode{Value: "alert"}, + Value: &parser.YamlNode{Value: "Foo"}, + }, + }, + }, + match: config.Match{ + Name: "Bar", + }, + isMatch: false, + }, + { + path: "foo.yaml", + rule: parser.Rule{ + RecordingRule: &parser.RecordingRule{ + Record: parser.YamlKeyValue{ + Key: &parser.YamlNode{Value: "record"}, + Value: &parser.YamlNode{Value: "Foo"}, + }, + }, + }, + match: config.Match{ + Name: "Bar", + }, + isMatch: false, + }, + { + path: "foo.yaml", + rule: parser.Rule{ + AlertingRule: &parser.AlertingRule{}, + }, + match: config.Match{ + Kind: "alerting", + }, + isMatch: true, + }, + { + path: "foo.yaml", + rule: parser.Rule{ + AlertingRule: &parser.AlertingRule{}, + }, + match: config.Match{}, + isMatch: true, + }, + { + path: "foo.yaml", + rule: parser.Rule{ + AlertingRule: &parser.AlertingRule{}, + }, + match: config.Match{ + Kind: "recording", + }, + isMatch: false, + }, + { + path: "foo.yaml", + rule: parser.Rule{ + RecordingRule: &parser.RecordingRule{}, + }, + match: config.Match{ + Kind: "recording", + }, + isMatch: true, + }, + { + path: "foo.yaml", + rule: parser.Rule{ + RecordingRule: &parser.RecordingRule{}, + }, + match: config.Match{ + Kind: "alerting", + }, + isMatch: false, + }, + } + + for i, tc := range testCases { + t.Run(strconv.Itoa(i+1), func(t *testing.T) { + assert := assert.New(t) + isMatch := tc.match.IsMatch(tc.path, tc.rule) + assert.Equal(tc.isMatch, isMatch) + }) + } +}