snap

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,

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)

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

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

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

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

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

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"

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 --

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=[]

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
 }
 

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,

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,15 +559,15 @@ 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,
 		},
 		{
 			description: "#2 {ALERTS=...} missing",
 			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{
 					{

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