diff --git a/cmd/pint/tests/0025_config.txt b/cmd/pint/tests/0025_config.txt index 2a5ef999..02d87c7b 100644 --- a/cmd/pint/tests/0025_config.txt +++ b/cmd/pint/tests/0025_config.txt @@ -38,7 +38,8 @@ level=INFO msg="Loading configuration file" path=.pint.hcl "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ], "disabled": [ "promql/fragile" diff --git a/cmd/pint/tests/0113_config_env_expand.txt b/cmd/pint/tests/0113_config_env_expand.txt index 6e0a01fe..bc3d6a38 100644 --- a/cmd/pint/tests/0113_config_env_expand.txt +++ b/cmd/pint/tests/0113_config_env_expand.txt @@ -43,7 +43,8 @@ level=INFO msg="Loading configuration file" path=.pint.hcl "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, diff --git a/cmd/pint/tests/0202_report_recording_rules.txt b/cmd/pint/tests/0202_report_recording_rules.txt new file mode 100644 index 00000000..a59dd37d --- /dev/null +++ b/cmd/pint/tests/0202_report_recording_rules.txt @@ -0,0 +1,33 @@ +! exec pint --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/1.yml:3-4 Bug: You cannot add any recording rules to this Prometheus server. (rule/report) + 3 | - record: bar + 4 | expr: sum(up) + +level=INFO msg="Problems found" Bug=1 +level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher" +-- rules/1.yml -- +- alert: foo + expr: up == 0 +- record: bar + expr: sum(up) + +-- .pint.hcl -- +parser { + relaxed = [".*"] +} +rule { + match { + kind = "recording" + } + report { + comment = "You cannot add any recording rules to this Prometheus server." + severity = "bug" + } +} + diff --git a/docs/changelog.md b/docs/changelog.md index 112eacaf..305a1aae 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,10 @@ ## v0.68.0 +### Added + +- Added [rule/report](checks/rule/report.md) check. + ### Changed - pint now uses [Prometheus 3.0](https://prometheus.io/blog/2024/11/14/prometheus-3-0/) libraries diff --git a/docs/checks/rule/report.md b/docs/checks/rule/report.md new file mode 100644 index 00000000..1d527fa7 --- /dev/null +++ b/docs/checks/rule/report.md @@ -0,0 +1,81 @@ +--- +layout: default +parent: Checks +grand_parent: Documentation +--- + +# rule/report + +This check will always report a problem for **every matching rule**. + +## Configuration + +Syntax: + +```js +report { + comment = "..." + severity = "bug|warning|info" +} +``` + +- `comment` - set a custom comment that will be added to reported problems. +- `severity` - set custom severity for reported issues. + +## 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 that matches some rules and +then add a `report` block there. + +Example where we block all recording rule: + +```js +rule { + match { + kind = "recording" + } + + report { + comment = "You cannot add any recording rules to this Prometheus server." + severity = "bug" + } +} +``` + +## How to disable it + +You can disable this check globally by adding this config block: + +```js +checks { + disabled = ["rule/report"] +} +``` + +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/report +``` + +Or you can disable it per rule by adding a comment to it. Example: + +```yaml +# pint disable rule/report +``` + +## How to snooze it + +You can disable this check until given time by adding a comment to it. Example: + +```yaml +# pint snooze $TIMESTAMP rule/report +``` + +Where `$TIMESTAMP` is either use [RFC3339](https://www.rfc-editor.org/rfc/rfc3339) +formatted or `YYYY-MM-DD`. +Adding this comment will disable `rule/report` _until_ `$TIMESTAMP`, after that +check will be re-enabled. diff --git a/internal/checks/base.go b/internal/checks/base.go index 8ee88042..d6c26ccd 100644 --- a/internal/checks/base.go +++ b/internal/checks/base.go @@ -37,6 +37,7 @@ var ( LabelCheckName, RuleLinkCheckName, RejectCheckName, + ReportCheckName, } OnlineChecks = []string{ AlertsAbsentCheckName, diff --git a/internal/checks/rule_report.go b/internal/checks/rule_report.go new file mode 100644 index 00000000..dae930b9 --- /dev/null +++ b/internal/checks/rule_report.go @@ -0,0 +1,51 @@ +package checks + +import ( + "context" + + "github.com/cloudflare/pint/internal/discovery" + "github.com/cloudflare/pint/internal/parser" +) + +const ( + ReportCheckName = "rule/report" +) + +func NewReportCheck(c string, s Severity) ReportCheck { + return ReportCheck{comment: c, severity: s} +} + +type ReportCheck struct { + comment string + severity Severity +} + +func (c ReportCheck) Meta() CheckMeta { + return CheckMeta{ + States: []discovery.ChangeType{ + discovery.Noop, + discovery.Added, + discovery.Modified, + discovery.Moved, + }, + IsOnline: false, + } +} + +func (c ReportCheck) String() string { + return ReportCheckName +} + +func (c ReportCheck) Reporter() string { + return ReportCheckName +} + +func (c ReportCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { + problems = append(problems, Problem{ + Lines: rule.Lines, + Reporter: c.Reporter(), + Text: c.comment, + Severity: c.severity, + }) + return problems +} diff --git a/internal/checks/rule_report_test.go b/internal/checks/rule_report_test.go new file mode 100644 index 00000000..d4a53636 --- /dev/null +++ b/internal/checks/rule_report_test.go @@ -0,0 +1,57 @@ +package checks_test + +import ( + "testing" + + "github.com/cloudflare/pint/internal/checks" + "github.com/cloudflare/pint/internal/parser" + "github.com/cloudflare/pint/internal/promapi" +) + +func TestReportCheck(t *testing.T) { + testCases := []checkTest{ + { + description: "report passed problem / warning", + content: "- alert: foo\n expr: sum(foo)\n annotations:\n alert: foo\n", + checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { + return checks.NewReportCheck("problem reported", checks.Warning) + }, + prometheus: noProm, + problems: func(_ string) []checks.Problem { + return []checks.Problem{ + { + Lines: parser.LineRange{ + First: 1, + Last: 4, + }, + Reporter: "rule/report", + Text: "problem reported", + Severity: checks.Warning, + }, + } + }, + }, + { + description: "report passed problem / info", + content: "- record: foo\n expr: sum(foo)\n", + checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { + return checks.NewReportCheck("problem reported", checks.Information) + }, + prometheus: noProm, + problems: func(_ string) []checks.Problem { + return []checks.Problem{ + { + Lines: parser.LineRange{ + First: 1, + Last: 2, + }, + Reporter: "rule/report", + Text: "problem reported", + Severity: checks.Information, + }, + } + }, + }, + } + runTests(t, testCases) +} diff --git a/internal/config/__snapshots__/config_test.snap b/internal/config/__snapshots__/config_test.snap index cafacae9..1109f4a7 100755 --- a/internal/config/__snapshots__/config_test.snap +++ b/internal/config/__snapshots__/config_test.snap @@ -32,7 +32,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {} @@ -72,7 +73,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -123,7 +125,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -178,7 +181,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ], "disabled": [ "alerts/template", @@ -242,7 +246,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -296,7 +301,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -353,7 +359,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -407,7 +414,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -461,7 +469,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -527,7 +536,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -570,7 +580,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -631,7 +642,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -693,7 +705,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -742,7 +755,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -813,7 +827,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -911,7 +926,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -997,7 +1013,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1059,7 +1076,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1120,7 +1138,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1181,7 +1200,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1242,7 +1262,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1303,7 +1324,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1364,7 +1386,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1425,7 +1448,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1488,7 +1512,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ], "disabled": [ "promql/counter", @@ -1725,7 +1750,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1780,7 +1806,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1836,7 +1863,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1891,7 +1919,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -1946,7 +1975,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2001,7 +2031,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2056,7 +2087,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2111,7 +2143,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2167,7 +2200,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2222,7 +2256,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2278,7 +2313,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2327,7 +2363,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ], "disabled": [ "alerts/template", @@ -2392,7 +2429,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ], "disabled": [ "alerts/template", @@ -2456,7 +2494,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ], "disabled": [ "alerts/template", @@ -2520,7 +2559,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2597,7 +2637,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2674,7 +2715,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2734,7 +2776,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2797,7 +2840,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2845,7 +2889,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -2924,7 +2969,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -3003,7 +3049,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ], "disabled": [ "alerts/template", @@ -3084,7 +3131,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ] }, "owners": {}, @@ -3136,7 +3184,8 @@ "rule/name", "rule/label", "rule/link", - "rule/reject" + "rule/reject", + "rule/report" ], "disabled": [ "alerts/template", @@ -3182,3 +3231,57 @@ ] } --- + +[TestGetChecksForRule/reject_rules#01 - 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": [ + { + "match": [ + { + "kind": "recording" + } + ], + "report": { + "comment": "You cannot add any recording rules to this Prometheus server.", + "severity": "bug" + } + } + ] +} +--- diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1a989741..3c232545 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2161,6 +2161,37 @@ rule { checks.AlertsExternalLabelsCheckName + "(prom2)", }, }, + { + title: "reject rules", + config: ` +rule { + match { + kind = "recording" + } + report { + comment = "You cannot add any recording rules to this Prometheus server." + severity = "bug" + } +} +`, + entry: discovery.Entry{ + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + }, + checks: []string{ + checks.SyntaxCheckName, + checks.AlertForCheckName, + checks.ComparisonCheckName, + checks.TemplateCheckName, + checks.FragileCheckName, + checks.RegexpCheckName, + checks.ReportCheckName, + }, + }, } dir := t.TempDir() diff --git a/internal/config/parsed_rule.go b/internal/config/parsed_rule.go index 2c0aeb93..79bb1ee4 100644 --- a/internal/config/parsed_rule.go +++ b/internal/config/parsed_rule.go @@ -409,5 +409,14 @@ func parseRule(rule Rule, prometheusServers []*promapi.FailoverGroup, defaultSta }) } + if rule.Report != nil { + rules = append(rules, parsedRule{ + match: defaultRuleMatch(rule.Match, defaultStates), + ignore: rule.Ignore, + name: checks.CostCheckName, + check: checks.NewReportCheck(rule.Report.Comment, rule.Report.getSeverity()), + }) + } + return rules } diff --git a/internal/config/report.go b/internal/config/report.go new file mode 100644 index 00000000..39dcffd5 --- /dev/null +++ b/internal/config/report.go @@ -0,0 +1,31 @@ +package config + +import ( + "errors" + + "github.com/cloudflare/pint/internal/checks" +) + +type ReportSettings struct { + Comment string `hcl:"comment" json:"comment"` + Severity string `hcl:"severity" json:"severity"` +} + +func (rs ReportSettings) validate() error { + if rs.Comment == "" { + return errors.New("report comment cannot be empty") + } + + if rs.Severity != "" { + if _, err := checks.ParseSeverity(rs.Severity); err != nil { + return err + } + } + + return nil +} + +func (rs ReportSettings) getSeverity() checks.Severity { + sev, _ := checks.ParseSeverity(rs.Severity) + return sev +} diff --git a/internal/config/rule.go b/internal/config/rule.go index 9b8699e2..ad8cee17 100644 --- a/internal/config/rule.go +++ b/internal/config/rule.go @@ -27,6 +27,7 @@ type Rule struct { Reject []RejectSettings `hcl:"reject,block" json:"reject,omitempty"` RuleLink []RuleLinkSettings `hcl:"link,block" json:"link,omitempty"` RuleName []RuleNameSettings `hcl:"name,block" json:"name,omitempty"` + Report *ReportSettings `hcl:"report,block" json:"report,omitempty"` } func (rule Rule) validate() (err error) { @@ -120,6 +121,12 @@ func (rule Rule) validate() (err error) { } } + if rule.Report != nil { + if err = rule.Report.validate(); err != nil { + return err + } + } + return nil }