[Prototyping] Lint dashboard in pull requests (#97199)

* Make lint issues comment

* Make it work

* Delete previous comments

* Move linting to separate package

* Fix issue with refactoring

* Update test fixture
This commit is contained in:
Roberto Jiménez Sánchez 2024-11-29 16:24:45 +01:00 committed by GitHub
parent a1eb1863e5
commit 48feba66e6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 349 additions and 41 deletions

3
go.mod
View File

@ -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/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 ( require (
cloud.google.com/go/longrunning v0.6.0 // indirect cloud.google.com/go/longrunning v0.6.0 // indirect
github.com/at-wat/mqtt-go v0.19.4 // 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/emirpasic/gods v1.18.1 // indirect
github.com/gammazero/deque v0.2.1 // indirect github.com/gammazero/deque v0.2.1 // indirect
github.com/golang/geo v0.0.0-20210211234256-740aa86cb551 // 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/jsonparser v0.0.0-20240425183733-ea80629e1a32 // indirect
github.com/grafana/loki/pkg/push v0.0.0-20231124142027-e52380921608 // indirect github.com/grafana/loki/pkg/push v0.0.0-20231124142027-e52380921608 // indirect
github.com/grafana/sqlds/v4 v4.1.0 // indirect github.com/grafana/sqlds/v4 v4.1.0 // indirect

4
go.sum
View File

@ -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/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 h1:I3IwBhF+UaQxRM79HnImtrAn8REGdb5M3+C4QrYHoWk=
github.com/grafana/cuetsy v0.1.11/go.mod h1:Ix97+CPD8ws9oSSxR3/Lf4ahU1I4Np83kjJmDVnLZvc= 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 h1:K9M5glueWyLoL4//H+EtTQq16lXuHLmOhb6DjSCahzA=
github.com/grafana/dataplane/examples v0.0.1/go.mod h1:h5YwY8s407/17XF5/dS8XrUtsTVV2RnuW8+m1Mp46mg= 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= 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-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 h1:F0O9eTy4jHjEd1Z3/qIza2GdY7PYpTddUeaq9p3NKGU=
github.com/grafana/grafana-cloud-migration-snapshot v1.3.0/go.mod h1:bd6Cm06EK0MzRO5ahUpbDz1SxNOKu+fzladbaRPHZPY= 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 h1:LKGY8z2DSxKjYfr2flZsWgTRTZ6HGQbTqewE3JvRaNA=
github.com/grafana/grafana-google-sdk-go v0.1.0/go.mod h1:Vo2TKWfDVmNTELBUM+3lkrZvFtBws0qSZdXhQxRdJrE= 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= github.com/grafana/grafana-openapi-client-go v0.0.0-20231213163343-bd475d63fb79 h1:r+mU5bGMzcXCRVAuOrTn54S80qbfVkvTdUJZfSfTNbs=

View File

@ -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 h1:qhugDMdQ4Vp68H0tp/0iN17DM2ehRo1rLEdOFe/gB8I=
github.com/grafana/cloudflare-go v0.0.0-20230110200409-c627cf6792f2/go.mod h1:w/aiO1POVIeXUQyl0VQSZjl5OAGDTL5aX+4v0RA1tcw= 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.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/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 h1:BOChP0h/jLeD+7F9mL7tq10xVkDG15he3T1zHuQaWak=
github.com/grafana/go-gelf/v2 v2.0.1/go.mod h1:lexHie0xzYGwCgiRGcvZ723bSNyNI8ZRD4s0CLobh90= 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/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 h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c=
github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= 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 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0=
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= 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= 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/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 h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI=
github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= 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 h1:4lbD8Mx2h7IvloP7r2C0D6ltZP6Ufip8Hn0wmSK5LR8=
github.com/zenazn/goji v1.0.1/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q= github.com/zenazn/goji v1.0.1/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q=
github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs= github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs=

View File

@ -63,6 +63,9 @@ type GitHubRepositoryConfig struct {
// By default, this is false (i.e. we will not create previews). // By default, this is false (i.e. we will not create previews).
// This option is a no-op if BranchWorkflow is `false` or default. // This option is a no-op if BranchWorkflow is `false` or default.
GenerateDashboardPreviews bool `json:"generateDashboardPreviews,omitempty"` 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 // RepositoryType defines the types of Repository

View File

@ -223,6 +223,13 @@ func schema_pkg_apis_provisioning_v0alpha1_GitHubRepositoryConfig(ref common.Ref
Format: "", Format: "",
}, },
}, },
"pullRequestLinter": {
SchemaProps: spec.SchemaProps{
Description: "PullRequestLinter enables the dashboard linter for this repository in Pull Requests",
Type: []string{"boolean"},
Format: "",
},
},
}, },
}, },
}, },

