Skip to content

Commit

Permalink
Refactor error check
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Aug 27, 2024
1 parent 6630e3d commit 86ffc83
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 26 deletions.
39 changes: 39 additions & 0 deletions cmd/pint/tests/0187_ci_noop_yaml_parse.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
mkdir testrepo
cd testrepo
exec git init --initial-branch=main .

cp ../src/rules.yml rules.yml
cp ../src/.pint.hcl .
env GIT_AUTHOR_NAME=pint
env GIT_AUTHOR_EMAIL=pint@example.com
env GIT_COMMITTER_NAME=pint
env GIT_COMMITTER_EMAIL=pint@example.com
exec git add .
exec git commit -am 'import rules and config'

exec git checkout -b v2
exec touch .keep
exec git add .keep
exec git commit -am 'v2'

pint.ok --no-color ci
! stdout .
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="error at line 1: top level field must be a groups key, got list" path=rules.yml lines=1-5
-- src/rules.yml --
- record: rule1
expr: sum(foo) by(job)
- record: rule2
expr: sum(foo)

-- src/.pint.hcl --
ci {
baseBranch = "main"
}
parser {
include = ["rules.yml"]
}
51 changes: 27 additions & 24 deletions internal/checks/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import (
)

const (
ErrorCheckName = "error"

yamlParseReporter = "yaml/parse"
ignoreFileReporter = "ignore/file"
pintCommentReporter = "pint/comment"
Expand All @@ -22,12 +20,14 @@ const (
This usually means that it's missing some required fields.`
)

func NewErrorCheck(err error) ErrorCheck {
return ErrorCheck{err: err}
func NewErrorCheck(entry discovery.Entry) ErrorCheck {
return ErrorCheck{
problem: parseRuleError(entry.Rule, entry.PathError),
}
}

type ErrorCheck struct {
err error
problem Problem
}

func (c ErrorCheck) Meta() CheckMeta {
Expand All @@ -45,65 +45,70 @@ func (c ErrorCheck) Meta() CheckMeta {
}

func (c ErrorCheck) String() string {
return ErrorCheckName
return c.problem.Reporter
}

func (c ErrorCheck) Reporter() string {
return ErrorCheckName
return c.problem.Reporter
}

func (c ErrorCheck) Check(_ context.Context, _ discovery.Path, _ parser.Rule, _ []discovery.Entry) (problems []Problem) {
problems = append(problems, c.problem)
return problems
}

func (c ErrorCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func parseRuleError(rule parser.Rule, err error) Problem {
var commentErr comments.CommentError
var ignoreErr discovery.FileIgnoreError

switch {
case errors.As(c.err, &ignoreErr):
case errors.As(err, &ignoreErr):
slog.Debug("ignore/file report", slog.Any("err", ignoreErr))
problems = append(problems, Problem{
return Problem{
Lines: parser.LineRange{
First: ignoreErr.Line,
Last: ignoreErr.Line,
},
Reporter: ignoreFileReporter,
Text: ignoreErr.Error(),
Severity: Information,
})
}

case errors.As(c.err, &commentErr):
case errors.As(err, &commentErr):
slog.Debug("invalid comment report", slog.Any("err", commentErr))
problems = append(problems, Problem{
return Problem{
Lines: parser.LineRange{
First: commentErr.Line,
Last: commentErr.Line,
},
Reporter: pintCommentReporter,
Text: "This comment is not a valid pint control comment: " + commentErr.Error(),
Severity: Warning,
})
}

case c.err != nil:
slog.Debug("yaml syntax report", slog.Any("err", c.err))
problems = append(problems, Problem{
case err != nil:
slog.Debug("yaml syntax report", slog.Any("err", err))
return Problem{
Lines: parser.LineRange{
First: 1,
Last: 1,
},
Reporter: yamlParseReporter,
Text: fmt.Sprintf("YAML parser returned an error when reading this file: `%s`.", c.err),
Text: fmt.Sprintf("YAML parser returned an error when reading this file: `%s`.", err),
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).
`,
Severity: Fatal,
})
}

case rule.Error.Err != nil:
default:
slog.Debug("rule error report", slog.Any("err", rule.Error.Err))
details := yamlDetails
if rule.Error.Details != "" {
details = rule.Error.Details
}
problems = append(problems, Problem{
return Problem{
Lines: parser.LineRange{
First: rule.Error.Line,
Last: rule.Error.Line,
Expand All @@ -112,8 +117,6 @@ If this file is a template that will be rendered into valid YAML then you can in
Text: fmt.Sprintf("This rule is not a valid Prometheus rule: `%s`.", rule.Error.Err.Error()),
Details: details,
Severity: Fatal,
})
}
}

return problems
}
5 changes: 3 additions & 2 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,11 @@ func (cfg *Config) GetChecksForEntry(ctx context.Context, gen *PrometheusGenerat
proms := gen.ServersForPath(entry.Path.Name)

if entry.PathError != nil || entry.Rule.Error.Err != nil {
check := checks.NewErrorCheck(entry)
enabled = parsedRule{
match: defaultMatch,
name: checks.ErrorCheckName,
check: checks.NewErrorCheck(entry.PathError),
name: check.Reporter(),
check: check,
}.entryChecks(ctx, cfg.Checks.Enabled, cfg.Checks.Disabled, enabled, entry)
} else {
for _, pr := range baseRules(proms, defaultMatch) {
Expand Down

0 comments on commit 86ffc83

Please sign in to comment.