Skip to content

Commit

Permalink
Get rid of rulefmt.Parse
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Jun 28, 2024
1 parent ef4d479 commit 3360d5b
Show file tree
Hide file tree
Showing 36 changed files with 988 additions and 192 deletions.
1 change: 1 addition & 0 deletions .github/spellcheck/wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ promql
PromQL
PRs
prymitive
rulefmt
samber
SNI
symlink
Expand Down
60 changes: 23 additions & 37 deletions cmd/pint/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,9 @@ import (
"errors"
"fmt"
"log/slog"
"regexp"
"strconv"
"sync"
"time"

"github.com/prometheus/prometheus/model/rulefmt"
"go.uber.org/atomic"

"github.com/cloudflare/pint/internal/checks"
Expand All @@ -22,42 +19,12 @@ import (
"github.com/cloudflare/pint/internal/reporter"
)

var (
yamlErrRe = regexp.MustCompile("^yaml: line (.+): (.+)")
yamlUnmarshalErrRe = regexp.MustCompile("^yaml: unmarshal errors:\n line (.+): (.+)")
rulefmtGroupRe = regexp.MustCompile("^([0-9]+):[0-9]+: group \".+\", rule [0-9]+, (.+)")
rulefmtGroupnameRe = regexp.MustCompile("^([0-9]+):[0-9]+: (groupname: .+)")
)

const (
yamlParseReporter = "yaml/parse"
ignoreFileReporter = "ignore/file"
pintCommentReporter = "pint/comment"
)

func tryDecodingYamlError(err error) (l int, s string) {
s = err.Error()

werr := &rulefmt.WrappedError{}
if errors.As(err, &werr) {
if uerr := werr.Unwrap(); uerr != nil {
s = uerr.Error()
}
}

for _, re := range []*regexp.Regexp{yamlErrRe, yamlUnmarshalErrRe, rulefmtGroupRe, rulefmtGroupnameRe} {
parts := re.FindStringSubmatch(err.Error())
if len(parts) > 2 {
line, err2 := strconv.Atoi(parts[1])
if err2 != nil || line <= 0 {
return 1, s
}
return line, parts[2]
}
}
return 1, s
}

func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.PrometheusGenerator, cfg config.Config, entries []discovery.Entry) (summary reporter.Summary, err error) {
if isOffline {
slog.Info("Offline mode, skipping Prometheus discovery")
Expand Down Expand Up @@ -186,7 +153,27 @@ 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{
Expand Down Expand Up @@ -224,7 +211,6 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
Owner: job.entry.Owner,
}
case job.entry.PathError != nil:
line, e := tryDecodingYamlError(job.entry.PathError)
results <- reporter.Report{
Path: discovery.Path{
Name: job.entry.Path.Name,
Expand All @@ -233,11 +219,11 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
ModifiedLines: job.entry.ModifiedLines,
Problem: checks.Problem{
Lines: parser.LineRange{
First: line,
Last: line,
First: 1,
Last: 1,
},
Reporter: yamlParseReporter,
Text: fmt.Sprintf("YAML parser returned an error when reading this file: `%s`.", e),
Text: fmt.Sprintf("YAML parser returned an error when reading this file: `%s`.", job.entry.PathError),
Details: `pint cannot read this file because YAML parser returned an error.
This usually means that you have an indention error or the file doesn't have the YAML structure required by Prometheus for [recording](https://prometheus.io/docs/prometheus/latest/configuration/recording_rules/) and [alerting](https://prometheus.io/docs/prometheus/latest/configuration/alerting_rules/) rules.
If this file is a template that will be rendered into valid YAML then you can instruct pint to ignore some lines using comments, see [pint docs](https://cloudflare.github.io/pint/ignoring.html).
Expand Down
4 changes: 2 additions & 2 deletions cmd/pint/tests/0004_fail_invalid_yaml.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ 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=ERROR msg="Failed to parse file content" err="yaml: line 4: did not find expected key" path=rules/bad.yaml lines=1-7
rules/bad.yaml:4 Fatal: YAML parser returned an error when reading this file: `did not find expected key`. (yaml/parse)
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)
4 |

rules/ok.yml:5 Fatal: Prometheus failed to parse the query with this PromQL error: unclosed left bracket. (promql/syntax)
Expand Down
4 changes: 2 additions & 2 deletions cmd/pint/tests/0010_syntax_check.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ 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=ERROR msg="Failed to parse file content" err="yaml: line 6: did not find expected '-' indicator" path=rules/1.yaml lines=1-12
rules/1.yaml:6 Fatal: YAML parser returned an error when reading this file: `did not find expected '-' indicator`. (yaml/parse)
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)
6 |

level=INFO msg="Problems found" Fatal=1
Expand Down
3 changes: 2 additions & 1 deletion cmd/pint/tests/0067_relaxed.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ cmp stderr stderr.txt
-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/strict.yml:2 Fatal: YAML parser returned an error when reading this file: `cannot unmarshal !!seq into rulefmt.RuleGroups`. (yaml/parse)
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)
2 | - alert: No Owner

level=INFO msg="Problems found" Fatal=1
Expand Down
1 change: 1 addition & 0 deletions cmd/pint/tests/0071_ci_owner.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ groups:

-- src/.pint.hcl --
ci {
include = [".+.yml"]
baseBranch = "main"
}
repository {
Expand Down
3 changes: 2 additions & 1 deletion cmd/pint/tests/0074_strict_error.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ cmp stderr stderr.txt

-- stderr.txt --
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/strict.yml:2 Fatal: YAML parser returned an error when reading this file: `field alert not found in type rulefmt.RuleGroup`. (yaml/parse)
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)
2 | - alert: Conntrack_Table_Almost_Full

level=INFO msg="Problems found" Fatal=1
Expand Down
1 change: 1 addition & 0 deletions cmd/pint/tests/0076_ci_group_errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ groups:
-- src/.pint.hcl --
ci {
baseBranch = "main"
include = [".+.yml"]
}
repository {
bitbucket {
Expand Down
3 changes: 2 additions & 1 deletion cmd/pint/tests/0078_repeated_group.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ cmp stderr stderr.txt

-- stderr.txt --
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/strict.yml:4 Fatal: YAML parser returned an error when reading this file: `groupname: "foo" is repeated in the same file`. (yaml/parse)
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)
4 | - name: foo

level=INFO msg="Problems found" Fatal=1
Expand Down
17 changes: 3 additions & 14 deletions cmd/pint/tests/0086_rulefmt_ignored_errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,11 @@ cmp stderr stderr.txt

-- stderr.txt --
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/strict.yml:4 Fatal: This rule is not a valid Prometheus rule: `incomplete rule, no alert or record key`. (yaml/parse)
4 | - expr: MissingAlertOrRecord

rules/strict.yml:7 Fatal: This rule is not a valid Prometheus rule: `expr value cannot be empty`. (yaml/parse)
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)
7 | expr:

rules/strict.yml:10 Fatal: Prometheus failed to parse the query with this PromQL error: unknown function with name "sumz". (promql/syntax)
10 | expr: sumz(0)

rules/strict.yml:15 Fatal: Template failed to parse with this error: `function "bogus" not defined`. (alerts/template)
15 | dashboard: '{{ bogus }}'

rules/strict.yml:20 Fatal: Template failed to parse with this error: `function "bogus" not defined`. (alerts/template)
20 | dashboard: '{{ bogus }}'

level=INFO msg="Problems found" Fatal=5
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 --
groups:
Expand Down
1 change: 1 addition & 0 deletions cmd/pint/tests/0123_ci_owner_allowed.txt
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ owners {
}
ci {
baseBranch = "main"
include = [".+.yml"]
}
repository {
bitbucket {
Expand Down
20 changes: 3 additions & 17 deletions cmd/pint/tests/0141_empty_keys.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,11 @@ cmp stderr stderr.txt

-- stderr.txt --
level=INFO msg="Finding all rules to check" paths=["rules.yml"]
rules.yml:4 Fatal: This rule is not a valid Prometheus rule: `record value cannot be empty`. (yaml/parse)
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:6 Fatal: This rule is not a valid Prometheus rule: `record value cannot be empty`. (yaml/parse)
6 | - record:

rules.yml:9 Fatal: This rule is not a valid Prometheus rule: `expr value cannot be empty`. (yaml/parse)
9 | expr:

rules.yml:10 Fatal: This rule is not a valid Prometheus rule: `alert value cannot be empty`. (yaml/parse)
10 | - alert:

rules.yml:12 Fatal: This rule is not a valid Prometheus rule: `alert value cannot be empty`. (yaml/parse)
12 | - alert:

rules.yml:15 Fatal: This rule is not a valid Prometheus rule: `expr value cannot be empty`. (yaml/parse)
15 | expr:

level=INFO msg="Problems found" Fatal=6
level=INFO msg="Problems found" Fatal=1
level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher"
-- rules.yml --
groups:
Expand Down
1 change: 1 addition & 0 deletions cmd/pint/tests/0160_ci_comment_edit.txt
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ groups:
-- src/.pint.hcl --
ci {
baseBranch = "main"
include = [".+.yml"]
}
prometheus "prom" {
uri = "http://127.0.0.1:7160"
Expand Down
1 change: 1 addition & 0 deletions cmd/pint/tests/0161_ci_move_files.txt
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ groups:
-- src/.pint.hcl --
ci {
baseBranch = "main"
include = [".+.yml"]
}
prometheus "prom1" {
uri = "http://127.0.0.1:7161/1"
Expand Down
1 change: 1 addition & 0 deletions cmd/pint/tests/0162_ci_deleted_dependency.txt
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ groups:
-- src/.pint.hcl --
ci {
baseBranch = "main"
include = [".+.yml"]
}
prometheus "prom" {
uri = "http://127.0.0.1:7162"
Expand Down
1 change: 1 addition & 0 deletions cmd/pint/tests/0167_rule_duplicate_symlink.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ 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 --
Expand Down
1 change: 1 addition & 0 deletions cmd/pint/tests/0173_rule_duplicate_move.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ 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=[]
Expand Down
4 changes: 4 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
- Don't suggest using `humanize` when alert template is already using printf on format the `$value`.
- Fixed git history parsing when running `pint ci` on a branch that include merge commits.

### Changed

- Refactored YAML syntax checks to avoid using [rulefmt.Parse](https://pkg.go.dev/github.com/prometheus/prometheus@v0.53.0/model/rulefmt#Parse) and effectively parsing rules twice. Some error messages will have different formatting.

## v0.60.0

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func runTests(t *testing.T, testCases []checkTest) {
}

func parseContent(content string) (entries []discovery.Entry, err error) {
p := parser.NewParser()
p := parser.NewParser(false)
rules, err := p.Parse([]byte(content))
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestTemplatedRegexpExpand(t *testing.T) {
}

func newMustRule(content string) parser.Rule {
p := parser.NewParser()
p := parser.NewParser(false)
rules, err := p.Parse([]byte(content))
if err != nil {
panic(err)
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func TestSetDisabledChecks(t *testing.T) {
}

func newRule(t *testing.T, content string) parser.Rule {
p := parser.NewParser()
p := parser.NewParser(false)
rules, err := p.Parse([]byte(content))
if err != nil {
t.Error(err)
Expand Down
Loading

0 comments on commit 3360d5b

Please sign in to comment.