Provisioning: unit test pull request job (#104058)

This commit is contained in:
Roberto Jiménez Sánchez
2025-04-17 15:33:32 +03:00
committed by GitHub
parent 2cf67806d9
commit d38cd9d6c4
12 changed files with 1956 additions and 232 deletions
@@ -0,0 +1,114 @@
// Code generated by mockery v2.52.4. DO NOT EDIT.
package pullrequest
import (
context "context"
grpc "google.golang.org/grpc"
mock "github.com/stretchr/testify/mock"
resource "github.com/grafana/grafana/pkg/storage/unified/resource"
)
// MockBlobStoreClient is an autogenerated mock type for the BlobStoreClient type
type MockBlobStoreClient struct {
mock.Mock
}
type MockBlobStoreClient_Expecter struct {
mock *mock.Mock
}
func (_m *MockBlobStoreClient) EXPECT() *MockBlobStoreClient_Expecter {
return &MockBlobStoreClient_Expecter{mock: &_m.Mock}
}
// PutBlob provides a mock function with given fields: ctx, in, opts
func (_m *MockBlobStoreClient) PutBlob(ctx context.Context, in *resource.PutBlobRequest, opts ...grpc.CallOption) (*resource.PutBlobResponse, error) {
_va := make([]interface{}, len(opts))
for _i := range opts {
_va[_i] = opts[_i]
}
var _ca []interface{}
_ca = append(_ca, ctx, in)
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
if len(ret) == 0 {
panic("no return value specified for PutBlob")
}
var r0 *resource.PutBlobResponse
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, *resource.PutBlobRequest, ...grpc.CallOption) (*resource.PutBlobResponse, error)); ok {
return rf(ctx, in, opts...)
}
if rf, ok := ret.Get(0).(func(context.Context, *resource.PutBlobRequest, ...grpc.CallOption) *resource.PutBlobResponse); ok {
r0 = rf(ctx, in, opts...)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*resource.PutBlobResponse)
}
}
if rf, ok := ret.Get(1).(func(context.Context, *resource.PutBlobRequest, ...grpc.CallOption) error); ok {
r1 = rf(ctx, in, opts...)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// MockBlobStoreClient_PutBlob_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'PutBlob'
type MockBlobStoreClient_PutBlob_Call struct {
*mock.Call
}
// PutBlob is a helper method to define mock.On call
// - ctx context.Context
// - in *resource.PutBlobRequest
// - opts ...grpc.CallOption
func (_e *MockBlobStoreClient_Expecter) PutBlob(ctx interface{}, in interface{}, opts ...interface{}) *MockBlobStoreClient_PutBlob_Call {
return &MockBlobStoreClient_PutBlob_Call{Call: _e.mock.On("PutBlob",
append([]interface{}{ctx, in}, opts...)...)}
}
func (_c *MockBlobStoreClient_PutBlob_Call) Run(run func(ctx context.Context, in *resource.PutBlobRequest, opts ...grpc.CallOption)) *MockBlobStoreClient_PutBlob_Call {
_c.Call.Run(func(args mock.Arguments) {
variadicArgs := make([]grpc.CallOption, len(args)-2)
for i, a := range args[2:] {
if a != nil {
variadicArgs[i] = a.(grpc.CallOption)
}
}
run(args[0].(context.Context), args[1].(*resource.PutBlobRequest), variadicArgs...)
})
return _c
}
func (_c *MockBlobStoreClient_PutBlob_Call) Return(_a0 *resource.PutBlobResponse, _a1 error) *MockBlobStoreClient_PutBlob_Call {
_c.Call.Return(_a0, _a1)
return _c
}
func (_c *MockBlobStoreClient_PutBlob_Call) RunAndReturn(run func(context.Context, *resource.PutBlobRequest, ...grpc.CallOption) (*resource.PutBlobResponse, error)) *MockBlobStoreClient_PutBlob_Call {
_c.Call.Return(run)
return _c
}
// NewMockBlobStoreClient creates a new instance of MockBlobStoreClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
// The first argument is typically a *testing.T value.
func NewMockBlobStoreClient(t interface {
mock.TestingT
Cleanup(func())
}) *MockBlobStoreClient {
mock := &MockBlobStoreClient{}
mock.Mock.Test(t)
t.Cleanup(func() { mock.AssertExpectations(t) })
return mock
}
@@ -28,7 +28,6 @@ type changeInfo struct {
// Requested image render, but it is not available
MissingImageRenderer bool
HasScreenshot bool
}
type fileChangeInfo struct {
@@ -72,66 +71,37 @@ func (e *evaluator) Evaluate(ctx context.Context, repo repository.Reader, opts p
return changeInfo{}, fmt.Errorf("failed to get parser for %s: %w", cfg.Name, err)
}
baseURL := e.urlProvider(cfg.Namespace)
rendererAvailable := e.render.IsAvailable(ctx)
shouldRender := rendererAvailable && len(changes) == 1 && cfg.Spec.GitHub.GenerateDashboardPreviews
info := changeInfo{
GrafanaBaseURL: baseURL,
}
var shouldRender bool
switch {
case e.render == nil:
shouldRender = false
case !e.render.IsAvailable(ctx):
info.MissingImageRenderer = true
shouldRender = false
case len(changes) > 1 || !cfg.Spec.GitHub.GenerateDashboardPreviews:
// Only render images when there is just one change
shouldRender = false
default:
shouldRender = true
GrafanaBaseURL: e.urlProvider(cfg.Namespace),
MissingImageRenderer: !rendererAvailable,
}
logger := logging.FromContext(ctx)
for i, change := range changes {
// process maximum 10 files
if i >= 10 {
info.SkippedFiles = len(changes) - i
logger.Info("skipping remaining files", "count", info.SkippedFiles)
break
}
progress.SetMessage(ctx, fmt.Sprintf("processing: %s", change.Path))
progress.SetMessage(ctx, fmt.Sprintf("process %s", change.Path))
logger.With("action", change.Action).With("path", change.Path)
v, err := calculateFileChangeInfo(ctx, repo, info.GrafanaBaseURL, change, opts, parser)
if err != nil {
return info, fmt.Errorf("error calculating changes %w", err)
}
// If everything applied OK, then render screenshots
if shouldRender && v.GrafanaURL != "" && v.Parsed != nil && v.Parsed.DryRunResponse != nil {
progress.SetMessage(ctx, fmt.Sprintf("rendering screenshots: %s", change.Path))
if err = v.renderScreenshots(ctx, info.GrafanaBaseURL, e.render); err != nil {
info.MissingImageRenderer = true
if v.Error == "" {
v.Error = "Error running image rendering"
}
if v.GrafanaScreenshotURL != "" || v.PreviewScreenshotURL != "" {
info.HasScreenshot = true
}
}
}
info.Changes = append(info.Changes, v)
info.Changes = append(info.Changes, e.evaluateFile(ctx, repo, info.GrafanaBaseURL, change, opts, parser, shouldRender))
}
return info, nil
}
var dashboardKind = dashboard.DashboardResourceInfo.GroupVersionKind().Kind
func calculateFileChangeInfo(ctx context.Context, repo repository.Reader, baseURL string, change repository.VersionedFileChange, opts provisioning.PullRequestJobOptions, parser resources.Parser) (fileChangeInfo, error) {
func (e *evaluator) evaluateFile(ctx context.Context, repo repository.Reader, baseURL string, change repository.VersionedFileChange, opts provisioning.PullRequestJobOptions, parser resources.Parser, shouldRender bool) fileChangeInfo {
if change.Action == repository.FileActionDeleted {
return calculateFileDeleteInfo(ctx, baseURL, change)
// TODO: read the old and verify
return fileChangeInfo{Change: change, Error: "delete feedback not yet implemented"}
}
info := fileChangeInfo{Change: change}
@@ -139,14 +109,14 @@ func calculateFileChangeInfo(ctx context.Context, repo repository.Reader, baseUR
if err != nil {
logger.Info("unable to read file", "err", err)
info.Error = err.Error()
return info, nil
return info
}
// Read the file as a resource
info.Parsed, err = parser.Parse(ctx, fileInfo)
if err != nil {
info.Error = err.Error()
return info, nil
return info
}
// Find a name within the file
@@ -158,11 +128,13 @@ func calculateFileChangeInfo(ctx context.Context, repo repository.Reader, baseUR
err = info.Parsed.DryRun(ctx)
if err != nil {
info.Error = err.Error()
return info, nil
return info
}
// Dashboards get special handling
if info.Parsed.GVK.Kind == dashboardKind {
// FIXME: extract the logic out of a dashboard URL builder/injector or similar
// for testability and decoupling
if info.Parsed.Existing != nil {
info.GrafanaURL = fmt.Sprintf("%sd/%s/%s", baseURL, obj.GetName(),
slugify.Slugify(info.Title))
@@ -178,31 +150,24 @@ func calculateFileChangeInfo(ctx context.Context, repo repository.Reader, baseUR
query.Set("pull_request_url", url.QueryEscape(opts.URL))
}
info.PreviewURL += "?" + query.Encode()
}
if shouldRender {
if info.GrafanaURL != "" {
info.GrafanaScreenshotURL, err = renderScreenshotFromGrafanaURL(ctx, baseURL, e.render, info.Parsed.Repo, info.GrafanaURL)
if err != nil {
info.Error = err.Error()
}
}
return info, nil
}
func calculateFileDeleteInfo(_ context.Context, _ string, change repository.VersionedFileChange) (fileChangeInfo, error) {
// TODO -- read the old and verify
return fileChangeInfo{Change: change, Error: "delete feedback not yet implemented"}, nil
}
// This will update render the linked screenshots and update the screenshotURLs
func (f *fileChangeInfo) renderScreenshots(ctx context.Context, baseURL string, renderer ScreenshotRenderer) (err error) {
if f.GrafanaURL != "" {
f.GrafanaScreenshotURL, err = renderScreenshotFromGrafanaURL(ctx, baseURL, renderer, f.Parsed.Repo, f.GrafanaURL)
if err != nil {
return err
if info.PreviewURL != "" {
info.PreviewScreenshotURL, err = renderScreenshotFromGrafanaURL(ctx, baseURL, e.render, info.Parsed.Repo, info.PreviewURL)
if err != nil {
info.Error = err.Error()
}
}
}
}
if f.PreviewURL != "" {
f.PreviewScreenshotURL, err = renderScreenshotFromGrafanaURL(ctx, baseURL, renderer, f.Parsed.Repo, f.PreviewURL)
if err != nil {
return err
}
}
return nil
return info
}
func renderScreenshotFromGrafanaURL(ctx context.Context,
@@ -219,7 +184,7 @@ func renderScreenshotFromGrafanaURL(ctx context.Context,
snap, err := renderer.RenderScreenshot(ctx, repo, strings.TrimPrefix(parsed.Path, "/"), parsed.Query())
if err != nil {
logging.FromContext(ctx).Warn("render failed", "url", grafanaURL, "err", err)
return "", fmt.Errorf("error rendering screenshot %w", err)
return "", fmt.Errorf("error rendering screenshot: %w", err)
}
if strings.Contains(snap, "://") {
return snap, nil // it is a full URL already (can happen when the blob storage returns CDN urls)
File diff suppressed because it is too large Load Diff
@@ -37,8 +37,9 @@ func (c *commenter) Comment(ctx context.Context, prRepo PullRequestRepo, pr int,
}
func (c *commenter) generateComment(_ context.Context, info changeInfo) (string, error) {
// TODO: should we comment even if there are no changes?
if len(info.Changes) == 0 {
return "no changes found", nil
return "Grafana didn't find any changes in this pull request.", nil
}
var buf bytes.Buffer
@@ -105,6 +106,7 @@ const commentTemplateMissingImageRenderer = `
NOTE: The image renderer is not configured
`
// TODO: does this have some value?
func (f *fileChangeInfo) Kind() string {
if f.Parsed == nil {
return filepath.Ext(f.Change.Path)
@@ -116,6 +118,7 @@ func (f *fileChangeInfo) Kind() string {
return f.Parsed.GVK.Kind
}
// TODO: does this have some value?
func (f *fileChangeInfo) ExistingLink() string {
if f.GrafanaURL != "" {
return fmt.Sprintf("[%s](%s)", f.Title, f.GrafanaURL)
@@ -2,11 +2,13 @@ package pullrequest
import (
"context"
"errors"
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -15,11 +17,21 @@ import (
"github.com/grafana/grafana/pkg/registry/apis/provisioning/resources"
)
func TestCommenter_Comment_FailedToComment(t *testing.T) {
repo := NewMockPullRequestRepo(t)
repo.On("CommentPullRequest", context.Background(), 1, mock.Anything).Return(errors.New("failed"))
commenter := NewCommenter()
err := commenter.Comment(context.Background(), repo, 1, changeInfo{})
require.Error(t, err)
}
func TestGenerateComment(t *testing.T) {
for _, tc := range []struct {
Name string
Input changeInfo
}{
{"no changes", changeInfo{}},
{"new dashboard", changeInfo{
GrafanaBaseURL: "http://host/",
Changes: []fileChangeInfo{
@@ -15,8 +15,14 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/rendering"
"github.com/grafana/grafana/pkg/storage/unified/resource"
"google.golang.org/grpc"
)
//go:generate mockery --name BlobStoreClient --structname MockBlobStoreClient --inpackage --filename blobstore_client_mock.go --with-expecter
type BlobStoreClient interface {
PutBlob(ctx context.Context, in *resource.PutBlobRequest, opts ...grpc.CallOption) (*resource.PutBlobResponse, error)
}
// ScreenshotRenderer is an interface for rendering a preview of a file
//
//go:generate mockery --name ScreenshotRenderer --structname MockScreenshotRenderer --inpackage --filename render_mock.go --with-expecter
@@ -27,10 +33,10 @@ type ScreenshotRenderer interface {
type screenshotRenderer struct {
render rendering.Service
blobstore resource.BlobStoreClient
blobstore BlobStoreClient
}
func NewScreenshotRenderer(render rendering.Service, blobstore resource.BlobStoreClient) ScreenshotRenderer {
func NewScreenshotRenderer(render rendering.Service, blobstore BlobStoreClient) ScreenshotRenderer {
return &screenshotRenderer{
render: render,
blobstore: blobstore,
@@ -0,0 +1,294 @@
package pullrequest
import (
"context"
"errors"
"net/url"
"os"
"path/filepath"
"testing"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/rendering"
"github.com/grafana/grafana/pkg/storage/unified/resource"
)
func setupTempFile(t *testing.T) (string, func()) {
t.Helper()
// Create a temporary directory
tmpDir, err := os.MkdirTemp("", "screenshot-renderer-test-*")
require.NoError(t, err)
// Create a temporary file
tmpFile := filepath.Join(tmpDir, "test.png")
err = os.WriteFile(tmpFile, []byte("test"), 0644)
require.NoError(t, err)
// Return cleanup function
cleanup := func() {
err := os.RemoveAll(tmpDir)
require.NoError(t, err)
}
return tmpFile, cleanup
}
func TestScreenshotRenderer_IsAvailable(t *testing.T) {
t.Run("should return false when render service is nil", func(t *testing.T) {
blobstore := NewMockBlobStoreClient(t)
renderer := NewScreenshotRenderer(nil, blobstore)
require.False(t, renderer.IsAvailable(context.Background()))
})
t.Run("should return false when render service is not available", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
render := rendering.NewMockService(ctrl)
render.EXPECT().IsAvailable(gomock.Any()).Return(false)
blobstore := NewMockBlobStoreClient(t)
renderer := NewScreenshotRenderer(render, blobstore)
require.False(t, renderer.IsAvailable(context.Background()))
})
t.Run("should return false when blobstore is nil", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
render := rendering.NewMockService(ctrl)
render.EXPECT().IsAvailable(gomock.Any()).Return(true)
renderer := NewScreenshotRenderer(render, nil)
require.False(t, renderer.IsAvailable(context.Background()))
})
t.Run("should return true when both services are available", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
render := rendering.NewMockService(ctrl)
render.EXPECT().IsAvailable(gomock.Any()).Return(true)
blobstore := NewMockBlobStoreClient(t)
renderer := NewScreenshotRenderer(render, blobstore)
require.True(t, renderer.IsAvailable(context.Background()))
})
}
func TestScreenshotRenderer_RenderScreenshot(t *testing.T) {
type testCase struct {
name string
path string
queryParams url.Values
repoInfo provisioning.ResourceRepositoryInfo
setupRender func(ctrl *gomock.Controller) rendering.Service
setupBlobstore func(t *testing.T) BlobStoreClient
expectedURL string
expectedError string
}
tests := []testCase{
{
name: "should fail when path contains protocol",
path: "http://test",
setupRender: func(ctrl *gomock.Controller) rendering.Service {
return rendering.NewMockService(ctrl)
},
setupBlobstore: func(t *testing.T) BlobStoreClient {
return NewMockBlobStoreClient(t)
},
expectedError: "path should be relative",
},
{
name: "should fail when path starts with slash",
path: "/test",
setupRender: func(ctrl *gomock.Controller) rendering.Service {
return rendering.NewMockService(ctrl)
},
setupBlobstore: func(t *testing.T) BlobStoreClient {
return NewMockBlobStoreClient(t)
},
expectedError: "path should not start with slash",
},
{
name: "should fail when render service fails",
path: "test",
setupRender: func(ctrl *gomock.Controller) rendering.Service {
render := rendering.NewMockService(ctrl)
render.EXPECT().Render(gomock.Any(), rendering.RenderPNG, gomock.Any(), gomock.Any()).
DoAndReturn(func(_ context.Context, _ rendering.RenderType, opts rendering.Opts, _ rendering.AuthOpts) (*rendering.RenderResult, error) {
require.Equal(t, "test?kiosk", opts.Path)
require.Equal(t, int64(1), opts.OrgID)
require.Equal(t, int64(1), opts.UserID)
require.Equal(t, 1024, opts.Width)
require.Equal(t, -1, opts.Height)
require.Equal(t, models.ThemeDark, opts.Theme)
return nil, errors.New("render error")
})
return render
},
setupBlobstore: func(t *testing.T) BlobStoreClient {
return NewMockBlobStoreClient(t)
},
expectedError: "render error",
},
{
name: "should fail when the rendered file does not exist",
path: "test",
setupRender: func(ctrl *gomock.Controller) rendering.Service {
render := rendering.NewMockService(ctrl)
render.EXPECT().Render(gomock.Any(), rendering.RenderPNG, gomock.Any(), gomock.Any()).
Return(&rendering.RenderResult{
FilePath: "/non/existent/file.png",
}, nil)
return render
},
setupBlobstore: func(t *testing.T) BlobStoreClient {
return NewMockBlobStoreClient(t)
},
expectedError: "no such file or directory",
},
{
name: "should fail when blobstore fails",
path: "test",
setupRender: func(ctrl *gomock.Controller) rendering.Service {
tmpFile, cleanup := setupTempFile(t)
t.Cleanup(cleanup)
render := rendering.NewMockService(ctrl)
render.EXPECT().Render(gomock.Any(), rendering.RenderPNG, gomock.Any(), gomock.Any()).
Return(&rendering.RenderResult{
FilePath: tmpFile,
}, nil)
return render
},
setupBlobstore: func(t *testing.T) BlobStoreClient {
blobstore := NewMockBlobStoreClient(t)
blobstore.On("PutBlob", mock.Anything, mock.MatchedBy(func(req *resource.PutBlobRequest) bool {
return req.Resource.Group == provisioning.GROUP &&
req.Resource.Resource == provisioning.RepositoryResourceInfo.GroupResource().Resource &&
req.Method == resource.PutBlobRequest_GRPC &&
req.ContentType == "image/png"
})).Return(nil, errors.New("blobstore error"))
return blobstore
},
expectedError: "blobstore error",
},
{
name: "should return URL when blobstore provides one",
path: "test",
repoInfo: provisioning.ResourceRepositoryInfo{
Name: "test-repo",
Namespace: "test-ns",
},
setupRender: func(ctrl *gomock.Controller) rendering.Service {
tmpFile, cleanup := setupTempFile(t)
t.Cleanup(cleanup)
render := rendering.NewMockService(ctrl)
render.EXPECT().Render(gomock.Any(), rendering.RenderPNG, gomock.Any(), gomock.Any()).
Return(&rendering.RenderResult{
FilePath: tmpFile,
}, nil)
return render
},
setupBlobstore: func(t *testing.T) BlobStoreClient {
blobstore := NewMockBlobStoreClient(t)
blobstore.On("PutBlob", mock.Anything, mock.Anything).
Return(&resource.PutBlobResponse{
Url: "https://example.com/test.png",
}, nil)
return blobstore
},
expectedURL: "https://example.com/test.png",
},
{
name: "should return API path when blobstore provides UID",
path: "test",
repoInfo: provisioning.ResourceRepositoryInfo{
Name: "test-repo",
Namespace: "test-ns",
},
setupRender: func(ctrl *gomock.Controller) rendering.Service {
tmpFile, cleanup := setupTempFile(t)
t.Cleanup(cleanup)
render := rendering.NewMockService(ctrl)
render.EXPECT().Render(gomock.Any(), rendering.RenderPNG, gomock.Any(), gomock.Any()).
Return(&rendering.RenderResult{
FilePath: tmpFile,
}, nil)
return render
},
setupBlobstore: func(t *testing.T) BlobStoreClient {
blobstore := NewMockBlobStoreClient(t)
blobstore.On("PutBlob", mock.Anything, mock.Anything).
Return(&resource.PutBlobResponse{
Uid: "test-uid",
}, nil)
return blobstore
},
expectedURL: "apis/provisioning.grafana.app/v0alpha1/namespaces/test-ns/repositories/test-repo/render/test-uid",
},
{
name: "should append query parameters correctly",
path: "test",
queryParams: url.Values{
"param1": []string{"value1"},
"param2": []string{"value2"},
},
setupRender: func(ctrl *gomock.Controller) rendering.Service {
tmpFile, cleanup := setupTempFile(t)
t.Cleanup(cleanup)
render := rendering.NewMockService(ctrl)
render.EXPECT().Render(gomock.Any(), rendering.RenderPNG, gomock.Any(), gomock.Any()).
DoAndReturn(func(_ context.Context, _ rendering.RenderType, opts rendering.Opts, _ rendering.AuthOpts) (*rendering.RenderResult, error) {
require.Equal(t, "test?param1=value1&param2=value2&kiosk", opts.Path)
return &rendering.RenderResult{
FilePath: tmpFile,
}, nil
})
return render
},
setupBlobstore: func(t *testing.T) BlobStoreClient {
blobstore := NewMockBlobStoreClient(t)
blobstore.On("PutBlob", mock.Anything, mock.Anything).
Return(&resource.PutBlobResponse{
Uid: "test-uid",
}, nil)
return blobstore
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
render := tc.setupRender(ctrl)
blobstore := tc.setupBlobstore(t)
renderer := NewScreenshotRenderer(render, blobstore)
url, err := renderer.RenderScreenshot(context.Background(), tc.repoInfo, tc.path, tc.queryParams)
if tc.expectedError != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tc.expectedError)
} else {
require.NoError(t, err)
if tc.expectedURL != "" {
require.Equal(t, tc.expectedURL, url)
}
}
if mock, ok := blobstore.(*MockBlobStoreClient); ok {
mock.AssertExpectations(t)
}
})
}
}
@@ -0,0 +1 @@
Grafana didn't find any changes in this pull request.
@@ -70,16 +70,16 @@ func (c *PullRequestWorker) Process(ctx context.Context,
return apierrors.NewBadRequest("expecting github configuration")
}
prRepo, ok := repo.(PullRequestRepo)
if !ok {
return fmt.Errorf("repository is not a github repository")
}
reader, ok := repo.(repository.Reader)
if !ok {
return errors.New("pull request job submitted targeting repository that is not a Reader")
}
prRepo, ok := repo.(PullRequestRepo)
if !ok {
return fmt.Errorf("repository is not a pull request repository")
}
logger := logging.FromContext(ctx).With("pr", opts.PR)
logger.Info("process pull request")
defer logger.Info("pull request processed")
@@ -114,20 +114,11 @@ func (c *PullRequestWorker) Process(ctx context.Context,
// Remove files we should not try to process
func onlySupportedFiles(files []repository.VersionedFileChange) (ret []repository.VersionedFileChange) {
for _, file := range files {
if file.Action == repository.FileActionIgnored {
if file.Action == repository.FileActionIgnored || resources.IsPathSupported(file.Path) != nil {
continue
}
if err := resources.IsPathSupported(file.Path); err == nil {
ret = append(ret, file)
continue
}
if file.PreviousPath != "" {
if err := resources.IsPathSupported(file.PreviousPath); err != nil {
ret = append(ret, file)
continue
}
}
ret = append(ret, file)
}
return
}
@@ -0,0 +1,437 @@
package pullrequest
import (
"context"
"errors"
"testing"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/repository"
)
func TestPullRequestWorker_IsSupported(t *testing.T) {
tests := []struct {
name string
job provisioning.Job
expected bool
}{
{
name: "pull request action is supported",
job: provisioning.Job{
Spec: provisioning.JobSpec{
Action: provisioning.JobActionPullRequest,
},
},
expected: true,
},
{
name: "non-pull request action is not supported",
job: provisioning.Job{
Spec: provisioning.JobSpec{
Action: provisioning.JobActionPush,
},
},
expected: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
evaluator := NewMockEvaluator(t)
commenter := NewMockCommenter(t)
worker := NewPullRequestWorker(evaluator, commenter)
result := worker.IsSupported(context.Background(), tt.job)
require.Equal(t, tt.expected, result)
})
}
}
func TestPullRequestWorker_Process_NotPullRequestRepository(t *testing.T) {
evaluator := NewMockEvaluator(t)
commenter := NewMockCommenter(t)
repo := repository.NewMockRepository(t)
progress := jobs.NewMockJobProgressRecorder(t)
// Configure the mock repository to return a GitHub config
repo.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
GitHub: &provisioning.GitHubRepositoryConfig{Branch: "main"},
},
})
worker := NewPullRequestWorker(evaluator, commenter)
job := provisioning.Job{
Spec: provisioning.JobSpec{
Action: provisioning.JobActionPullRequest,
PullRequest: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
},
}
// The repository is not a PullRequestRepo, so it should fail
err := worker.Process(context.Background(), repo, job, progress)
require.Error(t, err)
require.Contains(t, err.Error(), "repository is not a pull request repository")
repo.AssertExpectations(t)
}
func TestPullRequestWorker_Process_NotReaderRepository(t *testing.T) {
evaluator := NewMockEvaluator(t)
commenter := NewMockCommenter(t)
progress := jobs.NewMockJobProgressRecorder(t)
// Create a mock that implements PullRequestRepo but not Reader
repo := repository.NewMockConfigRepository(t)
// Configure the mock to return a GitHub config
repo.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
GitHub: &provisioning.GitHubRepositoryConfig{Branch: "main"},
},
})
worker := NewPullRequestWorker(evaluator, commenter)
job := provisioning.Job{
Spec: provisioning.JobSpec{
Action: provisioning.JobActionPullRequest,
PullRequest: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
},
}
// The repository is not a Reader, so it should fail
err := worker.Process(context.Background(), repo, job, progress)
require.Error(t, err)
require.Contains(t, err.Error(), "repository that is not a Reader")
repo.AssertExpectations(t)
}
func TestPullRequestWorker_Process(t *testing.T) {
tests := []struct {
name string
opts *provisioning.PullRequestJobOptions
setupMocks func(*MockEvaluator, *MockCommenter, *mockPullRequestRepo, *jobs.MockJobProgressRecorder)
expectedError string
}{
{
name: "missing pull request options",
opts: nil,
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
},
})
},
expectedError: "missing spec.pr",
},
{
name: "missing ref",
opts: &provisioning.PullRequestJobOptions{
PR: 123,
},
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
},
})
},
expectedError: "missing spec.ref",
},
{
name: "missing github configuration",
opts: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
},
})
},
expectedError: "expecting github configuration",
},
{
name: "failed to list pull request files",
opts: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
GitHub: &provisioning.GitHubRepositoryConfig{Branch: "main"},
},
})
progress.On("SetMessage", mock.Anything, "listing pull request files").Return()
repo.MockPullRequestRepo.On("CompareFiles", mock.Anything, "main", "test-ref").Return(nil, errors.New("failed to list files"))
},
expectedError: "failed to list pull request files: failed to list files",
},
{
name: "no files to process",
opts: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
GitHub: &provisioning.GitHubRepositoryConfig{Branch: "main"},
},
})
progress.On("SetMessage", mock.Anything, "listing pull request files").Return()
repo.MockPullRequestRepo.On("CompareFiles", mock.Anything, "main", "test-ref").Return([]repository.VersionedFileChange{}, nil)
progress.On("SetFinalMessage", mock.Anything, "no files to process").Return()
},
expectedError: "",
},
{
name: "ignored files are filtered out",
opts: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
GitHub: &provisioning.GitHubRepositoryConfig{Branch: "main"},
},
})
progress.On("SetMessage", mock.Anything, "listing pull request files").Return()
// Create a mix of ignored and supported files
files := []repository.VersionedFileChange{
{Path: "test.yaml"}, // Supported file
{Path: "ignored.txt", Action: repository.FileActionIgnored}, // Ignored file
{Path: "another.yaml"}, // Supported file
}
repo.MockPullRequestRepo.On("CompareFiles", mock.Anything, "main", "test-ref").Return(files, nil)
// Only non-ignored files should be passed to the evaluator
expectedFiles := []repository.VersionedFileChange{
{Path: "test.yaml"},
{Path: "another.yaml"},
}
evaluator.On("Evaluate", mock.Anything, mock.Anything, mock.Anything, expectedFiles, mock.Anything).Return(changeInfo{}, nil)
commenter.On("Comment", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
},
expectedError: "",
},
{
name: "files with unsupported paths are filtered out",
opts: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
GitHub: &provisioning.GitHubRepositoryConfig{Branch: "main"},
},
})
progress.On("SetMessage", mock.Anything, "listing pull request files").Return()
// Create a mix of supported and unsupported files
files := []repository.VersionedFileChange{
{Path: "test.yaml"}, // Supported file
{Path: "unsupported/path.txt"}, // Unsupported file
{Path: "another.yaml"}, // Supported file
{Path: "invalid.doc"}, // Unsupported file
{Path: ".github/something"}, // Unsupported file
}
repo.MockPullRequestRepo.On("CompareFiles", mock.Anything, "main", "test-ref").Return(files, nil)
// Only supported files should be passed to the evaluator
expectedFiles := []repository.VersionedFileChange{
{Path: "test.yaml"},
{Path: "another.yaml"},
}
evaluator.On("Evaluate", mock.Anything, mock.Anything, mock.Anything, expectedFiles, mock.Anything).Return(changeInfo{}, nil)
commenter.On("Comment", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
},
expectedError: "",
},
{
name: "evaluation fails",
opts: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
GitHub: &provisioning.GitHubRepositoryConfig{Branch: "main"},
},
})
progress.On("SetMessage", mock.Anything, "listing pull request files").Return()
files := []repository.VersionedFileChange{
{Path: "test.yaml"},
}
repo.MockPullRequestRepo.On("CompareFiles", mock.Anything, "main", "test-ref").Return(files, nil)
evaluator.On("Evaluate", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(changeInfo{}, errors.New("evaluation failed"))
},
expectedError: "calculate changes: evaluation failed",
},
{
name: "comment fails",
opts: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
GitHub: &provisioning.GitHubRepositoryConfig{Branch: "main"},
},
})
progress.On("SetMessage", mock.Anything, "listing pull request files").Return()
files := []repository.VersionedFileChange{
{Path: "test.yaml"},
}
repo.MockPullRequestRepo.On("CompareFiles", mock.Anything, "main", "test-ref").Return(files, nil)
evaluator.On("Evaluate", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(changeInfo{}, nil)
commenter.On("Comment", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(errors.New("comment failed"))
},
expectedError: "comment pull request: comment failed",
},
{
name: "successful process",
opts: &provisioning.PullRequestJobOptions{
PR: 123,
Ref: "test-ref",
},
setupMocks: func(evaluator *MockEvaluator, commenter *MockCommenter, repo *mockPullRequestRepo, progress *jobs.MockJobProgressRecorder) {
repo.MockRepository.On("Config").Return(&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
Title: "test-repo",
GitHub: &provisioning.GitHubRepositoryConfig{Branch: "main"},
},
})
progress.On("SetMessage", mock.Anything, "listing pull request files").Return()
files := []repository.VersionedFileChange{
{Path: "test.yaml"},
}
repo.MockPullRequestRepo.On("CompareFiles", mock.Anything, "main", "test-ref").Return(files, nil)
evaluator.On("Evaluate", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(changeInfo{}, nil)
commenter.On("Comment", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
},
expectedError: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
evaluator := NewMockEvaluator(t)
commenter := NewMockCommenter(t)
repo := mockPullRequestRepo{
MockRepository: repository.NewMockRepository(t),
MockPullRequestRepo: NewMockPullRequestRepo(t),
}
progress := jobs.NewMockJobProgressRecorder(t)
tt.setupMocks(evaluator, commenter, &repo, progress)
worker := NewPullRequestWorker(evaluator, commenter)
job := provisioning.Job{
Spec: provisioning.JobSpec{
Action: provisioning.JobActionPullRequest,
PullRequest: tt.opts,
},
}
err := worker.Process(context.Background(), repo, job, progress)
if tt.expectedError != "" {
require.EqualError(t, err, tt.expectedError)
} else {
require.NoError(t, err)
}
evaluator.AssertExpectations(t)
commenter.AssertExpectations(t)
repo.AssertExpectations(t)
progress.AssertExpectations(t)
})
}
}
type mockPullRequestRepo struct {
*repository.MockRepository
*MockPullRequestRepo
}
// implemented by both mocks
func (m mockPullRequestRepo) Config() *provisioning.Repository {
return m.MockRepository.Config()
}
// implemented by both mocks
func (m mockPullRequestRepo) Read(ctx context.Context, path, ref string) (*repository.FileInfo, error) {
return m.MockRepository.Read(ctx, path, ref)
}
// implemented by both mocks
func (m mockPullRequestRepo) AssertExpectations(t *testing.T) {
m.MockRepository.AssertExpectations(t)
m.MockPullRequestRepo.AssertExpectations(t)
}
@@ -0,0 +1,191 @@
// Code generated by mockery v2.52.4. DO NOT EDIT.
package repository
import (
context "context"
mock "github.com/stretchr/testify/mock"
field "k8s.io/apimachinery/pkg/util/validation/field"
v0alpha1 "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
)
// MockConfigRepository is an autogenerated mock type for the Repository type
type MockConfigRepository struct {
mock.Mock
}
type MockConfigRepository_Expecter struct {
mock *mock.Mock
}
func (_m *MockConfigRepository) EXPECT() *MockConfigRepository_Expecter {
return &MockConfigRepository_Expecter{mock: &_m.Mock}
}
// Config provides a mock function with no fields
func (_m *MockConfigRepository) Config() *v0alpha1.Repository {
ret := _m.Called()
if len(ret) == 0 {
panic("no return value specified for Config")
}
var r0 *v0alpha1.Repository
if rf, ok := ret.Get(0).(func() *v0alpha1.Repository); ok {
r0 = rf()
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*v0alpha1.Repository)
}
}
return r0
}
// MockConfigRepository_Config_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Config'
type MockConfigRepository_Config_Call struct {
*mock.Call
}
// Config is a helper method to define mock.On call
func (_e *MockConfigRepository_Expecter) Config() *MockConfigRepository_Config_Call {
return &MockConfigRepository_Config_Call{Call: _e.mock.On("Config")}
}
func (_c *MockConfigRepository_Config_Call) Run(run func()) *MockConfigRepository_Config_Call {
_c.Call.Run(func(args mock.Arguments) {
run()
})
return _c
}
func (_c *MockConfigRepository_Config_Call) Return(_a0 *v0alpha1.Repository) *MockConfigRepository_Config_Call {
_c.Call.Return(_a0)
return _c
}
func (_c *MockConfigRepository_Config_Call) RunAndReturn(run func() *v0alpha1.Repository) *MockConfigRepository_Config_Call {
_c.Call.Return(run)
return _c
}
// Test provides a mock function with given fields: ctx
func (_m *MockConfigRepository) Test(ctx context.Context) (*v0alpha1.TestResults, error) {
ret := _m.Called(ctx)
if len(ret) == 0 {
panic("no return value specified for Test")
}
var r0 *v0alpha1.TestResults
var r1 error
if rf, ok := ret.Get(0).(func(context.Context) (*v0alpha1.TestResults, error)); ok {
return rf(ctx)
}
if rf, ok := ret.Get(0).(func(context.Context) *v0alpha1.TestResults); ok {
r0 = rf(ctx)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*v0alpha1.TestResults)
}
}
if rf, ok := ret.Get(1).(func(context.Context) error); ok {
r1 = rf(ctx)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// MockConfigRepository_Test_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Test'
type MockConfigRepository_Test_Call struct {
*mock.Call
}
// Test is a helper method to define mock.On call
// - ctx context.Context
func (_e *MockConfigRepository_Expecter) Test(ctx interface{}) *MockConfigRepository_Test_Call {
return &MockConfigRepository_Test_Call{Call: _e.mock.On("Test", ctx)}
}
func (_c *MockConfigRepository_Test_Call) Run(run func(ctx context.Context)) *MockConfigRepository_Test_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context))
})
return _c
}
func (_c *MockConfigRepository_Test_Call) Return(_a0 *v0alpha1.TestResults, _a1 error) *MockConfigRepository_Test_Call {
_c.Call.Return(_a0, _a1)
return _c
}
func (_c *MockConfigRepository_Test_Call) RunAndReturn(run func(context.Context) (*v0alpha1.TestResults, error)) *MockConfigRepository_Test_Call {
_c.Call.Return(run)
return _c
}
// Validate provides a mock function with no fields
func (_m *MockConfigRepository) Validate() field.ErrorList {
ret := _m.Called()
if len(ret) == 0 {
panic("no return value specified for Validate")
}
var r0 field.ErrorList
if rf, ok := ret.Get(0).(func() field.ErrorList); ok {
r0 = rf()
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(field.ErrorList)
}
}
return r0
}
// MockConfigRepository_Validate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Validate'
type MockConfigRepository_Validate_Call struct {
*mock.Call
}
// Validate is a helper method to define mock.On call
func (_e *MockConfigRepository_Expecter) Validate() *MockConfigRepository_Validate_Call {
return &MockConfigRepository_Validate_Call{Call: _e.mock.On("Validate")}
}
func (_c *MockConfigRepository_Validate_Call) Run(run func()) *MockConfigRepository_Validate_Call {
_c.Call.Run(func(args mock.Arguments) {
run()
})
return _c
}
func (_c *MockConfigRepository_Validate_Call) Return(_a0 field.ErrorList) *MockConfigRepository_Validate_Call {
_c.Call.Return(_a0)
return _c
}
func (_c *MockConfigRepository_Validate_Call) RunAndReturn(run func() field.ErrorList) *MockConfigRepository_Validate_Call {
_c.Call.Return(run)
return _c
}
// NewMockConfigRepository creates a new instance of MockConfigRepository. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
// The first argument is typically a *testing.T value.
func NewMockConfigRepository(t interface {
mock.TestingT
Cleanup(func())
}) *MockConfigRepository {
mock := &MockConfigRepository{}
mock.Mock.Test(t)
t.Cleanup(func() { mock.AssertExpectations(t) })
return mock
}
@@ -15,6 +15,10 @@ import (
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
)
// FIXME: the name of the mock is different because there is another generated mock for Repository
// I don't know how it got generated.
//
//go:generate mockery --name Repository --structname MockConfigRepository --inpackage --filename config_repository_mock.go --with-expecter
type Repository interface {
// Config returns the saved Kubernetes object.
Config() *provisioning.Repository