diff --git a/pkg/apis/provisioning/v0alpha1/jobs.go b/pkg/apis/provisioning/v0alpha1/jobs.go index 5d0348864a7..ceea231400b 100644 --- a/pkg/apis/provisioning/v0alpha1/jobs.go +++ b/pkg/apis/provisioning/v0alpha1/jobs.go @@ -37,6 +37,9 @@ const ( // JobActionMigrate acts like JobActionExport, then JobActionPull. It also tries to preserve the history. JobActionMigrate JobAction = "migrate" + + // JobActionDelete deletes files in the remote repository + JobActionDelete JobAction = "delete" ) // +enum @@ -81,6 +84,9 @@ type JobSpec struct { // Required when the action is `migrate` Migrate *MigrateJobOptions `json:"migrate,omitempty"` + + // Delete when the action is `delete` + Delete *DeleteJobOptions `json:"delete,omitempty"` } type PullRequestJobOptions struct { @@ -109,9 +115,11 @@ type ExportJobOptions struct { // The source folder (or empty) to export Folder string `json:"folder,omitempty"` + // FIXME: we should validate this in admission hooks // Target branch for export (only git) Branch string `json:"branch,omitempty"` + // FIXME: we should validate this in admission hooks // Prefix in target file system Path string `json:"path,omitempty"` } @@ -124,6 +132,17 @@ type MigrateJobOptions struct { Message string `json:"message,omitempty"` } +type DeleteJobOptions struct { + // Ref to the branch or commit hash to delete from + Ref string `json:"ref,omitempty"` + // Paths to be deleted. Examples: + // - dashboard.json (for a file) + // - a/b/c/other-dashboard.json (for a file) + // - nested/deep/ (for a directory) + // FIXME: we should validate this in admission hooks + Paths []string `json:"paths,omitempty"` +} + // The job status type JobStatus struct { State JobState `json:"state,omitempty"` diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go b/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go index 30fd560a597..bed49953b63 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go @@ -48,6 +48,27 @@ func (in *BitbucketRepositoryConfig) DeepCopy() *BitbucketRepositoryConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeleteJobOptions) DeepCopyInto(out *DeleteJobOptions) { + *out = *in + if in.Paths != nil { + in, out := &in.Paths, &out.Paths + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeleteJobOptions. +func (in *DeleteJobOptions) DeepCopy() *DeleteJobOptions { + if in == nil { + return nil + } + out := new(DeleteJobOptions) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ErrorDetails) DeepCopyInto(out *ErrorDetails) { *out = *in @@ -370,6 +391,11 @@ func (in *JobSpec) DeepCopyInto(out *JobSpec) { *out = new(MigrateJobOptions) **out = **in } + if in.Delete != nil { + in, out := &in.Delete, &out.Delete + *out = new(DeleteJobOptions) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index 37db6def7d7..b81342c30c3 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -16,6 +16,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA return map[string]common.OpenAPIDefinition{ "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.Author": schema_pkg_apis_provisioning_v0alpha1_Author(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.BitbucketRepositoryConfig": schema_pkg_apis_provisioning_v0alpha1_BitbucketRepositoryConfig(ref), + "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.DeleteJobOptions": schema_pkg_apis_provisioning_v0alpha1_DeleteJobOptions(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.ErrorDetails": schema_pkg_apis_provisioning_v0alpha1_ErrorDetails(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.ExportJobOptions": schema_pkg_apis_provisioning_v0alpha1_ExportJobOptions(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.FileItem": schema_pkg_apis_provisioning_v0alpha1_FileItem(ref), @@ -155,6 +156,40 @@ func schema_pkg_apis_provisioning_v0alpha1_BitbucketRepositoryConfig(ref common. } } +func schema_pkg_apis_provisioning_v0alpha1_DeleteJobOptions(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "ref": { + SchemaProps: spec.SchemaProps{ + Description: "Ref to the branch or commit hash to delete from", + Type: []string{"string"}, + Format: "", + }, + }, + "paths": { + SchemaProps: spec.SchemaProps{ + Description: "Paths to be deleted. Examples: - dashboard.json (for a file) - a/b/c/other-dashboard.json (for a file) - nested/deep/ (for a directory) FIXME: we should validate this in admission hooks", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_provisioning_v0alpha1_ErrorDetails(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -209,14 +244,14 @@ func schema_pkg_apis_provisioning_v0alpha1_ExportJobOptions(ref common.Reference }, "branch": { SchemaProps: spec.SchemaProps{ - Description: "Target branch for export (only git)", + Description: "FIXME: we should validate this in admission hooks Target branch for export (only git)", Type: []string{"string"}, Format: "", }, }, "path": { SchemaProps: spec.SchemaProps{ - Description: "Prefix in target file system", + Description: "FIXME: we should validate this in admission hooks Prefix in target file system", Type: []string{"string"}, Format: "", }, @@ -843,10 +878,10 @@ func schema_pkg_apis_provisioning_v0alpha1_JobSpec(ref common.ReferenceCallback) Properties: map[string]spec.Schema{ "action": { SchemaProps: spec.SchemaProps{ - Description: "Possible enum values:\n - `\"migrate\"` acts like JobActionExport, then JobActionPull. It also tries to preserve the history.\n - `\"pr\"` adds additional useful information to a PR, such as comments with preview links and rendered images.\n - `\"pull\"` replicates the remote branch in the local copy of the repository.\n - `\"push\"` replicates the local copy of the repository in the remote branch.", + Description: "Possible enum values:\n - `\"delete\"` deletes files in the remote repository\n - `\"migrate\"` acts like JobActionExport, then JobActionPull. It also tries to preserve the history.\n - `\"pr\"` adds additional useful information to a PR, such as comments with preview links and rendered images.\n - `\"pull\"` replicates the remote branch in the local copy of the repository.\n - `\"push\"` replicates the local copy of the repository in the remote branch.", Type: []string{"string"}, Format: "", - Enum: []interface{}{"migrate", "pr", "pull", "push"}, + Enum: []interface{}{"delete", "migrate", "pr", "pull", "push"}, }, }, "repository": { @@ -880,11 +915,17 @@ func schema_pkg_apis_provisioning_v0alpha1_JobSpec(ref common.ReferenceCallback) Ref: ref("github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.MigrateJobOptions"), }, }, + "delete": { + SchemaProps: spec.SchemaProps{ + Description: "Delete when the action is `delete`", + Ref: ref("github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.DeleteJobOptions"), + }, + }, }, }, }, Dependencies: []string{ - "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.ExportJobOptions", "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.MigrateJobOptions", "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.PullRequestJobOptions", "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.SyncJobOptions"}, + "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.DeleteJobOptions", "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.ExportJobOptions", "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.MigrateJobOptions", "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.PullRequestJobOptions", "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.SyncJobOptions"}, } } diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list index 6c41753b403..fc982653184 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list @@ -1,3 +1,4 @@ +API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,DeleteJobOptions,Paths API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,FileList,Items API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,HistoryList,Items API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,JobResourceSummary,Errors diff --git a/pkg/registry/apis/provisioning/jobs/delete/worker.go b/pkg/registry/apis/provisioning/jobs/delete/worker.go new file mode 100644 index 00000000000..5489445d216 --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/delete/worker.go @@ -0,0 +1,96 @@ +package delete + +import ( + "context" + "errors" + "fmt" + "time" + + 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" +) + +type Worker struct { + syncWorker jobs.Worker + wrapFn repository.WrapWithStageFn +} + +func NewWorker(syncWorker jobs.Worker, wrapFn repository.WrapWithStageFn) *Worker { + return &Worker{ + syncWorker: syncWorker, + wrapFn: wrapFn, + } +} + +func (w *Worker) IsSupported(ctx context.Context, job provisioning.Job) bool { + return job.Spec.Action == provisioning.JobActionDelete +} + +func (w *Worker) Process(ctx context.Context, repo repository.Repository, job provisioning.Job, progress jobs.JobProgressRecorder) error { + if job.Spec.Delete == nil { + return errors.New("missing delete settings") + } + opts := *job.Spec.Delete + + paths := opts.Paths + progress.SetTotal(ctx, len(paths)) + progress.StrictMaxErrors(1) // Fail fast on any error during deletion + + fn := func(repo repository.Repository, _ bool) error { + rw, ok := repo.(repository.ReaderWriter) + if !ok { + return errors.New("delete job submitted targeting repository that is not a ReaderWriter") + } + + return w.deleteFiles(ctx, rw, progress, opts, paths...) + } + + stageOptions := repository.StageOptions{ + PushOnWrites: false, + Timeout: 10 * time.Minute, + } + + err := w.wrapFn(ctx, repo, stageOptions, fn) + if err != nil { + return fmt.Errorf("delete files from repository: %w", err) + } + + if opts.Ref == "" { + progress.ResetResults() + progress.SetMessage(ctx, "pull resources") + + syncJob := provisioning.Job{ + Spec: provisioning.JobSpec{ + Pull: &provisioning.SyncJobOptions{ + // Full sync because it's the only one that supports empty folder deletion + Incremental: false, + }, + }, + } + + if err := w.syncWorker.Process(ctx, repo, syncJob, progress); err != nil { + return fmt.Errorf("pull resources: %w", err) + } + } + + return nil +} + +func (w *Worker) deleteFiles(ctx context.Context, rw repository.ReaderWriter, progress jobs.JobProgressRecorder, opts provisioning.DeleteJobOptions, paths ...string) error { + for _, path := range paths { + result := jobs.JobResourceResult{ + Path: path, + Action: repository.FileActionDeleted, + } + + progress.SetMessage(ctx, "Deleting "+path) + result.Error = rw.Delete(ctx, path, opts.Ref, "Delete "+path) + progress.Record(ctx, result) + if err := progress.TooManyErrors(); err != nil { + return err + } + } + + return nil +} diff --git a/pkg/registry/apis/provisioning/jobs/delete/worker_test.go b/pkg/registry/apis/provisioning/jobs/delete/worker_test.go new file mode 100644 index 00000000000..d2ba09ba85d --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/delete/worker_test.go @@ -0,0 +1,373 @@ +package delete + +import ( + "context" + "errors" + "testing" + "time" + + 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/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type mockReaderWriter struct { + *repository.MockRepository +} + +func (m *mockReaderWriter) Delete(ctx context.Context, path, ref, message string) error { + args := m.Called(ctx, path, ref, message) + return args.Error(0) +} + +func TestDeleteWorker_IsSupported(t *testing.T) { + tests := []struct { + name string + job provisioning.Job + expected bool + }{ + { + name: "delete action is supported", + job: provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + }, + }, + expected: true, + }, + { + name: "pull action is not supported", + job: provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPull, + }, + }, + expected: false, + }, + { + name: "push 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) { + worker := NewWorker(nil, nil) + result := worker.IsSupported(context.Background(), tt.job) + require.Equal(t, tt.expected, result) + }) + } +} + +func TestDeleteWorker_ProcessMissingDeleteSettings(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + }, + } + + worker := NewWorker(nil, nil) + err := worker.Process(context.Background(), nil, job, nil) + require.EqualError(t, err, "missing delete settings") +} + +func TestDeleteWorker_ProcessNotReaderWriter(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"test/path"}, + }, + }, + } + + mockRepo := repository.NewMockRepository(t) + mockProgress := jobs.NewMockJobProgressRecorder(t) + mockWrapFn := repository.NewMockWrapWithStageFn(t) + + mockWrapFn.On("Execute", mock.Anything, mockRepo, mock.MatchedBy(func(opts repository.StageOptions) bool { + return !opts.PushOnWrites && opts.Timeout == 10*time.Minute + }), mock.Anything).Return(errors.New("delete job submitted targeting repository that is not a ReaderWriter")) + + mockProgress.On("SetTotal", mock.Anything, 1).Return() + mockProgress.On("StrictMaxErrors", 1).Return() + + worker := NewWorker(nil, mockWrapFn.Execute) + err := worker.Process(context.Background(), mockRepo, job, mockProgress) + require.EqualError(t, err, "delete files from repository: delete job submitted targeting repository that is not a ReaderWriter") +} + +func TestDeleteWorker_ProcessWrapFnError(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"test/path"}, + }, + }, + } + + mockRepo := repository.NewMockRepository(t) + mockProgress := jobs.NewMockJobProgressRecorder(t) + mockWrapFn := repository.NewMockWrapWithStageFn(t) + + mockWrapFn.On("Execute", mock.Anything, mockRepo, mock.Anything, mock.Anything).Return(errors.New("stage failed")) + mockProgress.On("SetTotal", mock.Anything, 1).Return() + mockProgress.On("StrictMaxErrors", 1).Return() + + worker := NewWorker(nil, mockWrapFn.Execute) + err := worker.Process(context.Background(), mockRepo, job, mockProgress) + require.EqualError(t, err, "delete files from repository: stage failed") +} + +func TestDeleteWorker_ProcessDeleteFilesSuccess(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"test/path1", "test/path2"}, + Ref: "main", + }, + }, + } + + mockRepo := &mockReaderWriter{ + MockRepository: repository.NewMockRepository(t), + } + mockProgress := jobs.NewMockJobProgressRecorder(t) + mockWrapFn := repository.NewMockWrapWithStageFn(t) + + mockWrapFn.On("Execute", mock.Anything, mockRepo, mock.MatchedBy(func(opts repository.StageOptions) bool { + return !opts.PushOnWrites && opts.Timeout == 10*time.Minute + }), mock.Anything).Return(func(ctx context.Context, repo repository.Repository, stageOptions repository.StageOptions, fn func(repository.Repository, bool) error) error { + return fn(mockRepo, false) + }) + + mockProgress.On("SetTotal", mock.Anything, 2).Return() + mockProgress.On("StrictMaxErrors", 1).Return() + mockProgress.On("SetMessage", mock.Anything, "Deleting test/path1").Return() + mockProgress.On("SetMessage", mock.Anything, "Deleting test/path2").Return() + mockProgress.On("TooManyErrors").Return(nil).Twice() + + mockRepo.On("Delete", mock.Anything, "test/path1", "main", "Delete test/path1").Return(nil) + mockRepo.On("Delete", mock.Anything, "test/path2", "main", "Delete test/path2").Return(nil) + + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == "test/path1" && result.Action == repository.FileActionDeleted && result.Error == nil + })).Return() + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == "test/path2" && result.Action == repository.FileActionDeleted && result.Error == nil + })).Return() + + worker := NewWorker(nil, mockWrapFn.Execute) + err := worker.Process(context.Background(), mockRepo, job, mockProgress) + require.NoError(t, err) +} + +func TestDeleteWorker_ProcessDeleteFilesWithError(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"test/path1", "test/path2"}, + Ref: "main", + }, + }, + } + + mockRepo := &mockReaderWriter{ + MockRepository: repository.NewMockRepository(t), + } + mockProgress := jobs.NewMockJobProgressRecorder(t) + mockWrapFn := repository.NewMockWrapWithStageFn(t) + + mockWrapFn.On("Execute", mock.Anything, mockRepo, mock.Anything, mock.Anything).Return(func(ctx context.Context, repo repository.Repository, stageOptions repository.StageOptions, fn func(repository.Repository, bool) error) error { + return fn(mockRepo, false) + }) + + mockProgress.On("SetTotal", mock.Anything, 2).Return() + mockProgress.On("StrictMaxErrors", 1).Return() + mockProgress.On("SetMessage", mock.Anything, "Deleting test/path1").Return() + + deleteError := errors.New("delete failed") + mockRepo.On("Delete", mock.Anything, "test/path1", "main", "Delete test/path1").Return(deleteError) + + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == "test/path1" && result.Action == repository.FileActionDeleted && errors.Is(result.Error, deleteError) + })).Return() + mockProgress.On("TooManyErrors").Return(errors.New("too many errors")) + + worker := NewWorker(nil, mockWrapFn.Execute) + err := worker.Process(context.Background(), mockRepo, job, mockProgress) + require.EqualError(t, err, "delete files from repository: too many errors") +} + +func TestDeleteWorker_ProcessWithSyncWorker(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"test/path"}, + }, + }, + } + + mockRepo := &mockReaderWriter{ + MockRepository: repository.NewMockRepository(t), + } + mockProgress := jobs.NewMockJobProgressRecorder(t) + mockSyncWorker := jobs.NewMockWorker(t) + mockWrapFn := repository.NewMockWrapWithStageFn(t) + + mockWrapFn.On("Execute", mock.Anything, mockRepo, mock.Anything, mock.Anything).Return(func(ctx context.Context, repo repository.Repository, stageOptions repository.StageOptions, fn func(repository.Repository, bool) error) error { + return fn(mockRepo, false) + }) + + mockProgress.On("SetTotal", mock.Anything, 1).Return() + mockProgress.On("StrictMaxErrors", 1).Return() + mockProgress.On("SetMessage", mock.Anything, "Deleting test/path").Return() + mockProgress.On("TooManyErrors").Return(nil) + + mockRepo.On("Delete", mock.Anything, "test/path", "", "Delete test/path").Return(nil) + + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == "test/path" && result.Action == repository.FileActionDeleted && result.Error == nil + })).Return() + + mockProgress.On("ResetResults").Return() + mockProgress.On("SetMessage", mock.Anything, "pull resources").Return() + + mockSyncWorker.On("Process", mock.Anything, mockRepo, mock.MatchedBy(func(syncJob provisioning.Job) bool { + return syncJob.Spec.Pull != nil && !syncJob.Spec.Pull.Incremental + }), mockProgress).Return(nil) + + worker := NewWorker(mockSyncWorker, mockWrapFn.Execute) + err := worker.Process(context.Background(), mockRepo, job, mockProgress) + require.NoError(t, err) +} + +func TestDeleteWorker_ProcessSyncWorkerError(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"test/path"}, + }, + }, + } + + mockRepo := &mockReaderWriter{ + MockRepository: repository.NewMockRepository(t), + } + mockProgress := jobs.NewMockJobProgressRecorder(t) + mockSyncWorker := jobs.NewMockWorker(t) + mockWrapFn := repository.NewMockWrapWithStageFn(t) + + mockWrapFn.On("Execute", mock.Anything, mockRepo, mock.Anything, mock.Anything).Return(func(ctx context.Context, repo repository.Repository, stageOptions repository.StageOptions, fn func(repository.Repository, bool) error) error { + return fn(mockRepo, false) + }) + + mockProgress.On("SetTotal", mock.Anything, 1).Return() + mockProgress.On("StrictMaxErrors", 1).Return() + mockProgress.On("SetMessage", mock.Anything, "Deleting test/path").Return() + mockProgress.On("TooManyErrors").Return(nil) + + mockRepo.On("Delete", mock.Anything, "test/path", "", "Delete test/path").Return(nil) + + mockProgress.On("Record", mock.Anything, mock.Anything).Return() + mockProgress.On("ResetResults").Return() + mockProgress.On("SetMessage", mock.Anything, "pull resources").Return() + + syncError := errors.New("sync failed") + mockSyncWorker.On("Process", mock.Anything, mockRepo, mock.Anything, mockProgress).Return(syncError) + + worker := NewWorker(mockSyncWorker, mockWrapFn.Execute) + err := worker.Process(context.Background(), mockRepo, job, mockProgress) + require.EqualError(t, err, "pull resources: sync failed") +} + +func TestDeleteWorker_deleteFiles(t *testing.T) { + tests := []struct { + name string + paths []string + deleteResults []error + tooManyErrors error + expectedError string + expectedCalls int + }{ + { + name: "single file success", + paths: []string{"test/file1.yaml"}, + deleteResults: []error{nil}, + expectedCalls: 1, + }, + { + name: "multiple files success", + paths: []string{"test/file1.yaml", "test/file2.yaml", "test/file3.yaml"}, + deleteResults: []error{nil, nil, nil}, + expectedCalls: 3, + }, + { + name: "single file with error continues", + paths: []string{"test/file1.yaml", "test/file2.yaml"}, + deleteResults: []error{errors.New("delete failed"), nil}, + expectedCalls: 2, + }, + { + name: "too many errors stops processing", + paths: []string{"test/file1.yaml", "test/file2.yaml", "test/file3.yaml"}, + deleteResults: []error{errors.New("delete failed")}, + tooManyErrors: errors.New("too many errors"), + expectedError: "too many errors", + expectedCalls: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockRepo := &mockReaderWriter{ + MockRepository: repository.NewMockRepository(t), + } + mockProgress := jobs.NewMockJobProgressRecorder(t) + + opts := provisioning.DeleteJobOptions{ + Ref: "main", + } + + for i, path := range tt.paths { + if i < len(tt.deleteResults) { + mockRepo.On("Delete", mock.Anything, path, "main", "Delete "+path).Return(tt.deleteResults[i]).Once() + mockProgress.On("SetMessage", mock.Anything, "Deleting "+path).Return().Once() + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == path && result.Action == repository.FileActionDeleted + })).Return().Once() + + if tt.tooManyErrors != nil && i == 0 { + mockProgress.On("TooManyErrors").Return(tt.tooManyErrors).Once() + } else { + mockProgress.On("TooManyErrors").Return(nil).Once() + } + } + } + + worker := NewWorker(nil, nil) + err := worker.deleteFiles(context.Background(), mockRepo, mockProgress, opts, tt.paths...) + + if tt.expectedError != "" { + require.EqualError(t, err, tt.expectedError) + } else { + require.NoError(t, err) + } + + mockRepo.AssertExpectations(t) + mockProgress.AssertExpectations(t) + }) + } +} diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 9b60c22df44..be4cf583824 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -43,6 +43,7 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy" "github.com/grafana/grafana/pkg/registry/apis/provisioning/controller" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" + deletepkg "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/delete" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/export" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/migrate" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/sync" @@ -616,7 +617,13 @@ func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartH b.storageStatus, ) - workers := []jobs.Worker{migrationWorker, syncWorker, exportWorker} + deleteWorker := deletepkg.NewWorker(syncWorker, stageIfPossible) + workers := []jobs.Worker{ + deleteWorker, + exportWorker, + migrationWorker, + syncWorker, + } // Add any extra workers for _, extra := range b.extras { diff --git a/pkg/registry/apis/provisioning/repository/mock_wrap_with_stage_fn.go b/pkg/registry/apis/provisioning/repository/mock_wrap_with_stage_fn.go new file mode 100644 index 00000000000..e4c6108e83c --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/mock_wrap_with_stage_fn.go @@ -0,0 +1,85 @@ +// Code generated by mockery v2.52.4. DO NOT EDIT. + +package repository + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// MockWrapWithStageFn is an autogenerated mock type for the WrapWithStageFn type +type MockWrapWithStageFn struct { + mock.Mock +} + +type MockWrapWithStageFn_Expecter struct { + mock *mock.Mock +} + +func (_m *MockWrapWithStageFn) EXPECT() *MockWrapWithStageFn_Expecter { + return &MockWrapWithStageFn_Expecter{mock: &_m.Mock} +} + +// Execute provides a mock function with given fields: ctx, repo, stageOptions, fn +func (_m *MockWrapWithStageFn) Execute(ctx context.Context, repo Repository, stageOptions StageOptions, fn func(Repository, bool) error) error { + ret := _m.Called(ctx, repo, stageOptions, fn) + + if len(ret) == 0 { + panic("no return value specified for Execute") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, Repository, StageOptions, func(Repository, bool) error) error); ok { + r0 = rf(ctx, repo, stageOptions, fn) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockWrapWithStageFn_Execute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Execute' +type MockWrapWithStageFn_Execute_Call struct { + *mock.Call +} + +// Execute is a helper method to define mock.On call +// - ctx context.Context +// - repo Repository +// - stageOptions StageOptions +// - fn func(Repository , bool) error +func (_e *MockWrapWithStageFn_Expecter) Execute(ctx interface{}, repo interface{}, stageOptions interface{}, fn interface{}) *MockWrapWithStageFn_Execute_Call { + return &MockWrapWithStageFn_Execute_Call{Call: _e.mock.On("Execute", ctx, repo, stageOptions, fn)} +} + +func (_c *MockWrapWithStageFn_Execute_Call) Run(run func(ctx context.Context, repo Repository, stageOptions StageOptions, fn func(Repository, bool) error)) *MockWrapWithStageFn_Execute_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(Repository), args[2].(StageOptions), args[3].(func(Repository, bool) error)) + }) + return _c +} + +func (_c *MockWrapWithStageFn_Execute_Call) Return(_a0 error) *MockWrapWithStageFn_Execute_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockWrapWithStageFn_Execute_Call) RunAndReturn(run func(context.Context, Repository, StageOptions, func(Repository, bool) error) error) *MockWrapWithStageFn_Execute_Call { + _c.Call.Return(run) + return _c +} + +// NewMockWrapWithStageFn creates a new instance of MockWrapWithStageFn. 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 NewMockWrapWithStageFn(t interface { + mock.TestingT + Cleanup(func()) +}) *MockWrapWithStageFn { + mock := &MockWrapWithStageFn{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/registry/apis/provisioning/repository/staged.go b/pkg/registry/apis/provisioning/repository/staged.go index bdcae0ab039..ca5b59c72c9 100644 --- a/pkg/registry/apis/provisioning/repository/staged.go +++ b/pkg/registry/apis/provisioning/repository/staged.go @@ -10,6 +10,9 @@ import ( "github.com/grafana/nanogit" ) +//go:generate mockery --name WrapWithStageFn --structname MockWrapWithStageFn --inpackage --filename mock_wrap_with_stage_fn.go --with-expecter +type WrapWithStageFn func(ctx context.Context, repo Repository, stageOptions StageOptions, fn func(repo Repository, staged bool) error) error + // StageMode defines the staging and commit behavior type StageMode int diff --git a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json index 0faaab04b78..bd54e67e82a 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -2631,6 +2631,23 @@ } } }, + "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.DeleteJobOptions": { + "type": "object", + "properties": { + "paths": { + "description": "Paths to be deleted. Examples: - dashboard.json (for a file) - a/b/c/other-dashboard.json (for a file) - nested/deep/ (for a directory) FIXME: we should validate this in admission hooks", + "type": "array", + "items": { + "type": "string", + "default": "" + } + }, + "ref": { + "description": "Ref to the branch or commit hash to delete from", + "type": "string" + } + } + }, "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.ErrorDetails": { "type": "object", "required": [ @@ -2653,7 +2670,7 @@ "type": "object", "properties": { "branch": { - "description": "Target branch for export (only git)", + "description": "FIXME: we should validate this in admission hooks Target branch for export (only git)", "type": "string" }, "folder": { @@ -2665,7 +2682,7 @@ "type": "string" }, "path": { - "description": "Prefix in target file system", + "description": "FIXME: we should validate this in admission hooks Prefix in target file system", "type": "string" } } @@ -3049,15 +3066,24 @@ "type": "object", "properties": { "action": { - "description": "Possible enum values:\n - `\"migrate\"` acts like JobActionExport, then JobActionPull. It also tries to preserve the history.\n - `\"pr\"` adds additional useful information to a PR, such as comments with preview links and rendered images.\n - `\"pull\"` replicates the remote branch in the local copy of the repository.\n - `\"push\"` replicates the local copy of the repository in the remote branch.", + "description": "Possible enum values:\n - `\"delete\"` deletes files in the remote repository\n - `\"migrate\"` acts like JobActionExport, then JobActionPull. It also tries to preserve the history.\n - `\"pr\"` adds additional useful information to a PR, such as comments with preview links and rendered images.\n - `\"pull\"` replicates the remote branch in the local copy of the repository.\n - `\"push\"` replicates the local copy of the repository in the remote branch.", "type": "string", "enum": [ + "delete", "migrate", "pr", "pull", "push" ] }, + "delete": { + "description": "Delete when the action is `delete`", + "allOf": [ + { + "$ref": "#/components/schemas/com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.DeleteJobOptions" + } + ] + }, "migrate": { "description": "Required when the action is `migrate`", "allOf": [ diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index a5edc751fbe..15049df999d 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -822,7 +822,7 @@ func TestIntegrationProvisioning_DeleteResources(t *testing.T) { }) } -func TestIntegrationProvisioning_MoveResources(t *testing.T) { +func TestIntegrationProvisioning_DeleteJob(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } @@ -830,6 +830,158 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) { helper := runGrafana(t) ctx := context.Background() + const repo = "delete-job-test-repo" + localTmp := helper.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{ + "Name": repo, + "SyncEnabled": true, + "SyncTarget": "instance", + }) + _, err := helper.Repositories.Resource.Create(ctx, localTmp, metav1.CreateOptions{}) + require.NoError(t, err) + // Copy multiple test files to the repository + helper.CopyToProvisioningPath(t, "testdata/all-panels.json", "dashboard1.json") + helper.CopyToProvisioningPath(t, "testdata/text-options.json", "dashboard2.json") + helper.CopyToProvisioningPath(t, "testdata/timeline-demo.json", "folder/dashboard3.json") + + // Trigger and wait for initial sync to populate resources + helper.SyncAndWait(t, repo, nil) + + // Verify initial state - should have 3 dashboards and 1 folder + dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, 3, len(dashboards.Items), "should have 3 dashboards after sync") + + folders, err := helper.Folders.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, 1, len(folders.Items), "should have 1 folder after sync") + + t.Run("delete single file", func(t *testing.T) { + // Create delete job for single file + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(asJSON(&provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"dashboard1.json"}, + }, + })). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error(), "should be able to create delete job") + + // Wait for job to complete + helper.AwaitJobs(t, repo) + + // Verify file is deleted from repository + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard1.json") + require.Error(t, err, "file should be deleted from repository") + require.True(t, apierrors.IsNotFound(err), "should be not found error") + + // Verify dashboard is removed from Grafana after sync + dashboards, err = helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, 2, len(dashboards.Items), "should have 2 dashboards after delete") + + // Verify other files still exist + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard2.json") + require.NoError(t, err, "other files should still exist") + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "folder", "dashboard3.json") + require.NoError(t, err, "nested files should still exist") + }) + + t.Run("delete multiple files", func(t *testing.T) { + // Create delete job for multiple files + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(asJSON(&provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"dashboard2.json", "folder/dashboard3.json"}, + }, + })). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error(), "should be able to create delete job") + + // Wait for job to complete + helper.AwaitJobs(t, repo) + + // Verify files are deleted from repository + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard2.json") + require.Error(t, err, "dashboard2.json should be deleted") + require.True(t, apierrors.IsNotFound(err)) + + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "folder", "dashboard3.json") + require.Error(t, err, "folder/dashboard3.json should be deleted") + require.True(t, apierrors.IsNotFound(err)) + + // Verify all dashboards are removed from Grafana after sync + dashboards, err = helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, 0, len(dashboards.Items), "should have 0 dashboards after deleting all") + }) + + t.Run("delete non-existent file", func(t *testing.T) { + // Create delete job for non-existent file + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(asJSON(&provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"non-existent.json"}, + }, + })). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error(), "should be able to create delete job") + + // Wait for job to complete - should fail due to strict error handling + require.EventuallyWithT(t, func(collect *assert.CollectT) { + list := &unstructured.UnstructuredList{} + err := helper.AdminREST.Get(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Do(ctx).Into(list) + assert.NoError(collect, err, "should be able to list jobs") + assert.NotEmpty(collect, list.Items, "expect at least one job") + + // Find the delete job specifically + var deleteJob *unstructured.Unstructured + for _, elem := range list.Items { + assert.Equal(collect, repo, elem.GetLabels()["provisioning.grafana.app/repository"], "should have repo label") + + action := mustNestedString(elem.Object, "spec", "action") + if action == "delete" { + deleteJob = &elem + break + } + } + assert.NotNil(collect, deleteJob, "should find a delete job") + + state := mustNestedString(deleteJob.Object, "status", "state") + assert.Equal(collect, "error", state, "delete job should have failed due to non-existent file") + }, time.Second*10, time.Millisecond*100, "Expected delete job to fail with error state") + }) +} + +func TestIntegrationProvisioning_MoveResources(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + helper := runGrafana(t) + ctx := context.Background() const repo = "move-test-repo" localTmp := helper.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{ "Name": repo, diff --git a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts index ecd79c6c52e..20a6c603f32 100644 --- a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts +++ b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts @@ -758,6 +758,12 @@ export type ObjectMeta = { Populated by the system. Read-only. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names#uids */ uid?: string; }; +export type DeleteJobOptions = { + /** Paths to be deleted. Examples: - dashboard.json (for a file) - a/b/c/other-dashboard.json (for a file) - nested/deep/ (for a directory) FIXME: we should validate this in admission hooks */ + paths?: string[]; + /** Ref to the branch or commit hash to delete from */ + ref?: string; +}; export type MigrateJobOptions = { /** Preserve history (if possible) */ history?: boolean; @@ -779,22 +785,26 @@ export type SyncJobOptions = { incremental: boolean; }; export type ExportJobOptions = { - /** Target branch for export (only git) */ + /** FIXME: we should validate this in admission hooks Target branch for export (only git) */ branch?: string; /** The source folder (or empty) to export */ folder?: string; /** Message to use when committing the changes in a single commit */ message?: string; + /** FIXME: we should validate this in admission hooks Prefix in target file system */ /** Prefix in target file system */ path?: string; }; export type JobSpec = { /** Possible enum values: + - `"delete"` deletes files in the remote repository - `"migrate"` acts like JobActionExport, then JobActionPull. It also tries to preserve the history. - `"pr"` adds additional useful information to a PR, such as comments with preview links and rendered images. - `"pull"` replicates the remote branch in the local copy of the repository. - `"push"` replicates the local copy of the repository in the remote branch. */ - action?: 'migrate' | 'pr' | 'pull' | 'push'; + action?: 'delete' | 'migrate' | 'pr' | 'pull' | 'push'; + /** Delete when the action is `delete` */ + delete?: DeleteJobOptions; /** Required when the action is `migrate` */ migrate?: MigrateJobOptions; /** Pull request options */