From 86ffc836b2b96a39b16f4f020f84280e92e4ed2c Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Tue, 27 Aug 2024 09:43:38 +0100 Subject: [PATCH] Refactor error check --- cmd/pint/tests/0187_ci_noop_yaml_parse.txt | 39 +++++++++++++++++ internal/checks/error.go | 51 ++++++++++++---------- internal/config/config.go | 5 ++- 3 files changed, 69 insertions(+), 26 deletions(-) create mode 100644 cmd/pint/tests/0187_ci_noop_yaml_parse.txt diff --git a/cmd/pint/tests/0187_ci_noop_yaml_parse.txt b/cmd/pint/tests/0187_ci_noop_yaml_parse.txt new file mode 100644 index 00000000..c271936e --- /dev/null +++ b/cmd/pint/tests/0187_ci_noop_yaml_parse.txt @@ -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"] +} diff --git a/internal/checks/error.go b/internal/checks/error.go index fa54e7b2..22c22a09 100644 --- a/internal/checks/error.go +++ b/internal/checks/error.go @@ -12,8 +12,6 @@ import ( ) const ( - ErrorCheckName = "error" - yamlParseReporter = "yaml/parse" ignoreFileReporter = "ignore/file" pintCommentReporter = "pint/comment" @@ -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 { @@ -45,21 +45,26 @@ 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, @@ -67,11 +72,11 @@ func (c ErrorCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, 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, @@ -79,31 +84,31 @@ func (c ErrorCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, 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, @@ -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 } diff --git a/internal/config/config.go b/internal/config/config.go index 83cff768..641b11f3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -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) {