diff --git a/go.mod b/go.mod index 4fa8986d9c2..63ddccd84c2 100644 --- a/go.mod +++ b/go.mod @@ -480,6 +480,8 @@ require github.com/openzipkin/zipkin-go v0.4.3 // @grafana/oss-big-tent require github.com/google/go-github/v66 v66.0.0 // @grafana/grafana-app-platform-squad +require github.com/grafana/dashboard-linter v0.0.0-20241106223805-1e7999311752 // @grafana/grafana-app-platform-squad + require ( cloud.google.com/go/longrunning v0.6.0 // indirect github.com/at-wat/mqtt-go v0.19.4 // indirect @@ -501,6 +503,7 @@ require ( github.com/emirpasic/gods v1.18.1 // indirect github.com/gammazero/deque v0.2.1 // indirect github.com/golang/geo v0.0.0-20210211234256-740aa86cb551 // indirect + github.com/grafana/grafana-foundation-sdk/go v0.0.0-20241101005901-83e3491f2a70 // indirect github.com/grafana/jsonparser v0.0.0-20240425183733-ea80629e1a32 // indirect github.com/grafana/loki/pkg/push v0.0.0-20231124142027-e52380921608 // indirect github.com/grafana/sqlds/v4 v4.1.0 // indirect diff --git a/go.sum b/go.sum index 714d9a6cfe3..643eea308ff 100644 --- a/go.sum +++ b/go.sum @@ -2302,6 +2302,8 @@ github.com/grafana/cue v0.0.0-20230926092038-971951014e3f h1:TmYAMnqg3d5KYEAaT6P github.com/grafana/cue v0.0.0-20230926092038-971951014e3f/go.mod h1:okjJBHFQFer+a41sAe2SaGm1glWS8oEb6CmJvn5Zdws= github.com/grafana/cuetsy v0.1.11 h1:I3IwBhF+UaQxRM79HnImtrAn8REGdb5M3+C4QrYHoWk= github.com/grafana/cuetsy v0.1.11/go.mod h1:Ix97+CPD8ws9oSSxR3/Lf4ahU1I4Np83kjJmDVnLZvc= +github.com/grafana/dashboard-linter v0.0.0-20241106223805-1e7999311752 h1:pIEPQPua6kCkMib/nOn//WLgtiA4urf2RPBViSF6CcE= +github.com/grafana/dashboard-linter v0.0.0-20241106223805-1e7999311752/go.mod h1:QRxC8NNAbpGx2vz9/XckJcWXtHU3My57byeudPrf0WU= github.com/grafana/dataplane/examples v0.0.1 h1:K9M5glueWyLoL4//H+EtTQq16lXuHLmOhb6DjSCahzA= github.com/grafana/dataplane/examples v0.0.1/go.mod h1:h5YwY8s407/17XF5/dS8XrUtsTVV2RnuW8+m1Mp46mg= github.com/grafana/dataplane/sdata v0.0.9 h1:AGL1LZnCUG4MnQtnWpBPbQ8ZpptaZs14w6kE/MWfg7s= @@ -2322,6 +2324,8 @@ github.com/grafana/grafana-azure-sdk-go/v2 v2.1.2 h1:fV6IgVtViXcYZ4VqTAMuVBTLuGA github.com/grafana/grafana-azure-sdk-go/v2 v2.1.2/go.mod h1:Cbh94bfL5o6mUSaHFiOkx4r4CRKlo/DJLx4dPL8XrE0= github.com/grafana/grafana-cloud-migration-snapshot v1.3.0 h1:F0O9eTy4jHjEd1Z3/qIza2GdY7PYpTddUeaq9p3NKGU= github.com/grafana/grafana-cloud-migration-snapshot v1.3.0/go.mod h1:bd6Cm06EK0MzRO5ahUpbDz1SxNOKu+fzladbaRPHZPY= +github.com/grafana/grafana-foundation-sdk/go v0.0.0-20241101005901-83e3491f2a70 h1:69GI3KsF851YnwYp6zHdsskcGp3ZnGsWc+ve8vMp1mc= +github.com/grafana/grafana-foundation-sdk/go v0.0.0-20241101005901-83e3491f2a70/go.mod h1:WtWosval1KCZP9BGa42b8aVoJmVXSg0EvQXi9LDSVZQ= github.com/grafana/grafana-google-sdk-go v0.1.0 h1:LKGY8z2DSxKjYfr2flZsWgTRTZ6HGQbTqewE3JvRaNA= github.com/grafana/grafana-google-sdk-go v0.1.0/go.mod h1:Vo2TKWfDVmNTELBUM+3lkrZvFtBws0qSZdXhQxRdJrE= github.com/grafana/grafana-openapi-client-go v0.0.0-20231213163343-bd475d63fb79 h1:r+mU5bGMzcXCRVAuOrTn54S80qbfVkvTdUJZfSfTNbs= diff --git a/go.work.sum b/go.work.sum index 6925ec372b5..7da4d90f98d 100644 --- a/go.work.sum +++ b/go.work.sum @@ -990,8 +990,6 @@ github.com/grafana/alerting v0.0.0-20240917171353-6c25eb6eff10/go.mod h1:GMLi6d0 github.com/grafana/cloudflare-go v0.0.0-20230110200409-c627cf6792f2 h1:qhugDMdQ4Vp68H0tp/0iN17DM2ehRo1rLEdOFe/gB8I= github.com/grafana/cloudflare-go v0.0.0-20230110200409-c627cf6792f2/go.mod h1:w/aiO1POVIeXUQyl0VQSZjl5OAGDTL5aX+4v0RA1tcw= github.com/grafana/cog v0.0.4/go.mod h1:lzetOuhGUl/JaSACiJoHvBokf9/fS6PEFaWZvnQu2vs= -github.com/grafana/cog v0.0.5 h1:BCa+10i3KvV+KMSQuxlN1DS9cZEwN+EAFc7ZmXqHxQE= -github.com/grafana/cog v0.0.5/go.mod h1:lzetOuhGUl/JaSACiJoHvBokf9/fS6PEFaWZvnQu2vs= github.com/grafana/cuetsy v0.1.10/go.mod h1:Ix97+CPD8ws9oSSxR3/Lf4ahU1I4Np83kjJmDVnLZvc= github.com/grafana/go-gelf/v2 v2.0.1 h1:BOChP0h/jLeD+7F9mL7tq10xVkDG15he3T1zHuQaWak= github.com/grafana/go-gelf/v2 v2.0.1/go.mod h1:lexHie0xzYGwCgiRGcvZ723bSNyNI8ZRD4s0CLobh90= @@ -1519,6 +1517,8 @@ github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c h1:u40Z8hqBAAQyv+vATcGgV github.com/xdg/stringprep v1.0.0 h1:d9X0esnoa3dFsV0FG35rAT0RIhYFlPq7MiP+DW89La0= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= +github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb h1:zGWFAtiMcyryUHoUjUJX0/lt1H2+i2Ka2n+D3DImSNo= +github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74= @@ -1545,6 +1545,8 @@ github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= +github.com/zeitlinger/conflate v0.0.0-20230622100834-279724abda8c h1:PtECnCzGLw8MuQ0tmPRaN5c95ZfNTFZOobvgC6A83zk= +github.com/zeitlinger/conflate v0.0.0-20230622100834-279724abda8c/go.mod h1:KsJBt1tGR0Q7u+3T7CLN+zITAI06GiXVi/cgP9Xrpb8= github.com/zenazn/goji v1.0.1 h1:4lbD8Mx2h7IvloP7r2C0D6ltZP6Ufip8Hn0wmSK5LR8= github.com/zenazn/goji v1.0.1/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q= github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs= diff --git a/pkg/apis/provisioning/v0alpha1/types.go b/pkg/apis/provisioning/v0alpha1/types.go index 1d317263562..afdf3b8455a 100644 --- a/pkg/apis/provisioning/v0alpha1/types.go +++ b/pkg/apis/provisioning/v0alpha1/types.go @@ -63,6 +63,9 @@ type GitHubRepositoryConfig struct { // By default, this is false (i.e. we will not create previews). // This option is a no-op if BranchWorkflow is `false` or default. GenerateDashboardPreviews bool `json:"generateDashboardPreviews,omitempty"` + + // PullRequestLinter enables the dashboard linter for this repository in Pull Requests + PullRequestLinter bool `json:"pullRequestLinter,omitempty"` } // RepositoryType defines the types of Repository diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index be8be00ca70..5a53be24efd 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -223,6 +223,13 @@ func schema_pkg_apis_provisioning_v0alpha1_GitHubRepositoryConfig(ref common.Ref Format: "", }, }, + "pullRequestLinter": { + SchemaProps: spec.SchemaProps{ + Description: "PullRequestLinter enables the dashboard linter for this repository in Pull Requests", + Type: []string{"boolean"}, + Format: "", + }, + }, }, }, }, diff --git a/pkg/registry/apis/provisioning/lint/dashboard.go b/pkg/registry/apis/provisioning/lint/dashboard.go new file mode 100644 index 00000000000..200567e9f26 --- /dev/null +++ b/pkg/registry/apis/provisioning/lint/dashboard.go @@ -0,0 +1,67 @@ +package lint + +import ( + "context" + "encoding/json" + "fmt" + "sort" + + "github.com/grafana/dashboard-linter/lint" +) + +type DashboardLinter struct { + rules lint.RuleSet +} + +// FIXME: what would be a good place to put all the schema validation? +type specData struct { + Spec json.RawMessage `json:"spec"` +} + +func NewDashboardLinter() *DashboardLinter { + // TODO: read rules from configuration and pass to the lint function + return &DashboardLinter{rules: lint.NewRuleSet()} +} + +func (l *DashboardLinter) Lint(ctx context.Context, fileData []byte) ([]Issue, error) { + var data specData + if err := json.Unmarshal(fileData, &data); err != nil { + return nil, fmt.Errorf("unmarshal file data into spec: %w", err) + } + + dashboard, err := lint.NewDashboard(data.Spec) + if err != nil { + return nil, fmt.Errorf("failed to parse dashboard with linter: %v", err) + } + + results, err := l.rules.Lint([]lint.Dashboard{dashboard}) + if err != nil { + return nil, fmt.Errorf("failed to lint dashboard: %v", err) + } + + byRule := results.ByRule() + rules := make([]string, 0, len(byRule)) + for r := range byRule { + rules = append(rules, r) + } + sort.Strings(rules) + + issues := make([]Issue, 0) + for _, rule := range rules { + for _, rr := range byRule[rule] { + for _, r := range rr.Result.Results { + if r.Severity != lint.Error && r.Severity != lint.Warning { + continue + } + + issues = append(issues, Issue{ + Rule: rule, + Severity: r.Severity, + Message: r.Message, + }) + } + } + } + + return issues, nil +} diff --git a/pkg/registry/apis/provisioning/lint/lint.go b/pkg/registry/apis/provisioning/lint/lint.go new file mode 100644 index 00000000000..c2b9131b8b4 --- /dev/null +++ b/pkg/registry/apis/provisioning/lint/lint.go @@ -0,0 +1,17 @@ +package lint + +import ( + "context" + + "github.com/grafana/dashboard-linter/lint" +) + +type Issue struct { + Severity lint.Severity + Rule string + Message string +} + +type Linter interface { + Lint(ctx context.Context, data []byte) ([]Issue, error) +} diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 046cb53e808..77551a8f5b7 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -30,6 +30,7 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" "github.com/grafana/grafana/pkg/registry/apis/provisioning/auth" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/lint" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" @@ -214,7 +215,8 @@ func (b *ProvisioningAPIBuilder) asRepository(ctx context.Context, obj runtime.O return nil, fmt.Errorf("invalid base URL: %w", err) } - return repository.NewGitHub(ctx, r, b.ghFactory, baseURL), nil + linter := lint.NewDashboardLinter() + return repository.NewGitHub(ctx, r, b.ghFactory, baseURL, linter), 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 ceb48771041..1492ac08de5 100644 --- a/pkg/registry/apis/provisioning/repository/github.go +++ b/pkg/registry/apis/provisioning/repository/github.go @@ -20,6 +20,7 @@ import ( "k8s.io/apiserver/pkg/registry/rest" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/lint" pgh "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" ) @@ -28,6 +29,7 @@ type githubRepository struct { config *provisioning.Repository gh pgh.Client baseURL *url.URL + linter lint.Linter } var _ Repository = (*githubRepository)(nil) @@ -37,12 +39,14 @@ func NewGitHub( config *provisioning.Repository, factory pgh.ClientFactory, baseURL *url.URL, + linter lint.Linter, ) *githubRepository { return &githubRepository{ config: config, logger: slog.Default().With("logger", "github-repository"), gh: factory.New(ctx, config.Spec.GitHub.Token), baseURL: baseURL, + linter: linter, } } @@ -484,18 +488,8 @@ func (r *githubRepository) onPushEvent(ctx context.Context, logger *slog.Logger, return nil } -const commentTemplate = `Hey there! 🎉 -Grafana spotted some changes for your resources in this pull request: - -| File Name | Type | Path | Action | Links | -|-----------|------|------|--------|-------| -{{- range .}} -| {{.Filename}} | {{.Type}} | {{.Path}} | {{.Action}} | {{if .Original}}[Original]({{.Original}}){{end}}{{if .Current}}, [Current]({{.Current}}){{end}}{{if .Preview}}, [Preview]({{.Preview}}){{end}}| -{{- end}} - -Click the preview links above to view how your changes will look and compare them with the original and current versions.` - -type commentFile struct { +// changedResource represents a resource that has changed in a pull request. +type changedResource struct { Filename string Path string Action string @@ -503,13 +497,27 @@ type commentFile struct { Original string Current string Preview string + Data []byte } // onPullRequestEvent is called when a pull request event is received // If the pull request is opened, reponed or synchronize, we read the files changed. func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog.Logger, event *github.PullRequestEvent, factory FileReplicatorFactory) error { action := event.GetAction() - r.logger.InfoContext(ctx, "processing pull request event", "number", event.GetNumber(), "action", action) + logger = logger.With("pull_request", event.GetNumber(), "action", action, "nnumber", event.GetNumber()) + logger.InfoContext( + ctx, + "processing pull request event", + "linter", + r.Config().Spec.GitHub.PullRequestLinter, + "previews", + r.Config().Spec.GitHub.GenerateDashboardPreviews, + ) + + if !r.Config().Spec.GitHub.PullRequestLinter && !r.Config().Spec.GitHub.GenerateDashboardPreviews { + logger.DebugContext(ctx, "no action required on pull request event") + return nil + } if event.GetRepo() == nil { return fmt.Errorf("missing repository in pull request event") @@ -530,7 +538,7 @@ func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog. return fmt.Errorf("list pull request files: %w", err) } - rows := make([]commentFile, 0) + changedResources := make([]changedResource, 0) baseBranch := event.GetPullRequest().GetBase().GetRef() mainBranch := r.config.Spec.GitHub.Branch @@ -542,9 +550,8 @@ func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog. prURL := event.GetPullRequest().GetHTMLURL() - // TODO: implement the real handling of the files for _, file := range files { - row := commentFile{ + resource := changedResource{ Filename: path.Base(file.GetFilename()), Path: file.GetFilename(), Action: file.GetStatus(), @@ -558,23 +565,23 @@ func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog. // reference: https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#get-a-commit switch file.GetStatus() { case "added": - row.Preview = r.previewURL(ref, path, prURL) + resource.Preview = r.previewURL(ref, path, prURL) case "modified": - row.Original = r.previewURL(baseBranch, path, prURL) - row.Current = r.previewURL(mainBranch, path, prURL) - row.Preview = r.previewURL(ref, path, prURL) + resource.Original = r.previewURL(baseBranch, path, prURL) + resource.Current = r.previewURL(mainBranch, path, prURL) + resource.Preview = r.previewURL(ref, path, prURL) case "removed": - row.Original = r.previewURL(baseBranch, path, prURL) - row.Current = r.previewURL(mainBranch, path, prURL) + resource.Original = r.previewURL(baseBranch, path, prURL) + resource.Current = r.previewURL(mainBranch, path, prURL) ref = baseBranch case "renamed": - row.Original = r.previewURL(baseBranch, file.GetPreviousFilename(), prURL) - row.Current = r.previewURL(mainBranch, file.GetPreviousFilename(), prURL) - row.Preview = r.previewURL(ref, path, prURL) + resource.Original = r.previewURL(baseBranch, file.GetPreviousFilename(), prURL) + resource.Current = r.previewURL(mainBranch, file.GetPreviousFilename(), prURL) + resource.Preview = r.previewURL(ref, path, prURL) case "changed": - row.Original = r.previewURL(baseBranch, path, prURL) - row.Current = r.previewURL(mainBranch, path, prURL) - row.Preview = r.previewURL(ref, path, prURL) + resource.Original = r.previewURL(baseBranch, path, prURL) + resource.Current = r.previewURL(mainBranch, path, prURL) + resource.Preview = r.previewURL(ref, path, prURL) case "unchanged": logger.InfoContext(ctx, "ignore unchanged file") continue @@ -589,6 +596,7 @@ func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog. continue } + // TODO: how does this validation works vs linting? ok, err := replicator.Validate(ctx, f) if err != nil { logger.ErrorContext(ctx, "failed to validate file", "error", err) @@ -599,34 +607,133 @@ func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog. continue } + resource.Data = f.Data logger.InfoContext(ctx, "resource changed") - rows = append(rows, row) + changedResources = append(changedResources, resource) } - if len(rows) == 0 { + if err := r.previewPullRequest(ctx, event.GetNumber(), changedResources); err != nil { + logger.ErrorContext(ctx, "failed to comment previews", "error", err) + } + + headSha := event.GetPullRequest().GetHead().GetSHA() + if err := r.lintPullRequest(ctx, logger, event.GetNumber(), headSha, changedResources); err != nil { + logger.ErrorContext(ctx, "failed to lint pull request resources", "error", err) + } + + return nil +} + +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 }}. +{{- end }} +` + +// lintPullRequest lints the files changed in the pull request and comments the issues found. +// The linter is disabled if the configuration does not have PullRequestLinter enabled. +// The only supported type of file to lint is a dashboard. +func (r *githubRepository) lintPullRequest(ctx context.Context, logger *slog.Logger, prNumber int, ref string, resources []changedResource) error { + if !r.Config().Spec.GitHub.PullRequestLinter { return nil } - tmpl, err := template.New("comment").Parse(commentTemplate) + logger.InfoContext(ctx, "lint pull request") + + // 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) + } + + for _, resource := range resources { + if resource.Action == "removed" || resource.Type != "dashboard" { + continue + } + + logger := logger.With("file", resource.Path) + if err := r.lintPullRequestFile(ctx, logger, prNumber, ref, resource); err != nil { + logger.ErrorContext(ctx, "failed to lint file", "error", err) + } + } + + return nil +} + +// 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) + if err != nil { + return fmt.Errorf("lint file: %w", err) + } + + if len(issues) == 0 { + return nil + } + + // FIXME: we should not be compiling this all the time + tmpl, err := template.New("comment").Parse(lintDashboardIssuesTemplate) + if err != nil { + return fmt.Errorf("parse lint comment template: %w", err) + } + + var buf bytes.Buffer + if err := tmpl.Execute(&buf, issues); err != nil { + return fmt.Errorf("execute lint comment template: %w", err) + } + + comment := pgh.FileComment{ + Content: buf.String(), + Path: resource.Path, + Position: 1, // create a top-level comment + Ref: ref, + } + + // FIXME: comment with Grafana Logo + // FIXME: comment author should be written by Grafana and not the user + if err := r.gh.CreatePullRequestFileComment(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, prNumber, comment); err != nil { + return fmt.Errorf("create pull request comment: %w", err) + } + logger.InfoContext(ctx, "lint comment created", "issues", len(issues)) + + return nil +} + +const previewsCommentTemplate = `Hey there! 🎉 +Grafana spotted some changes for your resources in this pull request: + +| File Name | Type | Path | Action | Links | +|-----------|------|------|--------|-------| +{{- range .}} +| {{.Filename}} | {{.Type}} | {{.Path}} | {{.Action}} | {{if .Original}}[Original]({{.Original}}){{end}}{{if .Current}}, [Current]({{.Current}}){{end}}{{if .Preview}}, [Preview]({{.Preview}}){{end}}| +{{- end}} + +Click the preview links above to view how your changes will look and compare them with the original and current versions.` + +func (r *githubRepository) previewPullRequest(ctx context.Context, prNumber int, resources []changedResource) error { + if !r.Config().Spec.GitHub.GenerateDashboardPreviews || len(resources) == 0 { + return nil + } + + // FIXME: we should not be compiling this all the time + tmpl, err := template.New("comment").Parse(previewsCommentTemplate) if err != nil { return fmt.Errorf("parse comment template: %w", err) } var buf bytes.Buffer - if err := tmpl.Execute(&buf, rows); err != nil { + if err := tmpl.Execute(&buf, resources); err != nil { return fmt.Errorf("execute comment template: %w", err) } comment := buf.String() - // TODO: comment with Grafana Logo - // TODO: comment author should be written by Grafana and not the user - if err := r.gh.CreatePullRequestComment(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, event.GetNumber(), comment); err != nil { + // FIXME: comment with Grafana Logo + // FIXME: comment author should be written by Grafana and not the user + if err := r.gh.CreatePullRequestComment(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, prNumber, comment); err != nil { return fmt.Errorf("create pull request comment: %w", err) } - r.logger.InfoContext(ctx, "comment created", "pull_request", event.GetNumber(), "num_changes", len(rows)) - return nil } diff --git a/pkg/registry/apis/provisioning/repository/github/client.go b/pkg/registry/apis/provisioning/repository/github/client.go index 043680ae294..56939156d1c 100644 --- a/pkg/registry/apis/provisioning/repository/github/client.go +++ b/pkg/registry/apis/provisioning/repository/github/client.go @@ -78,6 +78,8 @@ type Client interface { ListPullRequestFiles(ctx context.Context, owner, repository string, number int) ([]CommitFile, error) CreatePullRequestComment(ctx context.Context, owner, repository string, number int, body string) error + CreatePullRequestFileComment(ctx context.Context, owner, repository string, number int, comment FileComment) error + ClearAllPullRequestFileComments(ctx context.Context, owner, repository string, number int) error } type RepositoryContent interface { @@ -106,6 +108,13 @@ type CommitFile interface { GetStatus() string } +type FileComment struct { + Content string + Path string + Position int + Ref string +} + type CreateFileOptions struct { // The message of the commit. May be empty, in which case a default value is entered. Message string diff --git a/pkg/registry/apis/provisioning/repository/github/client_mock.go b/pkg/registry/apis/provisioning/repository/github/client_mock.go index d3aa09d684e..82445488f89 100644 --- a/pkg/registry/apis/provisioning/repository/github/client_mock.go +++ b/pkg/registry/apis/provisioning/repository/github/client_mock.go @@ -41,6 +41,24 @@ func (_m *MockClient) BranchExists(ctx context.Context, owner string, repository return r0, r1 } +// ClearAllPullRequestFileComments provides a mock function with given fields: ctx, owner, repository, number +func (_m *MockClient) ClearAllPullRequestFileComments(ctx context.Context, owner string, repository string, number int) error { + ret := _m.Called(ctx, owner, repository, number) + + if len(ret) == 0 { + panic("no return value specified for ClearAllPullRequestFileComments") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, int) error); ok { + r0 = rf(ctx, owner, repository, number) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // CreateBranch provides a mock function with given fields: ctx, owner, repository, sourceBranch, branchName func (_m *MockClient) CreateBranch(ctx context.Context, owner string, repository string, sourceBranch string, branchName string) error { ret := _m.Called(ctx, owner, repository, sourceBranch, branchName) @@ -95,6 +113,24 @@ func (_m *MockClient) CreatePullRequestComment(ctx context.Context, owner string return r0 } +// CreatePullRequestFileComment provides a mock function with given fields: ctx, owner, repository, number, comment +func (_m *MockClient) CreatePullRequestFileComment(ctx context.Context, owner string, repository string, number int, comment FileComment) error { + ret := _m.Called(ctx, owner, repository, number, comment) + + if len(ret) == 0 { + panic("no return value specified for CreatePullRequestFileComment") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, int, FileComment) error); ok { + r0 = rf(ctx, owner, repository, number, comment) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // CreateWebhook provides a mock function with given fields: ctx, owner, repository, cfg func (_m *MockClient) CreateWebhook(ctx context.Context, owner string, repository string, cfg WebhookConfig) error { ret := _m.Called(ctx, owner, repository, cfg) diff --git a/pkg/registry/apis/provisioning/repository/github/real.go b/pkg/registry/apis/provisioning/repository/github/real.go index ea8f08cbe28..542897a9705 100644 --- a/pkg/registry/apis/provisioning/repository/github/real.go +++ b/pkg/registry/apis/provisioning/repository/github/real.go @@ -344,6 +344,55 @@ func (r *realImpl) CreatePullRequestComment(ctx context.Context, owner, reposito return nil } +func (r *realImpl) CreatePullRequestFileComment(ctx context.Context, owner, repository string, number int, comment FileComment) error { + commentRequest := &github.PullRequestComment{ + Body: &comment.Content, + CommitID: &comment.Ref, + Path: &comment.Path, + Position: &comment.Position, + } + + if _, _, err := r.gh.PullRequests.CreateComment(ctx, owner, repository, number, commentRequest); err != nil { + var ghErr *github.ErrorResponse + if errors.As(err, &ghErr) && ghErr.Response.StatusCode == http.StatusServiceUnavailable { + return ErrServiceUnavailable + } + + return err + } + + return nil +} + +func (r *realImpl) ClearAllPullRequestFileComments(ctx context.Context, owner, repository string, number int) error { + comments, _, err := r.gh.PullRequests.ListComments(ctx, owner, repository, number, nil) + if err != nil { + var ghErr *github.ErrorResponse + if errors.As(err, &ghErr) && ghErr.Response.StatusCode == http.StatusServiceUnavailable { + return ErrServiceUnavailable + } + return err + } + + userLogin, _, err := r.gh.Users.Get(ctx, "") + if err != nil { + return fmt.Errorf("get user: %w", err) + } + + for _, c := range comments { + // skip if comments were not created by us + if c.User.GetLogin() != userLogin.GetLogin() { + continue + } + + if _, err := r.gh.PullRequests.DeleteComment(ctx, owner, repository, c.GetID()); err != nil { + return fmt.Errorf("delete comment: %w", err) + } + } + + return nil +} + type realRepositoryContent struct { real *github.RepositoryContent } diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index c90bcb44199..ffe3145670f 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -205,6 +205,7 @@ func TestIntegrationProvisioning(t *testing.T) { "branchWorkflow": true, "generateDashboardPreviews": true, "owner": "grafana", + "pullRequestLinter": true, "repository": "git-ui-sync-demo", "token": "github_pat_dummy", "webhookSecret": "dummyWebhookSecret", diff --git a/pkg/tests/apis/provisioning/testdata/github-example.yaml b/pkg/tests/apis/provisioning/testdata/github-example.yaml index df5abd3c1de..4f0732c2294 100644 --- a/pkg/tests/apis/provisioning/testdata/github-example.yaml +++ b/pkg/tests/apis/provisioning/testdata/github-example.yaml @@ -17,5 +17,6 @@ spec: branch: dummy-branch branchWorkflow: true generateDashboardPreviews: true + pullRequestLinter: true token: "github_pat_dummy" - webhookSecret: "dummyWebhookSecret" \ No newline at end of file + webhookSecret: "dummyWebhookSecret"