Merge pull request #1217 from cloudflare/lock

Add locked rule support

cmd/pint/tests/0207_locked_rule.txt

@@ -0,0 +1,35 @@
+! exec pint --no-color -l debug lint rules
+! stdout .
+cmp stderr stderr.txt
+
+-- stderr.txt --
+level=INFO msg="Loading configuration file" path=.pint.hcl
+level=DEBUG msg="Adding pint config to the parser exclude list" path=.pint.hcl
+level=INFO msg="Finding all rules to check" paths=["rules"]
+level=DEBUG msg="File parsed" path=rules/0001.yaml rules=1
+level=DEBUG msg="Glob finder completed" count=1
+level=DEBUG msg="Generated all Prometheus servers" count=0
+level=DEBUG msg="Found recording rule" path=rules/0001.yaml record=colo_job:up:byinstance lines=6-7 state=noop
+level=DEBUG msg="Configured checks for rule" enabled=["promql/syntax","alerts/for","alerts/comparison","alerts/template","promql/fragile","promql/regexp","promql/aggregate(job:true)"] path=rules/0001.yaml rule=colo_job:up:byinstance
+rules/0001.yaml:7 Bug: `job` label is required and should be preserved when aggregating `^.+$` rules, use `by(job, ...)`. (promql/aggregate)
+ 7 |     expr: sum(byinstance) by(instance)
+
+level=INFO msg="Problems found" Bug=1
+level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher"
+-- rules/0001.yaml --
+groups:
+- name: foo
+  rules:
+
+  # pint disable promql/aggregate(job:true)
+  - record: colo_job:up:byinstance
+    expr: sum(byinstance) by(instance)
+
+-- .pint.hcl --
+rule {
+    locked = true
+    aggregate ".+" {
+        keep     = [ "job" ]
+        severity = "bug"
+    }
+}

docs/changelog.md

@@ -15,6 +15,9 @@
   }
   ```
 
+- Rules configured in `pint` config can now be locked - when a rule is locked it cannot
+  be disabled by users by adding a `# pint disable ...` or `# pint snooze ...` comments.
+
 ### Fixed
 
 - The console reporter won't color the output if `--no-color` flag is set.

docs/configuration.md

@@ -527,6 +527,7 @@ Syntax:
 
 ```js
 rule {
+  locked    = true|false
   match {
     path    = "(.+)"
     state   = [ "any|added|modified|renamed|removed|unmodified", ... ]
@@ -569,6 +570,8 @@ rule {
 }
 ```
 
+- `locked` - if set to `true` this rule will be locked, which means that it cannot be disabled using
+  `# pint disable ...` or `# pint snooze ...` comments.
 - `match:path` - only files matching this pattern will be checked by this rule.
 - `match:state` - only match rules based on their state. Default value for `state` depends on the
   pint command that is being run, for `pint ci` the default value is `["added", "modified", "renamed"]`,

internal/config/__snapshots__/config_test.snap

@@ -3285,3 +3285,139 @@
   ]
 }
 ---
+
+[TestGetChecksForRule/multiple_checks_and_disable_comment_/_locked_rule - 1]
+{
+  "ci": {
+    "baseBranch": "master",
+    "maxCommits": 20
+  },
+  "parser": {},
+  "checks": {
+    "enabled": [
+      "alerts/absent",
+      "alerts/annotation",
+      "alerts/count",
+      "alerts/external_labels",
+      "alerts/for",
+      "alerts/template",
+      "labels/conflict",
+      "promql/aggregate",
+      "alerts/comparison",
+      "promql/fragile",
+      "promql/range_query",
+      "promql/rate",
+      "promql/regexp",
+      "promql/syntax",
+      "promql/vector_matching",
+      "query/cost",
+      "promql/counter",
+      "promql/series",
+      "rule/dependency",
+      "rule/duplicate",
+      "rule/for",
+      "rule/name",
+      "rule/label",
+      "rule/link",
+      "rule/reject",
+      "rule/report"
+    ]
+  },
+  "owners": {},
+  "rules": [
+    {
+      "aggregate": [
+        {
+          "name": ".+",
+          "severity": "bug",
+          "keep": [
+            "job"
+          ]
+        }
+      ]
+    },
+    {
+      "aggregate": [
+        {
+          "name": ".+",
+          "comment": "this is rule comment",
+          "severity": "bug",
+          "strip": [
+            "instance",
+            "rack"
+          ]
+        }
+      ],
+      "locked": true
+    }
+  ]
+}
+---
+
+[TestGetChecksForRule/multiple_checks_and_snooze_comment_/_locked_rule - 1]
+{
+  "ci": {
+    "baseBranch": "master",
+    "maxCommits": 20
+  },
+  "parser": {},
+  "checks": {
+    "enabled": [
+      "alerts/absent",
+      "alerts/annotation",
+      "alerts/count",
+      "alerts/external_labels",
+      "alerts/for",
+      "alerts/template",
+      "labels/conflict",
+      "promql/aggregate",
+      "alerts/comparison",
+      "promql/fragile",
+      "promql/range_query",
+      "promql/rate",
+      "promql/regexp",
+      "promql/syntax",
+      "promql/vector_matching",
+      "query/cost",
+      "promql/counter",
+      "promql/series",
+      "rule/dependency",
+      "rule/duplicate",
+      "rule/for",
+      "rule/name",
+      "rule/label",
+      "rule/link",
+      "rule/reject",
+      "rule/report"
+    ]
+  },
+  "owners": {},
+  "rules": [
+    {
+      "aggregate": [
+        {
+          "name": ".+",
+          "severity": "bug",
+          "keep": [
+            "job"
+          ]
+        }
+      ]
+    },
+    {
+      "aggregate": [
+        {
+          "name": ".+",
+          "comment": "this is rule comment",
+          "severity": "bug",
+          "strip": [
+            "instance",
+            "rack"
+          ]
+        }
+      ],
+      "locked": true
+    }
+  ]
+}
+---

