From 3923538ba4fb7442422203780f612ff8a8da2ac0 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Wed, 23 Apr 2025 13:30:30 +0300 Subject: [PATCH] Provisioning: Delete from repository, not the resource client on rename (#104291) Co-authored-by: Roberto Jimenez Sanchez --- .../apis/provisioning/jobs/export/all.go | 9 +- .../apis/provisioning/jobs/export/folders.go | 6 +- .../jobs/export/mock_export_fn.go | 23 +- .../apis/provisioning/jobs/export/worker.go | 10 +- .../provisioning/jobs/export/worker_test.go | 38 -- .../jobs/migrate/legacy_folders.go | 98 ------ .../jobs/migrate/legacy_folders_test.go | 326 ------------------ .../jobs/migrate/legacy_resources.go | 33 +- .../jobs/migrate/legacy_resources_test.go | 277 ++++++++++----- .../migrate/mock_legacy_folders_migrator.go | 240 ------------- pkg/registry/apis/provisioning/register.go | 4 +- .../provisioning/repository/go-git/wrapper.go | 19 +- 12 files changed, 263 insertions(+), 820 deletions(-) delete mode 100644 pkg/registry/apis/provisioning/jobs/migrate/legacy_folders.go delete mode 100644 pkg/registry/apis/provisioning/jobs/migrate/legacy_folders_test.go delete mode 100644 pkg/registry/apis/provisioning/jobs/migrate/mock_legacy_folders_migrator.go diff --git a/pkg/registry/apis/provisioning/jobs/export/all.go b/pkg/registry/apis/provisioning/jobs/export/all.go index 2f4b2253f76..1010c02f630 100644 --- a/pkg/registry/apis/provisioning/jobs/export/all.go +++ b/pkg/registry/apis/provisioning/jobs/export/all.go @@ -6,11 +6,16 @@ import ( 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/resources" - "k8s.io/client-go/dynamic" ) -func ExportAll(ctx context.Context, repoName string, options provisioning.ExportJobOptions, clients resources.ResourceClients, repositoryResources resources.RepositoryResources, folderClient dynamic.ResourceInterface, progress jobs.JobProgressRecorder) error { +func ExportAll(ctx context.Context, repoName string, options provisioning.ExportJobOptions, clients resources.ResourceClients, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder) error { // FIXME: should we sign with grafana user? + + folderClient, err := clients.Folder() + if err != nil { + return err + } + if err := ExportFolders(ctx, repoName, options, folderClient, repositoryResources, progress); err != nil { return err } diff --git a/pkg/registry/apis/provisioning/jobs/export/folders.go b/pkg/registry/apis/provisioning/jobs/export/folders.go index 954ff5dc358..afc7db05a87 100644 --- a/pkg/registry/apis/provisioning/jobs/export/folders.go +++ b/pkg/registry/apis/provisioning/jobs/export/folders.go @@ -5,15 +5,17 @@ import ( "errors" "fmt" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/dynamic" + "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" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/dynamic" ) +// ExportFolders will load the full folder tree into memory and update the repositoryResources tree func ExportFolders(ctx context.Context, repoName string, options provisioning.ExportJobOptions, folderClient dynamic.ResourceInterface, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder) error { // Load and write all folders // FIXME: we load the entire tree in memory diff --git a/pkg/registry/apis/provisioning/jobs/export/mock_export_fn.go b/pkg/registry/apis/provisioning/jobs/export/mock_export_fn.go index 7e7a405055a..e861e7da3c3 100644 --- a/pkg/registry/apis/provisioning/jobs/export/mock_export_fn.go +++ b/pkg/registry/apis/provisioning/jobs/export/mock_export_fn.go @@ -6,8 +6,6 @@ import ( context "context" jobs "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" - dynamic "k8s.io/client-go/dynamic" - mock "github.com/stretchr/testify/mock" resources "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" @@ -28,17 +26,17 @@ func (_m *MockExportFn) EXPECT() *MockExportFn_Expecter { return &MockExportFn_Expecter{mock: &_m.Mock} } -// Execute provides a mock function with given fields: ctx, repoName, options, clients, repositoryResources, folderClient, progress -func (_m *MockExportFn) Execute(ctx context.Context, repoName string, options v0alpha1.ExportJobOptions, clients resources.ResourceClients, repositoryResources resources.RepositoryResources, folderClient dynamic.ResourceInterface, progress jobs.JobProgressRecorder) error { - ret := _m.Called(ctx, repoName, options, clients, repositoryResources, folderClient, progress) +// Execute provides a mock function with given fields: ctx, repoName, options, clients, repositoryResources, progress +func (_m *MockExportFn) Execute(ctx context.Context, repoName string, options v0alpha1.ExportJobOptions, clients resources.ResourceClients, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder) error { + ret := _m.Called(ctx, repoName, options, clients, repositoryResources, progress) if len(ret) == 0 { panic("no return value specified for Execute") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, v0alpha1.ExportJobOptions, resources.ResourceClients, resources.RepositoryResources, dynamic.ResourceInterface, jobs.JobProgressRecorder) error); ok { - r0 = rf(ctx, repoName, options, clients, repositoryResources, folderClient, progress) + if rf, ok := ret.Get(0).(func(context.Context, string, v0alpha1.ExportJobOptions, resources.ResourceClients, resources.RepositoryResources, jobs.JobProgressRecorder) error); ok { + r0 = rf(ctx, repoName, options, clients, repositoryResources, progress) } else { r0 = ret.Error(0) } @@ -57,15 +55,14 @@ type MockExportFn_Execute_Call struct { // - options v0alpha1.ExportJobOptions // - clients resources.ResourceClients // - repositoryResources resources.RepositoryResources -// - folderClient dynamic.ResourceInterface // - progress jobs.JobProgressRecorder -func (_e *MockExportFn_Expecter) Execute(ctx interface{}, repoName interface{}, options interface{}, clients interface{}, repositoryResources interface{}, folderClient interface{}, progress interface{}) *MockExportFn_Execute_Call { - return &MockExportFn_Execute_Call{Call: _e.mock.On("Execute", ctx, repoName, options, clients, repositoryResources, folderClient, progress)} +func (_e *MockExportFn_Expecter) Execute(ctx interface{}, repoName interface{}, options interface{}, clients interface{}, repositoryResources interface{}, progress interface{}) *MockExportFn_Execute_Call { + return &MockExportFn_Execute_Call{Call: _e.mock.On("Execute", ctx, repoName, options, clients, repositoryResources, progress)} } -func (_c *MockExportFn_Execute_Call) Run(run func(ctx context.Context, repoName string, options v0alpha1.ExportJobOptions, clients resources.ResourceClients, repositoryResources resources.RepositoryResources, folderClient dynamic.ResourceInterface, progress jobs.JobProgressRecorder)) *MockExportFn_Execute_Call { +func (_c *MockExportFn_Execute_Call) Run(run func(ctx context.Context, repoName string, options v0alpha1.ExportJobOptions, clients resources.ResourceClients, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder)) *MockExportFn_Execute_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(v0alpha1.ExportJobOptions), args[3].(resources.ResourceClients), args[4].(resources.RepositoryResources), args[5].(dynamic.ResourceInterface), args[6].(jobs.JobProgressRecorder)) + run(args[0].(context.Context), args[1].(string), args[2].(v0alpha1.ExportJobOptions), args[3].(resources.ResourceClients), args[4].(resources.RepositoryResources), args[5].(jobs.JobProgressRecorder)) }) return _c } @@ -75,7 +72,7 @@ func (_c *MockExportFn_Execute_Call) Return(_a0 error) *MockExportFn_Execute_Cal return _c } -func (_c *MockExportFn_Execute_Call) RunAndReturn(run func(context.Context, string, v0alpha1.ExportJobOptions, resources.ResourceClients, resources.RepositoryResources, dynamic.ResourceInterface, jobs.JobProgressRecorder) error) *MockExportFn_Execute_Call { +func (_c *MockExportFn_Execute_Call) RunAndReturn(run func(context.Context, string, v0alpha1.ExportJobOptions, resources.ResourceClients, resources.RepositoryResources, jobs.JobProgressRecorder) error) *MockExportFn_Execute_Call { _c.Call.Return(run) return _c } diff --git a/pkg/registry/apis/provisioning/jobs/export/worker.go b/pkg/registry/apis/provisioning/jobs/export/worker.go index 73e4eaa6c2e..c8347746584 100644 --- a/pkg/registry/apis/provisioning/jobs/export/worker.go +++ b/pkg/registry/apis/provisioning/jobs/export/worker.go @@ -11,11 +11,10 @@ import ( "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" - "k8s.io/client-go/dynamic" ) //go:generate mockery --name ExportFn --structname MockExportFn --inpackage --filename mock_export_fn.go --with-expecter -type ExportFn func(ctx context.Context, repoName string, options provisioning.ExportJobOptions, clients resources.ResourceClients, repositoryResources resources.RepositoryResources, folderClient dynamic.ResourceInterface, progress jobs.JobProgressRecorder) error +type ExportFn func(ctx context.Context, repoName string, options provisioning.ExportJobOptions, clients resources.ResourceClients, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder) error //go:generate mockery --name WrapWithCloneFn --structname MockWrapWithCloneFn --inpackage --filename mock_wrap_with_clone_fn.go --with-expecter type WrapWithCloneFn func(ctx context.Context, repo repository.Repository, cloneOptions repository.CloneOptions, pushOptions repository.PushOptions, fn func(repo repository.Repository, cloned bool) error) error @@ -87,11 +86,6 @@ func (r *ExportWorker) Process(ctx context.Context, repo repository.Repository, return fmt.Errorf("create clients: %w", err) } - folderClient, err := clients.Folder() - if err != nil { - return fmt.Errorf("create folder client: %w", err) - } - rw, ok := repo.(repository.ReaderWriter) if !ok { return errors.New("export job submitted targeting repository that is not a ReaderWriter") @@ -102,7 +96,7 @@ func (r *ExportWorker) Process(ctx context.Context, repo repository.Repository, return fmt.Errorf("create repository resource client: %w", err) } - return r.exportFn(ctx, cfg.Name, *options, clients, repositoryResources, folderClient, progress) + return r.exportFn(ctx, cfg.Name, *options, clients, repositoryResources, progress) } return r.wrapWithCloneFn(ctx, repo, cloneOptions, pushOptions, fn) diff --git a/pkg/registry/apis/provisioning/jobs/export/worker_test.go b/pkg/registry/apis/provisioning/jobs/export/worker_test.go index 3e56f63047f..7a2bfaf0c4f 100644 --- a/pkg/registry/apis/provisioning/jobs/export/worker_test.go +++ b/pkg/registry/apis/provisioning/jobs/export/worker_test.go @@ -177,7 +177,6 @@ func TestExportWorker_ProcessNotReaderWriter(t *testing.T) { resourceClients := resources.NewMockResourceClients(t) mockClients := resources.NewMockClientFactory(t) mockClients.On("Clients", context.Background(), "test-namespace").Return(resourceClients, nil) - resourceClients.On("Folder").Return(nil, nil) mockProgress := jobs.NewMockJobProgressRecorder(t) mockCloneFn := NewMockWrapWithCloneFn(t) @@ -190,40 +189,6 @@ func TestExportWorker_ProcessNotReaderWriter(t *testing.T) { require.EqualError(t, err, "export job submitted targeting repository that is not a ReaderWriter") } -func TestExportWorker_ProcessFolderClientError(t *testing.T) { - job := v0alpha1.Job{ - Spec: v0alpha1.JobSpec{ - Action: v0alpha1.JobActionPush, - Push: &v0alpha1.ExportJobOptions{}, - }, - } - - mockRepo := repository.NewMockRepository(t) - mockRepo.On("Config").Return(&v0alpha1.Repository{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-repo", - Namespace: "test-namespace", - }, - Spec: v0alpha1.RepositorySpec{ - Workflows: []v0alpha1.Workflow{v0alpha1.WriteWorkflow}, - }, - }) - - resourceClients := resources.NewMockResourceClients(t) - mockClients := resources.NewMockClientFactory(t) - mockClients.On("Clients", context.Background(), "test-namespace").Return(resourceClients, nil) - resourceClients.On("Folder").Return(nil, fmt.Errorf("failed to create folder client")) - - mockProgress := jobs.NewMockJobProgressRecorder(t) - mockCloneFn := NewMockWrapWithCloneFn(t) - mockCloneFn.On("Execute", context.Background(), mockRepo, mock.Anything, mock.Anything, mock.Anything).Return(func(ctx context.Context, repo repository.Repository, cloneOpts repository.CloneOptions, pushOpts repository.PushOptions, fn func(repository.Repository, bool) error) error { - return fn(repo, true) - }) - r := NewExportWorker(mockClients, nil, nil, mockCloneFn.Execute) - err := r.Process(context.Background(), mockRepo, job, mockProgress) - require.EqualError(t, err, "create folder client: failed to create folder client") -} - func TestExportWorker_ProcessRepositoryResourcesError(t *testing.T) { job := v0alpha1.Job{ Spec: v0alpha1.JobSpec{ @@ -244,7 +209,6 @@ func TestExportWorker_ProcessRepositoryResourcesError(t *testing.T) { }) resourceClients := resources.NewMockResourceClients(t) - resourceClients.On("Folder").Return(nil, nil) mockClients := resources.NewMockClientFactory(t) mockClients.On("Clients", context.Background(), "test-namespace").Return(resourceClients, nil) @@ -288,7 +252,6 @@ func TestExportWorker_ProcessCloneAndPushOptions(t *testing.T) { mockClients := resources.NewMockClientFactory(t) mockResourceClients := resources.NewMockResourceClients(t) mockClients.On("Clients", mock.Anything, "test-namespace").Return(mockResourceClients, nil) - mockResourceClients.On("Folder").Return(nil, nil) mockRepoResources := resources.NewMockRepositoryResourcesFactory(t) mockRepoResourcesClient := resources.NewMockRepositoryResources(t) @@ -339,7 +302,6 @@ func TestExportWorker_ProcessExportFnError(t *testing.T) { mockClients := resources.NewMockClientFactory(t) mockResourceClients := resources.NewMockResourceClients(t) mockClients.On("Clients", mock.Anything, "test-namespace").Return(mockResourceClients, nil) - mockResourceClients.On("Folder").Return(nil, nil) mockRepoResources := resources.NewMockRepositoryResourcesFactory(t) mockRepoResourcesClient := resources.NewMockRepositoryResources(t) diff --git a/pkg/registry/apis/provisioning/jobs/migrate/legacy_folders.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_folders.go deleted file mode 100644 index 6f58eff634b..00000000000 --- a/pkg/registry/apis/provisioning/jobs/migrate/legacy_folders.go +++ /dev/null @@ -1,98 +0,0 @@ -package migrate - -import ( - "context" - "errors" - "fmt" - - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy" - "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/grafana/grafana/pkg/storage/unified/parquet" - "github.com/grafana/grafana/pkg/storage/unified/resource" -) - -const maxFolders = 10000 - -//go:generate mockery --name LegacyFoldersMigrator --structname MockLegacyFoldersMigrator --inpackage --filename mock_legacy_folders_migrator.go --with-expecter -type LegacyFoldersMigrator interface { - resource.BulkResourceWriter - Migrate(ctx context.Context, namespace string, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder) error -} - -type legacyFoldersMigrator struct { - tree resources.FolderTree - legacyMigrator legacy.LegacyMigrator -} - -func NewLegacyFoldersMigrator(legacyMigrator legacy.LegacyMigrator) LegacyFoldersMigrator { - return &legacyFoldersMigrator{ - legacyMigrator: legacyMigrator, - tree: resources.NewEmptyFolderTree(), - } -} - -// Close implements resource.BulkResourceWrite. -func (f *legacyFoldersMigrator) Close() error { - return nil -} - -// CloseWithResults implements resource.BulkResourceWrite. -func (f *legacyFoldersMigrator) CloseWithResults() (*resource.BulkResponse, error) { - return &resource.BulkResponse{}, nil -} - -// Write implements resource.BulkResourceWrite. -func (f *legacyFoldersMigrator) Write(ctx context.Context, key *resource.ResourceKey, value []byte) error { - item := &unstructured.Unstructured{} - err := item.UnmarshalJSON(value) - if err != nil { - return fmt.Errorf("unmarshal unstructured to JSON: %w", err) - } - - if f.tree.Count() > maxFolders { - return errors.New("too many folders") - } - - // TODO: should we check if managed already and abort migration? - - return f.tree.AddUnstructured(item) -} - -func (f *legacyFoldersMigrator) Migrate(ctx context.Context, namespace string, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder) error { - progress.SetMessage(ctx, "read folders from SQL") - if _, err := f.legacyMigrator.Migrate(ctx, legacy.MigrateOptions{ - Namespace: namespace, - Resources: []schema.GroupResource{resources.FolderResource.GroupResource()}, - Store: parquet.NewBulkResourceWriterClient(f), - }); err != nil { - return fmt.Errorf("read folders from SQL: %w", err) - } - - progress.SetMessage(ctx, "export folders from SQL") - // FIXME: we don't sign folders, not even with grafana user - if err := repositoryResources.EnsureFolderTreeExists(ctx, "", "", f.tree, func(folder resources.Folder, created bool, err error) error { - result := jobs.JobResourceResult{ - Action: repository.FileActionCreated, - Name: folder.ID, - Resource: resources.FolderResource.Resource, - Group: resources.FolderResource.Group, - Path: folder.Path, - Error: err, - } - - if !created { - result.Action = repository.FileActionIgnored - } - progress.Record(ctx, result) - return err - }); err != nil { - return fmt.Errorf("export folders from SQL: %w", err) - } - - return nil -} diff --git a/pkg/registry/apis/provisioning/jobs/migrate/legacy_folders_test.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_folders_test.go deleted file mode 100644 index c5c11850638..00000000000 --- a/pkg/registry/apis/provisioning/jobs/migrate/legacy_folders_test.go +++ /dev/null @@ -1,326 +0,0 @@ -package migrate - -import ( - "context" - "errors" - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - - "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy" - "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/grafana/grafana/pkg/storage/unified/resource" -) - -func TestLegacyFoldersMigrator_Write(t *testing.T) { - t.Run("should fail when json is invalid", func(t *testing.T) { - migrator := NewLegacyFoldersMigrator(legacy.NewMockLegacyMigrator(t)) - err := migrator.Write(context.Background(), nil, []byte("invalid json")) - require.Error(t, err) - require.Contains(t, err.Error(), "unmarshal unstructured to JSON") - }) - - t.Run("should fail when too many folders", func(t *testing.T) { - migrator := NewLegacyFoldersMigrator(legacy.NewMockLegacyMigrator(t)) - - // Write more than maxFolders - for i := 0; i <= maxFolders+1; i++ { - folder := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "folder.grafana.app/v1alpha1", - "kind": "Folder", - "metadata": map[string]interface{}{ - "name": fmt.Sprintf("test-folder-%d", i), - }, - }, - } - folder.SetKind("Folder") - folder.SetAPIVersion("folder.grafana.app/v1alpha1") - - data, err := folder.MarshalJSON() - require.NoError(t, err) - if i == maxFolders+1 { - err = migrator.Write(context.Background(), nil, data) - require.Error(t, err) - require.Equal(t, "too many folders", err.Error()) - return - } - err = migrator.Write(context.Background(), nil, data) - require.NoError(t, err) - } - }) - - t.Run("should add folder to tree", func(t *testing.T) { - migrator := NewLegacyFoldersMigrator(legacy.NewMockLegacyMigrator(t)) - folder := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "folder.grafana.app/v1alpha1", - "kind": "Folder", - "metadata": map[string]interface{}{ - "name": "test-folder", - "annotations": map[string]interface{}{ - "folder.grafana.app/uid": "test-folder-uid", - }, - }, - }, - } - folder.SetKind("Folder") - folder.SetAPIVersion("folder.grafana.app/v1alpha1") - - data, err := folder.MarshalJSON() - require.NoError(t, err) - - err = migrator.Write(context.Background(), nil, data) - require.NoError(t, err) - }) -} - -func TestLegacyFoldersMigrator_Migrate(t *testing.T) { - t.Run("should fail when legacy migrator fails", func(t *testing.T) { - mockLegacyMigrator := legacy.NewMockLegacyMigrator(t) - mockLegacyMigrator.On("Migrate", mock.Anything, mock.MatchedBy(func(opts legacy.MigrateOptions) bool { - return opts.Namespace == "test-namespace" && - len(opts.Resources) == 1 && - opts.Resources[0] == resources.FolderResource.GroupResource() - }), mock.Anything).Return(nil, errors.New("migration failed")) - - migrator := NewLegacyFoldersMigrator(mockLegacyMigrator) - progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, "read folders from SQL").Return() - - err := migrator.Migrate(context.Background(), "test-namespace", nil, progress) - require.Error(t, err) - require.Contains(t, err.Error(), "read folders from SQL: migration failed") - progress.AssertExpectations(t) - }) - - t.Run("should fail when folder tree creation fails", func(t *testing.T) { - mockLegacyMigrator := legacy.NewMockLegacyMigrator(t) - mockLegacyMigrator.On("Migrate", mock.Anything, mock.MatchedBy(func(opts legacy.MigrateOptions) bool { - return opts.Namespace == "test-namespace" && - len(opts.Resources) == 1 && - opts.Resources[0] == resources.FolderResource.GroupResource() - }), mock.Anything).Return(&resource.BulkResponse{}, nil) - - mockRepositoryResources := resources.NewMockRepositoryResources(t) - mockRepositoryResources.On("EnsureFolderTreeExists", mock.Anything, "", "", mock.Anything, mock.Anything). - Return(errors.New("folder tree creation failed")) - - migrator := NewLegacyFoldersMigrator(mockLegacyMigrator) - progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, "read folders from SQL").Return() - progress.On("SetMessage", mock.Anything, "export folders from SQL").Return() - - err := migrator.Migrate(context.Background(), "test-namespace", mockRepositoryResources, progress) - require.Error(t, err) - require.Contains(t, err.Error(), "export folders from SQL: folder tree creation failed") - - progress.AssertExpectations(t) - }) - - t.Run("should successfully migrate folders", func(t *testing.T) { - mockLegacyMigrator := legacy.NewMockLegacyMigrator(t) - mockLegacyMigrator.On("Migrate", mock.Anything, mock.MatchedBy(func(opts legacy.MigrateOptions) bool { - return opts.Namespace == "test-namespace" && - len(opts.Resources) == 1 && - opts.Resources[0] == resources.FolderResource.GroupResource() - }), mock.Anything).Run(func(args mock.Arguments) { - // Simulate writing a folder through the bulk writer - opts := args.Get(1).(legacy.MigrateOptions) - folder := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "folder.grafana.app/v1alpha1", - "kind": "Folder", - "metadata": map[string]interface{}{ - "name": "test-folder", - "annotations": map[string]interface{}{ - "folder.grafana.app/uid": "test-folder-uid", - }, - }, - }, - } - folder.SetKind("Folder") - folder.SetAPIVersion("folder.grafana.app/v1alpha1") - - data, err := folder.MarshalJSON() - require.NoError(t, err) - client, err := opts.Store.BulkProcess(context.Background()) - require.NoError(t, err) - require.NoError(t, client.Send(&resource.BulkRequest{ - Key: &resource.ResourceKey{Namespace: "test-namespace", Name: "test-folder"}, - Value: data, - })) - }).Return(&resource.BulkResponse{}, nil) - - mockRepositoryResources := resources.NewMockRepositoryResources(t) - mockRepositoryResources.On("EnsureFolderTreeExists", mock.Anything, "", "", mock.Anything, mock.Anything). - Run(func(args mock.Arguments) { - callback := args.Get(4).(func(folder resources.Folder, created bool, err error) error) - err := callback(resources.Folder{ - ID: "test-folder-uid", - Path: "/test-folder", - }, true, nil) - require.NoError(t, err) - }).Return(nil) - - migrator := NewLegacyFoldersMigrator(mockLegacyMigrator) - progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, "read folders from SQL").Return() - progress.On("SetMessage", mock.Anything, "export folders from SQL").Return() - progress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { - return result.Action == repository.FileActionCreated && - result.Name == "test-folder-uid" && - result.Resource == resources.FolderResource.Resource && - result.Group == resources.FolderResource.Group && - result.Path == "/test-folder" && - result.Error == nil - })).Return() - - err := migrator.Migrate(context.Background(), "test-namespace", mockRepositoryResources, progress) - require.NoError(t, err) - progress.AssertExpectations(t) - }) - t.Run("should ignore folders that already exist", func(t *testing.T) { - mockLegacyMigrator := legacy.NewMockLegacyMigrator(t) - mockLegacyMigrator.On("Migrate", mock.Anything, mock.MatchedBy(func(opts legacy.MigrateOptions) bool { - return opts.Namespace == "test-namespace" && - len(opts.Resources) == 1 && - opts.Resources[0] == resources.FolderResource.GroupResource() - }), mock.Anything).Run(func(args mock.Arguments) { - // Simulate writing a folder through the bulk writer - opts := args.Get(1).(legacy.MigrateOptions) - folder := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "folder.grafana.app/v1alpha1", - "kind": "Folder", - "metadata": map[string]interface{}{ - "name": "test-folder", - "annotations": map[string]interface{}{ - "folder.grafana.app/uid": "test-folder-uid", - }, - }, - }, - } - folder.SetKind("Folder") - folder.SetAPIVersion("folder.grafana.app/v1alpha1") - - data, err := folder.MarshalJSON() - require.NoError(t, err) - client, err := opts.Store.BulkProcess(context.Background()) - require.NoError(t, err) - require.NoError(t, client.Send(&resource.BulkRequest{ - Key: &resource.ResourceKey{Namespace: "test-namespace", Name: "test-folder"}, - Value: data, - })) - }).Return(&resource.BulkResponse{}, nil) - - mockRepositoryResources := resources.NewMockRepositoryResources(t) - mockRepositoryResources.On("EnsureFolderTreeExists", mock.Anything, "", "", mock.Anything, mock.Anything). - Run(func(args mock.Arguments) { - callback := args.Get(4).(func(folder resources.Folder, created bool, err error) error) - err := callback(resources.Folder{ - ID: "test-folder-uid", - Path: "/test-folder", - }, false, nil) - require.NoError(t, err) - }).Return(nil) - - migrator := NewLegacyFoldersMigrator(mockLegacyMigrator) - progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, "read folders from SQL").Return() - progress.On("SetMessage", mock.Anything, "export folders from SQL").Return() - progress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { - return result.Action == repository.FileActionIgnored && - result.Name == "test-folder-uid" && - result.Resource == resources.FolderResource.Resource && - result.Group == resources.FolderResource.Group && - result.Path == "/test-folder" && - result.Error == nil - })).Return() - - err := migrator.Migrate(context.Background(), "test-namespace", mockRepositoryResources, progress) - require.NoError(t, err) - progress.AssertExpectations(t) - }) - t.Run("should fail when folder creation fails", func(t *testing.T) { - mockLegacyMigrator := legacy.NewMockLegacyMigrator(t) - mockLegacyMigrator.On("Migrate", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - opts := args.Get(1).(legacy.MigrateOptions) - folder := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "folder.grafana.app/v1alpha1", - "kind": "Folder", - "metadata": map[string]interface{}{ - "name": "test-folder", - "annotations": map[string]interface{}{ - "folder.grafana.app/uid": "test-folder-uid", - }, - }, - }, - } - folder.SetKind("Folder") - folder.SetAPIVersion("folder.grafana.app/v1alpha1") - - data, err := folder.MarshalJSON() - require.NoError(t, err) - client, err := opts.Store.BulkProcess(context.Background()) - require.NoError(t, err) - require.NoError(t, client.Send(&resource.BulkRequest{ - Key: &resource.ResourceKey{Namespace: "test-namespace", Name: "test-folder"}, - Value: data, - })) - }).Return(&resource.BulkResponse{}, nil) - - mockRepositoryResources := resources.NewMockRepositoryResources(t) - expectedError := errors.New("folder creation failed") - mockRepositoryResources.On("EnsureFolderTreeExists", mock.Anything, "", "", mock.Anything, mock.Anything). - Run(func(args mock.Arguments) { - callback := args.Get(4).(func(folder resources.Folder, created bool, err error) error) - // Call the callback with an error and return its result - err := callback(resources.Folder{ - ID: "test-folder-uid", - Path: "/test-folder", - }, true, expectedError) - require.Equal(t, expectedError, err) - }).Return(expectedError) - - migrator := NewLegacyFoldersMigrator(mockLegacyMigrator) - progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, "read folders from SQL").Return() - progress.On("SetMessage", mock.Anything, "export folders from SQL").Return() - progress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { - return result.Action == repository.FileActionCreated && - result.Name == "test-folder-uid" && - result.Resource == resources.FolderResource.Resource && - result.Group == resources.FolderResource.Group && - result.Path == "/test-folder" && - assert.Equal(t, expectedError, result.Error) - })).Return() - - err := migrator.Migrate(context.Background(), "test-namespace", mockRepositoryResources, progress) - require.Error(t, err) - require.Contains(t, err.Error(), "export folders from SQL: folder creation failed") - progress.AssertExpectations(t) - }) -} - -func TestLegacyFoldersMigrator_Close(t *testing.T) { - t.Run("should close without error", func(t *testing.T) { - migrator := NewLegacyFoldersMigrator(legacy.NewMockLegacyMigrator(t)) - err := migrator.Close() - require.NoError(t, err) - }) - - t.Run("should close with results without error", func(t *testing.T) { - migrator := NewLegacyFoldersMigrator(legacy.NewMockLegacyMigrator(t)) - resp, err := migrator.CloseWithResults() - require.NoError(t, err) - require.NotNil(t, resp) - }) -} diff --git a/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go index 0a34cecb2f7..810d03a245b 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go @@ -10,6 +10,7 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/export" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources/signature" @@ -28,23 +29,26 @@ type legacyResourcesMigrator struct { repositoryResources resources.RepositoryResourcesFactory parsers resources.ParserFactory legacyMigrator legacy.LegacyMigrator - folderMigrator LegacyFoldersMigrator signerFactory signature.SignerFactory + clients resources.ClientFactory + exportFn export.ExportFn } func NewLegacyResourcesMigrator( repositoryResources resources.RepositoryResourcesFactory, parsers resources.ParserFactory, legacyMigrator legacy.LegacyMigrator, - folderMigrator LegacyFoldersMigrator, signerFactory signature.SignerFactory, + clients resources.ClientFactory, + exportFn export.ExportFn, ) LegacyResourcesMigrator { return &legacyResourcesMigrator{ repositoryResources: repositoryResources, parsers: parsers, legacyMigrator: legacyMigrator, - folderMigrator: folderMigrator, signerFactory: signerFactory, + clients: clients, + exportFn: exportFn, } } @@ -70,17 +74,25 @@ func (m *legacyResourcesMigrator) Migrate(ctx context.Context, rw repository.Rea } progress.SetMessage(ctx, "migrate folders from SQL") - if err := m.folderMigrator.Migrate(ctx, namespace, repositoryResources, progress); err != nil { + clients, err := m.clients.Clients(ctx, namespace) + if err != nil { + return err + } + + // nothing special for the export for now + exportOpts := provisioning.ExportJobOptions{} + if err = m.exportFn(ctx, rw.Config().Name, exportOpts, clients, repositoryResources, progress); err != nil { return fmt.Errorf("migrate folders from SQL: %w", err) } progress.SetMessage(ctx, "migrate resources from SQL") for _, kind := range resources.SupportedProvisioningResources { if kind == resources.FolderResource { - continue + continue // folders have special handling } - reader := NewLegacyResourceMigrator( + reader := newLegacyResourceMigrator( + rw, m.legacyMigrator, parser, repositoryResources, @@ -100,6 +112,7 @@ func (m *legacyResourcesMigrator) Migrate(ctx context.Context, rw repository.Rea } type legacyResourceResourceMigrator struct { + repo repository.ReaderWriter legacy legacy.LegacyMigrator parser resources.Parser progress jobs.JobProgressRecorder @@ -111,7 +124,8 @@ type legacyResourceResourceMigrator struct { history map[string]string // UID >> file path } -func NewLegacyResourceMigrator( +func newLegacyResourceMigrator( + repo repository.ReaderWriter, legacy legacy.LegacyMigrator, parser resources.Parser, resources resources.RepositoryResources, @@ -126,6 +140,7 @@ func NewLegacyResourceMigrator( history = make(map[string]string) } return &legacyResourceResourceMigrator{ + repo: repo, legacy: legacy, parser: parser, progress: progress, @@ -178,11 +193,11 @@ func (r *legacyResourceResourceMigrator) Write(ctx context.Context, key *resourc // When replaying history, the path to the file may change over time // This happens when the title or folder change - if r.history != nil { + if r.history != nil && err == nil { name := parsed.Meta.GetName() previous := r.history[name] if previous != "" && previous != fileName { - _, _, err = r.resources.RemoveResourceFromFile(ctx, previous, "") + err = r.repo.Delete(ctx, previous, "", fmt.Sprintf("moved to: %s", fileName)) } r.history[name] = fileName } diff --git a/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go index e73930bd718..50032583024 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "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" @@ -16,6 +17,7 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/export" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources/signature" @@ -29,13 +31,16 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { Return(nil, errors.New("parser factory error")) signerFactory := signature.NewMockSignerFactory(t) + mockClientFactory := resources.NewMockClientFactory(t) + mockExportFn := export.NewMockExportFn(t) migrator := NewLegacyResourcesMigrator( nil, mockParserFactory, nil, - nil, signerFactory, + mockClientFactory, + mockExportFn.Execute, ) err := migrator.Migrate(context.Background(), nil, "test-namespace", provisioning.MigrateJobOptions{}, jobs.NewMockJobProgressRecorder(t)) @@ -43,6 +48,8 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { require.EqualError(t, err, "get parser: parser factory error") mockParserFactory.AssertExpectations(t) + mockExportFn.AssertExpectations(t) + mockClientFactory.AssertExpectations(t) }) t.Run("should fail when repository resources factory fails", func(t *testing.T) { @@ -51,16 +58,19 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { Return(resources.NewMockParser(t), nil) mockRepoResourcesFactory := resources.NewMockRepositoryResourcesFactory(t) - mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything, mock.Anything). + mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything). Return(nil, errors.New("repo resources factory error")) signerFactory := signature.NewMockSignerFactory(t) + mockClientFactory := resources.NewMockClientFactory(t) + mockExportFn := export.NewMockExportFn(t) migrator := NewLegacyResourcesMigrator( mockRepoResourcesFactory, mockParserFactory, nil, - nil, signerFactory, + mockClientFactory, + mockExportFn.Execute, ) err := migrator.Migrate(context.Background(), nil, "test-namespace", provisioning.MigrateJobOptions{}, jobs.NewMockJobProgressRecorder(t)) @@ -69,46 +79,8 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { mockParserFactory.AssertExpectations(t) mockRepoResourcesFactory.AssertExpectations(t) - }) - - t.Run("should fail when folder migrator fails", func(t *testing.T) { - mockParserFactory := resources.NewMockParserFactory(t) - mockParserFactory.On("GetParser", mock.Anything, mock.Anything). - Return(resources.NewMockParser(t), nil) - - mockRepoResources := resources.NewMockRepositoryResources(t) - mockRepoResourcesFactory := resources.NewMockRepositoryResourcesFactory(t) - mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything, mock.Anything). - Return(mockRepoResources, nil) - - mockFolderMigrator := NewMockLegacyFoldersMigrator(t) - mockFolderMigrator.On("Migrate", mock.Anything, "test-namespace", mockRepoResources, mock.Anything). - Return(errors.New("folder migrator error")) - - progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, mock.Anything).Return() - - signer := signature.NewMockSigner(t) - signerFactory := signature.NewMockSignerFactory(t) - signerFactory.On("New", mock.Anything, mock.Anything). - Return(signer, nil) - - migrator := NewLegacyResourcesMigrator( - mockRepoResourcesFactory, - mockParserFactory, - nil, - mockFolderMigrator, - signerFactory, - ) - - err := migrator.Migrate(context.Background(), nil, "test-namespace", provisioning.MigrateJobOptions{}, progress) - require.Error(t, err) - require.Contains(t, err.Error(), "migrate folders from SQL") - - mockParserFactory.AssertExpectations(t) - mockRepoResourcesFactory.AssertExpectations(t) - mockFolderMigrator.AssertExpectations(t) - progress.AssertExpectations(t) + mockExportFn.AssertExpectations(t) + mockClientFactory.AssertExpectations(t) }) t.Run("should fail when resource migration fails", func(t *testing.T) { @@ -118,13 +90,9 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { mockRepoResources := resources.NewMockRepositoryResources(t) mockRepoResourcesFactory := resources.NewMockRepositoryResourcesFactory(t) - mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything, mock.Anything). + mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything). Return(mockRepoResources, nil) - mockFolderMigrator := NewMockLegacyFoldersMigrator(t) - mockFolderMigrator.On("Migrate", mock.Anything, "test-namespace", mockRepoResources, mock.Anything). - Return(nil) - mockLegacyMigrator := legacy.NewMockLegacyMigrator(t) mockLegacyMigrator.On("Migrate", mock.Anything, mock.MatchedBy(func(opts legacy.MigrateOptions) bool { return opts.OnlyCount && opts.Namespace == "test-namespace" @@ -138,23 +106,88 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { signerFactory.On("New", mock.Anything, mock.Anything). Return(signer, nil) + mockClients := resources.NewMockResourceClients(t) + mockClientFactory := resources.NewMockClientFactory(t) + mockClientFactory.On("Clients", mock.Anything, "test-namespace"). + Return(mockClients, nil) + mockExportFn := export.NewMockExportFn(t) + migrator := NewLegacyResourcesMigrator( mockRepoResourcesFactory, mockParserFactory, mockLegacyMigrator, - mockFolderMigrator, signerFactory, + mockClientFactory, + mockExportFn.Execute, ) + repo := repository.NewMockRepository(t) + repo.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-repo", + }, + }) + mockExportFn.On("Execute", mock.Anything, mock.Anything, provisioning.ExportJobOptions{}, mockClients, mockRepoResources, mock.Anything). + Return(nil) - err := migrator.Migrate(context.Background(), nil, "test-namespace", provisioning.MigrateJobOptions{}, progress) + err := migrator.Migrate(context.Background(), repo, "test-namespace", provisioning.MigrateJobOptions{}, progress) require.Error(t, err) require.Contains(t, err.Error(), "migrate resource") mockParserFactory.AssertExpectations(t) mockRepoResourcesFactory.AssertExpectations(t) - mockFolderMigrator.AssertExpectations(t) mockLegacyMigrator.AssertExpectations(t) progress.AssertExpectations(t) + mockExportFn.AssertExpectations(t) + mockClientFactory.AssertExpectations(t) + mockClients.AssertExpectations(t) + repo.AssertExpectations(t) + }) + t.Run("should fail when client creation fails", func(t *testing.T) { + mockParserFactory := resources.NewMockParserFactory(t) + mockParserFactory.On("GetParser", mock.Anything, mock.Anything). + Return(resources.NewMockParser(t), nil) + + mockRepoResources := resources.NewMockRepositoryResources(t) + mockRepoResourcesFactory := resources.NewMockRepositoryResourcesFactory(t) + mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything). + Return(mockRepoResources, nil) + + mockSigner := signature.NewMockSigner(t) + mockSignerFactory := signature.NewMockSignerFactory(t) + mockSignerFactory.On("New", mock.Anything, mock.Anything). + Return(mockSigner, nil) + + mockClientFactory := resources.NewMockClientFactory(t) + mockClientFactory.On("Clients", mock.Anything, "test-namespace"). + Return(nil, errors.New("client creation error")) + + mockExportFn := export.NewMockExportFn(t) + + progress := jobs.NewMockJobProgressRecorder(t) + progress.On("SetMessage", mock.Anything, "migrate folders from SQL").Return() + + migrator := NewLegacyResourcesMigrator( + mockRepoResourcesFactory, + mockParserFactory, + nil, + mockSignerFactory, + mockClientFactory, + mockExportFn.Execute, + ) + + repo := repository.NewMockRepository(t) + err := migrator.Migrate(context.Background(), repo, "test-namespace", provisioning.MigrateJobOptions{}, progress) + require.Error(t, err) + require.EqualError(t, err, "client creation error") + + mockParserFactory.AssertExpectations(t) + mockRepoResourcesFactory.AssertExpectations(t) + mockSignerFactory.AssertExpectations(t) + mockClientFactory.AssertExpectations(t) + progress.AssertExpectations(t) + mockExportFn.AssertExpectations(t) + repo.AssertExpectations(t) }) t.Run("should fail when signer factory fails", func(t *testing.T) { @@ -164,23 +197,26 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { mockRepoResources := resources.NewMockRepositoryResources(t) mockRepoResourcesFactory := resources.NewMockRepositoryResourcesFactory(t) - mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything, mock.Anything). + mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything). Return(mockRepoResources, nil) - mockFolderMigrator := NewMockLegacyFoldersMigrator(t) mockSignerFactory := signature.NewMockSignerFactory(t) mockSignerFactory.On("New", mock.Anything, signature.SignOptions{ Namespace: "test-namespace", History: true, }).Return(nil, fmt.Errorf("signer factory error")) + mockClientFactory := resources.NewMockClientFactory(t) + mockExportFn := export.NewMockExportFn(t) + progress := jobs.NewMockJobProgressRecorder(t) migrator := NewLegacyResourcesMigrator( mockRepoResourcesFactory, mockParserFactory, nil, - mockFolderMigrator, mockSignerFactory, + mockClientFactory, + mockExportFn.Execute, ) err := migrator.Migrate(context.Background(), nil, "test-namespace", provisioning.MigrateJobOptions{ @@ -191,8 +227,66 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { mockParserFactory.AssertExpectations(t) mockRepoResourcesFactory.AssertExpectations(t) - mockFolderMigrator.AssertExpectations(t) mockSignerFactory.AssertExpectations(t) + mockClientFactory.AssertExpectations(t) + progress.AssertExpectations(t) + mockExportFn.AssertExpectations(t) + }) + t.Run("should fail when folder export fails", func(t *testing.T) { + mockParser := resources.NewMockParser(t) + mockParserFactory := resources.NewMockParserFactory(t) + mockParserFactory.On("GetParser", mock.Anything, mock.Anything). + Return(mockParser, nil) + + mockRepoResources := resources.NewMockRepositoryResources(t) + mockRepoResourcesFactory := resources.NewMockRepositoryResourcesFactory(t) + mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything). + Return(mockRepoResources, nil) + + mockSigner := signature.NewMockSigner(t) + mockSignerFactory := signature.NewMockSignerFactory(t) + mockSignerFactory.On("New", mock.Anything, signature.SignOptions{ + Namespace: "test-namespace", + History: false, + }).Return(mockSigner, nil) + + mockClients := resources.NewMockResourceClients(t) + mockClientFactory := resources.NewMockClientFactory(t) + mockClientFactory.On("Clients", mock.Anything, "test-namespace"). + Return(mockClients, nil) + + mockExportFn := export.NewMockExportFn(t) + mockExportFn.On("Execute", mock.Anything, mock.Anything, provisioning.ExportJobOptions{}, mockClients, mockRepoResources, mock.Anything). + Return(fmt.Errorf("export error")) + + progress := jobs.NewMockJobProgressRecorder(t) + progress.On("SetMessage", mock.Anything, "migrate folders from SQL").Return() + + migrator := NewLegacyResourcesMigrator( + mockRepoResourcesFactory, + mockParserFactory, + nil, + mockSignerFactory, + mockClientFactory, + mockExportFn.Execute, + ) + repo := repository.NewMockRepository(t) + repo.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-repo", + }, + }) + + err := migrator.Migrate(context.Background(), repo, "test-namespace", provisioning.MigrateJobOptions{}, progress) + require.Error(t, err) + require.Contains(t, err.Error(), "migrate folders from SQL: export error") + + mockParserFactory.AssertExpectations(t) + mockRepoResourcesFactory.AssertExpectations(t) + mockSignerFactory.AssertExpectations(t) + mockClientFactory.AssertExpectations(t) + mockExportFn.AssertExpectations(t) progress.AssertExpectations(t) }) @@ -204,13 +298,9 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { mockRepoResources := resources.NewMockRepositoryResources(t) mockRepoResourcesFactory := resources.NewMockRepositoryResourcesFactory(t) - mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything, mock.Anything). + mockRepoResourcesFactory.On("Client", mock.Anything, mock.Anything). Return(mockRepoResources, nil) - mockFolderMigrator := NewMockLegacyFoldersMigrator(t) - mockFolderMigrator.On("Migrate", mock.Anything, "test-namespace", mockRepoResources, mock.Anything). - Return(nil) - mockSigner := signature.NewMockSigner(t) mockSignerFactory := signature.NewMockSignerFactory(t) mockSignerFactory.On("New", mock.Anything, signature.SignOptions{ @@ -235,6 +325,12 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { }, }, nil).Once() // Migration phase + mockClients := resources.NewMockResourceClients(t) + mockClientFactory := resources.NewMockClientFactory(t) + mockClientFactory.On("Clients", mock.Anything, "test-namespace"). + Return(mockClients, nil) + mockExportFn := export.NewMockExportFn(t) + progress := jobs.NewMockJobProgressRecorder(t) progress.On("SetMessage", mock.Anything, "migrate folders from SQL").Return() progress.On("SetMessage", mock.Anything, "migrate resources from SQL").Return() @@ -244,20 +340,33 @@ func TestLegacyResourcesMigrator_Migrate(t *testing.T) { mockRepoResourcesFactory, mockParserFactory, mockLegacyMigrator, - mockFolderMigrator, mockSignerFactory, + mockClientFactory, + mockExportFn.Execute, ) - err := migrator.Migrate(context.Background(), nil, "test-namespace", provisioning.MigrateJobOptions{ + repo := repository.NewMockRepository(t) + repo.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-repo", + }, + }) + mockExportFn.On("Execute", mock.Anything, mock.Anything, provisioning.ExportJobOptions{}, mockClients, mockRepoResources, mock.Anything). + Return(nil) + + err := migrator.Migrate(context.Background(), repo, "test-namespace", provisioning.MigrateJobOptions{ History: true, }, progress) require.NoError(t, err) mockParserFactory.AssertExpectations(t) mockRepoResourcesFactory.AssertExpectations(t) - mockFolderMigrator.AssertExpectations(t) mockLegacyMigrator.AssertExpectations(t) + mockClientFactory.AssertExpectations(t) + mockExportFn.AssertExpectations(t) progress.AssertExpectations(t) + mockClients.AssertExpectations(t) }) } @@ -269,7 +378,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { progress := jobs.NewMockJobProgressRecorder(t) - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, nil, mockParser, nil, @@ -318,7 +428,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { })).Return() progress.On("TooManyErrors").Return(nil) - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, nil, mockParser, mockRepoResources, @@ -360,7 +471,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { Return(nil, errors.New("signing error")) progress := jobs.NewMockJobProgressRecorder(t) - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, nil, mockParser, nil, @@ -419,7 +531,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { })).Return() progress.On("TooManyErrors").Return(nil) - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, nil, mockParser, mockRepoResources, @@ -459,10 +572,13 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { Obj: obj, }, nil) + mockRepo := repository.NewMockRepository(t) + mockRepoResources := resources.NewMockRepositoryResources(t) writeResourceFileFromObject := mockRepoResources.On("WriteResourceFileFromObject", mock.Anything, mock.Anything, mock.Anything) - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + mockRepo, nil, mockParser, mockRepoResources, @@ -482,8 +598,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { // Change the result file name writeResourceFileFromObject.Return("bbbb.json", nil) - mockRepoResources.On("RemoveResourceFromFile", mock.Anything, "aaaa.json", ""). - Return("", schema.GroupVersionKind{}, nil).Once() + mockRepo.On("Delete", mock.Anything, "aaaa.json", "", "moved to: bbbb.json"). + Return(nil).Once() err = migrator.Write(context.Background(), &resource.ResourceKey{}, []byte("")) require.NoError(t, err) @@ -557,7 +673,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { })).Return() progress.On("TooManyErrors").Return(nil) - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, nil, mockParser, mockRepoResources, @@ -602,7 +719,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { progress.On("Record", mock.Anything, mock.Anything).Return() progress.On("TooManyErrors").Return(errors.New("too many errors")) - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, nil, mockParser, mockRepoResources, @@ -632,7 +750,8 @@ func TestLegacyResourceResourceMigrator_Migrate(t *testing.T) { progress := jobs.NewMockJobProgressRecorder(t) progress.On("SetMessage", mock.Anything, mock.Anything).Return() - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, mockLegacyMigrator, nil, nil, @@ -663,7 +782,8 @@ func TestLegacyResourceResourceMigrator_Migrate(t *testing.T) { progress := jobs.NewMockJobProgressRecorder(t) progress.On("SetMessage", mock.Anything, mock.Anything).Return() - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, mockLegacyMigrator, nil, nil, @@ -694,7 +814,8 @@ func TestLegacyResourceResourceMigrator_Migrate(t *testing.T) { progress := jobs.NewMockJobProgressRecorder(t) progress.On("SetMessage", mock.Anything, mock.Anything).Return() - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, mockLegacyMigrator, nil, nil, @@ -733,7 +854,8 @@ func TestLegacyResourceResourceMigrator_Migrate(t *testing.T) { progress.On("SetMessage", mock.Anything, mock.Anything).Return() progress.On("SetTotal", mock.Anything, 100).Return() - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, mockLegacyMigrator, nil, nil, @@ -773,7 +895,8 @@ func TestLegacyResourceResourceMigrator_Migrate(t *testing.T) { progress.On("SetTotal", mock.Anything, 200).Return() signer := signature.NewMockSigner(t) - migrator := NewLegacyResourceMigrator( + migrator := newLegacyResourceMigrator( + nil, mockLegacyMigrator, nil, nil, diff --git a/pkg/registry/apis/provisioning/jobs/migrate/mock_legacy_folders_migrator.go b/pkg/registry/apis/provisioning/jobs/migrate/mock_legacy_folders_migrator.go deleted file mode 100644 index dca97b2467c..00000000000 --- a/pkg/registry/apis/provisioning/jobs/migrate/mock_legacy_folders_migrator.go +++ /dev/null @@ -1,240 +0,0 @@ -// Code generated by mockery v2.52.4. DO NOT EDIT. - -package migrate - -import ( - context "context" - - jobs "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" - mock "github.com/stretchr/testify/mock" - - resource "github.com/grafana/grafana/pkg/storage/unified/resource" - - resources "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" -) - -// MockLegacyFoldersMigrator is an autogenerated mock type for the LegacyFoldersMigrator type -type MockLegacyFoldersMigrator struct { - mock.Mock -} - -type MockLegacyFoldersMigrator_Expecter struct { - mock *mock.Mock -} - -func (_m *MockLegacyFoldersMigrator) EXPECT() *MockLegacyFoldersMigrator_Expecter { - return &MockLegacyFoldersMigrator_Expecter{mock: &_m.Mock} -} - -// Close provides a mock function with no fields -func (_m *MockLegacyFoldersMigrator) Close() error { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for Close") - } - - var r0 error - if rf, ok := ret.Get(0).(func() error); ok { - r0 = rf() - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// MockLegacyFoldersMigrator_Close_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Close' -type MockLegacyFoldersMigrator_Close_Call struct { - *mock.Call -} - -// Close is a helper method to define mock.On call -func (_e *MockLegacyFoldersMigrator_Expecter) Close() *MockLegacyFoldersMigrator_Close_Call { - return &MockLegacyFoldersMigrator_Close_Call{Call: _e.mock.On("Close")} -} - -func (_c *MockLegacyFoldersMigrator_Close_Call) Run(run func()) *MockLegacyFoldersMigrator_Close_Call { - _c.Call.Run(func(args mock.Arguments) { - run() - }) - return _c -} - -func (_c *MockLegacyFoldersMigrator_Close_Call) Return(_a0 error) *MockLegacyFoldersMigrator_Close_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *MockLegacyFoldersMigrator_Close_Call) RunAndReturn(run func() error) *MockLegacyFoldersMigrator_Close_Call { - _c.Call.Return(run) - return _c -} - -// CloseWithResults provides a mock function with no fields -func (_m *MockLegacyFoldersMigrator) CloseWithResults() (*resource.BulkResponse, error) { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for CloseWithResults") - } - - var r0 *resource.BulkResponse - var r1 error - if rf, ok := ret.Get(0).(func() (*resource.BulkResponse, error)); ok { - return rf() - } - if rf, ok := ret.Get(0).(func() *resource.BulkResponse); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*resource.BulkResponse) - } - } - - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// MockLegacyFoldersMigrator_CloseWithResults_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CloseWithResults' -type MockLegacyFoldersMigrator_CloseWithResults_Call struct { - *mock.Call -} - -// CloseWithResults is a helper method to define mock.On call -func (_e *MockLegacyFoldersMigrator_Expecter) CloseWithResults() *MockLegacyFoldersMigrator_CloseWithResults_Call { - return &MockLegacyFoldersMigrator_CloseWithResults_Call{Call: _e.mock.On("CloseWithResults")} -} - -func (_c *MockLegacyFoldersMigrator_CloseWithResults_Call) Run(run func()) *MockLegacyFoldersMigrator_CloseWithResults_Call { - _c.Call.Run(func(args mock.Arguments) { - run() - }) - return _c -} - -func (_c *MockLegacyFoldersMigrator_CloseWithResults_Call) Return(_a0 *resource.BulkResponse, _a1 error) *MockLegacyFoldersMigrator_CloseWithResults_Call { - _c.Call.Return(_a0, _a1) - return _c -} - -func (_c *MockLegacyFoldersMigrator_CloseWithResults_Call) RunAndReturn(run func() (*resource.BulkResponse, error)) *MockLegacyFoldersMigrator_CloseWithResults_Call { - _c.Call.Return(run) - return _c -} - -// Migrate provides a mock function with given fields: ctx, namespace, repositoryResources, progress -func (_m *MockLegacyFoldersMigrator) Migrate(ctx context.Context, namespace string, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder) error { - ret := _m.Called(ctx, namespace, repositoryResources, progress) - - if len(ret) == 0 { - panic("no return value specified for Migrate") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, resources.RepositoryResources, jobs.JobProgressRecorder) error); ok { - r0 = rf(ctx, namespace, repositoryResources, progress) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// MockLegacyFoldersMigrator_Migrate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Migrate' -type MockLegacyFoldersMigrator_Migrate_Call struct { - *mock.Call -} - -// Migrate is a helper method to define mock.On call -// - ctx context.Context -// - namespace string -// - repositoryResources resources.RepositoryResources -// - progress jobs.JobProgressRecorder -func (_e *MockLegacyFoldersMigrator_Expecter) Migrate(ctx interface{}, namespace interface{}, repositoryResources interface{}, progress interface{}) *MockLegacyFoldersMigrator_Migrate_Call { - return &MockLegacyFoldersMigrator_Migrate_Call{Call: _e.mock.On("Migrate", ctx, namespace, repositoryResources, progress)} -} - -func (_c *MockLegacyFoldersMigrator_Migrate_Call) Run(run func(ctx context.Context, namespace string, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder)) *MockLegacyFoldersMigrator_Migrate_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(resources.RepositoryResources), args[3].(jobs.JobProgressRecorder)) - }) - return _c -} - -func (_c *MockLegacyFoldersMigrator_Migrate_Call) Return(_a0 error) *MockLegacyFoldersMigrator_Migrate_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *MockLegacyFoldersMigrator_Migrate_Call) RunAndReturn(run func(context.Context, string, resources.RepositoryResources, jobs.JobProgressRecorder) error) *MockLegacyFoldersMigrator_Migrate_Call { - _c.Call.Return(run) - return _c -} - -// Write provides a mock function with given fields: ctx, key, value -func (_m *MockLegacyFoldersMigrator) Write(ctx context.Context, key *resource.ResourceKey, value []byte) error { - ret := _m.Called(ctx, key, value) - - if len(ret) == 0 { - panic("no return value specified for Write") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, *resource.ResourceKey, []byte) error); ok { - r0 = rf(ctx, key, value) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// MockLegacyFoldersMigrator_Write_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Write' -type MockLegacyFoldersMigrator_Write_Call struct { - *mock.Call -} - -// Write is a helper method to define mock.On call -// - ctx context.Context -// - key *resource.ResourceKey -// - value []byte -func (_e *MockLegacyFoldersMigrator_Expecter) Write(ctx interface{}, key interface{}, value interface{}) *MockLegacyFoldersMigrator_Write_Call { - return &MockLegacyFoldersMigrator_Write_Call{Call: _e.mock.On("Write", ctx, key, value)} -} - -func (_c *MockLegacyFoldersMigrator_Write_Call) Run(run func(ctx context.Context, key *resource.ResourceKey, value []byte)) *MockLegacyFoldersMigrator_Write_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(*resource.ResourceKey), args[2].([]byte)) - }) - return _c -} - -func (_c *MockLegacyFoldersMigrator_Write_Call) Return(_a0 error) *MockLegacyFoldersMigrator_Write_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *MockLegacyFoldersMigrator_Write_Call) RunAndReturn(run func(context.Context, *resource.ResourceKey, []byte) error) *MockLegacyFoldersMigrator_Write_Call { - _c.Call.Return(run) - return _c -} - -// NewMockLegacyFoldersMigrator creates a new instance of MockLegacyFoldersMigrator. 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 NewMockLegacyFoldersMigrator(t interface { - mock.TestingT - Cleanup(func()) -}) *MockLegacyFoldersMigrator { - mock := &MockLegacyFoldersMigrator{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index a65f2008813..a367f5ea2be 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -548,13 +548,13 @@ func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartH syncer, ) signerFactory := signature.NewSignerFactory(b.clients) - legacyFolders := migrate.NewLegacyFoldersMigrator(b.legacyMigrator) legacyResources := migrate.NewLegacyResourcesMigrator( b.repositoryResources, b.parsers, b.legacyMigrator, - legacyFolders, signerFactory, + b.clients, + export.ExportAll, ) storageSwapper := migrate.NewStorageSwapper(b.unified, b.storageStatus) legacyMigrator := migrate.NewLegacyMigrator( diff --git a/pkg/registry/apis/provisioning/repository/go-git/wrapper.go b/pkg/registry/apis/provisioning/repository/go-git/wrapper.go index d4d9af45c23..7a722cee2ce 100644 --- a/pkg/registry/apis/provisioning/repository/go-git/wrapper.go +++ b/pkg/registry/apis/provisioning/repository/go-git/wrapper.go @@ -336,7 +336,10 @@ func (g *GoGitRepo) Write(ctx context.Context, fpath string, ref string, data [] if err != nil { return err } + return g.maybeCommit(ctx, message) +} +func (g *GoGitRepo) maybeCommit(ctx context.Context, message string) error { // Skip commit for each file if !g.opts.PushOnWrites { return nil @@ -351,7 +354,7 @@ func (g *GoGitRepo) Write(ctx context.Context, fpath string, ref string, data [] When: sig.When, } } - _, err = g.tree.Commit(message, opts) + _, err := g.tree.Commit(message, opts) if errors.Is(err, git.ErrEmptyCommit) { return nil // empty commit is fine -- no change } @@ -359,16 +362,22 @@ func (g *GoGitRepo) Write(ctx context.Context, fpath string, ref string, data [] } // Delete implements repository.Repository. -func (g *GoGitRepo) Delete(ctx context.Context, path string, ref string, message string) error { - if _, err := g.tree.Remove(safepath.Join(g.config.Spec.GitHub.Path, path)); err != nil { +func (g *GoGitRepo) Delete(ctx context.Context, fpath string, ref string, message string) error { + fpath = safepath.Join(g.config.Spec.GitHub.Path, fpath) + if err := verifyPathWithoutRef(fpath, ref); err != nil { return err } - - return nil + if _, err := g.tree.Remove(fpath); err != nil { + return err + } + return g.maybeCommit(ctx, message) } // Read implements repository.Repository. func (g *GoGitRepo) Read(ctx context.Context, path string, ref string) (*repository.FileInfo, error) { + if err := verifyPathWithoutRef(path, ref); err != nil { + return nil, err + } readPath := safepath.Join(g.config.Spec.GitHub.Path, path) stat, err := g.tree.Filesystem.Lstat(readPath) if errors.Is(err, fs.ErrNotExist) {