View File

@ -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
}

View File

@ -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)
}

View File

@ -30,6 +30,7 @@ import (
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" 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/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.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" "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 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: case provisioning.S3RepositoryType:
return repository.NewS3(r), nil return repository.NewS3(r), nil
default: default:

View File

@ -20,6 +20,7 @@ import (
"k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/registry/rest"
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" 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" pgh "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github"
) )
@ -28,6 +29,7 @@ type githubRepository struct {
config *provisioning.Repository config *provisioning.Repository
gh pgh.Client gh pgh.Client
baseURL *url.URL baseURL *url.URL
linter lint.Linter
} }
var _ Repository = (*githubRepository)(nil) var _ Repository = (*githubRepository)(nil)
@ -37,12 +39,14 @@ func NewGitHub(
config *provisioning.Repository, config *provisioning.Repository,
factory pgh.ClientFactory, factory pgh.ClientFactory,
baseURL *url.URL, baseURL *url.URL,
linter lint.Linter,
) *githubRepository { ) *githubRepository {
return &githubRepository{ return &githubRepository{
config: config, config: config,
logger: slog.Default().With("logger", "github-repository"), logger: slog.Default().With("logger", "github-repository"),
gh: factory.New(ctx, config.Spec.GitHub.Token), gh: factory.New(ctx, config.Spec.GitHub.Token),
baseURL: baseURL, baseURL: baseURL,
linter: linter,
} }
} }
@ -484,18 +488,8 @@ func (r *githubRepository) onPushEvent(ctx context.Context, logger *slog.Logger,
return nil return nil
} }
const commentTemplate = `Hey there! 🎉 // changedResource represents a resource that has changed in a pull request.
Grafana spotted some changes for your resources in this pull request: type changedResource struct {
| 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 {
Filename string Filename string
Path string Path string
Action string Action string
@ -503,13 +497,27 @@ type commentFile struct {
Original string Original string
Current string Current string
Preview string Preview string
Data []byte
} }
// onPullRequestEvent is called when a pull request event is received // onPullRequestEvent is called when a pull request event is received
// If the pull request is opened, reponed or synchronize, we read the files changed. // 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 { func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog.Logger, event *github.PullRequestEvent, factory FileReplicatorFactory) error {
action := event.GetAction() 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 { if event.GetRepo() == nil {
return fmt.Errorf("missing repository in pull request event") 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) return fmt.Errorf("list pull request files: %w", err)
} }
rows := make([]commentFile, 0) changedResources := make([]changedResource, 0)
baseBranch := event.GetPullRequest().GetBase().GetRef() baseBranch := event.GetPullRequest().GetBase().GetRef()
mainBranch := r.config.Spec.GitHub.Branch mainBranch := r.config.Spec.GitHub.Branch
@ -542,9 +550,8 @@ func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog.
prURL := event.GetPullRequest().GetHTMLURL() prURL := event.GetPullRequest().GetHTMLURL()
// TODO: implement the real handling of the files
for _, file := range files { for _, file := range files {
row := commentFile{ resource := changedResource{
Filename: path.Base(file.GetFilename()), Filename: path.Base(file.GetFilename()),
Path: file.GetFilename(), Path: file.GetFilename(),
Action: file.GetStatus(), 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 // reference: https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#get-a-commit
switch file.GetStatus() { switch file.GetStatus() {
case "added": case "added":
row.Preview = r.previewURL(ref, path, prURL) resource.Preview = r.previewURL(ref, path, prURL)
case "modified": case "modified":
row.Original = r.previewURL(baseBranch, path, prURL) resource.Original = r.previewURL(baseBranch, path, prURL)
row.Current = r.previewURL(mainBranch, path, prURL) resource.Current = r.previewURL(mainBranch, path, prURL)
row.Preview = r.previewURL(ref, path, prURL) resource.Preview = r.previewURL(ref, path, prURL)
case "removed": case "removed":
row.Original = r.previewURL(baseBranch, path, prURL) resource.Original = r.previewURL(baseBranch, path, prURL)
row.Current = r.previewURL(mainBranch, path, prURL) resource.Current = r.previewURL(mainBranch, path, prURL)
ref = baseBranch ref = baseBranch
case "renamed": case "renamed":
row.Original = r.previewURL(baseBranch, file.GetPreviousFilename(), prURL) resource.Original = r.previewURL(baseBranch, file.GetPreviousFilename(), prURL)
row.Current = r.previewURL(mainBranch, file.GetPreviousFilename(), prURL) resource.Current = r.previewURL(mainBranch, file.GetPreviousFilename(), prURL)
row.Preview = r.previewURL(ref, path, prURL) resource.Preview = r.previewURL(ref, path, prURL)
case "changed": case "changed":
row.Original = r.previewURL(baseBranch, path, prURL) resource.Original = r.previewURL(baseBranch, path, prURL)
row.Current = r.previewURL(mainBranch, path, prURL) resource.Current = r.previewURL(mainBranch, path, prURL)
row.Preview = r.previewURL(ref, path, prURL) resource.Preview = r.previewURL(ref, path, prURL)
case "unchanged": case "unchanged":
logger.InfoContext(ctx, "ignore unchanged file") logger.InfoContext(ctx, "ignore unchanged file")
continue continue
@ -589,6 +596,7 @@ func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog.
continue continue
} }
// TODO: how does this validation works vs linting?
ok, err := replicator.Validate(ctx, f) ok, err := replicator.Validate(ctx, f)
if err != nil { if err != nil {
logger.ErrorContext(ctx, "failed to validate file", "error", err) logger.ErrorContext(ctx, "failed to validate file", "error", err)
@ -599,34 +607,133 @@ func (r *githubRepository) onPullRequestEvent(ctx context.Context, logger *slog.
continue continue
} }
resource.Data = f.Data
logger.InfoContext(ctx, "resource changed") 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 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 { if err != nil {
return fmt.Errorf("parse comment template: %w", err) return fmt.Errorf("parse comment template: %w", err)
} }
var buf bytes.Buffer 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) return fmt.Errorf("execute comment template: %w", err)
} }
comment := buf.String() comment := buf.String()
// TODO: comment with Grafana Logo // FIXME: comment with Grafana Logo
// TODO: comment author should be written by Grafana and not the user // 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, event.GetNumber(), comment); err != nil { 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) 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 return nil
} }

View File

@ -78,6 +78,8 @@ type Client interface {
ListPullRequestFiles(ctx context.Context, owner, repository string, number int) ([]CommitFile, error) ListPullRequestFiles(ctx context.Context, owner, repository string, number int) ([]CommitFile, error)
CreatePullRequestComment(ctx context.Context, owner, repository string, number int, body string) 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 { type RepositoryContent interface {
@ -106,6 +108,13 @@ type CommitFile interface {
GetStatus() string GetStatus() string
} }
type FileComment struct {
Content string
Path string
Position int
Ref string
}
type CreateFileOptions struct { type CreateFileOptions struct {
// The message of the commit. May be empty, in which case a default value is entered. // The message of the commit. May be empty, in which case a default value is entered.
Message string Message string

View File

@ -41,6 +41,24 @@ func (_m *MockClient) BranchExists(ctx context.Context, owner string, repository
return r0, r1 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 // 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 { func (_m *MockClient) CreateBranch(ctx context.Context, owner string, repository string, sourceBranch string, branchName string) error {
ret := _m.Called(ctx, owner, repository, sourceBranch, branchName) ret := _m.Called(ctx, owner, repository, sourceBranch, branchName)
@ -95,6 +113,24 @@ func (_m *MockClient) CreatePullRequestComment(ctx context.Context, owner string
return r0 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 // 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 { func (_m *MockClient) CreateWebhook(ctx context.Context, owner string, repository string, cfg WebhookConfig) error {
ret := _m.Called(ctx, owner, repository, cfg) ret := _m.Called(ctx, owner, repository, cfg)

View File

@ -344,6 +344,55 @@ func (r *realImpl) CreatePullRequestComment(ctx context.Context, owner, reposito
return nil 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 { type realRepositoryContent struct {
real *github.RepositoryContent real *github.RepositoryContent
} }

View File

@ -205,6 +205,7 @@ func TestIntegrationProvisioning(t *testing.T) {
"branchWorkflow": true, "branchWorkflow": true,
"generateDashboardPreviews": true, "generateDashboardPreviews": true,
"owner": "grafana", "owner": "grafana",
"pullRequestLinter": true,
"repository": "git-ui-sync-demo", "repository": "git-ui-sync-demo",
"token": "github_pat_dummy", "token": "github_pat_dummy",
"webhookSecret": "dummyWebhookSecret", "webhookSecret": "dummyWebhookSecret",

View File

@ -17,5 +17,6 @@ spec:
branch: dummy-branch branch: dummy-branch
branchWorkflow: true branchWorkflow: true
generateDashboardPreviews: true generateDashboardPreviews: true
pullRequestLinter: true
token: "github_pat_dummy" token: "github_pat_dummy"
webhookSecret: "dummyWebhookSecret" webhookSecret: "dummyWebhookSecret"