internal/config/config.go

@@ -104,7 +104,7 @@ func (cfg *Config) GetChecksForEntry(ctx context.Context, gen *PrometheusGenerat
 		if !isMatch(ctx, entry, pr.ignore, pr.match) {
 			continue
 		}
-		if pr.isEnabled(ctx, cfg.Checks.Enabled, cfg.Checks.Disabled, enabled, entry, cfg.Rules) {
+		if pr.isEnabled(ctx, cfg.Checks.Enabled, cfg.Checks.Disabled, enabled, entry, cfg.Rules, pr.locked) {
 			enabled = append(enabled, pr.check)
 		}
 	}

internal/config/config_test.go

@@ -2192,6 +2192,88 @@ rule {
 				checks.ReportCheckName,
 			},
 		},
+		{
+			title: "multiple checks and disable comment / locked rule",
+			config: `
+rule {
+  locked = false
+  aggregate ".+" {
+    severity = "bug"
+	keep     = ["job"]
+  }
+}
+rule {
+  locked = true
+  aggregate ".+" {
+	comment  = "this is rule comment"
+    severity = "bug"
+	strip    = ["instance", "rack"]
+  }
+}`,
+			entry: discovery.Entry{
+				State: discovery.Modified,
+				Path: discovery.Path{
+					Name:          "rules.yml",
+					SymlinkTarget: "rules.yml",
+				},
+				Rule: newRule(t, `
+- record: foo
+  # pint disable promql/aggregate
+  expr: sum(foo)
+`),
+			},
+			checks: []string{
+				checks.SyntaxCheckName,
+				checks.AlertForCheckName,
+				checks.ComparisonCheckName,
+				checks.TemplateCheckName,
+				checks.FragileCheckName,
+				checks.RegexpCheckName,
+				checks.AggregationCheckName + "(instance:false)",
+				checks.AggregationCheckName + "(rack:false)",
+			},
+		},
+		{
+			title: "multiple checks and snooze comment / locked rule",
+			config: `
+rule {
+  locked = false
+  aggregate ".+" {
+    severity = "bug"
+	keep     = ["job"]
+  }
+}
+rule {
+  locked = true
+  aggregate ".+" {
+	comment  = "this is rule comment"
+    severity = "bug"
+	strip    = ["instance", "rack"]
+  }
+}`,
+			entry: discovery.Entry{
+				State: discovery.Modified,
+				Path: discovery.Path{
+					Name:          "rules.yml",
+					SymlinkTarget: "rules.yml",
+				},
+				Rule: newRule(t, `
+- record: foo
+  # pint snooze 2099-11-28 promql/aggregate
+  expr: sum(foo)
+`),
+			},
+			checks: []string{
+				checks.SyntaxCheckName,
+				checks.AlertForCheckName,
+				checks.ComparisonCheckName,
+				checks.TemplateCheckName,
+				checks.FragileCheckName,
+				checks.RegexpCheckName,
+				checks.AggregationCheckName + "(instance:false)",
+				checks.AggregationCheckName + "(rack:false)",
+			},
+		},
 	}
 
 	dir := t.TempDir()