diff --git a/cmd/pint/scan.go b/cmd/pint/scan.go index c56b3350..031157ea 100644 --- a/cmd/pint/scan.go +++ b/cmd/pint/scan.go @@ -23,6 +23,9 @@ const ( yamlParseReporter = "yaml/parse" ignoreFileReporter = "ignore/file" pintCommentReporter = "pint/comment" + + yamlDetails = `This Prometheus rule is not valid. +This usually means that it's missing some required fields.` ) func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.PrometheusGenerator, cfg config.Config, entries []discovery.Entry) (summary reporter.Summary, err error) { @@ -153,27 +156,7 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte default: var commentErr comments.CommentError var ignoreErr discovery.FileIgnoreError - var strictErr parser.StrictError switch { - case errors.As(job.entry.PathError, &strictErr): - results <- reporter.Report{ - Path: discovery.Path{ - Name: job.entry.Path.Name, - SymlinkTarget: job.entry.Path.SymlinkTarget, - }, - ModifiedLines: job.entry.ModifiedLines, - Problem: checks.Problem{ - Lines: parser.LineRange{ - First: strictErr.Line, - Last: strictErr.Line, - }, - Reporter: yamlParseReporter, - Text: strictErr.Err.Error(), - Details: strictErr.Details, - Severity: checks.Fatal, - }, - Owner: job.entry.Owner, - } case errors.As(job.entry.PathError, &ignoreErr): results <- reporter.Report{ Path: discovery.Path{ @@ -233,6 +216,10 @@ If this file is a template that will be rendered into valid YAML then you can in Owner: job.entry.Owner, } case job.entry.Rule.Error.Err != nil: + details := yamlDetails + if job.entry.Rule.Error.Details != "" { + details = job.entry.Rule.Error.Details + } results <- reporter.Report{ Path: discovery.Path{ Name: job.entry.Path.Name, @@ -247,8 +234,7 @@ If this file is a template that will be rendered into valid YAML then you can in }, Reporter: yamlParseReporter, Text: fmt.Sprintf("This rule is not a valid Prometheus rule: `%s`.", job.entry.Rule.Error.Err.Error()), - Details: `This Prometheus rule is not valid. -This usually means that it's missing some required fields.`, + Details: details, Severity: checks.Fatal, }, Owner: job.entry.Owner, diff --git a/cmd/pint/tests/0004_fail_invalid_yaml.txt b/cmd/pint/tests/0004_fail_invalid_yaml.txt index 28b40874..5bda1e67 100644 --- a/cmd/pint/tests/0004_fail_invalid_yaml.txt +++ b/cmd/pint/tests/0004_fail_invalid_yaml.txt @@ -5,8 +5,7 @@ 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"] -level=WARN msg="Failed to parse file content" err="YAML syntax error at line 4: did not find expected key" path=rules/bad.yaml lines=1-7 -rules/bad.yaml:4 Fatal: did not find expected key (yaml/parse) +rules/bad.yaml:4 Fatal: This rule is not a valid Prometheus rule: `did not find expected key`. (yaml/parse) 4 | rules/ok.yml:5 Fatal: Prometheus failed to parse the query with this PromQL error: unclosed left bracket. (promql/syntax) diff --git a/cmd/pint/tests/0010_syntax_check.txt b/cmd/pint/tests/0010_syntax_check.txt index b13cf69f..830b9532 100644 --- a/cmd/pint/tests/0010_syntax_check.txt +++ b/cmd/pint/tests/0010_syntax_check.txt @@ -5,8 +5,7 @@ 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"] -level=WARN msg="Failed to parse file content" err="YAML syntax error at line 6: did not find expected '-' indicator" path=rules/1.yaml lines=1-12 -rules/1.yaml:6 Fatal: did not find expected '-' indicator (yaml/parse) +rules/1.yaml:6 Fatal: This rule is not a valid Prometheus rule: `did not find expected '-' indicator`. (yaml/parse) 6 | level=INFO msg="Problems found" Fatal=1 diff --git a/cmd/pint/tests/0067_relaxed.txt b/cmd/pint/tests/0067_relaxed.txt index 1a07f073..39e73475 100644 --- a/cmd/pint/tests/0067_relaxed.txt +++ b/cmd/pint/tests/0067_relaxed.txt @@ -5,8 +5,11 @@ 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"] -level=WARN msg="Failed to parse file content" err="YAML syntax error at line 2: YAML list is not allowed here, expected a YAML mapping" path=rules/strict.yml lines=1-4 -rules/strict.yml:2 Fatal: YAML list is not allowed here, expected a YAML mapping (yaml/parse) +rules/strict.yml:2 Fatal: This rule is not a valid Prometheus rule: `This file is not a valid Prometheus rule file. +Errors found: + +- line 2: YAML list is not allowed here, expected a YAML mapping +`. (yaml/parse) 2 | - alert: No Owner level=INFO msg="Problems found" Fatal=1 diff --git a/cmd/pint/tests/0074_strict_error.txt b/cmd/pint/tests/0074_strict_error.txt index f6605a78..b3bf5829 100644 --- a/cmd/pint/tests/0074_strict_error.txt +++ b/cmd/pint/tests/0074_strict_error.txt @@ -4,8 +4,17 @@ cmp stderr stderr.txt -- stderr.txt -- level=INFO msg="Finding all rules to check" paths=["rules"] -level=WARN msg="Failed to parse file content" err="YAML syntax error at line 2: unexpected key `alert`" path=rules/strict.yml lines=1-9 -rules/strict.yml:2 Fatal: unexpected key `alert` (yaml/parse) +rules/strict.yml:2 Fatal: This rule is not a valid Prometheus rule: `This file is not a valid Prometheus rule file. +Errors found: + +- line 2: unexpected key `alert` +- line 3: unexpected key `expr` +- line 4: unexpected key `for` +- line 5: unexpected key `labels` +- line 8: unexpected key `annotations` +- line 2: `name` key is required and must be set +- line 2: `rules` key is required and must be set +`. (yaml/parse) 2 | - alert: Conntrack_Table_Almost_Full level=INFO msg="Problems found" Fatal=1 diff --git a/cmd/pint/tests/0078_repeated_group.txt b/cmd/pint/tests/0078_repeated_group.txt index c9c67b7c..c3508ff9 100644 --- a/cmd/pint/tests/0078_repeated_group.txt +++ b/cmd/pint/tests/0078_repeated_group.txt @@ -4,12 +4,21 @@ cmp stderr stderr.txt -- stderr.txt -- level=INFO msg="Finding all rules to check" paths=["rules"] -level=WARN msg="Failed to parse file content" err="YAML syntax error at line 4: duplicated group name `name`" path=rules/strict.yml lines=1-5 -rules/strict.yml:4 Fatal: duplicated group name `name` (yaml/parse) +rules/strict.yml:1-4 Bug: `rule/owner` comments are required in all files, please add a `# pint file/owner $owner` somewhere in this file and/or `# pint rule/owner $owner` on top of each rule. (rule/owner) + 1 | groups: + 2 | - name: foo + 3 | rules: [] 4 | - name: foo -level=INFO msg="Problems found" Fatal=1 -level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher" +rules/strict.yml:4 Fatal: This rule is not a valid Prometheus rule: `This file is not a valid Prometheus rule file. +Errors found: + +- line 4: duplicated group name `foo` +`. (yaml/parse) + 4 | - name: foo + +level=INFO msg="Problems found" Fatal=1 Bug=1 +level=ERROR msg="Fatal error" err="found 2 problem(s) with severity Bug or higher" -- rules/strict.yml -- groups: - name: foo diff --git a/cmd/pint/tests/0086_rulefmt_ignored_errors.txt b/cmd/pint/tests/0086_rulefmt_ignored_errors.txt index 566677ac..18367549 100644 --- a/cmd/pint/tests/0086_rulefmt_ignored_errors.txt +++ b/cmd/pint/tests/0086_rulefmt_ignored_errors.txt @@ -4,8 +4,11 @@ cmp stderr stderr.txt -- stderr.txt -- level=INFO msg="Finding all rules to check" paths=["rules"] -level=WARN msg="Failed to parse file content" err="YAML syntax error at line 7: expected a YAML string here, got null instead" path=rules/strict.yml lines=1-20 -rules/strict.yml:7 Fatal: expected a YAML string here, got null instead (yaml/parse) +rules/strict.yml:7 Fatal: This rule is not a valid Prometheus rule: `This file is not a valid Prometheus rule file. +Errors found: + +- line 7: expected a YAML string here, got null instead +`. (yaml/parse) 7 | expr: level=INFO msg="Problems found" Fatal=1 diff --git a/cmd/pint/tests/0141_empty_keys.txt b/cmd/pint/tests/0141_empty_keys.txt index 3da69e11..3772cc04 100644 --- a/cmd/pint/tests/0141_empty_keys.txt +++ b/cmd/pint/tests/0141_empty_keys.txt @@ -4,9 +4,19 @@ cmp stderr stderr.txt -- stderr.txt -- level=INFO msg="Finding all rules to check" paths=["rules.yml"] -level=WARN msg="Failed to parse file content" err="YAML syntax error at line 4: expected a YAML string here, got null instead" path=rules.yml lines=1-15 -rules.yml:4 Fatal: expected a YAML string here, got null instead (yaml/parse) - 4 | - record: +rules.yml:15 Fatal: This rule is not a valid Prometheus rule: `This file is not a valid Prometheus rule file. +Errors found: + +- line 4: expected a YAML string here, got null instead +- line 5: expected a YAML string here, got null instead +- line 6: expected a YAML string here, got null instead +- line 9: expected a YAML string here, got null instead +- line 10: expected a YAML string here, got null instead +- line 11: expected a YAML string here, got null instead +- line 12: expected a YAML string here, got null instead +- line 15: expected a YAML string here, got null instead +`. (yaml/parse) + 15 | expr: level=INFO msg="Problems found" Fatal=1 level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher" diff --git a/cmd/pint/tests/0167_rule_duplicate_symlink.txt b/cmd/pint/tests/0167_rule_duplicate_symlink.txt index 1f15f5ee..c992a505 100644 --- a/cmd/pint/tests/0167_rule_duplicate_symlink.txt +++ b/cmd/pint/tests/0167_rule_duplicate_symlink.txt @@ -28,7 +28,6 @@ cmp stderr ../stderrV1.txt -- stderrV1.txt -- level=INFO msg="Loading configuration file" path=.pint.hcl level=INFO msg="Finding all rules to check on current git branch" base=main -level=WARN msg="Failed to parse file content" err="YAML syntax error at line 1: YAML scalar value is not allowed here, expected a YAML mapping" path=.pint.hcl lines=1-20 level=INFO msg="Configured new Prometheus server" name=prom1 uris=1 uptime=up tags=[] include=["^rules.yml$"] exclude=[] level=INFO msg="Configured new Prometheus server" name=prom2 uris=1 uptime=up tags=[] include=["^symlink.yml$"] exclude=[] -- stderrV2.txt -- diff --git a/cmd/pint/tests/0173_rule_duplicate_move.txt b/cmd/pint/tests/0173_rule_duplicate_move.txt index d408fd9d..b2c55999 100644 --- a/cmd/pint/tests/0173_rule_duplicate_move.txt +++ b/cmd/pint/tests/0173_rule_duplicate_move.txt @@ -25,7 +25,6 @@ cmp stderr ../stderr.txt -- stderr.txt -- level=INFO msg="Loading configuration file" path=.pint.hcl level=INFO msg="Finding all rules to check on current git branch" base=main -level=WARN msg="Failed to parse file content" err="YAML syntax error at line 1: YAML scalar value is not allowed here, expected a YAML mapping" path=.pint.hcl lines=1-24 level=INFO msg="Configured new Prometheus server" name=prom1 uris=1 uptime=up tags=[] include=["^rules/alert.*$"] exclude=[] level=INFO msg="Configured new Prometheus server" name=prom2a uris=1 uptime=up tags=[] include=["^rules/record.*$"] exclude=[] level=INFO msg="Configured new Prometheus server" name=prom2b uris=1 uptime=up tags=[] include=["^rules/record.*$"] exclude=[] diff --git a/internal/checks/base_test.go b/internal/checks/base_test.go index 35a761ae..170a6d4a 100644 --- a/internal/checks/base_test.go +++ b/internal/checks/base_test.go @@ -136,8 +136,7 @@ func runTests(t *testing.T, testCases []checkTest) { defer prom.Close(reg) } - entries, err := parseContent(tc.content) - require.NoError(t, err, "cannot parse rule content") + entries := parseContent(tc.content) for _, entry := range entries { ctx := context.Background() if tc.ctx != nil { @@ -154,7 +153,7 @@ func runTests(t *testing.T, testCases []checkTest) { }) // broken rules to test check against rules with syntax error - entries, err := parseContent(` + entries := parseContent(` - alert: foo expr: 'foo{}{} > 0' annotations: @@ -163,7 +162,6 @@ func runTests(t *testing.T, testCases []checkTest) { - record: foo expr: 'foo{}{}' `) - require.NoError(t, err, "cannot parse rule content") t.Run(tc.description+" (bogus rules)", func(_ *testing.T) { for _, entry := range entries { _ = tc.checker(newSimpleProm("prom")).Check(context.Background(), entry.Path, entry.Rule, tc.entries) @@ -172,12 +170,9 @@ func runTests(t *testing.T, testCases []checkTest) { } } -func parseContent(content string) (entries []discovery.Entry, err error) { +func parseContent(content string) (entries []discovery.Entry) { p := parser.NewParser(false) - rules, err := p.Parse([]byte(content)) - if err != nil { - return nil, err - } + rules := p.Parse([]byte(content)) for _, rule := range rules { entries = append(entries, discovery.Entry{ @@ -190,14 +185,6 @@ func parseContent(content string) (entries []discovery.Entry, err error) { }) } - return entries, nil -} - -func mustParseContent(content string) (entries []discovery.Entry) { - entries, err := parseContent(content) - if err != nil { - panic(err) - } return entries } diff --git a/internal/checks/promql_rate_test.go b/internal/checks/promql_rate_test.go index 18a5ef4f..bb55957f 100644 --- a/internal/checks/promql_rate_test.go +++ b/internal/checks/promql_rate_test.go @@ -664,7 +664,7 @@ func TestRateCheck(t *testing.T) { { description: "rate_over_sum", content: "- alert: my alert\n expr: rate(my:sum[5m])\n", - entries: mustParseContent("- record: my:sum\n expr: sum(foo)\n"), + entries: parseContent("- record: my:sum\n expr: sum(foo)\n"), checker: newRateCheck, prometheus: newSimpleProm, problems: func(_ string) []checks.Problem { @@ -706,7 +706,7 @@ func TestRateCheck(t *testing.T) { { description: "rate_over_sum_error", content: "- alert: my alert\n expr: rate(my:sum[5m])\n", - entries: mustParseContent("- record: my:sum\n expr: sum(foo)\n"), + entries: parseContent("- record: my:sum\n expr: sum(foo)\n"), checker: newRateCheck, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -746,7 +746,7 @@ func TestRateCheck(t *testing.T) { { description: "rate_over_sum_on_gauge", content: "- alert: my alert\n expr: rate(my:sum[5m])\n", - entries: mustParseContent("- record: my:sum\n expr: sum(foo)\n"), + entries: parseContent("- record: my:sum\n expr: sum(foo)\n"), checker: newRateCheck, prometheus: newSimpleProm, problems: noProblems, @@ -776,7 +776,7 @@ func TestRateCheck(t *testing.T) { { description: "sum_over_rate", content: "- alert: my alert\n expr: sum(my:rate:5m)\n", - entries: mustParseContent("- record: my:rate:5m\n expr: rate(foo[5m])\n"), + entries: parseContent("- record: my:rate:5m\n expr: rate(foo[5m])\n"), checker: newRateCheck, prometheus: newSimpleProm, problems: noProblems, diff --git a/internal/checks/promql_series_test.go b/internal/checks/promql_series_test.go index 4fa94b77..a65d7569 100644 --- a/internal/checks/promql_series_test.go +++ b/internal/checks/promql_series_test.go @@ -471,7 +471,7 @@ func TestSeriesCheck(t *testing.T) { content: "- record: foo\n expr: sum(foo:bar{job=\"xxx\"})\n", checker: newSeriesCheck, prometheus: newSimpleProm, - entries: mustParseContent("- record: foo:bar\n expr: sum(foo:bar)\n"), + entries: parseContent("- record: foo:bar\n expr: sum(foo:bar)\n"), problems: func(uri string) []checks.Problem { return []checks.Problem{ { @@ -515,7 +515,7 @@ func TestSeriesCheck(t *testing.T) { content: "- record: foo\n expr: sum(foo:bar{job=\"xxx\"})\n", checker: newSeriesCheck, prometheus: newSimpleProm, - entries: mustParseContent("- record: foo:bar\n expr: sum(foo)\n"), + entries: parseContent("- record: foo:bar\n expr: sum(foo)\n"), problems: func(uri string) []checks.Problem { return []checks.Problem{ { @@ -559,7 +559,7 @@ func TestSeriesCheck(t *testing.T) { content: "- record: foo\n expr: count(ALERTS{alertname=\"myalert\"})\n", checker: newSeriesCheck, prometheus: newSimpleProm, - entries: mustParseContent("- alert: myalert\n expr: sum(foo) == 0\n"), + entries: parseContent("- alert: myalert\n expr: sum(foo) == 0\n"), problems: noProblems, }, { @@ -567,7 +567,7 @@ func TestSeriesCheck(t *testing.T) { content: "- record: foo\n expr: count(ALERTS{alertname=\"myalert\"})\n", checker: newSeriesCheck, prometheus: newSimpleProm, - entries: mustParseContent("- alert: notmyalert\n expr: sum(foo) == 0\n"), + entries: parseContent("- alert: notmyalert\n expr: sum(foo) == 0\n"), problems: func(_ string) []checks.Problem { return []checks.Problem{ { @@ -588,7 +588,7 @@ func TestSeriesCheck(t *testing.T) { content: "- record: foo\n expr: sum(foo:bar{job=\"xxx\"})\n", checker: newSeriesCheck, prometheus: newSimpleProm, - entries: mustParseContent("- record: foo:bar\n expr: sum(foo:bar)\n"), + entries: parseContent("- record: foo:bar\n expr: sum(foo:bar)\n"), problems: func(uri string) []checks.Problem { return []checks.Problem{ { @@ -3096,7 +3096,7 @@ func TestSeriesCheck(t *testing.T) { content: "- alert: foo\n expr: sum(foo:count) / sum(foo:sum) > 120\n", checker: newSeriesCheck, prometheus: newSimpleProm, - entries: mustParseContent("- record: foo:count\n expr: count(foo)\n- record: foo:sum\n expr: sum(foo)\n"), + entries: parseContent("- record: foo:count\n expr: count(foo)\n- record: foo:sum\n expr: sum(foo)\n"), problems: func(uri string) []checks.Problem { return []checks.Problem{ { diff --git a/internal/checks/rule_dependency_test.go b/internal/checks/rule_dependency_test.go index f88e6295..c3b8f129 100644 --- a/internal/checks/rule_dependency_test.go +++ b/internal/checks/rule_dependency_test.go @@ -26,7 +26,7 @@ func detailsDependencyRule(kind, name, broken string) string { func TestRuleDependencyCheck(t *testing.T) { parseWithState := func(input string, state discovery.ChangeType, sp, rp string) []discovery.Entry { - entries := mustParseContent(input) + entries := parseContent(input) for i := range entries { entries[i].State = state entries[i].Path.Name = sp diff --git a/internal/checks/rule_duplicate_test.go b/internal/checks/rule_duplicate_test.go index 0a99d658..98cceb1b 100644 --- a/internal/checks/rule_duplicate_test.go +++ b/internal/checks/rule_duplicate_test.go @@ -16,7 +16,7 @@ func textDuplicateRule(path string, line int) string { } func TestRuleDuplicateCheck(t *testing.T) { - xxxEntries := mustParseContent("- record: foo\n expr: up == 0\n") + xxxEntries := parseContent("- record: foo\n expr: up == 0\n") for i := range xxxEntries { xxxEntries[i].Path.SymlinkTarget = "xxx.yml" } @@ -48,7 +48,7 @@ func TestRuleDuplicateCheck(t *testing.T) { }, prometheus: newSimpleProm, problems: noProblems, - entries: mustParseContent("- record: foo\n expr: up == 0\n"), + entries: parseContent("- record: foo\n expr: up == 0\n"), }, { description: "skip alerting entries", @@ -58,7 +58,7 @@ func TestRuleDuplicateCheck(t *testing.T) { }, prometheus: newSimpleProm, problems: noProblems, - entries: mustParseContent(` + entries: parseContent(` - alert: foo expr: up == 0 - record: baz @@ -75,7 +75,7 @@ func TestRuleDuplicateCheck(t *testing.T) { }, prometheus: newSimpleProm, problems: noProblems, - entries: mustParseContent(` + entries: parseContent(` # foo - record: foo expr: up == @@ -91,7 +91,7 @@ func TestRuleDuplicateCheck(t *testing.T) { }, prometheus: newSimpleProm, problems: noProblems, - entries: mustParseContent(` + entries: parseContent(` - record: bar expr: up == 0 labels: @@ -110,7 +110,7 @@ func TestRuleDuplicateCheck(t *testing.T) { }, prometheus: newSimpleProm, problems: noProblems, - entries: mustParseContent(` + entries: parseContent(` - record: foo expr: up == 0 labels: @@ -141,7 +141,7 @@ func TestRuleDuplicateCheck(t *testing.T) { }, } }, - entries: mustParseContent(` + entries: parseContent(` - record: foo expr: up == 0 labels: diff --git a/internal/checks/template_test.go b/internal/checks/template_test.go index e7d05b68..4e623710 100644 --- a/internal/checks/template_test.go +++ b/internal/checks/template_test.go @@ -106,10 +106,7 @@ func TestTemplatedRegexpExpand(t *testing.T) { func newMustRule(content string) parser.Rule { p := parser.NewParser(false) - rules, err := p.Parse([]byte(content)) - if err != nil { - panic(err) - } + rules := p.Parse([]byte(content)) for _, rule := range rules { return rule } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 08c46b55..2e3f0389 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -132,13 +132,9 @@ func TestSetDisabledChecks(t *testing.T) { require.Equal(t, []string{checks.SyntaxCheckName, checks.RateCheckName}, cfg.Checks.Disabled) } -func newRule(t *testing.T, content string) parser.Rule { +func newRule(content string) parser.Rule { p := parser.NewParser(false) - rules, err := p.Parse([]byte(content)) - if err != nil { - t.Error(err) - t.FailNow() - } + rules := p.Parse([]byte(content)) return rules[0] } @@ -161,7 +157,7 @@ func TestGetChecksForRule(t *testing.T) { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -186,7 +182,7 @@ prometheus "prom" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -219,7 +215,7 @@ prometheus "prom" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -258,7 +254,7 @@ checks { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` # pint disable promql/counter # pint disable promql/rate # pint disable promql/series @@ -293,7 +289,7 @@ prometheus "prom" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -320,7 +316,7 @@ prometheus "prom" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -346,7 +342,7 @@ prometheus "prom" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -372,7 +368,7 @@ prometheus "prom" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -410,7 +406,7 @@ prometheus "ignore" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -437,7 +433,7 @@ prometheus "ignore" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -467,7 +463,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -500,7 +496,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` - record: foo # pint disable promql/aggregate(instance:false) expr: sum(foo) @@ -533,7 +529,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -567,7 +563,7 @@ prometheus "prom2" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` # pint disable promql/series(prom1) # pint disable query/cost(prom2) - record: foo @@ -648,7 +644,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -700,7 +696,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` # pint disable promql/counter # pint disable promql/series # pint disable promql/rate @@ -755,7 +751,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -792,7 +788,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -826,7 +822,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -860,7 +856,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n labels:\n cluster: dev\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n labels:\n cluster: dev\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -894,7 +890,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n labels:\n cluster: prod\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n labels:\n cluster: prod\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -928,7 +924,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -962,7 +958,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n annotations:\n cluster: dev\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n annotations:\n cluster: dev\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -998,7 +994,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n annotations:\n cluster: prod\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n annotations:\n cluster: prod\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1041,7 +1037,7 @@ prometheus "prom1" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` - record: foo expr: sum(foo) # pint disable promql/series @@ -1083,7 +1079,7 @@ prometheus "prom1" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` - record: foo expr: sum(foo) `), @@ -1134,7 +1130,7 @@ prometheus "prom1" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` - record: foo expr: sum(foo) `), @@ -1175,7 +1171,7 @@ checks { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` - alert: foo expr: sum(foo) `), @@ -1216,7 +1212,7 @@ checks { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` - alert: foo expr: sum(foo) `), @@ -1243,7 +1239,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 16m\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n for: 16m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1274,7 +1270,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 14m\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n for: 14m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1303,7 +1299,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n keep_firing_for: 16m\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n keep_firing_for: 16m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1333,7 +1329,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 16m\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n for: 16m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1362,7 +1358,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n keep_firing_for: 14m\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n keep_firing_for: 14m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1391,7 +1387,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1420,7 +1416,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 16m\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n for: 16m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1451,7 +1447,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 14m\n"), + Rule: newRule("- alert: foo\n expr: sum(foo)\n for: 14m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1480,7 +1476,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1513,7 +1509,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1546,7 +1542,7 @@ checks { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` # Some extra comment # pint disable promql/series # Some extra comment @@ -1585,7 +1581,7 @@ checks { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` # pint snooze 2099-11-AB labels/conflict # pint snooze 2099-11-28 labels/conflict won't work # pint snooze 2099-11-28 @@ -1633,7 +1629,7 @@ checks { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` # pint snooze 2000-11-28 promql/series(prom1) # pint snooze 2000-11-28T10:24:18Z promql/range_query # pint snooze 2000-11-28 rule/duplicate @@ -1687,7 +1683,7 @@ prometheus "prom3" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` # pint disable alerts/count(+disable) # pint disable alerts/external_labels(+disable) # pint disable labels/conflict(+disable) @@ -1748,7 +1744,7 @@ prometheus "prom3" { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, ` + Rule: newRule(` # pint snooze 2099-11-28 alerts/count(+disable) # pint snooze 2099-11-28 alerts/external_labels(+disable) # pint snooze 2099-11-28 labels/conflict(+disable) @@ -1809,7 +1805,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1853,7 +1849,7 @@ rule { Name: "rules.yml", SymlinkTarget: "rules.yml", }, - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + Rule: newRule("- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go index 152cefa3..a2fad439 100644 --- a/internal/discovery/discovery.go +++ b/internal/discovery/discovery.go @@ -158,38 +158,24 @@ func readRules(reportedPath, sourcePath string, r io.Reader, isStrict bool) (ent } p := parser.NewParser(isStrict) - rules, err := p.Parse(content.Body) - if err != nil { - slog.Warn( - "Failed to parse file content", - slog.Any("err", err), - slog.String("path", sourcePath), - slog.String("lines", contentLines.String()), - ) - entries = append(entries, Entry{ - Path: Path{ - Name: sourcePath, - SymlinkTarget: reportedPath, - }, - PathError: err, - Owner: fileOwner, - ModifiedLines: contentLines.Expand(), - }) - return entries, nil - } - - for _, rule := range rules { + for _, rule := range p.Parse(content.Body) { ruleOwner := fileOwner for _, owner := range comments.Only[comments.Owner](rule.Comments, comments.RuleOwnerType) { ruleOwner = owner.Name } + var ml []int + if rule.Error.IsFatal { + ml = contentLines.Expand() + } else { + ml = rule.Lines.Expand() + } entries = append(entries, Entry{ Path: Path{ Name: sourcePath, SymlinkTarget: reportedPath, }, Rule: rule, - ModifiedLines: rule.Lines.Expand(), + ModifiedLines: ml, Owner: ruleOwner, DisabledChecks: disabledChecks, }) diff --git a/internal/discovery/discovery_test.go b/internal/discovery/discovery_test.go index cedc17aa..007621af 100644 --- a/internal/discovery/discovery_test.go +++ b/internal/discovery/discovery_test.go @@ -25,13 +25,13 @@ func (r failingReader) Read(_ []byte) (int, error) { func TestReadRules(t *testing.T) { mustParse := func(offset int, s string) parser.Rule { p := parser.NewParser(false) - r, err := p.Parse([]byte(strings.Repeat("\n", offset) + s)) - if err != nil { - panic(fmt.Sprintf("failed to parse rule:\n---\n%s\n---\nerror: %s", s, err)) - } + r := p.Parse([]byte(strings.Repeat("\n", offset) + s)) if len(r) != 1 { panic(fmt.Sprintf("wrong number of rules returned: %d\n---\n%s\n---", len(r), s)) } + if r[0].Error.Err != nil { + panic(r[0].Error) + } return r[0] } diff --git a/internal/discovery/git_branch_test.go b/internal/discovery/git_branch_test.go index 954de60f..7eb4bc82 100644 --- a/internal/discovery/git_branch_test.go +++ b/internal/discovery/git_branch_test.go @@ -39,13 +39,13 @@ func TestGitBranchFinder(t *testing.T) { mustParse := func(offset int, s string) parser.Rule { p := parser.NewParser(false) - r, err := p.Parse([]byte(strings.Repeat("\n", offset) + s)) - if err != nil { - panic(fmt.Sprintf("failed to parse rule:\n---\n%s\n---\nerror: %s", s, err)) - } + r := p.Parse([]byte(strings.Repeat("\n", offset) + s)) if len(r) != 1 { panic(fmt.Sprintf("wrong number of rules returned: %d\n---\n%s\n---", len(r), s)) } + if r[0].Error.Err != nil { + panic(r[0].Error) + } return r[0] } @@ -719,9 +719,12 @@ groups: SymlinkTarget: "rules.yml", }, ModifiedLines: []int{3}, - PathError: parser.StrictError{ - Err: errors.New(`mapping key "expr" already defined at line 6`), - Line: 7, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 2, Last: 7}, + Error: parser.ParseError{ + Err: errors.New(`mapping key "expr" already defined at line 6`), + Line: 7, + }, }, }, }, diff --git a/internal/discovery/glob_test.go b/internal/discovery/glob_test.go index a5593b13..a1746a79 100644 --- a/internal/discovery/glob_test.go +++ b/internal/discovery/glob_test.go @@ -28,8 +28,7 @@ func TestGlobPathFinder(t *testing.T) { p := parser.NewParser(false) testRuleBody := "# pint file/owner bob\n\n- record: foo\n expr: sum(foo)\n" - testRules, err := p.Parse([]byte(testRuleBody)) - require.NoError(t, err) + testRules := p.Parse([]byte(testRuleBody)) testCases := []testCaseT{ { @@ -99,11 +98,14 @@ func TestGlobPathFinder(t *testing.T) { Name: "bar.yml", SymlinkTarget: "bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 3, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 3, Last: 3}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 3, + }, }, - ModifiedLines: []int{1, 2, 3, 4}, + ModifiedLines: []int{3}, Owner: "bob", }, }, @@ -118,9 +120,13 @@ func TestGlobPathFinder(t *testing.T) { Name: "bar.yml", SymlinkTarget: "bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("mapping values are not allowed in this context"), - Line: 2, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 2, Last: 2}, + Error: parser.ParseError{ + Err: errors.New("mapping values are not allowed in this context"), + Line: 2, + IsFatal: true, + }, }, ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", @@ -138,11 +144,14 @@ func TestGlobPathFinder(t *testing.T) { Name: "bar.yml", SymlinkTarget: "bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 3, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 3, Last: 3}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 3, + }, }, - ModifiedLines: []int{1, 2, 3, 4}, + ModifiedLines: []int{3}, Owner: "bob", }, { @@ -151,11 +160,14 @@ func TestGlobPathFinder(t *testing.T) { Name: "link.yml", SymlinkTarget: "bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 3, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 3, Last: 3}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 3, + }, }, - ModifiedLines: []int{1, 2, 3, 4}, + ModifiedLines: []int{3}, Owner: "bob", }, }, @@ -174,11 +186,14 @@ func TestGlobPathFinder(t *testing.T) { Name: "a/bar.yml", SymlinkTarget: "a/bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 3, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 3, Last: 3}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 3, + }, }, - ModifiedLines: []int{1, 2, 3, 4}, + ModifiedLines: []int{3}, Owner: "bob", }, { @@ -187,11 +202,14 @@ func TestGlobPathFinder(t *testing.T) { Name: "b/c/link.yml", SymlinkTarget: "a/bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 3, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 3, Last: 3}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 3, + }, }, - ModifiedLines: []int{1, 2, 3, 4}, + ModifiedLines: []int{3}, Owner: "bob", }, { @@ -200,11 +218,14 @@ func TestGlobPathFinder(t *testing.T) { Name: "b/link.yml", SymlinkTarget: "a/bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 3, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 3, Last: 3}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 3, + }, }, - ModifiedLines: []int{1, 2, 3, 4}, + ModifiedLines: []int{3}, Owner: "bob", }, }, @@ -238,9 +259,12 @@ func TestGlobPathFinder(t *testing.T) { Name: "a/bar.yml", SymlinkTarget: "a/bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 1, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 1, Last: 2}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 2, + }, }, ModifiedLines: []int{1, 2}, Owner: "", @@ -251,9 +275,12 @@ func TestGlobPathFinder(t *testing.T) { Name: "b/c/link.yml", SymlinkTarget: "a/bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 1, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 1, Last: 2}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 2, + }, }, ModifiedLines: []int{1, 2}, Owner: "", @@ -287,11 +314,14 @@ func TestGlobPathFinder(t *testing.T) { Name: "a/bar.yml", SymlinkTarget: "a/bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 3, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 3, Last: 3}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 3, + }, }, - ModifiedLines: []int{1, 2, 3, 4}, + ModifiedLines: []int{3}, Owner: "bob", }, { @@ -300,11 +330,14 @@ func TestGlobPathFinder(t *testing.T) { Name: "b/c/link.yml", SymlinkTarget: "a/bar.yml", }, - PathError: parser.StrictError{ - Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), - Line: 3, + Rule: parser.Rule{ + Lines: parser.LineRange{First: 3, Last: 3}, + Error: parser.ParseError{ + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 3, + }, }, - ModifiedLines: []int{1, 2, 3, 4}, + ModifiedLines: []int{3}, Owner: "bob", }, }, diff --git a/internal/parser/fuzz_test.go b/internal/parser/fuzz_test.go index b5a7ec3c..1f98fc79 100644 --- a/internal/parser/fuzz_test.go +++ b/internal/parser/fuzz_test.go @@ -279,6 +279,6 @@ labels: p := parser.NewParser(false) f.Fuzz(func(t *testing.T, s string) { t.Logf("Parsing: [%s]\n", s) - _, _ = p.Parse([]byte(s)) + _ = p.Parse([]byte(s)) }) } diff --git a/internal/parser/models.go b/internal/parser/models.go index 97e1ef48..abe38728 100644 --- a/internal/parser/models.go +++ b/internal/parser/models.go @@ -238,10 +238,12 @@ func (rr *RecordingRule) IsIdentical(b *RecordingRule) bool { return true } +// Use insread of StrictError. type ParseError struct { - Err error - Fragment string - Line int + Err error + Details string + Line int + IsFatal bool } type LineRange struct { diff --git a/internal/parser/models_test.go b/internal/parser/models_test.go index 95bf5a0a..c48ec7d7 100644 --- a/internal/parser/models_test.go +++ b/internal/parser/models_test.go @@ -11,11 +11,7 @@ import ( func newMustRule(content string) parser.Rule { p := parser.NewParser(false) - rules, err := p.Parse([]byte(content)) - if err != nil { - panic(err) - } - for _, rule := range rules { + for _, rule := range p.Parse([]byte(content)) { return rule } return parser.Rule{} diff --git a/internal/parser/parser.go b/internal/parser/parser.go index e316f760..a1c31343 100644 --- a/internal/parser/parser.go +++ b/internal/parser/parser.go @@ -38,14 +38,21 @@ type Parser struct { isStrict bool } -func (p Parser) Parse(content []byte) (rules []Rule, err error) { +func (p Parser) Parse(content []byte) (rules []Rule) { if len(content) == 0 { - return nil, nil + return nil } defer func() { if r := recover(); r != nil { - err = fmt.Errorf("unable to parse YAML file: %s", r) + rules = append(rules, Rule{ + Lines: LineRange{First: 1, Last: 1}, + Error: ParseError{ + Line: 1, + Err: fmt.Errorf("unable to parse YAML file: %s", r), + IsFatal: true, + }, + }) } }() @@ -58,26 +65,58 @@ func (p Parser) Parse(content []byte) (rules []Rule, err error) { break } if decodeErr != nil { - return nil, tryDecodingYamlError(decodeErr) + err := tryDecodingYamlError(decodeErr) + rules = append(rules, Rule{ + Lines: LineRange{First: err.Line, Last: err.Line}, + Error: err, + }) + break } index++ if p.isStrict { - if err = validateRuleFile(&doc); err != nil { - return nil, err + errs := validateRuleFile(&doc) + if len(errs) > 0 { + var line int + var buf strings.Builder + buf.WriteString("This file is not a valid Prometheus rule file.\n") + buf.WriteString("Errors found:\n\n") + lr := LineRange{First: doc.Line, Last: doc.Line} + for _, err := range errs { + line = err.Line + lr.First = min(lr.First, err.Line) + lr.Last = max(lr.Last, err.Line) + buf.WriteString("- line ") + buf.WriteString(strconv.Itoa(err.Line)) + buf.WriteString(": ") + buf.WriteString(err.Err.Error()) + buf.WriteString("\n") + } + rules = append(rules, Rule{ + Lines: lr, + Error: ParseError{ + Line: line, + Err: errors.New(buf.String()), + IsFatal: true, + }, + }) + break } } rules = append(rules, parseNode(content, &doc, 0)...) if index > 1 && p.isStrict { - return nil, StrictError{ - Err: errors.New("multi-document YAML files are not allowed"), - Details: `This is a multi-document YAML file. Prometheus will only parse the first document and silently ignore the rest. + rules = append(rules, Rule{ + Lines: LineRange{First: doc.Line, Last: doc.Line}, + Error: ParseError{ + Err: errors.New("multi-document YAML files are not allowed"), + Details: `This is a multi-document YAML file. Prometheus will only parse the first document and silently ignore the rest. To allow for multi-document YAML files set parser->relaxed option in pint config file.`, - Line: doc.Line, - } + Line: doc.Line, + }, + }) } } - return rules, err + return rules } func parseNode(content []byte, node *yaml.Node, offset int) (rules []Rule) { @@ -608,19 +647,18 @@ var ( yamlUnmarshalErrRe = regexp.MustCompile("^yaml: unmarshal errors:\n line (.+): (.+)") ) -func tryDecodingYamlError(err error) error { +func tryDecodingYamlError(err error) ParseError { for _, re := range []*regexp.Regexp{yamlErrRe, yamlUnmarshalErrRe} { parts := re.FindStringSubmatch(err.Error()) if len(parts) > 2 { - line, err2 := strconv.Atoi(parts[1]) - if err2 != nil || line <= 0 { - return err - } - return StrictError{ - Line: line, - Err: errors.New(parts[2]), + if line, err2 := strconv.Atoi(parts[1]); line > 0 && err2 == nil { + return ParseError{ + Line: line, + Err: errors.New(parts[2]), + IsFatal: true, + } } } } - return err + return ParseError{Line: 1, Err: err, IsFatal: true} } diff --git a/internal/parser/parser_test.go b/internal/parser/parser_test.go index f11b30b3..31e22dd3 100644 --- a/internal/parser/parser_test.go +++ b/internal/parser/parser_test.go @@ -5,8 +5,6 @@ import ( "strconv" "testing" - "github.com/stretchr/testify/require" - "github.com/cloudflare/pint/internal/comments" "github.com/cloudflare/pint/internal/parser" @@ -15,7 +13,6 @@ import ( func TestParse(t *testing.T) { type testCaseT struct { - err string content []byte output []parser.Rule strict bool @@ -32,8 +29,15 @@ func TestParse(t *testing.T) { }, { content: []byte(string("! !00 \xf6")), - output: nil, - err: "yaml: incomplete UTF-8 octet sequence", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 1, Last: 1}, + Error: parser.ParseError{ + Line: 1, + Err: errors.New("yaml: incomplete UTF-8 octet sequence"), + }, + }, + }, }, { content: []byte("- 0: 0\n 00000000: 000000\n 000000:00000000000: 00000000\n 00000000000:000000: 0000000000000000000000000000000000\n 000000: 0000000\n expr: |"), @@ -115,15 +119,39 @@ func TestParse(t *testing.T) { }, { content: []byte("- record: - foo\n"), - err: "yaml: block sequence entries are not allowed in this context", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 1, Last: 1}, + Error: parser.ParseError{ + Line: 1, + Err: errors.New("yaml: block sequence entries are not allowed in this context"), + }, + }, + }, }, { content: []byte("- record: foo expr: sum(\n"), - err: "yaml: mapping values are not allowed in this context", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 1, Last: 1}, + Error: parser.ParseError{ + Line: 1, + Err: errors.New("yaml: mapping values are not allowed in this context"), + }, + }, + }, }, { content: []byte("- record\n\texpr: foo\n"), - err: "YAML syntax error at line 2: found a tab character that violates indentation", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 2}, + Error: parser.ParseError{ + Line: 2, + Err: errors.New("found a tab character that violates indentation"), + }, + }, + }, }, { content: []byte(` @@ -1396,7 +1424,15 @@ data: foo: ` + string("\xed\xbf\xbf")), // Label values are invalid only if they aren't valid UTF-8 strings // which also makes them unparsable by YAML. - err: "yaml: invalid Unicode character", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 1, Last: 1}, + Error: parser.ParseError{ + Line: 1, + Err: errors.New("yaml: invalid Unicode character"), + }, + }, + }, }, { content: []byte(` @@ -1678,7 +1714,10 @@ data: output: []parser.Rule{ { Lines: parser.LineRange{First: 2, Last: 4}, - Error: parser.ParseError{Err: errors.New("labels value must be a YAML mapping, got binary data instead"), Line: 4}, + Error: parser.ParseError{ + Err: errors.New("labels value must be a YAML mapping, got binary data instead"), + Line: 4, + }, }, }, }, @@ -1691,7 +1730,10 @@ data: output: []parser.Rule{ { Lines: parser.LineRange{First: 2, Last: 4}, - Error: parser.ParseError{Err: errors.New("for value must be a YAML string, got float instead"), Line: 4}, + Error: parser.ParseError{ + Err: errors.New("for value must be a YAML string, got float instead"), + Line: 4, + }, }, }, }, @@ -1714,7 +1756,15 @@ data: expr: bar labels: !! "SGVsbG8sIFdvcmxkIQ==" `), - err: "YAML syntax error at line 4: did not find expected tag URI", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 4, Last: 4}, + Error: parser.ParseError{ + Line: 4, + Err: errors.New("did not find expected tag URI"), + }, + }, + }, }, { content: []byte(` @@ -1725,7 +1775,10 @@ data: output: []parser.Rule{ { Lines: parser.LineRange{First: 2, Last: 4}, - Error: parser.ParseError{Err: errors.New("labels value must be a YAML mapping, got string instead"), Line: 4}, + Error: parser.ParseError{ + Err: errors.New("labels value must be a YAML mapping, got string instead"), + Line: 4, + }, }, }, }, @@ -1755,7 +1808,10 @@ data: }, { Lines: parser.LineRange{First: 5, Last: 5}, - Error: parser.ParseError{Err: errors.New("incomplete rule, no alert or record key"), Line: 5}, + Error: parser.ParseError{ + Err: errors.New("incomplete rule, no alert or record key"), + Line: 5, + }, }, }, }, @@ -1900,7 +1956,20 @@ groups: expr: count(up) `), strict: true, - err: "YAML syntax error at line 2: YAML list is not allowed here, expected a YAML mapping", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 2}, + Error: parser.ParseError{ + Line: 2, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 2: YAML list is not allowed here, expected a YAML mapping +`), + }, + }, + }, }, { content: []byte(` @@ -1909,7 +1978,19 @@ rules: expr: count(up) `), strict: true, - err: "YAML syntax error at line 2: unexpected key `rules`", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 2}, + Error: parser.ParseError{ + Line: 2, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 2: unexpected key ` + "`rules`\n"), + }, + }, + }, }, { content: []byte(` @@ -1918,7 +1999,22 @@ groups: expr: count(up) `), strict: true, - err: "YAML syntax error at line 3: unexpected key `record`", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 4}, + Error: parser.ParseError{ + Line: 3, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 3: unexpected key ` + "`record`" + ` +- line 4: unexpected key ` + "`expr`" + ` +- line 3: ` + "`name` key is required and must be set" + ` +- line 3: ` + "`rules` key is required and must be set\n"), + }, + }, + }, }, { content: []byte(` @@ -1928,7 +2024,19 @@ groups: expr: count(up) `), strict: true, - err: "YAML syntax error at line 3: `name` key is required and must be set", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 3}, + Error: parser.ParseError{ + Line: 3, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 3: ` + "`name` key is required and must be set\n"), + }, + }, + }, }, { content: []byte(` @@ -1936,14 +2044,39 @@ groups: - name: foo `), strict: true, - err: "YAML syntax error at line 3: `rules` key is required and must be set", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 3}, + Error: parser.ParseError{ + Line: 3, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 3: ` + "`rules` key is required and must be set\n"), + }, + }, + }, }, { content: []byte(` groups: {} `), strict: true, - err: `YAML syntax error at line 2: YAML mapping is not allowed here, expected a YAML list`, + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 2}, + Error: parser.ParseError{ + Line: 2, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 2: YAML mapping is not allowed here, expected a YAML list +`), + }, + }, + }, }, { content: []byte(` @@ -1951,16 +2084,44 @@ groups: - name: [] `), strict: true, - err: `YAML syntax error at line 3: YAML list is not allowed here, expected a YAML scalar value`, + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 3}, + Error: parser.ParseError{ + Line: 3, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 3: YAML list is not allowed here, expected a YAML scalar value +- line 3: ` + "`rules` key is required and must be set\n"), + }, + }, + }, }, { content: []byte(` groups: - name: foo name: bar + name: bob `), strict: true, - err: "YAML syntax error at line 4: duplicated key `name`", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 5}, + Error: parser.ParseError{ + Line: 3, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 4: duplicated key ` + "`name`" + ` +- line 5: duplicated key ` + "`name`" + ` +- line 3: ` + "`rules` key is required and must be set\n"), + }, + }, + }, }, { content: []byte(` @@ -1972,7 +2133,20 @@ groups: expr: count(up) `), strict: true, - err: `YAML syntax error at line 5: YAML mapping is not allowed here, expected a YAML list`, + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 5}, + Error: parser.ParseError{ + Line: 5, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 5: YAML mapping is not allowed here, expected a YAML list +`), + }, + }, + }, }, { content: []byte(` @@ -1984,7 +2158,18 @@ groups: expr: count(up) `), strict: true, - err: "YAML syntax error at line 5: unexpected key `rules`", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 2, Last: 5}, + Error: parser.ParseError{ + Line: 5, + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 5: unexpected key ` + "`rules`\n"), + }, + }, + }, }, { content: []byte(` @@ -1996,7 +2181,15 @@ groups: expr: count(up) `), strict: true, - err: "YAML syntax error at line 6: found a tab character that violates indentation", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 6, Last: 6}, + Error: parser.ParseError{ + Line: 6, // it's really 7 but go-yaml gives us 6 + Err: errors.New("found a tab character that violates indentation"), + }, + }, + }, }, { content: []byte(` @@ -2015,7 +2208,33 @@ groups: expr: count(up) `), strict: true, - err: "YAML syntax error at line 12: unexpected key `rules`", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 6, Last: 7}, + RecordingRule: &parser.RecordingRule{ + Record: parser.YamlNode{ + Lines: parser.LineRange{First: 6, Last: 6}, + Value: "up:count", + }, + Expr: parser.PromQLExpr{ + Value: &parser.YamlNode{ + Lines: parser.LineRange{First: 7, Last: 7}, + Value: "count(up)", + }, + }, + }, + }, + { + Lines: parser.LineRange{First: 8, Last: 12}, + Error: parser.ParseError{ + Line: 12, + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 12: unexpected key ` + "`rules`\n"), + }, + }, + }, }, { content: []byte(` @@ -2028,7 +2247,20 @@ groups: - labels: !!binary "SGVsbG8sIFdvcmxkIQ==" `), strict: true, - err: "YAML syntax error at line 8: YAML scalar value is not allowed here, expected a YAML mapping", + output: []parser.Rule{ + { + Lines: parser.LineRange{First: 4, Last: 8}, + Error: parser.ParseError{ + Line: 8, + // nolint error-strings + Err: errors.New(`This file is not a valid Prometheus rule file. +Errors found: + +- line 8: YAML scalar value is not allowed here, expected a YAML mapping +`), + }, + }, + }, }, } @@ -2055,13 +2287,7 @@ groups: t.Logf("\n--- Content ---%s--- END ---", tc.content) p := parser.NewParser(tc.strict) - output, err := p.Parse(tc.content) - - if tc.err != "" { - require.EqualError(t, err, tc.err) - } else { - require.NoError(t, err) - } + output := p.Parse(tc.content) if diff := cmp.Diff(tc.output, output, ignorePrometheusExpr, sameErrorText); diff != "" { t.Errorf("Parse() returned wrong output (-want +got):\n%s", diff) diff --git a/internal/parser/strict.go b/internal/parser/strict.go index e7235970..d98eccd3 100644 --- a/internal/parser/strict.go +++ b/internal/parser/strict.go @@ -21,18 +21,8 @@ const ( mergeTag = "!!merge" ) -type StrictError struct { - Err error - Details string - Line int -} - -func (se StrictError) Error() string { - return fmt.Sprintf("YAML syntax error at line %d: %s", se.Line, se.Err.Error()) -} - type validator interface { - validate(*yaml.Node) error + validate(*yaml.Node) []ParseError } func mustKind(node *yaml.Node, kind yaml.Kind) error { @@ -59,25 +49,23 @@ type requireDocument struct { v validator } -func (rd requireDocument) validate(node *yaml.Node) (err error) { - if err = mustKind(node, yaml.DocumentNode); err != nil { - return StrictError{Err: err, Line: node.Line} +func (rd requireDocument) validate(node *yaml.Node) (errs []ParseError) { + if err := mustKind(node, yaml.DocumentNode); err != nil { + return []ParseError{{Err: err, Line: node.Line}} } for _, child := range node.Content { - if err = rd.v.validate(child); err != nil { - return err - } + errs = append(errs, rd.v.validate(child)...) } - return nil + return errs } type requireScalar struct { tag string } -func (rs requireScalar) validate(node *yaml.Node) (err error) { - if err = mustKindTag(node, yaml.ScalarNode, rs.tag); err != nil { - return StrictError{Err: err, Line: node.Line} +func (rs requireScalar) validate(node *yaml.Node) (errs []ParseError) { + if err := mustKindTag(node, yaml.ScalarNode, rs.tag); err != nil { + return []ParseError{{Err: err, Line: node.Line}} } return nil } @@ -86,44 +74,40 @@ type requireList struct { v validator } -func (rl requireList) validate(node *yaml.Node) (err error) { - if err = mustKind(node, yaml.SequenceNode); err != nil { - return StrictError{Err: err, Line: node.Line} +func (rl requireList) validate(node *yaml.Node) (errs []ParseError) { + if err := mustKind(node, yaml.SequenceNode); err != nil { + return []ParseError{{Err: err, Line: node.Line}} } for _, n := range unpackNodes(node) { - if err = rl.v.validate(n); err != nil { - return err - } + errs = append(errs, rl.v.validate(n)...) } - return nil + return errs } type requireMap struct { v validator } -func (rm requireMap) validate(node *yaml.Node) (err error) { - if err = mustKind(node, yaml.MappingNode); err != nil { - return StrictError{Err: err, Line: node.Line} +func (rm requireMap) validate(node *yaml.Node) (errs []ParseError) { + if err := mustKind(node, yaml.MappingNode); err != nil { + return []ParseError{{Err: err, Line: node.Line}} } setKeys := map[string]struct{}{} var ok bool for _, n := range unpackNodes(node) { - if err = rm.v.validate(n); err != nil { - return err - } + errs = append(errs, rm.v.validate(n)...) if _, ok = setKeys[n.Value]; ok { - return StrictError{ + errs = append(errs, ParseError{ Err: fmt.Errorf("duplicated key `%s`", n.Value), Line: n.Line, - } + }) } setKeys[n.Value] = struct{}{} } - return nil + return errs } type requireExactMap struct { @@ -132,57 +116,64 @@ type requireExactMap struct { required []string } -func (rm requireExactMap) validate(node *yaml.Node) (err error) { - if err = mustKind(node, yaml.MappingNode); err != nil { - return StrictError{Err: err, Line: node.Line} +func (rm requireExactMap) validate(node *yaml.Node) (errs []ParseError) { + if err := mustKind(node, yaml.MappingNode); err != nil { + return []ParseError{{Err: err, Line: node.Line}} } setKeys := make(map[string]struct{}, len(rm.required)) var v validator var ok bool + var fns []func(string, int) for i, n := range unpackNodes(node) { if i%2 == 0 { - if err = mustKindTag(n, yaml.ScalarNode, strTag); err != nil { - return StrictError{Err: err, Line: n.Line} + if err := mustKindTag(n, yaml.ScalarNode, strTag); err != nil { + errs = append(errs, ParseError{Err: err, Line: n.Line}) + continue } v, ok = rm.keys[n.Value] if !ok { - return StrictError{Err: fmt.Errorf("unexpected key `%s`", n.Value), Line: n.Line} + errs = append(errs, ParseError{ + Err: fmt.Errorf("unexpected key `%s`", n.Value), + Line: n.Line, + }) } if _, ok = setKeys[n.Value]; ok { - return StrictError{ + errs = append(errs, ParseError{ Err: fmt.Errorf("duplicated key `%s`", n.Value), Line: n.Line, - } + }) } setKeys[n.Value] = struct{}{} for name, fn := range rm.nameCallback { if name == n.Value { - fn(n.Value, n.Line) + fns = append(fns, fn) } } - } else { - if err = v.validate(n); err != nil { - return err + } else if v != nil { + errs = append(errs, v.validate(n)...) + for _, fn := range fns { + fn(n.Value, n.Line) } + fns = nil } } for _, key := range rm.required { if _, ok = setKeys[key]; !ok { - return StrictError{ + errs = append(errs, ParseError{ Err: fmt.Errorf("`%s` key is required and must be set", key), Line: node.Line, - } + }) } } - return nil + return errs } -func validateRuleFile(node *yaml.Node) (err error) { +func validateRuleFile(node *yaml.Node) (errs []ParseError) { groupNames := map[string][]int{} nameCallback := func(s string, l int) { groupNames[s] = append(groupNames[s], l) @@ -222,20 +213,21 @@ func validateRuleFile(node *yaml.Node) (err error) { }, } - if err = v.validate(node); err != nil { - return err - } + errs = append(errs, v.validate(node)...) for name, lines := range groupNames { - if len(lines) > 1 { - return StrictError{ - Err: fmt.Errorf("duplicated group name `%s`", name), - Line: lines[len(lines)-1], + for i, line := range lines { + if i == 0 { + continue } + errs = append(errs, ParseError{ + Err: fmt.Errorf("duplicated group name `%s`", name), + Line: line, + }) } } - return nil + return errs } func descirbeTag(tag string) string { diff --git a/internal/parser/strict_test.go b/internal/parser/strict_test.go index 31f93b8c..27e4173e 100644 --- a/internal/parser/strict_test.go +++ b/internal/parser/strict_test.go @@ -1,11 +1,13 @@ package parser import ( + "errors" "io" "strconv" "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" ) @@ -13,46 +15,69 @@ import ( func TestValidateRuleFile(t *testing.T) { type testCaseT struct { content string - err string - line int + errs []ParseError } testCases := []testCaseT{ { content: "[]", - err: "YAML list is not allowed here, expected a YAML mapping", - line: 1, + errs: []ParseError{ + { + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 1, + }, + }, }, { content: "\n\n[]", - err: "YAML list is not allowed here, expected a YAML mapping", - line: 3, + errs: []ParseError{ + { + Err: errors.New("YAML list is not allowed here, expected a YAML mapping"), + Line: 3, + }, + }, }, { content: "groups: {}", - err: "YAML mapping is not allowed here, expected a YAML list", - line: 1, + errs: []ParseError{ + { + Err: errors.New("YAML mapping is not allowed here, expected a YAML list"), + Line: 1, + }, + }, }, { content: "groups: []", }, { content: "xgroups: {}", - err: "unexpected key `xgroups`", - line: 1, + errs: []ParseError{ + { + Err: errors.New("unexpected key `xgroups`"), + Line: 1, + }, + }, }, { content: "\nbob\n", - err: "YAML scalar value is not allowed here, expected a YAML mapping", - line: 2, + errs: []ParseError{ + { + Err: errors.New("YAML scalar value is not allowed here, expected a YAML mapping"), + Line: 2, + }, + }, }, { content: `groups: [] rules: [] `, - err: "unexpected key `rules`", - line: 3, + errs: []ParseError{ + { + Err: errors.New("unexpected key `rules`"), + Line: 3, + }, + }, }, { content: ` @@ -67,8 +92,12 @@ groups: - name: rules: [] `, - err: "expected a YAML string here, got null instead", - line: 3, + errs: []ParseError{ + { + Err: errors.New("expected a YAML string here, got null instead"), + Line: 3, + }, + }, }, { content: ` @@ -92,8 +121,12 @@ groups: labels: job: foo `, - err: "unexpected key `xxx`", - line: 7, + errs: []ParseError{ + { + Err: errors.New("unexpected key `xxx`"), + Line: 7, + }, + }, }, { content: ` @@ -106,8 +139,12 @@ groups: labels: job: foo `, - err: "YAML mapping is not allowed here, expected a YAML list", - line: 5, + errs: []ParseError{ + { + Err: errors.New("YAML mapping is not allowed here, expected a YAML list"), + Line: 5, + }, + }, }, { content: ` @@ -120,8 +157,12 @@ groups: job: foo: bar `, - err: "YAML mapping is not allowed here, expected a YAML scalar value", - line: 9, + errs: []ParseError{ + { + Err: errors.New("YAML mapping is not allowed here, expected a YAML scalar value"), + Line: 9, + }, + }, }, { content: ` @@ -132,8 +173,12 @@ groups: expr: sum: sum(up) `, - err: "YAML mapping is not allowed here, expected a YAML scalar value", - line: 7, + errs: []ParseError{ + { + Err: errors.New("YAML mapping is not allowed here, expected a YAML scalar value"), + Line: 7, + }, + }, }, { content: ` @@ -143,8 +188,12 @@ groups: - name: foo rules: [] `, - err: "duplicated group name `name`", - line: 5, + errs: []ParseError{ + { + Err: errors.New("duplicated group name `foo`"), + Line: 5, + }, + }, }, { content: ` @@ -157,8 +206,12 @@ groups: foo: bob foo: bar `, - err: "duplicated key `foo`", - line: 9, + errs: []ParseError{ + { + Err: errors.New("duplicated key `foo`"), + Line: 9, + }, + }, }, { content: ` @@ -168,8 +221,12 @@ groups: - record: up:count expr: count(up) expr: sum(up)`, - err: "duplicated key `expr`", - line: 7, + errs: []ParseError{ + { + Err: errors.New("duplicated key `expr`"), + Line: 7, + }, + }, }, { content: ` @@ -180,8 +237,12 @@ groups: expr: count(up) bogus: 1 `, - err: "unexpected key `bogus`", - line: 7, + errs: []ParseError{ + { + Err: errors.New("unexpected key `bogus`"), + Line: 7, + }, + }, }, { content: ` @@ -192,8 +253,12 @@ groups: expr: count(up) bogus: 1 `, - err: "unexpected key `bogus`", - line: 7, + errs: []ParseError{ + { + Err: errors.New("unexpected key `bogus`"), + Line: 7, + }, + }, }, { content: ` @@ -203,8 +268,12 @@ groups: rules: `, - err: "YAML scalar value is not allowed here, expected a YAML list", - line: 6, + errs: []ParseError{ + { + Err: errors.New("YAML scalar value is not allowed here, expected a YAML list"), + Line: 6, + }, + }, }, { content: ` @@ -213,8 +282,12 @@ groups: rules: expr: 1 `, - err: "YAML mapping is not allowed here, expected a YAML list", - line: 5, + errs: []ParseError{ + { + Err: errors.New("YAML mapping is not allowed here, expected a YAML list"), + Line: 5, + }, + }, }, { content: ` @@ -223,8 +296,12 @@ groups: rules: - expr: 1 `, - err: "expected a YAML string here, got integer instead", - line: 5, + errs: []ParseError{ + { + Err: errors.New("expected a YAML string here, got integer instead"), + Line: 5, + }, + }, }, { content: ` @@ -233,8 +310,12 @@ groups: rules: - expr: null `, - err: "expected a YAML string here, got null instead", - line: 5, + errs: []ParseError{ + { + Err: errors.New("expected a YAML string here, got null instead"), + Line: 5, + }, + }, }, { content: ` @@ -243,8 +324,12 @@ groups: rules: - 1: null `, - err: "expected a YAML string here, got integer instead", - line: 5, + errs: []ParseError{ + { + Err: errors.New("expected a YAML string here, got integer instead"), + Line: 5, + }, + }, }, { content: ` @@ -253,8 +338,12 @@ groups: rules: - true: !!binary "SGVsbG8sIFdvcmxkIQ==" `, - err: "expected a YAML string here, got bool instead", - line: 5, + errs: []ParseError{ + { + Err: errors.New("expected a YAML string here, got bool instead"), + Line: 5, + }, + }, }, { content: ` @@ -263,8 +352,12 @@ groups: rules: - expr: !!binary "SGVsbG8sIFdvcmxkIQ==" `, - err: "expected a YAML string here, got binary data instead", - line: 5, + errs: []ParseError{ + { + Err: errors.New("expected a YAML string here, got binary data instead"), + Line: 5, + }, + }, }, { content: ` @@ -273,8 +366,12 @@ groups: rules: - labels: !!binary "SGVsbG8sIFdvcmxkIQ==" `, - err: "YAML scalar value is not allowed here, expected a YAML mapping", - line: 5, + errs: []ParseError{ + { + Err: errors.New("YAML scalar value is not allowed here, expected a YAML mapping"), + Line: 5, + }, + }, }, { content: ` @@ -294,6 +391,17 @@ groups: }, } + cmpErrorText := cmp.Comparer(func(x, y interface{}) bool { + xe := x.(error) + ye := y.(error) + return xe.Error() == ye.Error() + }) + sameErrorText := cmp.FilterValues(func(x, y interface{}) bool { + _, xe := x.(error) + _, ye := y.(error) + return xe && ye + }, cmpErrorText) + for i, tc := range testCases { t.Run(strconv.Itoa(i+1), func(t *testing.T) { t.Logf("\n--- Content ---%s--- END ---", tc.content) @@ -307,15 +415,10 @@ groups: break } - err := validateRuleFile(&doc) - if tc.err == "" { - require.NoError(t, err) - } else { - require.Error(t, err) - var se StrictError - require.ErrorAs(t, err, &se) - require.EqualError(t, se.Err, tc.err) - require.Equal(t, tc.line, se.Line) + errs := validateRuleFile(&doc) + if diff := cmp.Diff(tc.errs, errs, sameErrorText); diff != "" { + t.Errorf("validateRuleFile() returned wrong output (-want +got):\n%s", diff) + return } } }) diff --git a/internal/reporter/bitbucket_test.go b/internal/reporter/bitbucket_test.go index c7a9b579..51dc4a6e 100644 --- a/internal/reporter/bitbucket_test.go +++ b/internal/reporter/bitbucket_test.go @@ -42,7 +42,7 @@ func TestBitBucketReporter(t *testing.T) { } p := parser.NewParser(false) - mockRules, _ := p.Parse([]byte(` + mockRules := p.Parse([]byte(` - record: target is down expr: up == 0 - record: sum errors diff --git a/internal/reporter/comments_test.go b/internal/reporter/comments_test.go index 48e34f9a..48d78101 100644 --- a/internal/reporter/comments_test.go +++ b/internal/reporter/comments_test.go @@ -67,7 +67,7 @@ func (tc testCommenter) IsEqual(e ExistingCommentV2, p PendingCommentV2) bool { func TestCommenter(t *testing.T) { p := parser.NewParser(false) - mockRules, _ := p.Parse([]byte(` + mockRules := p.Parse([]byte(` - record: target is down expr: up == 0 - record: sum errors diff --git a/internal/reporter/github_test.go b/internal/reporter/github_test.go index eb7ce0e8..beb80e27 100644 --- a/internal/reporter/github_test.go +++ b/internal/reporter/github_test.go @@ -38,7 +38,7 @@ func TestGithubReporter(t *testing.T) { } p := parser.NewParser(false) - mockRules, _ := p.Parse([]byte(` + mockRules := p.Parse([]byte(` - record: target is down expr: up == 0 - record: sum errors diff --git a/internal/reporter/gitlab_test.go b/internal/reporter/gitlab_test.go index 55ebffb6..52d3bdfa 100644 --- a/internal/reporter/gitlab_test.go +++ b/internal/reporter/gitlab_test.go @@ -50,7 +50,7 @@ func TestGitLabReporter(t *testing.T) { } p := parser.NewParser(false) - mockRules, _ := p.Parse([]byte(` + mockRules := p.Parse([]byte(` - record: target is down expr: up == 0 - record: sum errors diff --git a/internal/reporter/teamcity_test.go b/internal/reporter/teamcity_test.go index 2f4aa7ae..007d2947 100644 --- a/internal/reporter/teamcity_test.go +++ b/internal/reporter/teamcity_test.go @@ -23,7 +23,7 @@ func TestTeamCityReporter(t *testing.T) { } p := parser.NewParser(false) - mockRules, _ := p.Parse([]byte(` + mockRules := p.Parse([]byte(` - record: target is down expr: up == 0 `))