Merge pull request #9 from cloudflare/disable-comments

Allow disabling individual checks via comments

cmd/pint/tests/0011_ignore_rules.txt

@@ -0,0 +1,62 @@
+pint.error 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/1.yaml rules=10
+rules/1.yaml:5: syntax error: unexpected right parenthesis ')' (promql/syntax)
+  expr: sum(errors_total) by )
+
+rules/1.yaml:16: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/without)
+  expr: sum(errors_total) without(job)
+
+rules/1.yaml:22: syntax error: unexpected right parenthesis ')' (promql/syntax)
+  expr: sum(errors_total) by )
+
+rules/1.yaml:33: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/without)
+  expr: sum(errors_total) without(job)
+
+level=info msg="Problems found" Fatal=2 Warning=2
+level=fatal msg="Fatal error" error="problems found"
+-- rules/1.yaml --
+- record: disabled
+  expr: sum(errors_total) by ) # pint disable promql/syntax
+
+- record: active
+  expr: sum(errors_total) by )
+
+- record: disabled
+  # pint disable promql/without(job:true)
+  expr: sum(errors_total) without(job)
+
+- record: disabled
+  # pint disable promql/without
+  expr: sum(errors_total) without(job)
+
+- record: active
+  expr: sum(errors_total) without(job)
+
+- alert: disabled
+  expr: sum(errors_total) by ) # pint disable promql/syntax
+
+- alert: active
+  expr: sum(errors_total) by )
+
+- alert: disabled
+  # pint disable promql/without(job:true)
+  expr: sum(errors_total) without(job)
+
+- alert: disabled
+  # pint disable promql/without
+  expr: sum(errors_total) without(job)
+
+- alert: active
+  expr: sum(errors_total) without(job)
+
+-- .pint.hcl --
+rule {
+    aggregate ".+" {
+        keep = [ "job" ]
+    }
+}

docs/CONFIGURATION.md

@@ -652,3 +652,31 @@ groups:
     - record: job:http_inprogress_requests:sum
       expr: sum by (job) (http_inprogress_requests)
 ```
+
+## Disabling individual checks for specific rules
+
+To disable individual check for a specific rule use `# pint disable ...` comments.
+A single comment can only disable one check, so repeat it for every check you wish
+to disable.
+
+To disable `query/cost` check add `# pint disable query/cost` comment anywhere in
+the rule.
+
+Example:
+
+```YAML
+groups:
+  - name: example
+    rules:
+    - record: instance:http_requests_total:avg_over_time:1w
+      # pint disable query/cost
+      expr: avg_over_time(http_requests_total[1w]) by (instance)
+```
+
+```YAML
+groups:
+  - name: example
+    rules:
+    - record: instance:http_requests_total:avg_over_time:1w
+      expr: avg_over_time(http_requests_total[1w]) by (instance) # pint disable query/cost
+```

internal/config/config.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/cloudflare/pint/internal/checks"
@@ -53,7 +54,7 @@ func (cfg Config) String() string {
 func (cfg Config) GetChecksForRule(path string, r parser.Rule) []checks.RuleChecker {
 	enabled := []checks.RuleChecker{}
 
-	if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.SyntaxCheckName) {
+	if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.SyntaxCheckName, r) {
 		enabled = append(enabled, checks.NewSyntaxCheck())
 	}
 
@@ -64,7 +65,17 @@ func (cfg Config) GetChecksForRule(path string, r parser.Rule) []checks.RuleChec
 		}
 	}
 	for _, rule := range cfg.Rules {
-		enabled = append(enabled, rule.resolveChecks(path, r, cfg.Checks.Enabled, cfg.Checks.Disabled, proms)...)
+		for _, c := range rule.resolveChecks(path, r, cfg.Checks.Enabled, cfg.Checks.Disabled, proms) {
+			if r.HasComment(fmt.Sprintf("disable %s", removeRedundantSpaces(c.String()))) {
+				log.Debug().
+					Str("path", path).
+					Str("check", c.String()).
+					Msg("Check disabled by comment")
+				continue
+			}
+			enabled = append(enabled, c)
+
+		}
 	}
 
 	el := []string{}
@@ -200,3 +211,7 @@ func parseDuration(d string) (time.Duration, error) {
 	}
 	return time.Duration(mdur), nil
 }
+
+func removeRedundantSpaces(line string) string {
+	return strings.Join(strings.Fields(line), " ")
+}

internal/config/rule.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/cloudflare/pint/internal/checks"
 	"github.com/cloudflare/pint/internal/parser"
