diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/blobstore_client_mock.go b/pkg/registry/apis/provisioning/jobs/pullrequest/blobstore_client_mock.go new file mode 100644 index 00000000000..ad69c8c9b57 --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/blobstore_client_mock.go @@ -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 +} diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/changes.go b/pkg/registry/apis/provisioning/jobs/pullrequest/changes.go index 961485b1eb6..227f6f52fff 100644 --- a/pkg/registry/apis/provisioning/jobs/pullrequest/changes.go +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/changes.go @@ -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) diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/changes_test.go b/pkg/registry/apis/provisioning/jobs/pullrequest/changes_test.go index c45374cc1f4..d31198eb595 100644 --- a/pkg/registry/apis/provisioning/jobs/pullrequest/changes_test.go +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/changes_test.go @@ -7,166 +7,786 @@ import ( "fmt" "testing" - "github.com/grafana/grafana/pkg/apimachinery/utils" - "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" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/grafana/grafana/pkg/apimachinery/utils" + 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" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" ) func TestCalculateChanges(t *testing.T) { - parser := resources.NewMockParser(t) - reader := repository.NewMockReader(t) - progress := jobs.NewMockJobProgressRecorder(t) + tests := []struct { + name string + setupMocks func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) + changes []repository.VersionedFileChange + expectedInfo changeInfo + expectedError string + grafanaBaseURL string + }{ + { + name: "with screenshot", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + finfo := &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + } + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": resources.DashboardResource.GroupVersion().String(), + "kind": dashboardKind, + "metadata": map[string]interface{}{ + "name": "the-uid", + }, + "spec": map[string]interface{}{ + "title": "hello world", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) - finfo := &repository.FileInfo{ - Path: "path/to/file.json", - Ref: "ref", - Data: []byte("xxxx"), // not a valid JSON! - } - obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": resources.DashboardResource.GroupVersion().String(), - "kind": dashboardKind, // will trigger creating a URL - "metadata": map[string]interface{}{ - "name": "the-uid", + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + Spec: provisioning.RepositorySpec{ + GitHub: &provisioning.GitHubRepositoryConfig{ + GenerateDashboardPreviews: true, + }, + }, + }) + parser.On("Parse", mock.Anything, finfo).Return(&resources.ParsedResource{ + Info: finfo, + Repo: provisioning.ResourceRepositoryInfo{ + Namespace: "x", + Name: "y", + }, + GVK: schema.GroupVersionKind{ + Kind: dashboardKind, + }, + Obj: obj, + Existing: obj, + Meta: meta, + DryRunResponse: obj, + }, nil) + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(true) + renderer.On("RenderScreenshot", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(getDummyRenderedURL("x"), nil) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) }, - "spec": map[string]interface{}{ - "title": "hello world", // has spaces + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + GrafanaURL: "http://host/d/the-uid/hello-world", + PreviewURL: "http://host/admin/provisioning/y/dashboard/preview/path/to/file.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", + GrafanaScreenshotURL: "https://cdn2.thecatapi.com/images/9e2.jpg", + PreviewScreenshotURL: "https://cdn2.thecatapi.com/images/9e2.jpg", + }}, + }, + }, + { + name: "without screenshot", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + finfo := &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + } + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": resources.DashboardResource.GroupVersion().String(), + "kind": dashboardKind, + "metadata": map[string]interface{}{ + "name": "the-uid", + }, + "spec": map[string]interface{}{ + "title": "hello world", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) + + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + Spec: provisioning.RepositorySpec{ + GitHub: &provisioning.GitHubRepositoryConfig{ + GenerateDashboardPreviews: true, + }, + }, + }) + parser.On("Parse", mock.Anything, finfo).Return(&resources.ParsedResource{ + Info: finfo, + Repo: provisioning.ResourceRepositoryInfo{ + Namespace: "x", + Name: "y", + }, + GVK: schema.GroupVersionKind{ + Kind: dashboardKind, + }, + Obj: obj, + Existing: obj, + Meta: meta, + DryRunResponse: obj, + }, nil) + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(false) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + GrafanaURL: "http://host/d/the-uid/hello-world", + PreviewURL: "http://host/admin/provisioning/y/dashboard/preview/path/to/file.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", + GrafanaScreenshotURL: "", + PreviewScreenshotURL: "", + }}, + }, + }, + { + name: "process first 10 files", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + finfo := &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + } + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": resources.DashboardResource.GroupVersion().String(), + "kind": dashboardKind, + "metadata": map[string]interface{}{ + "name": "the-uid", + }, + "spec": map[string]interface{}{ + "title": "hello world", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) + + progress.On("SetMessage", mock.Anything, mock.Anything).Return() + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + Spec: provisioning.RepositorySpec{ + GitHub: &provisioning.GitHubRepositoryConfig{ + GenerateDashboardPreviews: true, + }, + }, + }) + parser.On("Parse", mock.Anything, finfo).Return(&resources.ParsedResource{ + Info: finfo, + Repo: provisioning.ResourceRepositoryInfo{ + Namespace: "x", + Name: "y", + }, + GVK: schema.GroupVersionKind{ + Kind: dashboardKind, + }, + Obj: obj, + Existing: obj, + Meta: meta, + DryRunResponse: obj, + }, nil) + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(true) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + }, + changes: func() []repository.VersionedFileChange { + changes := []repository.VersionedFileChange{} + for range 15 { + changes = append(changes, repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }) + } + return changes + }(), + expectedInfo: changeInfo{ + SkippedFiles: 5, + Changes: func() []fileChangeInfo { + changes := []fileChangeInfo{} + for range 10 { + changes = append(changes, fileChangeInfo{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + GrafanaURL: "http://host/d/the-uid/hello-world", + PreviewURL: "http://host/admin/provisioning/y/dashboard/preview/path/to/file.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", + }) + } + return changes + }(), + }, + }, + { + name: "parser factory error", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + }) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("parser factory error")) + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedError: "failed to get parser for test-repo: parser factory error", + }, + { + name: "file read error", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + }) + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(false) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(nil, fmt.Errorf("read error")) + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + Error: "read error", + }}, + }, + }, + { + name: "parse error", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + }) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(false) + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + + finfo := &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("invalid json"), + } + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) + parser.On("Parse", mock.Anything, finfo).Return(nil, fmt.Errorf("parse error")) + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + Error: "parse error", + }}, + }, + }, + { + name: "dry run error", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + }) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(false) + + finfo := &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + } + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": resources.DashboardResource.GroupVersion().String(), + "kind": dashboardKind, + "metadata": map[string]interface{}{ + "name": "the-uid", + }, + "spec": map[string]interface{}{ + "title": "hello world", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) + + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) + parsed := &resources.ParsedResource{ + Info: finfo, + Repo: provisioning.ResourceRepositoryInfo{ + Namespace: "x", + Name: "y", + }, + GVK: schema.GroupVersionKind{ + Kind: dashboardKind, + }, + Obj: obj, + Existing: obj, + Meta: meta, + } + parser.On("Parse", mock.Anything, finfo).Return(parsed, nil) + parsed.DryRunResponse = nil // This will cause a dry run error + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + Error: "no client configured", + Title: "hello world", + Parsed: &resources.ParsedResource{ + Info: &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + }, + GVK: schema.GroupVersionKind{ + Kind: dashboardKind, + }, + }, + }}, + }, + }, + { + name: "screenshot render error", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + finfo := &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + } + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": resources.DashboardResource.GroupVersion().String(), + "kind": dashboardKind, + "metadata": map[string]interface{}{ + "name": "the-uid", + }, + "spec": map[string]interface{}{ + "title": "hello world", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) + + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(true) + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + Spec: provisioning.RepositorySpec{ + GitHub: &provisioning.GitHubRepositoryConfig{ + GenerateDashboardPreviews: true, + }, + }, + }) + parser.On("Parse", mock.Anything, finfo).Return(&resources.ParsedResource{ + Info: finfo, + Repo: provisioning.ResourceRepositoryInfo{ + Namespace: "x", + Name: "y", + }, + GVK: schema.GroupVersionKind{ + Kind: dashboardKind, + }, + Obj: obj, + Existing: obj, + Meta: meta, + DryRunResponse: obj, + }, nil) + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(true) + renderer.On("RenderScreenshot", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return("", fmt.Errorf("render error")) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + MissingImageRenderer: true, + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + Error: "error rendering screenshot: render error", + GrafanaURL: "http://host/d/the-uid/hello-world", + PreviewURL: "http://host/admin/provisioning/y/dashboard/preview/path/to/file.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", + }}, + }, + }, + { + name: "non-dashboard resource", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + finfo := &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + } + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "test/v1", + "kind": "TestResource", + "metadata": map[string]interface{}{ + "name": "test-resource", + }, + "spec": map[string]interface{}{ + "title": "Test Resource", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(false) + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + }) + parser.On("Parse", mock.Anything, finfo).Return(&resources.ParsedResource{ + Info: finfo, + Repo: provisioning.ResourceRepositoryInfo{ + Namespace: "x", + Name: "y", + }, + GVK: schema.GroupVersionKind{ + Kind: "TestResource", + }, + Obj: obj, + Existing: obj, + Meta: meta, + DryRunResponse: obj, + }, nil) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + Title: "Test Resource", + Parsed: &resources.ParsedResource{ + Info: &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + }, + GVK: schema.GroupVersionKind{ + Kind: "TestResource", + }, + }, + }}, + }, + }, + { + name: "deleted file", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + }) + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(false) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionDeleted, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionDeleted, + Path: "path/to/file.json", + Ref: "ref", + }, + Error: "delete feedback not yet implemented", + }}, + }, + }, + { + name: "invalid grafana url", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + finfo := &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + } + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": resources.DashboardResource.GroupVersion().String(), + "kind": dashboardKind, + "metadata": map[string]interface{}{ + "name": "the:uid", // Invalid character in UID + }, + "spec": map[string]interface{}{ + "title": "hello world", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) + + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(true) + renderer.On("RenderScreenshot", mock.Anything, mock.MatchedBy(func(repo provisioning.ResourceRepositoryInfo) bool { + return repo.Namespace == "x" && repo.Name == "y" + }), "d/the:uid/hello-world", mock.Anything).Return("", fmt.Errorf("invalid URL")) + renderer.On("RenderScreenshot", mock.Anything, mock.MatchedBy(func(repo provisioning.ResourceRepositoryInfo) bool { + return repo.Namespace == "x" && repo.Name == "y" + }), "admin/provisioning/y/dashboard/preview/path/to/file.json", mock.Anything).Return("", fmt.Errorf("invalid preview URL")) + + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + Spec: provisioning.RepositorySpec{ + GitHub: &provisioning.GitHubRepositoryConfig{ + GenerateDashboardPreviews: true, + }, + }, + }) + parser.On("Parse", mock.Anything, finfo).Return(&resources.ParsedResource{ + Info: finfo, + Repo: provisioning.ResourceRepositoryInfo{ + Namespace: "x", + Name: "y", + }, + GVK: schema.GroupVersionKind{ + Kind: dashboardKind, + }, + Obj: obj, + Existing: obj, + Meta: meta, + DryRunResponse: obj, + }, nil) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + MissingImageRenderer: true, + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + Error: "error rendering screenshot: invalid preview URL", + GrafanaURL: "http://host/d/the:uid/hello-world", // Invalid URL + PreviewURL: "http://host/admin/provisioning/y/dashboard/preview/path/to/file.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", + }}, + }, + }, + { + name: "malformed grafana url", + grafanaBaseURL: "ht tp://bad url/", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + finfo := &repository.FileInfo{ + Path: "path/to/file.json", + Ref: "ref", + Data: []byte("xxxx"), + } + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": resources.DashboardResource.GroupVersion().String(), + "kind": dashboardKind, + "metadata": map[string]interface{}{ + "name": "the-uid", + }, + "spec": map[string]interface{}{ + "title": "hello world", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) + + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(false) + progress.On("SetMessage", mock.Anything, "process path/to/file.json").Return() + reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + Spec: provisioning.RepositorySpec{ + GitHub: &provisioning.GitHubRepositoryConfig{ + GenerateDashboardPreviews: true, + }, + }, + }) + parsed := &resources.ParsedResource{ + Info: finfo, + Repo: provisioning.ResourceRepositoryInfo{ + Namespace: "x", + Name: "y", + }, + GVK: schema.GroupVersionKind{ + Kind: dashboardKind, + }, + Obj: obj, + Existing: obj, + Meta: meta, + DryRunResponse: obj, + } + parser.On("Parse", mock.Anything, finfo).Return(parsed, nil) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + MissingImageRenderer: true, + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file.json", + Ref: "ref", + }, + GrafanaURL: "ht tp://bad url/d/the-uid/hello-world", // Malformed URL + PreviewURL: "ht tp://bad url/admin/provisioning/y/dashboard/preview/path/to/file.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", + }}, }, }, } - meta, _ := utils.MetaAccessor(obj) - progress.On("SetMessage", mock.Anything, mock.Anything).Return() - reader.On("Read", mock.Anything, "path/to/file.json", "ref").Return(finfo, nil) - reader.On("Config").Return(&v0alpha1.Repository{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-repo", - Namespace: "x", - }, - Spec: v0alpha1.RepositorySpec{ - GitHub: &v0alpha1.GitHubRepositoryConfig{ - GenerateDashboardPreviews: true, - }, - }, - }) - parser.On("Parse", mock.Anything, finfo).Return(&resources.ParsedResource{ - Info: finfo, - Repo: v0alpha1.ResourceRepositoryInfo{ - Namespace: "x", - Name: "y", - }, - GVK: schema.GroupVersionKind{ - Kind: dashboardKind, - }, - Obj: obj, - Existing: obj, - Meta: meta, - DryRunResponse: obj, // avoid hitting the client - }, nil) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parser := resources.NewMockParser(t) + reader := repository.NewMockReader(t) + progress := jobs.NewMockJobProgressRecorder(t) + renderer := NewMockScreenshotRenderer(t) + parserFactory := resources.NewMockParserFactory(t) - pullRequest := v0alpha1.PullRequestJobOptions{ - Ref: "ref", - PR: 123, - URL: "http://github.com/pr/", + tt.setupMocks(parser, reader, progress, renderer, parserFactory) + + evaluator := NewEvaluator(renderer, parserFactory, func(_ string) string { + if tt.grafanaBaseURL != "" { + return tt.grafanaBaseURL + } + + return "http://host/" + }) + + pullRequest := provisioning.PullRequestJobOptions{ + Ref: "ref", + PR: 123, + URL: "http://github.com/pr/", + } + + info, err := evaluator.Evaluate(context.Background(), reader, pullRequest, tt.changes, progress) + if tt.expectedError != "" { + require.EqualError(t, err, tt.expectedError) + return + } + + require.NoError(t, err) + require.Equal(t, len(tt.expectedInfo.Changes), len(info.Changes)) + require.Equal(t, tt.expectedInfo.SkippedFiles, info.SkippedFiles) + + // compare change URLs + for i, change := range info.Changes { + require.Equal(t, tt.expectedInfo.Changes[i].GrafanaURL, change.GrafanaURL) + require.Equal(t, tt.expectedInfo.Changes[i].PreviewURL, change.PreviewURL) + require.Equal(t, tt.expectedInfo.Changes[i].GrafanaScreenshotURL, change.GrafanaScreenshotURL) + require.Equal(t, tt.expectedInfo.Changes[i].PreviewScreenshotURL, change.PreviewScreenshotURL) + require.Equal(t, tt.expectedInfo.Changes[i].Error, change.Error) + } + }) } - createdFileChange := repository.VersionedFileChange{ - Action: repository.FileActionCreated, - Path: "path/to/file.json", - Ref: "ref", - } - - t.Run("with-screenshot", func(t *testing.T) { - renderer := NewMockScreenshotRenderer(t) - renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(true) - renderer.On("RenderScreenshot", mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(getDummyRenderedURL("x"), nil) - changes := []repository.VersionedFileChange{createdFileChange} - - parserFactory := resources.NewMockParserFactory(t) - parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) - evaluator := NewEvaluator(renderer, parserFactory, func(_ string) string { - return "http://host/" - }) - - info, err := evaluator.Evaluate(context.Background(), reader, pullRequest, changes, progress) - require.NoError(t, err) - - require.False(t, info.MissingImageRenderer) - require.Equal(t, map[string]string{ - "Grafana": "http://host/d/the-uid/hello-world", - "GrafanaSnapshot": "https://cdn2.thecatapi.com/images/9e2.jpg", - "Preview": "http://host/admin/provisioning/y/dashboard/preview/path/to/file.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", - "PreviewSnapshot": "https://cdn2.thecatapi.com/images/9e2.jpg", - }, map[string]string{ - "Grafana": info.Changes[0].GrafanaURL, - "GrafanaSnapshot": info.Changes[0].GrafanaScreenshotURL, - "Preview": info.Changes[0].PreviewURL, - "PreviewSnapshot": info.Changes[0].PreviewScreenshotURL, - }) - }) - - t.Run("without-screenshot", func(t *testing.T) { - renderer := NewMockScreenshotRenderer(t) - renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(false) - changes := []repository.VersionedFileChange{createdFileChange} - parserFactory := resources.NewMockParserFactory(t) - parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) - evaluator := NewEvaluator(renderer, parserFactory, func(_ string) string { - return "http://host/" - }) - - info, err := evaluator.Evaluate(context.Background(), reader, pullRequest, changes, progress) - require.NoError(t, err) - - require.True(t, info.MissingImageRenderer) - require.Equal(t, map[string]string{ - "Grafana": "http://host/d/the-uid/hello-world", - "GrafanaSnapshot": "", - "Preview": "http://host/admin/provisioning/y/dashboard/preview/path/to/file.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", - "PreviewSnapshot": "", - }, map[string]string{ - "Grafana": info.Changes[0].GrafanaURL, - "GrafanaSnapshot": info.Changes[0].GrafanaScreenshotURL, - "Preview": info.Changes[0].PreviewURL, - "PreviewSnapshot": info.Changes[0].PreviewScreenshotURL, - }) - }) - - t.Run("process first 10 files", func(t *testing.T) { - renderer := NewMockScreenshotRenderer(t) - renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(true) - - changes := []repository.VersionedFileChange{} - for range 15 { - changes = append(changes, createdFileChange) - } - - parserFactory := resources.NewMockParserFactory(t) - parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) - evaluator := NewEvaluator(renderer, parserFactory, func(_ string) string { - return "http://host/" - }) - - info, err := evaluator.Evaluate(context.Background(), reader, pullRequest, changes, progress) - require.NoError(t, err) - - require.False(t, info.MissingImageRenderer) - require.Equal(t, 10, len(info.Changes)) - require.Equal(t, 5, info.SkippedFiles) - - // Make sure we linked a URL, but no screenshot for each item - for _, change := range info.Changes { - require.NotEmpty(t, change.GrafanaURL) - require.Empty(t, change.GrafanaScreenshotURL) - } - }) } func TestDummyImageURL(t *testing.T) { @@ -206,3 +826,89 @@ func getDummyRenderedURL(url string) string { } return dummy[idx] } + +// FIXME: test these cases from the public interface once the component is refactored +func TestRenderScreenshotFromGrafanaURL(t *testing.T) { + tests := []struct { + name string + baseURL string + grafanaURL string + setupMock func(renderer *MockScreenshotRenderer) + wantSnap string + wantErr string + }{ + { + name: "invalid grafana url", + baseURL: "http://host/", + grafanaURL: "ht tp://host/d/uid/dashboard", + setupMock: func(renderer *MockScreenshotRenderer) {}, + wantErr: `parse "ht tp://host/d/uid/dashboard": first path segment in URL cannot contain colon`, + }, + { + name: "invalid base url", + baseURL: "ht tp://bad host/", + grafanaURL: "http://host/d/uid/dashboard", + setupMock: func(renderer *MockScreenshotRenderer) { + renderer.On("RenderScreenshot", mock.Anything, mock.MatchedBy(func(repo provisioning.ResourceRepositoryInfo) bool { + return repo.Namespace == "test" && repo.Name == "repo" + }), "d/uid/dashboard", mock.Anything).Return("screenshot.png", nil) + }, + wantErr: `parse "ht tp://bad host/": first path segment in URL cannot contain colon`, + }, + { + name: "render error", + baseURL: "http://host/", + grafanaURL: "http://host/d/uid/dashboard", + setupMock: func(renderer *MockScreenshotRenderer) { + renderer.On("RenderScreenshot", mock.Anything, mock.MatchedBy(func(repo provisioning.ResourceRepositoryInfo) bool { + return repo.Namespace == "test" && repo.Name == "repo" + }), "d/uid/dashboard", mock.Anything).Return("", fmt.Errorf("render failed")) + }, + wantErr: "error rendering screenshot: render failed", + }, + { + name: "cdn url returned", + baseURL: "http://host/", + grafanaURL: "http://host/d/uid/dashboard", + setupMock: func(renderer *MockScreenshotRenderer) { + renderer.On("RenderScreenshot", mock.Anything, mock.MatchedBy(func(repo provisioning.ResourceRepositoryInfo) bool { + return repo.Namespace == "test" && repo.Name == "repo" + }), "d/uid/dashboard", mock.Anything).Return("https://cdn.example.com/screenshot.png", nil) + }, + wantSnap: "https://cdn.example.com/screenshot.png", + }, + { + name: "successful render with relative path", + baseURL: "http://host/", + grafanaURL: "http://host/d/uid/dashboard", + setupMock: func(renderer *MockScreenshotRenderer) { + renderer.On("RenderScreenshot", mock.Anything, mock.MatchedBy(func(repo provisioning.ResourceRepositoryInfo) bool { + return repo.Namespace == "test" && repo.Name == "repo" + }), "d/uid/dashboard", mock.Anything).Return("screenshots/123.png", nil) + }, + wantSnap: "http://host/screenshots/123.png", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + renderer := NewMockScreenshotRenderer(t) + tt.setupMock(renderer) + + repo := provisioning.ResourceRepositoryInfo{ + Namespace: "test", + Name: "repo", + } + + got, err := renderScreenshotFromGrafanaURL(context.Background(), tt.baseURL, renderer, repo, tt.grafanaURL) + if tt.wantErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + require.Equal(t, tt.wantSnap, got) + }) + } +} diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/comment.go b/pkg/registry/apis/provisioning/jobs/pullrequest/comment.go index 12003b73305..9229df5a433 100644 --- a/pkg/registry/apis/provisioning/jobs/pullrequest/comment.go +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/comment.go @@ -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) diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/comment_test.go b/pkg/registry/apis/provisioning/jobs/pullrequest/comment_test.go index ec0d0da27bf..fdd8e3f6dd4 100644 --- a/pkg/registry/apis/provisioning/jobs/pullrequest/comment_test.go +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/comment_test.go @@ -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{ diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/render.go b/pkg/registry/apis/provisioning/jobs/pullrequest/render.go index 644a8660973..31e35b86673 100644 --- a/pkg/registry/apis/provisioning/jobs/pullrequest/render.go +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/render.go @@ -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, diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/render_test.go b/pkg/registry/apis/provisioning/jobs/pullrequest/render_test.go new file mode 100644 index 00000000000..f21be8d1c18 --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/render_test.go @@ -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¶m2=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) + } + }) + } +} diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/testdata/no-changes.md b/pkg/registry/apis/provisioning/jobs/pullrequest/testdata/no-changes.md new file mode 100644 index 00000000000..5109fbbab25 --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/testdata/no-changes.md @@ -0,0 +1 @@ +Grafana didn't find any changes in this pull request. \ No newline at end of file diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/worker.go b/pkg/registry/apis/provisioning/jobs/pullrequest/worker.go index c95a37dbb7e..28db3d37f06 100644 --- a/pkg/registry/apis/provisioning/jobs/pullrequest/worker.go +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/worker.go @@ -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 } diff --git a/pkg/registry/apis/provisioning/jobs/pullrequest/worker_test.go b/pkg/registry/apis/provisioning/jobs/pullrequest/worker_test.go new file mode 100644 index 00000000000..4ba1cae7cb3 --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/pullrequest/worker_test.go @@ -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) +} diff --git a/pkg/registry/apis/provisioning/repository/config_repository_mock.go b/pkg/registry/apis/provisioning/repository/config_repository_mock.go new file mode 100644 index 00000000000..40ea3dcd3de --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/config_repository_mock.go @@ -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 +} diff --git a/pkg/registry/apis/provisioning/repository/repository.go b/pkg/registry/apis/provisioning/repository/repository.go index 0d15d28a9d8..a02ae539b08 100644 --- a/pkg/registry/apis/provisioning/repository/repository.go +++ b/pkg/registry/apis/provisioning/repository/repository.go @@ -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