From 97107d450e2b53eb86af498b79530b8a1fe6fa8e Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Fri, 11 Nov 2022 10:28:29 +0000 Subject: [PATCH] Add support for file/disable comments --- cmd/pint/scan.go | 2 +- cmd/pint/tests/0103_file_disable.txt | 34 +++ cmd/pint/tests/0104_file_ignore_prom.txt | 35 +++ docs/changelog.md | 2 + docs/checks/alerts/annotation.md | 7 +- docs/checks/alerts/comparison.md | 7 +- docs/checks/alerts/count.md | 7 +- docs/checks/alerts/for.md | 7 +- docs/checks/alerts/template.md | 7 +- docs/checks/ignore/file.md | 2 +- docs/checks/promql/aggregate.md | 7 +- docs/checks/promql/fragile.md | 7 +- docs/checks/promql/range_query.md | 7 +- docs/checks/promql/rate.md | 8 +- docs/checks/promql/regexp.md | 8 +- docs/checks/promql/series.md | 7 +- docs/checks/promql/syntax.md | 7 +- docs/checks/promql/vector_matching.md | 7 +- docs/checks/query/cost.md | 7 +- docs/checks/rule/label.md | 7 +- docs/checks/rule/link.md | 7 +- docs/checks/rule/reject.md | 7 +- docs/ignoring.md | 7 + .../config/__snapshots__/config_test.snap | 270 ++++++++++++++++++ internal/config/config.go | 7 +- internal/config/config_test.go | 46 ++- internal/discovery/discovery.go | 38 ++- internal/discovery/symlinks.go | 11 +- internal/parser/models.go | 4 +- internal/parser/read.go | 22 +- internal/parser/read_test.go | 16 +- 31 files changed, 563 insertions(+), 54 deletions(-) create mode 100644 cmd/pint/tests/0103_file_disable.txt create mode 100644 cmd/pint/tests/0104_file_ignore_prom.txt diff --git a/cmd/pint/scan.go b/cmd/pint/scan.go index d3c6159c..abe8c71f 100644 --- a/cmd/pint/scan.go +++ b/cmd/pint/scan.go @@ -108,7 +108,7 @@ func checkRules(ctx context.Context, workers int, cfg config.Config, entries []d Msg("Found alerting rule") } - checkList := cfg.GetChecksForRule(ctx, entry.SourcePath, entry.Rule) + checkList := cfg.GetChecksForRule(ctx, entry.SourcePath, entry.Rule, entry.DisabledChecks) for _, check := range checkList { checkIterationChecks.Inc() check := check diff --git a/cmd/pint/tests/0103_file_disable.txt b/cmd/pint/tests/0103_file_disable.txt new file mode 100644 index 00000000..617b8da2 --- /dev/null +++ b/cmd/pint/tests/0103_file_disable.txt @@ -0,0 +1,34 @@ +pint.ok -l debug --no-color lint rules +! stdout . +cmp stderr stderr.txt + +-- stderr.txt -- +level=info msg="Loading configuration file" path=.pint.hcl +level=debug msg="File parsed" path=rules/0001.yml rules=1 +level=debug msg="Starting query workers" name=prom uri=http://127.0.0.1:7103 workers=16 +level=debug msg="Found recording rule" lines=9-10 path=rules/0001.yml record=colo:test1 +level=debug msg="Configured checks for rule" enabled=["promql/syntax","alerts/for","alerts/comparison","alerts/template","promql/fragile","promql/regexp","promql/vector_matching(prom)"] path=rules/0001.yml rule=colo:test1 +level=debug msg="Stopping query workers" name=prom uri=http://127.0.0.1:7103 +-- rules/0001.yml -- +# This should skip all online checks +# pint file/disable promql/series +# pint file/disable promql/rate +# +# pint file/disable alerts/count +# pint file/disable promql/range_query +# + +- record: "colo:test1" + expr: sum(foo) without(job) + + +-- .pint.hcl -- +prometheus "prom" { + uri = "http://127.0.0.1:7103" + failover = [] + timeout = "5s" + required = true +} +parser { + relaxed = [".*"] +} diff --git a/cmd/pint/tests/0104_file_ignore_prom.txt b/cmd/pint/tests/0104_file_ignore_prom.txt new file mode 100644 index 00000000..8a9341a0 --- /dev/null +++ b/cmd/pint/tests/0104_file_ignore_prom.txt @@ -0,0 +1,35 @@ +http response prometheus / 502 Bad Gateway +http start prometheus 127.0.0.1:7104 + +pint.error --no-color lint rules +! stdout . +stderr 'level=error msg="Query returned an error" error="server error: 502" query=/api/v1/status/config uri=http://127.0.0.1:7104' +stderr 'level=error msg="Query returned an error" error="server error: 502" query=/api/v1/status/flags uri=http://127.0.0.1:7104' +stderr 'level=error msg="Query returned an error" error="server error: 502" query=count\(foo\) uri=http://127.0.0.1:7104' +stderr 'level=info msg="Problems found" Bug=3' +-- rules/0001.yml -- +# This should skip all online checks +# pint file/disable promql/series +# pint file/disable promql/rate +# +# pint file/disable alerts/count +# pint file/disable promql/range_query +# + +- record: "colo:test1" + expr: sum(foo) without(job) + +-- rules/0002.yml -- +- record: "colo:test2" + expr: sum(foo) without(job) + +-- .pint.hcl -- +prometheus "prom" { + uri = "http://127.0.0.1:7104" + failover = [] + timeout = "5s" + required = true +} +parser { + relaxed = [".*"] +} diff --git a/docs/changelog.md b/docs/changelog.md index ae867390..beb60ad5 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -6,6 +6,8 @@ - Added `pint_prometheus_cache_evictions_total` metric tracking the number of times cache results were evicted from query cache. +- Allow disabling individual checks for the entire file using + `# pint file/disable ...` comments. ### Changed diff --git a/docs/checks/alerts/annotation.md b/docs/checks/alerts/annotation.md index d4a1ed26..d79706ce 100644 --- a/docs/checks/alerts/annotation.md +++ b/docs/checks/alerts/annotation.md @@ -85,7 +85,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable alerts/annotation` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable alerts/annotation` diff --git a/docs/checks/alerts/comparison.md b/docs/checks/alerts/comparison.md index 7c59b038..204db636 100644 --- a/docs/checks/alerts/comparison.md +++ b/docs/checks/alerts/comparison.md @@ -41,6 +41,11 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable alerts/comparison` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable alerts/comparison` diff --git a/docs/checks/alerts/count.md b/docs/checks/alerts/count.md index 63825091..9a38bc38 100644 --- a/docs/checks/alerts/count.md +++ b/docs/checks/alerts/count.md @@ -65,7 +65,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable alerts/count` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable alerts/count` diff --git a/docs/checks/alerts/for.md b/docs/checks/alerts/for.md index 4386215a..8885322d 100644 --- a/docs/checks/alerts/for.md +++ b/docs/checks/alerts/for.md @@ -27,6 +27,11 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable alerts/for` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable alerts/for` diff --git a/docs/checks/alerts/template.md b/docs/checks/alerts/template.md index ad3983fa..0a431c0a 100644 --- a/docs/checks/alerts/template.md +++ b/docs/checks/alerts/template.md @@ -44,6 +44,11 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable alerts/template` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable alerts/template` diff --git a/docs/checks/ignore/file.md b/docs/checks/ignore/file.md index 69f49f39..cb26555a 100644 --- a/docs/checks/ignore/file.md +++ b/docs/checks/ignore/file.md @@ -7,7 +7,7 @@ grand_parent: Documentation # ignore/file You will see this check reports for files that are excludes from pint using a -`file/ignore` comment. +`ignore/file` comment. See this [page](../../ignoring.md) for details on how such comments work. Those reports are informational to make it obvious that pint didn't run any diff --git a/docs/checks/promql/aggregate.md b/docs/checks/promql/aggregate.md index 67a1b659..37eff3d0 100644 --- a/docs/checks/promql/aggregate.md +++ b/docs/checks/promql/aggregate.md @@ -86,7 +86,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it: +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable promql/aggregate` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable promql/aggregate` diff --git a/docs/checks/promql/fragile.md b/docs/checks/promql/fragile.md index 340b5382..9a07078f 100644 --- a/docs/checks/promql/fragile.md +++ b/docs/checks/promql/fragile.md @@ -113,6 +113,11 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable promql/fragile` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable promql/fragile` diff --git a/docs/checks/promql/range_query.md b/docs/checks/promql/range_query.md index c826dcee..dba19220 100644 --- a/docs/checks/promql/range_query.md +++ b/docs/checks/promql/range_query.md @@ -73,7 +73,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it: +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable promql/range_query` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable promql/range_query` diff --git a/docs/checks/promql/rate.md b/docs/checks/promql/rate.md index b0df9e39..7c7480e1 100644 --- a/docs/checks/promql/rate.md +++ b/docs/checks/promql/rate.md @@ -82,7 +82,13 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it: + +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable promql/rate` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable promql/rate` diff --git a/docs/checks/promql/regexp.md b/docs/checks/promql/regexp.md index 57d5d624..0b4c7cea 100644 --- a/docs/checks/promql/regexp.md +++ b/docs/checks/promql/regexp.md @@ -52,6 +52,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. + +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable promql/regexp` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable promql/regexp` diff --git a/docs/checks/promql/series.md b/docs/checks/promql/series.md index 29b71fd1..57b91463 100644 --- a/docs/checks/promql/series.md +++ b/docs/checks/promql/series.md @@ -253,7 +253,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it: +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable promql/series` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable promql/series` diff --git a/docs/checks/promql/syntax.md b/docs/checks/promql/syntax.md index 713a2f4a..9d1d0329 100644 --- a/docs/checks/promql/syntax.md +++ b/docs/checks/promql/syntax.md @@ -27,6 +27,11 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable promql/syntax` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable promql/syntax` diff --git a/docs/checks/promql/vector_matching.md b/docs/checks/promql/vector_matching.md index 1f2ed020..19d2a160 100644 --- a/docs/checks/promql/vector_matching.md +++ b/docs/checks/promql/vector_matching.md @@ -86,7 +86,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it: +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable promql/vector_matching` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable promql/vector_matching` diff --git a/docs/checks/query/cost.md b/docs/checks/query/cost.md index ab67387b..eb4210ec 100644 --- a/docs/checks/query/cost.md +++ b/docs/checks/query/cost.md @@ -78,7 +78,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it: +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable query/cost` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable query/cost` diff --git a/docs/checks/rule/label.md b/docs/checks/rule/label.md index 40b187ef..10e3e550 100644 --- a/docs/checks/rule/label.md +++ b/docs/checks/rule/label.md @@ -83,7 +83,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it: +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable rule/label` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable rule/label` diff --git a/docs/checks/rule/link.md b/docs/checks/rule/link.md index 6da26800..d5456f8c 100644 --- a/docs/checks/rule/link.md +++ b/docs/checks/rule/link.md @@ -98,7 +98,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable rule/link` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable rule/link` diff --git a/docs/checks/rule/reject.md b/docs/checks/rule/reject.md index ffa64b01..ec0c24d0 100644 --- a/docs/checks/rule/reject.md +++ b/docs/checks/rule/reject.md @@ -96,7 +96,12 @@ checks { } ``` -Or you can disable it per rule by adding a comment to it. +You can also disable it for all rules inside given file by adding +a comment anywhere in that file. Example: + +`# pint file/disable rule/reject` + +Or you can disable it per rule by adding a comment to it. Example: `# pint disable rule/reject` diff --git a/docs/ignoring.md b/docs/ignoring.md index ddd47af7..f30fab06 100644 --- a/docs/ignoring.md +++ b/docs/ignoring.md @@ -91,6 +91,13 @@ checks { } ``` +## Disabling invididual checks for specific files + +To disable individual check for a specific rule use `# pint file/disable ...` comments +anywhere in the file. This will disable given check for all rules in that file. + +See each individual [check](checks/index.md) documentation for details. + ## Disabling individual checks for specific rules To disable individual check for a specific rule use `# pint disable ...` comments. diff --git a/internal/config/__snapshots__/config_test.snap b/internal/config/__snapshots__/config_test.snap index c5ed7e43..3bb77193 100755 --- a/internal/config/__snapshots__/config_test.snap +++ b/internal/config/__snapshots__/config_test.snap @@ -8803,3 +8803,273 @@ ] } --- + +[TestGetChecksForRule/two_prometheus_servers_/_disable_checks_via_file/disable_comment - 1] +{ + "ci": { + "maxCommits": 20, + "baseBranch": "master" + }, + "parser": {}, + "prometheus": [ + { + "name": "prom1", + "uri": "http://localhost/1", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + }, + { + "name": "prom2", + "uri": "http://localhost/2", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + } + ], + "checks": { + "enabled": [ + "alerts/annotation", + "alerts/count", + "alerts/for", + "alerts/template", + "promql/aggregate", + "alerts/comparison", + "promql/fragile", + "promql/range_query", + "promql/rate", + "promql/regexp", + "promql/syntax", + "promql/vector_matching", + "query/cost", + "promql/series", + "rule/label", + "rule/link", + "rule/reject" + ], + "disabled": [ + "alerts/template" + ] + } +} +--- + +[TestGetChecksForRule/two_prometheus_servers_/_disable_checks_via_file/disable_comment - 2] +{ + "ci": { + "maxCommits": 20, + "baseBranch": "master" + }, + "parser": {}, + "prometheus": [ + { + "name": "prom1", + "uri": "http://localhost/1", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + }, + { + "name": "prom2", + "uri": "http://localhost/2", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + } + ], + "checks": { + "enabled": [ + "alerts/annotation", + "alerts/count", + "alerts/for", + "alerts/template", + "promql/aggregate", + "alerts/comparison", + "promql/fragile", + "promql/range_query", + "promql/rate", + "promql/regexp", + "promql/syntax", + "promql/vector_matching", + "query/cost", + "promql/series", + "rule/label", + "rule/link", + "rule/reject" + ], + "disabled": [ + "alerts/template" + ] + } +} +--- + +[TestGetChecksForRule/two_prometheus_servers_/_disable_checks_via_file/disable_comment - 3] +{ + "ci": { + "maxCommits": 20, + "baseBranch": "master" + }, + "parser": {}, + "prometheus": [ + { + "name": "prom1", + "uri": "http://localhost/1", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + }, + { + "name": "prom2", + "uri": "http://localhost/2", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + } + ], + "checks": { + "enabled": [ + "alerts/annotation", + "alerts/count", + "alerts/for", + "alerts/template", + "promql/aggregate", + "alerts/comparison", + "promql/fragile", + "promql/range_query", + "promql/rate", + "promql/regexp", + "promql/syntax", + "promql/vector_matching", + "query/cost", + "promql/series", + "rule/label", + "rule/link", + "rule/reject" + ], + "disabled": [ + "alerts/template" + ] + } +} +--- + +[TestGetChecksForRule/two_prometheus_servers_/_disable_checks_via_file/disable_comment - 4] +{ + "ci": { + "maxCommits": 20, + "baseBranch": "master" + }, + "parser": {}, + "prometheus": [ + { + "name": "prom1", + "uri": "http://localhost/1", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + }, + { + "name": "prom2", + "uri": "http://localhost/2", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + } + ], + "checks": { + "enabled": [ + "alerts/annotation", + "alerts/count", + "alerts/for", + "alerts/template", + "promql/aggregate", + "alerts/comparison", + "promql/fragile", + "promql/range_query", + "promql/rate", + "promql/regexp", + "promql/syntax", + "promql/vector_matching", + "query/cost", + "promql/series", + "rule/label", + "rule/link", + "rule/reject" + ], + "disabled": [ + "alerts/template" + ] + } +} +--- + +[TestGetChecksForRule/two_prometheus_servers_/_disable_checks_via_file/disable_comment - 5] +{ + "ci": { + "maxCommits": 20, + "baseBranch": "master" + }, + "parser": {}, + "prometheus": [ + { + "name": "prom1", + "uri": "http://localhost/1", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + }, + { + "name": "prom2", + "uri": "http://localhost/2", + "timeout": "1s", + "concurrency": 16, + "rateLimit": 100, + "cache": 10000, + "required": false + } + ], + "checks": { + "enabled": [ + "alerts/annotation", + "alerts/count", + "alerts/for", + "alerts/template", + "promql/aggregate", + "alerts/comparison", + "promql/fragile", + "promql/range_query", + "promql/rate", + "promql/regexp", + "promql/syntax", + "promql/vector_matching", + "query/cost", + "promql/series", + "rule/label", + "rule/link", + "rule/reject" + ], + "disabled": [ + "alerts/template" + ] + } +} +--- diff --git a/internal/config/config.go b/internal/config/config.go index 11f1e8e6..f69a1b63 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -72,7 +72,7 @@ func (cfg Config) String() string { return string(content) } -func (cfg *Config) GetChecksForRule(ctx context.Context, path string, r parser.Rule) []checks.RuleChecker { +func (cfg *Config) GetChecksForRule(ctx context.Context, path string, r parser.Rule, disabledChecks []string) []checks.RuleChecker { enabled := []checks.RuleChecker{} allChecks := []checkMeta{ @@ -139,6 +139,11 @@ func (cfg *Config) GetChecksForRule(ctx context.Context, path string, r parser.R } for _, cm := range allChecks { + // check if check is disabled for specific rule + if !isEnabled(cfg.Checks.Enabled, disabledChecks, r, cm.name, cm.check) { + continue + } + // check if rule was disabled if !isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, r, cm.name, cm.check) { continue diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 0a7a77cd..1432a160 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -123,11 +123,12 @@ func newRule(t *testing.T, content string) parser.Rule { func TestGetChecksForRule(t *testing.T) { type testCaseT struct { - title string - config string - path string - rule parser.Rule - checks []string + title string + config string + path string + rule parser.Rule + checks []string + disabledChecks []string } testCases := []testCaseT{ @@ -1140,6 +1141,39 @@ rule { checks.RuleLinkCheckName + "(^https?://(.+)$)", }, }, + { + title: "two prometheus servers / disable checks via file/disable comment", + config: ` +prometheus "prom1" { + uri = "http://localhost/1" + timeout = "1s" +} +prometheus "prom2" { + uri = "http://localhost/2" + timeout = "1s" +} +checks { + disabled = [ "alerts/template" ] +} +`, + path: "rules.yml", + rule: newRule(t, ` +# Some extra comment +# pint disable promql/series +# Some extra comment +# pint disable promql/range_query +- record: foo + expr: sum(foo) +`), + checks: []string{ + checks.SyntaxCheckName, + checks.AlertForCheckName, + checks.ComparisonCheckName, + checks.FragileCheckName, + checks.RegexpCheckName, + }, + disabledChecks: []string{"promql/rate", "promql/vector_matching"}, + }, } dir := t.TempDir() @@ -1155,7 +1189,7 @@ rule { cfg, err := config.Load(path, false) require.NoError(t, err) - checks := cfg.GetChecksForRule(ctx, tc.path, tc.rule) + checks := cfg.GetChecksForRule(ctx, tc.path, tc.rule, tc.disabledChecks) checkNames := make([]string, 0, len(checks)) for _, c := range checks { checkNames = append(checkNames, c.String()) diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go index 72b6c974..56a7eb60 100644 --- a/internal/discovery/discovery.go +++ b/internal/discovery/discovery.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/prometheus/prometheus/model/rulefmt" + "golang.org/x/exp/slices" "github.com/cloudflare/pint/internal/output" "github.com/cloudflare/pint/internal/parser" @@ -15,8 +16,9 @@ import ( ) const ( - FileOwnerComment = "file/owner" - RuleOwnerComment = "rule/owner" + FileOwnerComment = "file/owner" + FileDisabledCheckComment = "file/disable" + RuleOwnerComment = "rule/owner" ) var ignoredErrors = []string{ @@ -51,12 +53,13 @@ type RuleFinder interface { } type Entry struct { - ReportedPath string - SourcePath string - PathError error - ModifiedLines []int - Rule parser.Rule - Owner string + ReportedPath string + SourcePath string + PathError error + ModifiedLines []int + Rule parser.Rule + Owner string + DisabledChecks []string } func readFile(path string, isStrict bool) (entries []Entry, err error) { @@ -78,7 +81,15 @@ func readFile(path string, isStrict bool) (entries []Entry, err error) { contentLines = append(contentLines, i) } - fileOwner, _ := parser.GetComment(string(content.Body), FileOwnerComment) + body := string(content.Body) + fileOwner, _ := parser.GetLastComment(body, FileOwnerComment) + + var disabledChecks []string + for _, comment := range parser.GetComments(body, FileDisabledCheckComment) { + if !slices.Contains(disabledChecks, comment.Value) { + disabledChecks = append(disabledChecks, comment.Value) + } + } if content.Ignored { entries = append(entries, Entry{ @@ -139,10 +150,11 @@ func readFile(path string, isStrict bool) (entries []Entry, err error) { owner = fileOwner } entries = append(entries, Entry{ - ReportedPath: path, - SourcePath: path, - Rule: rule, - Owner: owner.Value, + ReportedPath: path, + SourcePath: path, + Rule: rule, + Owner: owner.Value, + DisabledChecks: disabledChecks, }) } diff --git a/internal/discovery/symlinks.go b/internal/discovery/symlinks.go index 55bf48de..f2cfbfab 100644 --- a/internal/discovery/symlinks.go +++ b/internal/discovery/symlinks.go @@ -67,11 +67,12 @@ func addSymlinkedEntries(entries []Entry) ([]Entry, error) { if sl.to == entry.SourcePath { log.Debug().Str("to", sl.to).Str("from", sl.from).Msg("Found a symlink") nentries = append(nentries, Entry{ - ReportedPath: sl.to, - SourcePath: sl.from, - ModifiedLines: entry.ModifiedLines, - Rule: entry.Rule, - Owner: entry.Owner, + ReportedPath: sl.to, + SourcePath: sl.from, + ModifiedLines: entry.ModifiedLines, + Rule: entry.Rule, + Owner: entry.Owner, + DisabledChecks: entry.DisabledChecks, }) } } diff --git a/internal/parser/models.go b/internal/parser/models.go index 8bd41b3e..74d684cb 100644 --- a/internal/parser/models.go +++ b/internal/parser/models.go @@ -396,7 +396,7 @@ func (r Rule) GetComment(comment ...string) (s Comment, ok bool) { comments = r.AlertingRule.Comments() } for _, c := range comments { - if val, ok := GetComment(c, comment...); ok { + if val, ok := GetLastComment(c, comment...); ok { return val, ok } } @@ -413,7 +413,7 @@ func (r Rule) GetComments(key string) (cs []Comment) { for _, c := range comments { sc := bufio.NewScanner(strings.NewReader(c)) for sc.Scan() { - if val, ok := GetComment(sc.Text(), key); ok { + if val, ok := GetLastComment(sc.Text(), key); ok { cs = append(cs, val) } } diff --git a/internal/parser/read.go b/internal/parser/read.go index 72eef8ec..0bf9dd1b 100644 --- a/internal/parser/read.go +++ b/internal/parser/read.go @@ -160,8 +160,8 @@ func (c Comment) String() string { return c.Key + " " + c.Value } -func GetComment(line string, comment ...string) (s Comment, ok bool) { - sc := bufio.NewScanner(strings.NewReader(line)) +func GetComments(text string, comment ...string) (comments []Comment) { + sc := bufio.NewScanner(strings.NewReader(text)) for sc.Scan() { elems := strings.Split(sc.Text(), "#") lastComment := elems[len(elems)-1] @@ -181,11 +181,21 @@ func GetComment(line string, comment ...string) (s Comment, ok bool) { for i := len(comment) + 1; i < len(parts); i++ { values = append(values, parts[i]) } - ok = true - s.Key = strings.Join(keys, " ") - s.Value = strings.Join(values, " ") + comments = append(comments, Comment{ + Key: strings.Join(keys, " "), + Value: strings.Join(values, " "), + }) } NEXT: } - return + + return comments +} + +func GetLastComment(text string, comment ...string) (Comment, bool) { + comments := GetComments(text, comment...) + if len(comments) == 0 { + return Comment{}, false + } + return comments[len(comments)-1], true } diff --git a/internal/parser/read_test.go b/internal/parser/read_test.go index 54cfe53e..9457e18a 100644 --- a/internal/parser/read_test.go +++ b/internal/parser/read_test.go @@ -161,7 +161,7 @@ func TestReadContent(t *testing.T) { } } -func TestGetComment(t *testing.T) { +func TestGetLastComment(t *testing.T) { type testCaseT struct { input string comment []string @@ -242,6 +242,18 @@ func TestGetComment(t *testing.T) { ok: true, output: parser.Comment{Key: "set promql/series min-age", Value: "1w"}, }, + { + input: "# pint set promql/series min-age 1d\n# pint set promql/series min-age 1w\n", + comment: []string{"set", "promql/series", "min-age"}, + ok: true, + output: parser.Comment{Key: "set promql/series min-age", Value: "1w"}, + }, + { + input: "# pint set promql/series min-age 2d\n# pint set promql/series min-age 1w\n# pint set promql/series min-age 1s\n", + comment: []string{"set", "promql/series", "min-age"}, + ok: true, + output: parser.Comment{Key: "set promql/series min-age", Value: "1s"}, + }, { input: "# pint set promql/series min-age 1w ", comment: []string{"set", "promql/series", "min-age"}, @@ -262,7 +274,7 @@ func TestGetComment(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d/%s", i, tc.input), func(t *testing.T) { - output, ok := parser.GetComment(tc.input, tc.comment...) + output, ok := parser.GetLastComment(tc.input, tc.comment...) require.Equal(t, tc.ok, ok) require.Equal(t, tc.output, output) })