Merge pull request #1048 from cloudflare/rule/name

Add rule/name check

cmd/pint/tests/0025_config.txt

@@ -35,6 +35,7 @@ level=INFO msg="Loading configuration file" path=.pint.hcl
       "rule/dependency",
       "rule/duplicate",
       "rule/for",
+      "rule/name",
       "rule/label",
       "rule/link",
       "rule/reject"

cmd/pint/tests/0113_config_env_expand.txt

@@ -40,6 +40,7 @@ level=INFO msg="Loading configuration file" path=.pint.hcl
       "rule/dependency",
       "rule/duplicate",
       "rule/for",
+      "rule/name",
       "rule/label",
       "rule/link",
       "rule/reject"

cmd/pint/tests/0177_rule_name.txt

@@ -0,0 +1,28 @@
+pint.error --no-color lint rules
+! stdout .
+cmp stderr stderr.txt
+
+-- stderr.txt --
+level=INFO msg="Loading configuration file" path=.pint.hcl
+level=INFO msg="Finding all rules to check" paths=["rules"]
+rules/01.yml:4 Bug: alerting rule name must match `^rec:.+$`. (rule/name)
+ 4 |   - alert: foo
+
+level=INFO msg="Problems found" Bug=1
+level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher"
+-- rules/01.yml --
+groups:
+- name: foo
+  rules:
+  - alert: foo
+    expr: up == 0
+  - alert: rec:foo
+    expr: up == 0
+
+-- .pint.hcl --
+rule {
+    name "rec:.+" {
+        severity = "bug"
+        comment  = "must use rec: prefix"
+    }
+}

docs/changelog.md

@@ -5,6 +5,7 @@
 ### Added
 
 - Added [alerts/absent](checks/alerts/absent.md) check.
+- Added [rule/name](checks/rule/name.md) check - #1020.
 
 ### Changed
 

docs/checks/rule/name.md

