feat: add support for disabling validations using comments on group a…

…nd file level

Signed-off-by: Martin Chodur <m.chodur@seznam.cz>

CHANGELOG.md

@@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+- Added: support for disabling validation using comments in yaml rules per rule group and for the whole file.
+- Docs: Updated/improved documentation on how to disable validations and validation rules.
 
 ## [2.12.0] - 2024-03-07
 - Fixed resolving of the path in `paramsFromFile`. Formerly it was resolved from the current working directory, now it must be a relative path, that will be resolved from the config file location.

README.md

@@ -185,71 +185,122 @@ Or using [Docker image](https://hub.docker.com/r/fusakla/promruval)
 docker run -it -v $PWD:/rules fusakla/promruval validate --config-file=/rules/examples/validation.yaml /rules/examples/rules.yaml
 ```
 
-#### Validation using live Prometheus instance
+### Validation using live Prometheus instance
 
 Event though these validations are useful, they may be flaky and dangerous for the Prometheus instance.
 If you have large number of rules and run the check often the number of queries can be huge or the instance might go
 down and your validation
 would be flaky.
 
 Therefore, it's recommended to use these check as a warning and do not fail if it does not succeed.
+Also consider running it rather periodically (for example once per day) instead of running it on every commit in CI.
 
-### Disabling validations per rule
+### Disabling validations
+There are three ways you can disable certain validation:
+ - Using cmd line flag `--disable-rule`
+ - Using YAML comments
+ - Using comments in the PromQL expression
+ - Using alert annotation
 
-If you want to disable particular validation for a certain rule, you can add a comment above it with a list of
-validation names to ignore. Alternatively, the comment can be put on its own line _inside_ the `expr` of the rule.
-The in-expression comment can be present multiple times.
+> The later two are useful if you yse for example jsonnet to generate the rules.
+> Then you can't use the YAML comments, but you can set the comments in the expression or alert annotations.
+> Unfortunately those have limited scope of usage (recording rules cannot have annotations, cannot be disabled on the group or file level).
 
-By default, the comment prefix is `ignore_validations` but can be changed using the `customDisableComment` config option
-in [config](#configuration).
-Value of the comment should be comma separated list of [validation names](./docs/validations.md)
+#### Using cmd line flag
+If you want to temporarily disable any of the validation rules for all the tested files,
+you can use the `--disable-rule` flag with value corresponding to the `name`
+of the validation rule you want to disable. Can be passed multiple times.
 
 Example:
+```yaml
+# Promruval validation configuration
+validationRules:
+  - name: check-irate
+    scope: Alert
+    validations:
+      - type: expressionDoesNotUseIrate
+```
+
+```bash
+promruval validate --config-file examples/validation.yaml --disable-rule check-irate examples/rules.yaml
+```
+
+#### Using YAML comments
+You can use comments in YAML to disable certain validations. This can be done on the file, group or rule level.
+The comment should be in format `# ignore_validations: validationName1, validationName2, ...` where the `validationName`
+is the name of the validation as defined in the [docs/validations.md](./docs/validations.md).
 
+> The `ignore_validations` prefix can be changed using the `customDisableComment` config option in the [config](#configuration).
+
+Example:
 ```yaml
+# Disable for the whole file
+# ignore_validations: expressionDoesNotUseIrate
 groups:
-  - name: foo
+  # Disable only for the following rule group
+  # ignore_validations: expressionDoesNotUseIrate
+  - name: group1
+    partial_response_strategy: abort
+    interval: 1m
+    limit: 10
     rules:
-      # The following validations will be ignored in the rule that immediately follows.
-      # ignore_validations: expressionSelectorsMatchesAnything, expressionDoesNotUseOlderDataThan
-      - record: bar
+      # Disable only for the following rule
+      # ignore_validations: expressionDoesNotUseIrate
+      - record: recorded_metrics
         expr: 1
-      # The same validations are disabled for the following rule, but the comments are in the expression.
-      - name: baz
-        expr: |
-          # ignore_validations: expressionSelectorsMatchesAnything
-          up{
-            # ignore_validations: expressionDoesNotUseOlderDataThan
-          }
+        labels:
+          foo: bar
 ```
 
-### Disabling rules
+#### Using PromQL expression comments
+Same way as in the YAML comments, you can use comments in the PromQL expression to disable certain validations.
+The comment should be in the same format `# ignore_validations: validationName1, validationName2, ...` where the `validationName`
+is the name of the validation as defined in the [docs/validations.md](./docs/validations.md).
+The comment can be present multiple times in the expression and can be anywhere in the expression.
 
-If you want to temporarily disable any of the rules for all the tested rules,
-you can use the `--disable-rule` flag with value corresponding to the `name`
-of the rule you want to disable. Can be passed multiple times.
+> The `ignore_validations` prefix can be changed using the `customDisableComment` config option in the [config](#configuration).
 
-```bash
-promruval validate --config-file examples/validation.yaml --disable-rule check-team-label examples/rules.yaml
+Example:
+```yaml
+groups:
+  - name: test-group
+    rules:
+      - alert: test-alert
+        expr: |
+          # ignore_validations: expressionDoesNotUseIrate
+          irate(http_requests_total[5m]) # ignore_validations: expressionDoesNotUseIrate
 ```
 
-If you want to disable permanently for some Prometheus rule, you can use the special annotation
-`disabled_validation_rules`(can be changed in the [config](#configuration)) that represents comma separated list of
-rule names to be skipped for the particular rule.
+#### Using alert annotation
+If you can't(or don't want to) use the comments to disable validations, you can use the special annotation
+`disabled_validation_rules`. It represents comma separated list of **validation rule names** to be skipped for the particular alert.
+Since annotations are only available for alerts, **this method can be used only for alerts!**
+
+> The `disabled_validation_rules` annotation name can be changed using the `customExcludeAnnotation` config option in the [config](#configuration).
 
-Example Prometheus rule:
+Example:
 
 ```yaml
+# Promruval validation configuration
+validationRules:
+  - name: check-irate
+    scope: Alert
+    validations:
+      - type: expressionDoesNotUseIrate
+```
+
+```yaml
+# Prometheus rule file
 groups:
-  - name: ...
+  - name: test-group
     rules:
-      - alert: ...
-        expr: ...
+      - alert: test-alert
+        expr: 1
         annotations:
-          disabled_validation_rules: team-label-check,title-annotation-check
+          disabled_validation_rules: check-irate # Will disable the check-irate validation rule check for this alert
 ```
 
-### Readable validation description
+### Human readable validation description
 
 If you want more human readable validation summary (for a documentation or generating readable git pages)
 you can use the `validation-docs` command, see the [usage](#usage).

examples/rules/rules.yaml

@@ -1,3 +1,4 @@
+# ignore_validations: hasAllowedLimit
 groups:
   - name: group1
     partial_response_strategy: abort
@@ -10,10 +11,11 @@ groups:
         labels:
           foo: bar
 
+  # ignore_validations: labelHasAllowedValue
   - name: testGroup
     partial_response_strategy: "warn"
     source_tenants: ["tenant1", "tenant2"]
-    limit: 100
+    limit: 1000
     rules:
       # Comment before.
       # Comment on the same line. ignore_validations: expressionSelectorsMatchesAnything, expressionDoesNotUseOlderDataThan
@@ -23,7 +25,7 @@ groups:
         for: 4w
         keep_firing_for: 5m
         labels:
-          severity: critical
+          severity: critica
           team: sre@mail.com
           page: true
           foo: "{{ $labels.foo }}"

pkg/config/config.go

@@ -16,10 +16,11 @@ const (
 	AlertScope         ValidationScope = "Alert"
 	RecordingRuleScope ValidationScope = "Recording rule"
 	AllRulesScope      ValidationScope = "All rules"
-	Group              ValidationScope = "Group"
+	GroupScope         ValidationScope = "Group"
+	AllScope           ValidationScope = "All"
 )
 
-var ValidationScopes = []ValidationScope{Group, AlertScope, RecordingRuleScope, AllRulesScope}
+var ValidationScopes = []ValidationScope{GroupScope, AlertScope, RecordingRuleScope, AllRulesScope}
 
 // Ugly hack with a global variable to be able to use it in UnmarshalYAML.
 // Not sure how to better propagate some context to the UnmarshalYAML function.

pkg/unmarshaler/unmarshaler.go

@@ -1,6 +1,7 @@
 package unmarshaler
 
 import (
+	"slices"
 	"strings"
 
 	"github.com/prometheus/common/model"
@@ -16,8 +17,40 @@ type fakeTestFile struct {
 }
 
 type RulesFile struct {
-	Groups       []RuleGroup `yaml:"groups"`
-	fakeTestFile             // Just so we can unmarshal also PromQL test files but ignore them because it has no Groups
+	Groups       GroupsWithComment `yaml:"groups"`
+	fakeTestFile                   // Just so we can unmarshal also PromQL test files but ignore them because it has no Groups
+}
+
+type RulesFileWithComment struct {
+	node           yaml.Node
+	groupsComments []string
+	RulesFile
+}
+
+func (r *RulesFileWithComment) UnmarshalYAML(value *yaml.Node) error {
+	for _, field := range value.Content {
+		if field.Kind == yaml.ScalarNode && field.Value == "groups" {
+			r.groupsComments = strings.Split(field.HeadComment, "\n")
+		}
+	}
+	return unmarshalToNodeAndStruct(value, &r.node, &r.RulesFile)
+}
+
+func (r *RulesFileWithComment) DisabledValidators(commentPrefix string) []string {
+	return disabledValidatorsFromComments(slices.Concat(getYamlNodeComments(r.node, commentPrefix), r.groupsComments), commentPrefix)
+}
+
+type GroupsWithComment struct {
+	node   yaml.Node
+	Groups []RuleGroupWithComment
+}
+
+func (g *GroupsWithComment) UnmarshalYAML(value *yaml.Node) error {
+	return unmarshalToNodeAndStruct(value, &g.node, &g.Groups)
+}
+
+func (g *GroupsWithComment) DisabledValidators(commentPrefix string) []string {
+	return disabledValidatorsFromComments(getYamlNodeComments(g.node, commentPrefix), commentPrefix)
 }
 
 type RuleGroup struct {
@@ -29,6 +62,19 @@ type RuleGroup struct {
 	Limit                   int               `yaml:"limit,omitempty"`
 }
 
+type RuleGroupWithComment struct {
+	node yaml.Node
+	RuleGroup
+}
+
+func (r *RuleGroupWithComment) UnmarshalYAML(value *yaml.Node) error {
+	return unmarshalToNodeAndStruct(value, &r.node, &r.RuleGroup)
+}
+
+func (r *RuleGroupWithComment) DisabledValidators(commentPrefix string) []string {
+	return disabledValidatorsFromComments(getYamlNodeComments(r.node, commentPrefix), commentPrefix)
+}
+
 type RuleWithComment struct {
 	node yaml.Node
 	rule rulefmt.RuleNode
@@ -46,38 +92,11 @@ func (r *RuleWithComment) OriginalRule() rulefmt.Rule {
 }
 
 func (r *RuleWithComment) UnmarshalYAML(value *yaml.Node) error {
-	err := value.Decode(&r.node)
-	if err != nil {
-		return err
-	}
-	err = value.Decode(&r.rule)
-	if err != nil {
-		return err
-	}
-	return nil
+	return unmarshalToNodeAndStruct(value, &r.node, &r.rule)
 }
 
 func (r *RuleWithComment) DisabledValidators(commentPrefix string) []string {
-	commentPrefix += ":"
-	var disabledValidators []string
-	allComments := strings.Split(r.node.HeadComment, "\n")
-	for _, line := range strings.Split(r.rule.Expr.Value, "\n") {
-		before, comment, found := strings.Cut(line, "#")
-		if !found || strings.TrimSpace(before) != "" {
-			continue
-		}
-		allComments = append(allComments, comment)
-	}
-	for _, comment := range allComments {
-		_, csv, found := strings.Cut(comment, commentPrefix)
-		if !found {
-			continue
-		}
-		validators := strings.Split(csv, ",")
-		for _, v := range validators {
-			vv := strings.TrimSpace(v)
-			disabledValidators = append(disabledValidators, vv)
-		}
-	}
-	return disabledValidators
+	ruleComments := getYamlNodeComments(r.node, commentPrefix)
+	exprComments := getExpressionComments(r.rule.Expr.Value, commentPrefix)
+	return disabledValidatorsFromComments(slices.Concat(ruleComments, exprComments), commentPrefix)
 }

pkg/validate/validate.go

@@ -55,8 +55,9 @@ func Files(fileNames []string, validationRules []*validationrule.ValidationRule,
 			fileReport.Errors = []error{fmt.Errorf("cannot read file %s: %w", fileName, err)}
 			continue
 		}
-		var rf unmarshaler.RulesFile
+		var rf unmarshaler.RulesFileWithComment
 		decoder := yaml.NewDecoder(f)
+		decoder.KnownFields(true)
 		err = decoder.Decode(&rf)
 		if err != nil {
 			if errors.Is(err, io.EOF) {
@@ -67,15 +68,25 @@ func Files(fileNames []string, validationRules []*validationrule.ValidationRule,
 			fileReport.Errors = []error{fmt.Errorf("invalid file %s: %w", fileName, err)}
 			continue
 		}
-		for _, group := range rf.Groups {
+		fileDisabledValidators := rf.DisabledValidators(disableValidationsComment)
+		allGroupsDisabledValidators := rf.Groups.DisabledValidators(disableValidationsComment)
+		for _, group := range rf.Groups.Groups {
 			validationReport.GroupsCount++
 			groupReport := fileReport.NewGroupReport(group.Name)
+			groupDisabledValidators := group.DisabledValidators(disableValidationsComment)
+			if err := validator.KnownValidators(config.AllScope, groupDisabledValidators); err != nil {
+				groupReport.Errors = append(groupReport.Errors, fmt.Errorf("invalid disabled validators: %w", err))
+			}
+			groupDisabledValidators = slices.Concat(groupDisabledValidators, fileDisabledValidators, allGroupsDisabledValidators)
 			for _, rule := range validationRules {
-				if rule.Scope() != config.Group {
+				if rule.Scope() != config.GroupScope {
 					continue
 				}
 				for _, v := range rule.Validators() {
-					groupReport.Errors = append(groupReport.Errors, validateWithDetails(v, group, rulefmt.Rule{}, prometheusClient)...)
+					if slices.Contains(groupDisabledValidators, v.Name()) {
+						continue
+					}
+					groupReport.Errors = append(groupReport.Errors, validateWithDetails(v, group.RuleGroup, rulefmt.Rule{}, prometheusClient)...)
 				}
 			}
 			if len(groupReport.Errors) > 0 {
@@ -97,11 +108,12 @@ func Files(fileNames []string, validationRules []*validationrule.ValidationRule,
 					excludedRules = strings.Split(excludedRulesText, ",")
 				}
 				disabledValidators := ruleNode.DisabledValidators(disableValidationsComment)
-				if err := validator.KnownValidators(config.AllRulesScope, disabledValidators); err != nil {
-					ruleReport.Errors = append(ruleReport.Errors, err)
+				if err := validator.KnownValidators(config.AllScope, disabledValidators); err != nil {
+					ruleReport.Errors = append(ruleReport.Errors, fmt.Errorf("invalid disabled validators: %w", err))
 				}
+				disabledValidators = append(disabledValidators, groupDisabledValidators...)
 				for _, rule := range validationRules {
-					if rule.Scope() == config.Group {
+					if rule.Scope() == config.GroupScope {
 						continue
 					}
 					skipRule := false
@@ -121,7 +133,7 @@ func Files(fileNames []string, validationRules []*validationrule.ValidationRule,
 						if slices.Contains(disabledValidators, validatorName) {
 							continue
 						}
-						ruleReport.Errors = append(ruleReport.Errors, validateWithDetails(v, group, originalRule, prometheusClient)...)
+						ruleReport.Errors = append(ruleReport.Errors, validateWithDetails(v, group.RuleGroup, originalRule, prometheusClient)...)
 						log.Debugf("validation of file %s group %s using \"%s\" took %s", fileName, group.Name, v, time.Since(start))
 					}
 					if len(ruleReport.Errors) > 0 {