diff --git a/pkg/apis/provisioning/v0alpha1/jobs.go b/pkg/apis/provisioning/v0alpha1/jobs.go index 66c141ebbc8..771085861d6 100644 --- a/pkg/apis/provisioning/v0alpha1/jobs.go +++ b/pkg/apis/provisioning/v0alpha1/jobs.go @@ -40,6 +40,9 @@ const ( // JobActionDelete deletes files in the remote repository JobActionDelete JobAction = "delete" + + // JobActionMove moves files in the remote repository + JobActionMove JobAction = "move" ) // +enum @@ -87,6 +90,9 @@ type JobSpec struct { // Delete when the action is `delete` Delete *DeleteJobOptions `json:"delete,omitempty"` + + // Move when the action is `move` + Move *MoveJobOptions `json:"move,omitempty"` } type PullRequestJobOptions struct { @@ -160,6 +166,20 @@ type ResourceRef struct { Group string `json:"group,omitempty"` } +type MoveJobOptions struct { + // Ref to the branch or commit hash that should move + 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"` + + // Destination path for the move (e.g. "new-location/") + TargetPath string `json:"targetPath,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 a39d8f42d72..ce5320bc548 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go @@ -401,6 +401,11 @@ func (in *JobSpec) DeepCopyInto(out *JobSpec) { *out = new(DeleteJobOptions) (*in).DeepCopyInto(*out) } + if in.Move != nil { + in, out := &in.Move, &out.Move + *out = new(MoveJobOptions) + (*in).DeepCopyInto(*out) + } return } @@ -499,6 +504,27 @@ func (in *MigrateJobOptions) DeepCopy() *MigrateJobOptions { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MoveJobOptions) DeepCopyInto(out *MoveJobOptions) { + *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 MoveJobOptions. +func (in *MoveJobOptions) DeepCopy() *MoveJobOptions { + if in == nil { + return nil + } + out := new(MoveJobOptions) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PullRequestJobOptions) DeepCopyInto(out *PullRequestJobOptions) { *out = *in diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index 8c497089383..aeedcedf468 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -35,6 +35,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.LocalRepositoryConfig": schema_pkg_apis_provisioning_v0alpha1_LocalRepositoryConfig(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.ManagerStats": schema_pkg_apis_provisioning_v0alpha1_ManagerStats(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.MigrateJobOptions": schema_pkg_apis_provisioning_v0alpha1_MigrateJobOptions(ref), + "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.MoveJobOptions": schema_pkg_apis_provisioning_v0alpha1_MoveJobOptions(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.PullRequestJobOptions": schema_pkg_apis_provisioning_v0alpha1_PullRequestJobOptions(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.RefItem": schema_pkg_apis_provisioning_v0alpha1_RefItem(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.RefList": schema_pkg_apis_provisioning_v0alpha1_RefList(ref), @@ -895,10 +896,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 - `\"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.", + 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 - `\"move\"` moves files in the remote repository\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{}{"delete", "migrate", "pr", "pull", "push"}, + Enum: []interface{}{"delete", "migrate", "move", "pr", "pull", "push"}, }, }, "repository": { @@ -938,11 +939,17 @@ func schema_pkg_apis_provisioning_v0alpha1_JobSpec(ref common.ReferenceCallback) Ref: ref("github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.DeleteJobOptions"), }, }, + "move": { + SchemaProps: spec.SchemaProps{ + Description: "Move when the action is `move`", + Ref: ref("github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.MoveJobOptions"), + }, + }, }, }, }, Dependencies: []string{ - "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"}, + "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.MoveJobOptions", "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.PullRequestJobOptions", "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.SyncJobOptions"}, } } @@ -1108,6 +1115,47 @@ func schema_pkg_apis_provisioning_v0alpha1_MigrateJobOptions(ref common.Referenc } } +func schema_pkg_apis_provisioning_v0alpha1_MoveJobOptions(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 that should move", + 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: "", + }, + }, + }, + }, + }, + "targetPath": { + SchemaProps: spec.SchemaProps{ + Description: "Destination path for the move (e.g. \"new-location/\")", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_provisioning_v0alpha1_PullRequestJobOptions(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ 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 847f8aaab1a..67010afc96f 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list @@ -6,6 +6,7 @@ API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provis API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,JobStatus,Errors API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,JobStatus,Summary API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,ManagerStats,Stats +API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,MoveJobOptions,Paths API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,RefList,Items API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,RepositoryList,Items API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,RepositorySpec,Workflows diff --git a/pkg/registry/apis/provisioning/jobs/move/worker.go b/pkg/registry/apis/provisioning/jobs/move/worker.go new file mode 100644 index 00000000000..9dd25aab41e --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/move/worker.go @@ -0,0 +1,127 @@ +package move + +import ( + "context" + "errors" + "fmt" + "path/filepath" + "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/grafana/grafana/pkg/registry/apis/provisioning/safepath" +) + +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.JobActionMove +} + +func (w *Worker) Process(ctx context.Context, repo repository.Repository, job provisioning.Job, progress jobs.JobProgressRecorder) error { + if job.Spec.Move == nil { + return errors.New("missing move settings") + } + opts := *job.Spec.Move + + if opts.TargetPath == "" { + return errors.New("target path is required for move operation") + } + + // Validate that target path is a directory (ends with slash) + if !safepath.IsDir(opts.TargetPath) { + return errors.New("target path must be a directory (should end with '/')") + } + + paths := opts.Paths + progress.SetTotal(ctx, len(paths)) + progress.StrictMaxErrors(1) // Fail fast on any error during move + + fn := func(repo repository.Repository, _ bool) error { + rw, ok := repo.(repository.ReaderWriter) + if !ok { + return errors.New("move job submitted targeting repository that is not a ReaderWriter") + } + + return w.moveFiles(ctx, rw, progress, opts, paths...) + } + + msg := fmt.Sprintf("Move files from Grafana %s", job.Name) + stageOptions := repository.StageOptions{ + Mode: repository.StageModeCommitOnlyOnce, + CommitOnlyOnceMessage: msg, + PushOnWrites: false, + Timeout: 10 * time.Minute, + } + + err := w.wrapFn(ctx, repo, stageOptions, fn) + if err != nil { + return fmt.Errorf("move files in 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) moveFiles(ctx context.Context, rw repository.ReaderWriter, progress jobs.JobProgressRecorder, opts provisioning.MoveJobOptions, paths ...string) error { + for _, path := range paths { + result := jobs.JobResourceResult{ + Path: path, + Action: repository.FileActionRenamed, + } + + // Construct the target path by combining the job's target path with the file/folder name + targetPath := w.constructTargetPath(opts.TargetPath, path) + + progress.SetMessage(ctx, "Moving "+path+" to "+targetPath) + result.Error = rw.Move(ctx, path, targetPath, opts.Ref, "Move "+path+" to "+targetPath) + progress.Record(ctx, result) + if err := progress.TooManyErrors(); err != nil { + return err + } + } + + return nil +} + +// constructTargetPath combines the job's target path with the file/folder name from the source path +func (w *Worker) constructTargetPath(jobTargetPath, sourcePath string) string { + // Extract the file/folder name from the source path + fileName := filepath.Base(sourcePath) + + // If the source path is a directory (ends with slash), preserve the trailing slash in target + if safepath.IsDir(sourcePath) { + return jobTargetPath + fileName + "/" + } + + // For files, just append the filename + return jobTargetPath + fileName +} diff --git a/pkg/registry/apis/provisioning/jobs/move/worker_test.go b/pkg/registry/apis/provisioning/jobs/move/worker_test.go new file mode 100644 index 00000000000..2a6720f5964 --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/move/worker_test.go @@ -0,0 +1,490 @@ +package move + +import ( + "context" + "errors" + "path/filepath" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type mockReaderWriter struct { + *repository.MockRepository +} + +func (m *mockReaderWriter) Move(ctx context.Context, oldPath, newPath, ref, message string) error { + args := m.Called(ctx, oldPath, newPath, ref, message) + return args.Error(0) +} + +func TestMoveWorker_IsSupported(t *testing.T) { + tests := []struct { + name string + job provisioning.Job + expected bool + }{ + { + name: "move action is supported", + job: provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + }, + }, + expected: true, + }, + { + name: "delete action is not supported", + job: provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + }, + }, + expected: false, + }, + { + 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 TestMoveWorker_ProcessMissingMoveSettings(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + }, + } + + worker := NewWorker(nil, nil) + err := worker.Process(context.Background(), nil, job, nil) + require.EqualError(t, err, "missing move settings") +} + +func TestMoveWorker_ProcessMissingTargetPath(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"test/path"}, + }, + }, + } + + worker := NewWorker(nil, nil) + err := worker.Process(context.Background(), nil, job, nil) + require.EqualError(t, err, "target path is required for move operation") +} + +func TestMoveWorker_ProcessInvalidTargetPath(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"test/path"}, + TargetPath: "target.txt", // This is not a directory path + }, + }, + } + + worker := NewWorker(nil, nil) + err := worker.Process(context.Background(), nil, job, nil) + require.EqualError(t, err, "target path must be a directory (should end with '/')") +} + +func TestMoveWorker_ProcessNotReaderWriter(t *testing.T) { + job := provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"test/path"}, + TargetPath: "new/location/", + }, + }, + } + + 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 && + opts.Mode == repository.StageModeCommitOnlyOnce && + opts.CommitOnlyOnceMessage == "Move files from Grafana test-job" + }), mock.Anything).Return(errors.New("move 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, "move files in repository: move job submitted targeting repository that is not a ReaderWriter") +} + +func TestMoveWorker_ProcessWrapFnError(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"test/path"}, + TargetPath: "new/location/", + }, + }, + } + + 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, "move files in repository: stage failed") +} + +func TestMoveWorker_ProcessMoveFilesSuccess(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"test/path1", "test/path2"}, + TargetPath: "new/location/", + 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 && + opts.Mode == repository.StageModeCommitOnlyOnce && + opts.CommitOnlyOnceMessage != "" + }), 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, "Moving test/path1 to new/location/path1").Return() + mockProgress.On("SetMessage", mock.Anything, "Moving test/path2 to new/location/path2").Return() + mockProgress.On("TooManyErrors").Return(nil).Twice() + + mockRepo.On("Move", mock.Anything, "test/path1", "new/location/path1", "main", "Move test/path1 to new/location/path1").Return(nil) + mockRepo.On("Move", mock.Anything, "test/path2", "new/location/path2", "main", "Move test/path2 to new/location/path2").Return(nil) + + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == "test/path1" && result.Action == repository.FileActionRenamed && result.Error == nil + })).Return() + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == "test/path2" && result.Action == repository.FileActionRenamed && result.Error == nil + })).Return() + + worker := NewWorker(nil, mockWrapFn.Execute) + err := worker.Process(context.Background(), mockRepo, job, mockProgress) + require.NoError(t, err) +} + +func TestMoveWorker_ProcessMoveFilesWithError(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"test/path1", "test/path2"}, + TargetPath: "new/location/", + 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, "Moving test/path1 to new/location/path1").Return() + + moveError := errors.New("move failed") + mockRepo.On("Move", mock.Anything, "test/path1", "new/location/path1", "main", "Move test/path1 to new/location/path1").Return(moveError) + + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == "test/path1" && result.Action == repository.FileActionRenamed && errors.Is(result.Error, moveError) + })).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, "move files in repository: too many errors") +} + +func TestMoveWorker_ProcessWithSyncWorker(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"test/path"}, + TargetPath: "new/location/", + }, + }, + } + + 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, "Moving test/path to new/location/path").Return() + mockProgress.On("TooManyErrors").Return(nil) + + mockRepo.On("Move", mock.Anything, "test/path", "new/location/path", "", "Move test/path to new/location/path").Return(nil) + + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == "test/path" && result.Action == repository.FileActionRenamed && 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 TestMoveWorker_ProcessSyncWorkerError(t *testing.T) { + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"test/path"}, + TargetPath: "new/location/", + }, + }, + } + + 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, "Moving test/path to new/location/path").Return() + mockProgress.On("TooManyErrors").Return(nil) + + mockRepo.On("Move", mock.Anything, "test/path", "new/location/path", "", "Move test/path to new/location/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 TestMoveWorker_moveFiles(t *testing.T) { + tests := []struct { + name string + paths []string + moveResults []error + tooManyErrors error + expectedError string + expectedCalls int + }{ + { + name: "single file success", + paths: []string{"test/file1.yaml"}, + moveResults: []error{nil}, + expectedCalls: 1, + }, + { + name: "multiple files success", + paths: []string{"test/file1.yaml", "test/file2.yaml", "test/file3.yaml"}, + moveResults: []error{nil, nil, nil}, + expectedCalls: 3, + }, + { + name: "mixed files and folders", + paths: []string{"file.json", "folder/", "nested/file.yaml"}, + moveResults: []error{nil, nil, nil}, + expectedCalls: 3, + }, + { + name: "single file with error continues", + paths: []string{"test/file1.yaml", "test/file2.yaml"}, + moveResults: []error{errors.New("move failed"), nil}, + expectedCalls: 2, + }, + { + name: "too many errors stops processing", + paths: []string{"test/file1.yaml", "test/file2.yaml", "test/file3.yaml"}, + moveResults: []error{errors.New("move 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.MoveJobOptions{ + TargetPath: "new/location/", + Ref: "main", + } + + for i, path := range tt.paths { + if i < len(tt.moveResults) { + // Use the same logic as constructTargetPath to build expected target + expectedTarget := "new/location/" + filepath.Base(path) + if safepath.IsDir(path) { + expectedTarget += "/" + } + mockRepo.On("Move", mock.Anything, path, expectedTarget, "main", "Move "+path+" to "+expectedTarget).Return(tt.moveResults[i]).Once() + mockProgress.On("SetMessage", mock.Anything, "Moving "+path+" to "+expectedTarget).Return().Once() + mockProgress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { + return result.Path == path && result.Action == repository.FileActionRenamed + })).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.moveFiles(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) + }) + } +} + +func TestMoveWorker_constructTargetPath(t *testing.T) { + tests := []struct { + name string + jobTargetPath string + sourcePath string + expectedTarget string + }{ + { + name: "simple file in directory", + jobTargetPath: "moved/", + sourcePath: "dashboard.json", + expectedTarget: "moved/dashboard.json", + }, + { + name: "nested file in directory", + jobTargetPath: "archived/", + sourcePath: "folder/dashboard.json", + expectedTarget: "archived/dashboard.json", + }, + { + name: "deeply nested directory target", + jobTargetPath: "deep/nested/target/", + sourcePath: "source/file.yaml", + expectedTarget: "deep/nested/target/file.yaml", + }, + { + name: "folder to folder", + jobTargetPath: "new-location/", + sourcePath: "old-folder/", + expectedTarget: "new-location/old-folder/", + }, + { + name: "nested folder move", + jobTargetPath: "archive/", + sourcePath: "deep/nested/folder/", + expectedTarget: "archive/folder/", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + worker := NewWorker(nil, nil) + result := worker.constructTargetPath(tt.jobTargetPath, tt.sourcePath) + require.Equal(t, tt.expectedTarget, result) + }) + } +} diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 96a5145fd30..6d87177b247 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -46,6 +46,7 @@ import ( 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" + movepkg "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/move" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/sync" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/git" @@ -618,10 +619,12 @@ func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartH ) deleteWorker := deletepkg.NewWorker(syncWorker, stageIfPossible, b.repositoryResources) + moveWorker := movepkg.NewWorker(syncWorker, stageIfPossible) workers := []jobs.Worker{ deleteWorker, exportWorker, migrationWorker, + moveWorker, syncWorker, } diff --git a/pkg/registry/apis/provisioning/repository/git/staged.go b/pkg/registry/apis/provisioning/repository/git/staged.go index 335193716b0..a3f79567029 100644 --- a/pkg/registry/apis/provisioning/repository/git/staged.go +++ b/pkg/registry/apis/provisioning/repository/git/staged.go @@ -91,22 +91,37 @@ func (r *stagedGitRepository) ReadTree(ctx context.Context, ref string) ([]repos return r.gitRepository.ReadTree(ctx, ref) } -// handleCommitAndPush handles the commit and push logic based on the StageMode +// handleCommitAndPush handles the commit and push logic based on the StageMode and PushOnWrites flag func (r *stagedGitRepository) handleCommitAndPush(ctx context.Context, message string) error { switch r.opts.Mode { case repository.StageModeCommitOnEach: - return r.commit(ctx, r.writer, message) + if err := r.commit(ctx, r.writer, message); err != nil { + return err + } + // Only push if PushOnWrites is enabled + if r.opts.PushOnWrites { + return r.Push(ctx) + } + return nil case repository.StageModeCommitAndPushOnEach: if err := r.commit(ctx, r.writer, message); err != nil { return err } + // Always push for this mode (explicit push-on-each mode) return r.Push(ctx) case repository.StageModeCommitOnlyOnce: // No immediate commit, will commit on Push return nil default: // Default to StageModeCommitOnEach for backward compatibility - return r.commit(ctx, r.writer, message) + if err := r.commit(ctx, r.writer, message); err != nil { + return err + } + // Only push if PushOnWrites is enabled + if r.opts.PushOnWrites { + return r.Push(ctx) + } + return nil } } @@ -180,6 +195,18 @@ func (r *stagedGitRepository) Delete(ctx context.Context, path, ref, message str return r.handleCommitAndPush(ctx, message) } +func (r *stagedGitRepository) Move(ctx context.Context, oldPath, newPath, ref, message string) error { + if !r.isRefSupported(ref) { + return errors.New("ref is not supported for staged repository") + } + + if err := r.move(ctx, oldPath, newPath, r.writer); err != nil { + return err + } + + return r.handleCommitAndPush(ctx, message) +} + func (r *stagedGitRepository) Push(ctx context.Context) error { if r.opts.Timeout > 0 { var cancel context.CancelFunc diff --git a/pkg/registry/apis/provisioning/repository/git/staged_test.go b/pkg/registry/apis/provisioning/repository/git/staged_test.go index 0a614b94c57..4501a087e7a 100644 --- a/pkg/registry/apis/provisioning/repository/git/staged_test.go +++ b/pkg/registry/apis/provisioning/repository/git/staged_test.go @@ -1028,6 +1028,319 @@ func TestStagedGitRepository_Push(t *testing.T) { } } +func TestStagedGitRepository_Move(t *testing.T) { + tests := []struct { + name string + setupMock func(*mocks.FakeStagedWriter) + opts repository.StageOptions + oldPath string + newPath string + ref string + message string + wantError error + expectPush bool + expectCommit bool + }{ + { + name: "succeeds with file move and CommitOnEach with PushOnWrites false", + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.MoveTreeReturns(hash.Hash{1, 2, 3}, nil) + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnEach, + PushOnWrites: false, + }, + oldPath: "folder/", + newPath: "newfolder/", + ref: "", + message: "Move folder to newfolder", + wantError: nil, + expectPush: false, + expectCommit: true, + }, + { + name: "succeeds with file move and CommitOnEach with PushOnWrites true", + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.MoveBlobReturns(hash.Hash{1, 2, 3}, nil) + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + mockWriter.PushReturns(nil) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnEach, + PushOnWrites: true, + }, + oldPath: "test.yaml", + newPath: "newtest.yaml", + ref: "", + message: "Move test to newtest", + wantError: nil, + expectPush: true, + expectCommit: true, + }, + { + name: "succeeds with CommitAndPushOnEach mode", + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.MoveBlobReturns(hash.Hash{1, 2, 3}, nil) + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + mockWriter.PushReturns(nil) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitAndPushOnEach, + PushOnWrites: false, // Should be ignored in this mode + }, + oldPath: "test.yaml", + newPath: "newtest.yaml", + ref: "", + message: "Move test to newtest", + wantError: nil, + expectPush: true, + expectCommit: true, + }, + { + name: "succeeds with CommitOnlyOnce mode", + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.MoveBlobReturns(hash.Hash{1, 2, 3}, nil) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnlyOnce, + PushOnWrites: true, // Should be ignored in this mode + }, + oldPath: "test.yaml", + newPath: "newtest.yaml", + ref: "", + message: "Move test to newtest", + wantError: nil, + expectPush: false, + expectCommit: false, + }, + { + name: "fails with unsupported ref", + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + // No setup needed as error occurs before writer calls + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnEach, + }, + oldPath: "test.yaml", + newPath: "newtest.yaml", + ref: "feature-branch", + message: "Move test to newtest", + wantError: errors.New("ref is not supported for staged repository"), + }, + { + name: "fails with move error", + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.MoveBlobReturns(hash.Hash{}, errors.New("move failed")) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnEach, + }, + oldPath: "test.yaml", + newPath: "newtest.yaml", + ref: "", + message: "Move test to newtest", + wantError: errors.New("move blob: move failed"), + }, + { + name: "fails with commit error", + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.MoveBlobReturns(hash.Hash{1, 2, 3}, nil) + mockWriter.CommitReturns(&nanogit.Commit{}, errors.New("commit failed")) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnEach, + }, + oldPath: "test.yaml", + newPath: "newtest.yaml", + ref: "", + message: "Move test to newtest", + wantError: errors.New("commit changes: commit failed"), + }, + { + name: "fails with push error", + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.MoveBlobReturns(hash.Hash{1, 2, 3}, nil) + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + mockWriter.PushReturns(errors.New("push failed")) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitAndPushOnEach, + PushOnWrites: false, + }, + oldPath: "test.yaml", + newPath: "newtest.yaml", + ref: "", + message: "Move test to newtest", + wantError: errors.New("push failed"), + expectPush: true, + expectCommit: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockWriter := &mocks.FakeStagedWriter{} + tt.setupMock(mockWriter) + + stagedRepo := createTestStagedRepositoryWithWriter(mockWriter, tt.opts) + + err := stagedRepo.Move(context.Background(), tt.oldPath, tt.newPath, tt.ref, tt.message) + + if tt.wantError != nil { + require.EqualError(t, err, tt.wantError.Error()) + } else { + require.NoError(t, err) + } + + // Verify push behavior + if tt.expectPush { + require.Equal(t, 1, mockWriter.PushCallCount()) + } else if tt.wantError == nil { + require.Equal(t, 0, mockWriter.PushCallCount()) + } + + // Verify commit behavior + if tt.expectCommit { + require.Equal(t, 1, mockWriter.CommitCallCount()) + } else if tt.wantError == nil { + require.Equal(t, 0, mockWriter.CommitCallCount()) + } + }) + } +} + +func TestStagedGitRepository_handleCommitAndPush(t *testing.T) { + tests := []struct { + name string + opts repository.StageOptions + setupMock func(*mocks.FakeStagedWriter) + message string + wantError error + expectCommit bool + expectPush bool + }{ + { + name: "StageModeCommitOnEach with PushOnWrites false", + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnEach, + PushOnWrites: false, + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + }, + message: "test message", + wantError: nil, + expectCommit: true, + expectPush: false, + }, + { + name: "StageModeCommitOnEach with PushOnWrites true", + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnEach, + PushOnWrites: true, + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + mockWriter.PushReturns(nil) + }, + message: "test message", + wantError: nil, + expectCommit: true, + expectPush: true, + }, + { + name: "StageModeCommitAndPushOnEach always pushes regardless of PushOnWrites", + opts: repository.StageOptions{ + Mode: repository.StageModeCommitAndPushOnEach, + PushOnWrites: false, // Should be ignored + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + mockWriter.PushReturns(nil) + }, + message: "test message", + wantError: nil, + expectCommit: true, + expectPush: true, + }, + { + name: "StageModeCommitOnlyOnce does nothing", + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnlyOnce, + PushOnWrites: true, // Should be ignored + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + // No setup needed as no calls should be made + }, + message: "test message", + wantError: nil, + expectCommit: false, + expectPush: false, + }, + { + name: "Default mode (backward compatibility) with PushOnWrites false", + opts: repository.StageOptions{ + Mode: repository.StageMode(99), // Unknown mode defaults to StageModeCommitOnEach + PushOnWrites: false, + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + }, + message: "test message", + wantError: nil, + expectCommit: true, + expectPush: false, + }, + { + name: "Default mode (backward compatibility) with PushOnWrites true", + opts: repository.StageOptions{ + Mode: repository.StageMode(99), // Unknown mode defaults to StageModeCommitOnEach + PushOnWrites: true, + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + mockWriter.PushReturns(nil) + }, + message: "test message", + wantError: nil, + expectCommit: true, + expectPush: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockWriter := &mocks.FakeStagedWriter{} + tt.setupMock(mockWriter) + + stagedRepo := createTestStagedRepositoryWithWriter(mockWriter, tt.opts) + + err := stagedRepo.handleCommitAndPush(context.Background(), tt.message) + + if tt.wantError != nil { + require.EqualError(t, err, tt.wantError.Error()) + } else { + require.NoError(t, err) + } + + // Verify commit behavior + if tt.expectCommit { + require.Equal(t, 1, mockWriter.CommitCallCount()) + } else { + require.Equal(t, 0, mockWriter.CommitCallCount()) + } + + // Verify push behavior + if tt.expectPush { + require.Equal(t, 1, mockWriter.PushCallCount()) + } else { + require.Equal(t, 0, mockWriter.PushCallCount()) + } + }) + } +} + func TestStagedGitRepository_Remove(t *testing.T) { t.Run("succeeds with remove", func(t *testing.T) { mockWriter := &mocks.FakeStagedWriter{} 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 c4d341ea41d..006294e1795 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -3078,11 +3078,12 @@ "type": "object", "properties": { "action": { - "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.", + "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 - `\"move\"` moves files in the remote repository\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", + "move", "pr", "pull", "push" @@ -3104,6 +3105,14 @@ } ] }, + "move": { + "description": "Move when the action is `move`", + "allOf": [ + { + "$ref": "#/components/schemas/com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.MoveJobOptions" + } + ] + }, "pr": { "description": "Pull request options", "allOf": [ @@ -3229,6 +3238,27 @@ } } }, + "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.MoveJobOptions": { + "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 that should move", + "type": "string" + }, + "targetPath": { + "description": "Destination path for the move (e.g. \"new-location/\")", + "type": "string" + } + } + }, "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.PullRequestJobOptions": { "type": "object", "properties": { diff --git a/pkg/tests/apis/provisioning/helper_test.go b/pkg/tests/apis/provisioning/helper_test.go index cff6dbeeabf..9c7949c2032 100644 --- a/pkg/tests/apis/provisioning/helper_test.go +++ b/pkg/tests/apis/provisioning/helper_test.go @@ -115,6 +115,8 @@ func (h *provisioningTestHelper) AwaitJobSuccess(t *testing.T, ctx context.Conte state := mustNestedString(result.Object, "status", "state") require.Equal(t, string(provisioning.JobStateSuccess), state, "historic job '%s' was not successful", job.GetName()) + errors := mustNestedStringSlice(result.Object, "status", "errors") + require.Empty(t, errors, "historic job '%s' has errors: %v", job.GetName(), errors) }, time.Second*10, time.Millisecond*25) { // We also want to add the job details to the error when it fails. job, err := h.Jobs.Resource.Get(ctx, job.GetName(), metav1.GetOptions{}) @@ -324,6 +326,14 @@ func mustNestedString(obj map[string]interface{}, fields ...string) string { return v } +func mustNestedStringSlice(obj map[string]interface{}, fields ...string) []string { + v, _, err := unstructured.NestedStringSlice(obj, fields...) + if err != nil { + panic(err) + } + return v +} + func asJSON(obj any) []byte { jj, _ := json.Marshal(obj) return jj diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index ecd3f5755c1..2bf6a9fba70 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -1309,6 +1309,267 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { }) } +func TestIntegrationProvisioning_MoveJob(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, + "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("move single file", func(t *testing.T) { + // Create move job for single file + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(asJSON(&provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"dashboard1.json"}, + TargetPath: "moved/", + }, + })). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error(), "should be able to create move job") + + raw, err := result.Raw() + require.NoError(t, err) + + obj := &unstructured.Unstructured{} + err = json.Unmarshal(raw, obj) + require.NoError(t, err) + + // Wait for job to complete + helper.AwaitJobSuccess(t, ctx, obj) + + // TODO: This additional sync should not be necessary - the move job should handle sync properly + helper.SyncAndWait(t, repo, nil) + + // Verify file is moved in repository + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "moved", "dashboard1.json") + require.NoError(t, err, "file should exist at new location in repository") + + // Verify original file is gone from repository + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard1.json") + require.Error(t, err, "original file should be gone from repository") + require.True(t, apierrors.IsNotFound(err), "should be not found error") + + // Verify dashboard still exists in Grafana after sync + dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Len(t, dashboards.Items, 3, "should still have 3 dashboards after move") + + // Verify that dashboards have the correct source paths + foundPaths := make(map[string]bool) + for _, dashboard := range dashboards.Items { + sourcePath := dashboard.GetAnnotations()["grafana.app/sourcePath"] + foundPaths[sourcePath] = true + } + + require.True(t, foundPaths["moved/dashboard1.json"], "should have dashboard with moved source path") + require.True(t, foundPaths["dashboard2.json"], "should have dashboard2 in original location") + require.True(t, foundPaths["folder/dashboard3.json"], "should have dashboard3 in original nested location") + + // Verify other files still exist at original locations + _, 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("move multiple files and folder", func(t *testing.T) { + // Create move job for multiple files including a folder + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(asJSON(&provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"dashboard2.json", "folder/"}, + TargetPath: "archived/", + }, + })). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error(), "should be able to create move job") + + raw, err := result.Raw() + require.NoError(t, err) + obj := &unstructured.Unstructured{} + err = json.Unmarshal(raw, obj) + require.NoError(t, err) + + // Wait for job to complete + helper.AwaitJobSuccess(t, ctx, obj) + + // TODO: This additional sync should not be necessary - the move job should handle sync properly + helper.SyncAndWait(t, repo, nil) + + // Verify files are moved in repository + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "archived", "dashboard2.json") + require.NoError(t, err, "dashboard2.json should exist at new location") + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "archived", "folder", "dashboard3.json") + require.NoError(t, err, "folder/dashboard3.json should exist at new nested location") + + // Verify original files are gone from repository + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard2.json") + require.Error(t, err, "dashboard2.json should be gone from original location") + require.True(t, apierrors.IsNotFound(err)) + + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "folder", "dashboard3.json") + require.Error(t, err, "folder should be gone from original location") + require.True(t, apierrors.IsNotFound(err), err.Error()) + + // Verify dashboards still exist in Grafana after sync + // Note: Since dashboard1.json was moved in the previous test, we now expect all 3 dashboards + // to be accessible from their moved locations (dashboard1 from moved/, dashboard2 and dashboard3 from archived/) + dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Len(t, dashboards.Items, 3, "should still have 3 dashboards after move") + + // Verify that dashboards have the correct source paths after cumulative moves + foundPaths := make(map[string]bool) + for _, dashboard := range dashboards.Items { + sourcePath := dashboard.GetAnnotations()["grafana.app/sourcePath"] + foundPaths[sourcePath] = true + } + + require.True(t, foundPaths["moved/dashboard1.json"], "should have dashboard1 from first move") + require.True(t, foundPaths["archived/dashboard2.json"], "should have dashboard2 in archived location") + require.True(t, foundPaths["archived/folder/dashboard3.json"], "should have dashboard3 in archived nested location") + }) + + t.Run("move non-existent file", func(t *testing.T) { + // Create move job for non-existent file + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(asJSON(&provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"non-existent.json"}, + TargetPath: "moved/", + }, + })). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error(), "should be able to create move 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 move job specifically + var moveJob *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 == "move" { + // Check if this is the specific job we're looking for + paths, found, err := unstructured.NestedStringSlice(elem.Object, "spec", "move", "paths") + if err == nil && found && len(paths) > 0 && paths[0] == "non-existent.json" { + moveJob = &elem + break + } + } + } + assert.NotNil(collect, moveJob, "should find a move job for non-existent file") + + state := mustNestedString(moveJob.Object, "status", "state") + assert.Equal(collect, "error", state, "move job should have failed due to non-existent file") + }, time.Second*10, time.Millisecond*100, "Expected move job to fail with error state") + }) + + t.Run("move without target path", func(t *testing.T) { + // Create move job without target path (should fail) + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(asJSON(&provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"moved/dashboard1.json"}, + // TargetPath intentionally omitted + }, + })). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error(), "should be able to create move job") + + // Wait for job to complete - should fail due to missing target path + 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 move job specifically + var moveJob *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 == "move" { + // Check if this is the job without target path + targetPath, found, _ := unstructured.NestedString(elem.Object, "spec", "move", "targetPath") + if !found || targetPath == "" { + moveJob = &elem + break + } + } + } + assert.NotNil(collect, moveJob, "should find a move job without target path") + + state := mustNestedString(moveJob.Object, "status", "state") + assert.Equal(collect, "error", state, "move job should have failed due to missing target path") + }, time.Second*10, time.Millisecond*100, "Expected move job to fail with error state") + }) +} + func TestIntegrationProvisioning_MoveResources(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") diff --git a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts index 0cba5dbf96a..0b239287e85 100644 --- a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts +++ b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts @@ -780,6 +780,14 @@ export type MigrateJobOptions = { /** Message to use when committing the changes in a single commit */ message?: string; }; +export type MoveJobOptions = { + /** 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 that should move */ + ref?: string; + /** Destination path for the move (e.g. "new-location/") */ + targetPath?: string; +}; export type PullRequestJobOptions = { /** The specific commit hash that triggered this notice */ hash?: string; @@ -808,14 +816,17 @@ 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. + - `"move"` moves files in the remote repository - `"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?: 'delete' | 'migrate' | 'pr' | 'pull' | 'push'; + action?: 'delete' | 'migrate' | 'move' | 'pr' | 'pull' | 'push'; /** Delete when the action is `delete` */ delete?: DeleteJobOptions; /** Required when the action is `migrate` */ migrate?: MigrateJobOptions; + /** Move when the action is `move` */ + move?: MoveJobOptions; /** Pull request options */ pr?: PullRequestJobOptions; /** Required when the action is `pull` */