+	"github.com/rs/zerolog/log"
 )
 
 var (
@@ -152,40 +153,40 @@ func (rule Rule) resolveChecks(path string, r parser.Rule, enabledChecks, disabl
 			}
 			severity := aggr.getSeverity(checks.Warning)
 			for _, label := range aggr.Keep {
-				if isEnabled(enabledChecks, disabledChecks, checks.WithoutCheckName) {
+				if isEnabled(enabledChecks, disabledChecks, checks.WithoutCheckName, r) {
 					enabled = append(enabled, checks.NewWithoutCheck(nameRegex, label, true, severity))
 				}
-				if isEnabled(enabledChecks, disabledChecks, checks.ByCheckName) {
+				if isEnabled(enabledChecks, disabledChecks, checks.ByCheckName, r) {
 					enabled = append(enabled, checks.NewByCheck(nameRegex, label, true, severity))
 				}
 			}
 			for _, label := range aggr.Strip {
-				if isEnabled(enabledChecks, disabledChecks, checks.WithoutCheckName) {
+				if isEnabled(enabledChecks, disabledChecks, checks.WithoutCheckName, r) {
 					enabled = append(enabled, checks.NewWithoutCheck(nameRegex, label, false, severity))
 				}
-				if isEnabled(enabledChecks, disabledChecks, checks.ByCheckName) {
+				if isEnabled(enabledChecks, disabledChecks, checks.ByCheckName, r) {
 					enabled = append(enabled, checks.NewByCheck(nameRegex, label, false, severity))
 				}
 			}
 		}
 	}
 
-	if rule.Rate != nil && isEnabled(enabledChecks, disabledChecks, checks.RateCheckName) {
+	if rule.Rate != nil && isEnabled(enabledChecks, disabledChecks, checks.RateCheckName, r) {
 		for _, prom := range proms {
 			timeout, _ := parseDuration(prom.Timeout)
 			enabled = append(enabled, checks.NewRateCheck(prom.Name, prom.URI, timeout))
 		}
 	}
 
-	if rule.Cost != nil && isEnabled(enabledChecks, disabledChecks, checks.CostCheckName) {
+	if rule.Cost != nil && isEnabled(enabledChecks, disabledChecks, checks.CostCheckName, r) {
 		severity := rule.Cost.getSeverity(checks.Bug)
 		for _, prom := range proms {
 			timeout, _ := parseDuration(prom.Timeout)
 			enabled = append(enabled, checks.NewCostCheck(prom.Name, prom.URI, timeout, rule.Cost.BytesPerSample, rule.Cost.MaxSeries, severity))
 		}
 	}
 
-	if len(rule.Annotation) > 0 && isEnabled(enabledChecks, disabledChecks, checks.AnnotationCheckName) {
+	if len(rule.Annotation) > 0 && isEnabled(enabledChecks, disabledChecks, checks.AnnotationCheckName, r) {
 		for _, ann := range rule.Annotation {
 			var valueRegex *regexp.Regexp
 			if ann.Value != "" {
@@ -195,7 +196,7 @@ func (rule Rule) resolveChecks(path string, r parser.Rule, enabledChecks, disabl
 			enabled = append(enabled, checks.NewAnnotationCheck(ann.Key, valueRegex, ann.Required, severity))
 		}
 	}
-	if len(rule.Label) > 0 && isEnabled(enabledChecks, disabledChecks, checks.LabelCheckName) {
+	if len(rule.Label) > 0 && isEnabled(enabledChecks, disabledChecks, checks.LabelCheckName, r) {
 		for _, lab := range rule.Label {
 			var valueRegex *regexp.Regexp
 			if lab.Value != "" {
@@ -206,15 +207,15 @@ func (rule Rule) resolveChecks(path string, r parser.Rule, enabledChecks, disabl
 		}
 	}
 
-	if rule.Series != nil && isEnabled(enabledChecks, disabledChecks, checks.SeriesCheckName) {
+	if rule.Series != nil && isEnabled(enabledChecks, disabledChecks, checks.SeriesCheckName, r) {
 		severity := rule.Series.getSeverity(checks.Warning)
 		for _, prom := range proms {
 			timeout, _ := parseDuration(prom.Timeout)
 			enabled = append(enabled, checks.NewSeriesCheck(prom.Name, prom.URI, timeout, severity))
 		}
 	}
 
-	if rule.Alerts != nil && isEnabled(enabledChecks, disabledChecks, checks.AlertsCheckName) {
+	if rule.Alerts != nil && isEnabled(enabledChecks, disabledChecks, checks.AlertsCheckName, r) {
 		qRange := time.Hour * 24
 		if rule.Alerts.Range != "" {
 			qRange, _ = parseDuration(rule.Alerts.Range)
@@ -233,12 +234,12 @@ func (rule Rule) resolveChecks(path string, r parser.Rule, enabledChecks, disabl
 		}
 	}
 
-	if rule.Value != nil && isEnabled(enabledChecks, disabledChecks, checks.ValueCheckName) {
+	if rule.Value != nil && isEnabled(enabledChecks, disabledChecks, checks.ValueCheckName, r) {
 		severity := rule.Value.getSeverity(checks.Bug)
 		enabled = append(enabled, checks.NewValueCheck(severity))
 	}
 
-	if len(rule.Reject) > 0 && isEnabled(enabledChecks, disabledChecks, checks.RejectCheckName) {
+	if len(rule.Reject) > 0 && isEnabled(enabledChecks, disabledChecks, checks.RejectCheckName, r) {
 		for _, reject := range rule.Reject {
 			severity := reject.getSeverity(checks.Bug)
 			if reject.LabelKeys {
@@ -263,7 +264,14 @@ func (rule Rule) resolveChecks(path string, r parser.Rule, enabledChecks, disabl
 	return enabled
 }
 
-func isEnabled(enabledChecks, disabledChecks []string, name string) bool {
+func isEnabled(enabledChecks, disabledChecks []string, name string, rule parser.Rule) bool {
+	if rule.HasComment(fmt.Sprintf("disable %s", removeRedundantSpaces(name))) {
+		log.Debug().
+			Str("check", name).
+			Msg("Check disabled by comment")
+		return false
+	}
+
 	for _, c := range disabledChecks {
 		if c == name {
 			return false

internal/parser/models.go

@@ -69,22 +69,38 @@ func (fp FilePosition) LastLine() (line int) {
 	return
 }
 
+func mergeComments(node *yaml.Node) (comments []string) {
+	if node.HeadComment != "" {
+		comments = append(comments, node.HeadComment)
+	}
+	if node.LineComment != "" {
+		comments = append(comments, node.LineComment)
+	}
+	if node.FootComment != "" {
+		comments = append(comments, node.FootComment)
+	}
+	return
+}
+
 type YamlNode struct {
 	Position FilePosition
 	Value    string
+	Comments []string
 }
 
 func newYamlNode(node *yaml.Node) *YamlNode {
 	return &YamlNode{
 		Position: NewFilePosition(nodeLines(node)),
 		Value:    node.Value,
+		Comments: mergeComments(node),
 	}
 }
 
 func newYamlNodeWithParent(parent, node *yaml.Node) *YamlNode {
 	return &YamlNode{
 		Position: NewFilePosition(nodeLines(node)),
 		Value:    node.Value,
+		Comments: mergeComments(node),
 	}
 }
 
@@ -225,6 +241,32 @@ func (ar AlertingRule) Lines() (lines []int) {
 	return
 }
 
+func (ar AlertingRule) Comments() (comments []string) {
+	comments = append(comments, ar.Alert.Key.Comments...)
+	comments = append(comments, ar.Alert.Value.Comments...)
+	comments = append(comments, ar.Expr.Key.Comments...)
+	comments = append(comments, ar.Expr.Value.Comments...)
+	if ar.For != nil {
+		comments = append(comments, ar.For.Key.Comments...)
+		comments = append(comments, ar.For.Value.Comments...)
+	}
+	if ar.Labels != nil {
+		comments = append(comments, ar.Labels.Key.Comments...)
+		for _, label := range ar.Labels.Items {
+			comments = append(comments, label.Key.Comments...)
+			comments = append(comments, label.Value.Comments...)
+		}
+	}
+	if ar.Annotations != nil {
+		comments = append(comments, ar.Annotations.Key.Comments...)
+		for _, annotation := range ar.Annotations.Items {
+			comments = append(comments, annotation.Key.Comments...)
+			comments = append(comments, annotation.Value.Comments...)
+		}
+	}
+	return
+}
+
 type RecordingRule struct {
 	Record YamlKeyValue
 	Expr   PromQLExpr
@@ -240,6 +282,21 @@ func (rr RecordingRule) Lines() (lines []int) {
 	return
 }
 
+func (rr RecordingRule) Comments() (comments []string) {
+	comments = append(comments, rr.Record.Key.Comments...)
+	comments = append(comments, rr.Record.Value.Comments...)
+	comments = append(comments, rr.Expr.Key.Comments...)
+	comments = append(comments, rr.Expr.Value.Comments...)
+	if rr.Labels != nil {
+		comments = append(comments, rr.Labels.Key.Comments...)
+		for _, label := range rr.Labels.Items {
+			comments = append(comments, label.Key.Comments...)
+			comments = append(comments, label.Value.Comments...)
+		}
+	}
+	return
+}
+
 type ParseError struct {
 	Fragment string
 	Err      error
@@ -269,6 +326,21 @@ func (r Rule) Lines() []int {
 	return []int{r.Error.Line}
 }
 
+func (r Rule) HasComment(comment string) bool {
+	var comments []string
+	if r.RecordingRule != nil {
+		comments = r.RecordingRule.Comments()
+	} else if r.AlertingRule != nil {
+		comments = r.AlertingRule.Comments()
+	}
+	for _, c := range comments {
+		if hasComment(c, comment) {
+			return true
+		}
+	}
+	return false
+}
+
 type Result struct {
 	Path    string
 	Error   error

internal/parser/parser.go

@@ -95,6 +95,12 @@ func parseRule(content []byte, node *yaml.Node) (rule Rule, isEmpty bool, err er
 	var key *yaml.Node
 	unknownKeys := []*yaml.Node{}
 	for i, part := range node.Content {
+		if i == 0 && node.HeadComment != "" {
+			part.HeadComment = node.HeadComment
+		}
+		if i == len(node.Content)-1 && node.FootComment != "" {
+			part.FootComment = node.FootComment
+		}
 		if i%2 == 0 {
 			key = part
 		} else {