@@ -0,0 +1,109 @@
+---
+layout: default
+parent: Checks
+grand_parent: Documentation
+---
+
+# rule/name
+
+This check allows to match rule names:
+
+- `alert` for alerting rules
+- `record` for recording rules
+
+## Configuration
+
+Syntax:
+
+```js
+name "$pattern" {
+  comment  = "..."
+  severity = "bug|warning|info"
+}
+```
+
+- `$pattern` - regexp pattern to match the name on, this can be templated
+  to reference checked rule fields, see [Configuration](../../configuration.md)
+  for details.
+- `comment` - set a custom comment that will be added to reported problems.
+- `severity` - set custom severity for reported issues, defaults to a information.
+
+## How to enable it
+
+This check is not enabled by default as it requires explicit configuration
+to work.
+To enable it add one or more `rule {...}` blocks and specify all required
+labels there.
+
+Example that will require all recording rules to have `rec:` prefix:
+
+```js
+rule {
+  match {
+    kind = "recording"
+  }
+
+  label "rec:.+" {
+    comment  = "ALl recording rules must use the `rec:` prefix."
+    severity = "bug"
+  }
+}
+```
+
+## How to disable it
+
+You can disable this check globally by adding this config block:
+
+```js
+checks {
+  disabled = ["rule/name"]
+}
+```
+
+You can also disable it for all rules inside given file by adding
+a comment anywhere in that file. Example:
+
+```yaml
+# pint file/disable rule/name
+```
+
+Or you can disable it per rule by adding a comment to it. Example:
+
+```yaml
+# pint disable rule/name
+```
+
+If you want to disable only individual instances of this check
+you can add a more specific comment.
+
+```yaml
+# pint disable rule/name($pattern)
+```
+
+Example pint rule:
+
+```js
+label "rec:.+" {
+  comment  = "ALl recording rules must use the `rec:` prefix."
+  severity = "bug"
+}
+```
+
+Example comment disabling that rule:
+
+```yaml
+# pint disable rule/name($rec:.+$)
+```
+
+## How to snooze it
+
+You can disable this check until given time by adding a comment to it. Example:
+
+```yaml
+# pint snooze $TIMESTAMP rule/name
+```
+
+Where `$TIMESTAMP` is either use [RFC3339](https://www.rfc-editor.org/rfc/rfc3339)
+formatted or `YYYY-MM-DD`.
+Adding this comment will disable `rule/name` _until_ `$TIMESTAMP`, after that
+check will be re-enabled.

internal/checks/base.go

@@ -33,6 +33,7 @@ var (
 		RuleDependencyCheckName,
 		RuleDuplicateCheckName,
 		RuleForCheckName,
+		RuleNameCheckName,
 		LabelCheckName,
 		RuleLinkCheckName,
 		RejectCheckName,

internal/checks/rule_name.go

@@ -0,0 +1,69 @@
+package checks
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/cloudflare/pint/internal/discovery"
+	"github.com/cloudflare/pint/internal/parser"
+)
+
+const (
+	RuleNameCheckName = "rule/name"
+)
+
+func NewRuleNameCheck(re *TemplatedRegexp, comment string, severity Severity) RuleNameCheck {
+	return RuleNameCheck{
+		re:       re,
+		comment:  comment,
+		severity: severity,
+	}
+}
+
+type RuleNameCheck struct {
+	re       *TemplatedRegexp
+	comment  string
+	severity Severity
+}
+
+func (c RuleNameCheck) Meta() CheckMeta {
+	return CheckMeta{
+		States: []discovery.ChangeType{
+			discovery.Noop,
+			discovery.Added,
+			discovery.Modified,
+			discovery.Moved,
+		},
+		IsOnline: false,
+	}
+}
+
+func (c RuleNameCheck) String() string {
+	return fmt.Sprintf("%s(%s)", RuleNameCheckName, c.re.anchored)
+}
+
+func (c RuleNameCheck) Reporter() string {
+	return RuleNameCheckName
+}
+
+func (c RuleNameCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
+	if rule.AlertingRule != nil && !c.re.MustExpand(rule).MatchString(rule.AlertingRule.Alert.Value) {
+		problems = append(problems, Problem{
+			Lines:    rule.AlertingRule.Alert.Lines,
+			Reporter: c.Reporter(),
+			Text:     fmt.Sprintf("alerting rule name must match `%s`.", c.re.anchored),
+			Details:  maybeComment(c.comment),
+			Severity: c.severity,
+		})
+	}
+	if rule.RecordingRule != nil && !c.re.MustExpand(rule).MatchString(rule.RecordingRule.Record.Value) {
+		problems = append(problems, Problem{
+			Lines:    rule.RecordingRule.Record.Lines,
+			Reporter: c.Reporter(),
+			Text:     fmt.Sprintf("recording rule name must match `%s`.", c.re.anchored),
+			Details:  maybeComment(c.comment),
+			Severity: c.severity,
+		})
+	}
+	return problems
+}

internal/checks/rule_name_test.go

@@ -0,0 +1,77 @@
+package checks_test
+
+import (
+	"testing"
+
+	"github.com/cloudflare/pint/internal/checks"
+	"github.com/cloudflare/pint/internal/parser"
+	"github.com/cloudflare/pint/internal/promapi"
+)
+
+func TestRuleName(t *testing.T) {
+	testCases := []checkTest{
+		{
+			description: "doesn't ignore rules with syntax errors",
+			content:     "- record: foo\n  expr: sum(foo) without(\n",
+			checker: func(_ *promapi.FailoverGroup) checks.RuleChecker {
+				return checks.NewRuleNameCheck(checks.MustTemplatedRegexp("total:.+"), "some text", checks.Warning)
+			},
+			prometheus: noProm,
+			problems: func(_ string) []checks.Problem {
+				return []checks.Problem{
+					{
+						Lines: parser.LineRange{
+							First: 1,
+							Last:  1,
+						},
+						Reporter: checks.RuleNameCheckName,
+						Text:     "recording rule name must match `^total:.+$`.",
+						Details:  "Rule comment: some text",
+						Severity: checks.Warning,
+					},
+				}
+			},
+		},
+		{
+			description: "doesn't ignore rules with syntax errors",
+			content:     "- record: total:foo\n  expr: sum(foo) without(\n",
+			checker: func(_ *promapi.FailoverGroup) checks.RuleChecker {
+				return checks.NewRuleNameCheck(checks.MustTemplatedRegexp("total:.+"), "some text", checks.Warning)
+			},
+			prometheus: noProm,
+			problems:   noProblems,
+		},
+		{
+			description: "doesn't ignore rules with syntax errors",
+			content:     "- alert: foo\n  expr: sum(foo) without(\n",
+			checker: func(_ *promapi.FailoverGroup) checks.RuleChecker {
+				return checks.NewRuleNameCheck(checks.MustTemplatedRegexp("total:.+"), "some text", checks.Warning)
+			},
+			prometheus: noProm,
+			problems: func(_ string) []checks.Problem {
+				return []checks.Problem{
+					{
+						Lines: parser.LineRange{
+							First: 1,
+							Last:  1,
+						},
+						Reporter: checks.RuleNameCheckName,
+						Text:     "alerting rule name must match `^total:.+$`.",
+						Details:  "Rule comment: some text",
+						Severity: checks.Warning,
+					},
+				}
+			},
+		},
+		{
+			description: "doesn't ignore rules with syntax errors",
+			content:     "- alert: total:foo\n  expr: sum(foo) without(\n",
+			checker: func(_ *promapi.FailoverGroup) checks.RuleChecker {
+				return checks.NewRuleNameCheck(checks.MustTemplatedRegexp("total:.+"), "some text", checks.Warning)
+			},
+			prometheus: noProm,
+			problems:   noProblems,
+		},
+	}
+	runTests(t, testCases)
+}