From 8058b3fa892595c98823e7629f4ef6ccfa312df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Jim=C3=A9nez=20S=C3=A1nchez?= Date: Wed, 4 Dec 2024 09:23:46 +0100 Subject: [PATCH] [Prototyping] load linter config from repository (#97325) --- .../apis/provisioning/lint/dashboard.go | 27 ++++++++++ pkg/registry/apis/provisioning/lint/lint.go | 6 +++ pkg/registry/apis/provisioning/register.go | 5 +- .../apis/provisioning/repository/github.go | 50 ++++++++++++------- .../apis/provisioning/resources/parser.go | 1 + 5 files changed, 69 insertions(+), 20 deletions(-) diff --git a/pkg/registry/apis/provisioning/lint/dashboard.go b/pkg/registry/apis/provisioning/lint/dashboard.go index 9a92c592146..97a4d9e0139 100644 --- a/pkg/registry/apis/provisioning/lint/dashboard.go +++ b/pkg/registry/apis/provisioning/lint/dashboard.go @@ -8,10 +8,35 @@ import ( "github.com/grafana/dashboard-linter/lint" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" + "gopkg.in/yaml.v3" ) +type DashboardLinterFactory struct{} + +func NewDashboardLinterFactory() *DashboardLinterFactory { + return &DashboardLinterFactory{} +} + +func (f *DashboardLinterFactory) New() (Linter, error) { + return &DashboardLinter{rules: lint.NewRuleSet()}, nil +} + +func (f *DashboardLinterFactory) NewFromConfig(cfg []byte) (Linter, error) { + var lintCfg lint.ConfigurationFile + if err := yaml.Unmarshal(cfg, &lintCfg); err != nil { + return nil, fmt.Errorf("unmarshal linter config: %w", err) + } + + return &DashboardLinter{rules: lint.NewRuleSet(), cfg: lintCfg}, nil +} + +func (f *DashboardLinterFactory) ConfigPath() string { + return ".dashboard-lint" +} + type DashboardLinter struct { rules lint.RuleSet + cfg lint.ConfigurationFile } // FIXME: what would be a good place to put all the schema validation? @@ -40,6 +65,8 @@ func (l *DashboardLinter) Lint(ctx context.Context, fileData []byte) ([]provisio return nil, fmt.Errorf("failed to lint dashboard: %v", err) } + results.Configure(&l.cfg) + byRule := results.ByRule() rules := make([]string, 0, len(byRule)) for r := range byRule { diff --git a/pkg/registry/apis/provisioning/lint/lint.go b/pkg/registry/apis/provisioning/lint/lint.go index 88bf7e29b8c..3a269bf6f7b 100644 --- a/pkg/registry/apis/provisioning/lint/lint.go +++ b/pkg/registry/apis/provisioning/lint/lint.go @@ -9,3 +9,9 @@ import ( type Linter interface { Lint(ctx context.Context, data []byte) ([]provisioning.LintIssue, error) } + +type LinterFactory interface { + ConfigPath() string + NewFromConfig(cfg []byte) (Linter, error) + New() (Linter, error) +} diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 977208b8409..45128e11786 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -242,15 +242,14 @@ func (b *ProvisioningAPIBuilder) asRepository(ctx context.Context, obj runtime.O return nil, fmt.Errorf("invalid base URL: %w", err) } - linter := lint.NewDashboardLinter() - // HACK: replace resource name placeholder in the webhook URL with the actual resource name // In case the name was automatically generated by k8s if r.Spec.GitHub.WebhookURL != "" { r.Spec.GitHub.WebhookURL = strings.ReplaceAll(r.Spec.GitHub.WebhookURL, resourceNamePlaceholder, r.GetName()) } - return repository.NewGitHub(ctx, r, b.ghFactory, baseURL, linter, b.renderer), nil + linterFactory := lint.NewDashboardLinterFactory() + return repository.NewGitHub(ctx, r, b.ghFactory, baseURL, linterFactory, b.renderer), nil case provisioning.S3RepositoryType: return repository.NewS3(r), nil default: diff --git a/pkg/registry/apis/provisioning/repository/github.go b/pkg/registry/apis/provisioning/repository/github.go index c59affc147b..a639c8db9c8 100644 --- a/pkg/registry/apis/provisioning/repository/github.go +++ b/pkg/registry/apis/provisioning/repository/github.go @@ -25,12 +25,12 @@ import ( ) type githubRepository struct { - logger *slog.Logger - config *provisioning.Repository - gh pgh.Client - baseURL *url.URL - linter lint.Linter - renderer PreviewRenderer + logger *slog.Logger + config *provisioning.Repository + gh pgh.Client + baseURL *url.URL + linterFactory lint.LinterFactory + renderer PreviewRenderer } var _ Repository = (*githubRepository)(nil) @@ -40,16 +40,16 @@ func NewGitHub( config *provisioning.Repository, factory pgh.ClientFactory, baseURL *url.URL, - linter lint.Linter, + linterFactory lint.LinterFactory, renderer PreviewRenderer, ) *githubRepository { return &githubRepository{ - config: config, - logger: slog.Default().With("logger", "github-repository"), - gh: factory.New(ctx, config.Spec.GitHub.Token), - baseURL: baseURL, - linter: linter, - renderer: renderer, + config: config, + logger: slog.Default().With("logger", "github-repository"), + gh: factory.New(ctx, config.Spec.GitHub.Token), + baseURL: baseURL, + linterFactory: linterFactory, + renderer: renderer, } } @@ -681,7 +681,7 @@ func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog. var lintDashboardIssuesTemplate = `Hey there! 👋 Grafana found some linting issues in this dashboard you may want to check: {{ range .}} -{{ if eq .Severity 4 }}❌{{ else if eq .Severity 3 }}⚠️ {{ end }} [dashboard-linter/{{ .Rule }}](https://github.com/grafana/dashboard-linter/blob/main/docs/rules/{{ .Rule }}.md): {{ .Message }}. +{{ if eq .Severity "error" }}❌{{ else if eq .Severity "warning" }}⚠️ {{ end }} [dashboard-linter/{{ .Rule }}](https://github.com/grafana/dashboard-linter/blob/main/docs/rules/{{ .Rule }}.md): {{ .Message }}. {{- end }} ` @@ -695,6 +695,22 @@ func (r *githubRepository) lintPullRequest(ctx context.Context, logger *slog.Log logger.InfoContext(ctx, "lint pull request") + // Load linter config + cfg, err := r.Read(ctx, logger, r.linterFactory.ConfigPath(), ref) + switch { + case err == nil: + logger.InfoContext(ctx, "linter config found", "config", string(cfg.Data)) + case errors.Is(err, pgh.ErrResourceNotFound): + logger.InfoContext(ctx, "no linter config found") + default: + return fmt.Errorf("read linter config: %w", err) + } + + linter, err := r.linterFactory.NewFromConfig(cfg.Data) + if err != nil { + return fmt.Errorf("create linter: %w", err) + } + // Clear all previous comments because we don't know if the files have changed if err := r.gh.ClearAllPullRequestFileComments(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, prNumber); err != nil { return fmt.Errorf("clear pull request comments: %w", err) @@ -706,7 +722,7 @@ func (r *githubRepository) lintPullRequest(ctx context.Context, logger *slog.Log } logger := logger.With("file", resource.Path) - if err := r.lintPullRequestFile(ctx, logger, prNumber, ref, resource); err != nil { + if err := r.lintPullRequestFile(ctx, logger, prNumber, ref, resource, linter); err != nil { logger.ErrorContext(ctx, "failed to lint file", "error", err) } } @@ -715,8 +731,8 @@ func (r *githubRepository) lintPullRequest(ctx context.Context, logger *slog.Log } // lintPullRequestFile lints a file and comments the issues found. -func (r *githubRepository) lintPullRequestFile(ctx context.Context, logger *slog.Logger, prNumber int, ref string, resource changedResource) error { - issues, err := r.linter.Lint(ctx, resource.Data) +func (r *githubRepository) lintPullRequestFile(ctx context.Context, logger *slog.Logger, prNumber int, ref string, resource changedResource, linter lint.Linter) error { + issues, err := linter.Lint(ctx, resource.Data) if err != nil { return fmt.Errorf("lint file: %w", err) } diff --git a/pkg/registry/apis/provisioning/resources/parser.go b/pkg/registry/apis/provisioning/resources/parser.go index 5c02d082b0e..7017afc511c 100644 --- a/pkg/registry/apis/provisioning/resources/parser.go +++ b/pkg/registry/apis/provisioning/resources/parser.go @@ -148,6 +148,7 @@ func (r *FileParser) Parse(ctx context.Context, logger *slog.Logger, info *repos } } linter := lint.NewDashboardLinter() + // TODO: pass the config file parsed.Lint, err = linter.Lint(ctx, raw) if err != nil { parsed.Errors = append(parsed.Errors, err)