diff --git a/pkg/apis/provisioning/v0alpha1/jobs.go b/pkg/apis/provisioning/v0alpha1/jobs.go index f4fa20e8934..5d0348864a7 100644 --- a/pkg/apis/provisioning/v0alpha1/jobs.go +++ b/pkg/apis/provisioning/v0alpha1/jobs.go @@ -103,6 +103,9 @@ type SyncJobOptions struct { } type ExportJobOptions struct { + // Message to use when committing the changes in a single commit + Message string `json:"message,omitempty"` + // The source folder (or empty) to export Folder string `json:"folder,omitempty"` @@ -116,6 +119,9 @@ type ExportJobOptions struct { type MigrateJobOptions struct { // Preserve history (if possible) History bool `json:"history,omitempty"` + + // Message to use when committing the changes in a single commit + Message string `json:"message,omitempty"` } // The job status diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index 58a67095b53..737d2cc29c0 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -193,6 +193,13 @@ func schema_pkg_apis_provisioning_v0alpha1_ExportJobOptions(ref common.Reference SchemaProps: spec.SchemaProps{ Type: []string{"object"}, Properties: map[string]spec.Schema{ + "message": { + SchemaProps: spec.SchemaProps{ + Description: "Message to use when committing the changes in a single commit", + Type: []string{"string"}, + Format: "", + }, + }, "folder": { SchemaProps: spec.SchemaProps{ Description: "The source folder (or empty) to export", @@ -1030,6 +1037,13 @@ func schema_pkg_apis_provisioning_v0alpha1_MigrateJobOptions(ref common.Referenc Format: "", }, }, + "message": { + SchemaProps: spec.SchemaProps{ + Description: "Message to use when committing the changes in a single commit", + Type: []string{"string"}, + Format: "", + }, + }, }, }, }, diff --git a/pkg/registry/apis/provisioning/jobs/export/worker.go b/pkg/registry/apis/provisioning/jobs/export/worker.go index 5ded8555257..87405a881ef 100644 --- a/pkg/registry/apis/provisioning/jobs/export/worker.go +++ b/pkg/registry/apis/provisioning/jobs/export/worker.go @@ -56,9 +56,15 @@ func (r *ExportWorker) Process(ctx context.Context, repo repository.Repository, return err } + msg := options.Message + if msg == "" { + msg = fmt.Sprintf("Export from Grafana %s", job.Name) + } + cloneOptions := repository.StageOptions{ - Timeout: 10 * time.Minute, - PushOnWrites: false, + Mode: repository.StageModeCommitOnlyOnce, + CommitOnlyOnceMessage: msg, + Timeout: 10 * time.Minute, } fn := func(repo repository.Repository, _ bool) error { diff --git a/pkg/registry/apis/provisioning/jobs/export/worker_test.go b/pkg/registry/apis/provisioning/jobs/export/worker_test.go index aa5f2ea1c5e..af419538a42 100644 --- a/pkg/registry/apis/provisioning/jobs/export/worker_test.go +++ b/pkg/registry/apis/provisioning/jobs/export/worker_test.go @@ -260,7 +260,7 @@ func TestExportWorker_ProcessStageOptions(t *testing.T) { mockStageFn := NewMockWrapWithStageFn(t) // Verify clone and push options mockStageFn.On("Execute", mock.Anything, mockRepo, mock.MatchedBy(func(opts repository.StageOptions) bool { - return opts.Timeout == 10*time.Minute && !opts.PushOnWrites + return opts.Timeout == 10*time.Minute && opts.Mode == repository.StageModeCommitOnlyOnce }), mock.Anything).Return(func(ctx context.Context, repo repository.Repository, stageOpts repository.StageOptions, fn func(repository.Repository, bool) error) error { return fn(repo, true) }) @@ -406,7 +406,7 @@ func TestExportWorker_ProcessGitRepository(t *testing.T) { mockStageFn := NewMockWrapWithStageFn(t) // Verify clone and push options mockStageFn.On("Execute", mock.Anything, mockRepo, mock.MatchedBy(func(opts repository.StageOptions) bool { - return opts.Timeout == 10*time.Minute && !opts.PushOnWrites + return opts.Timeout == 10*time.Minute && opts.Mode == repository.StageModeCommitOnlyOnce }), mock.Anything).Return(func(ctx context.Context, repo repository.Repository, stageOpts repository.StageOptions, fn func(repository.Repository, bool) error) error { return fn(repo, true) }) diff --git a/pkg/registry/apis/provisioning/jobs/migrate/legacy.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy.go index f2a78b5181e..8b253bebd7f 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/legacy.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/legacy.go @@ -35,8 +35,18 @@ func NewLegacyMigrator( func (m *LegacyMigrator) Migrate(ctx context.Context, rw repository.ReaderWriter, options provisioning.MigrateJobOptions, progress jobs.JobProgressRecorder) error { namespace := rw.Config().Namespace + var stageMode repository.StageMode + if options.History { + // When History is true, we want to commit and push each file (previous PushOnWrites: true) + stageMode = repository.StageModeCommitAndPushOnEach + } else { + // When History is false, we want to commit only once (previous CommitOnlyOnce: true) + stageMode = repository.StageModeCommitOnlyOnce + } + stageOptions := repository.StageOptions{ - PushOnWrites: options.History, + Mode: stageMode, + CommitOnlyOnceMessage: options.Message, // TODO: make this configurable Timeout: 10 * time.Minute, } diff --git a/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage.go b/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage.go index d0b9739d72e..4f9f33b96ec 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage.go @@ -37,7 +37,9 @@ func (m *UnifiedStorageMigrator) Migrate(ctx context.Context, repo repository.Re exportJob := provisioning.Job{ Spec: provisioning.JobSpec{ - Push: &provisioning.ExportJobOptions{}, + Push: &provisioning.ExportJobOptions{ + Message: options.Message, + }, }, } if err := m.exportWorker.Process(ctx, repo, exportJob, progress); err != nil { diff --git a/pkg/registry/apis/provisioning/repository/git/repository_test.go b/pkg/registry/apis/provisioning/repository/git/repository_test.go index 0f13c2f9854..2c731b4c9f3 100644 --- a/pkg/registry/apis/provisioning/repository/git/repository_test.go +++ b/pkg/registry/apis/provisioning/repository/git/repository_test.go @@ -2256,7 +2256,7 @@ func TestGitRepository_Stage(t *testing.T) { t.Run("calls NewStagedGitRepository", func(t *testing.T) { ctx := context.Background() opts := repository.StageOptions{ - PushOnWrites: true, + Mode: repository.StageModeCommitAndPushOnEach, } // Since NewStagedGitRepository is not mocked and may panic, we expect this to fail diff --git a/pkg/registry/apis/provisioning/repository/git/staged.go b/pkg/registry/apis/provisioning/repository/git/staged.go index e11044230b2..e122bc8d33b 100644 --- a/pkg/registry/apis/provisioning/repository/git/staged.go +++ b/pkg/registry/apis/provisioning/repository/git/staged.go @@ -69,6 +69,25 @@ 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 +func (r *stagedGitRepository) handleCommitAndPush(ctx context.Context, message string) error { + switch r.opts.Mode { + case repository.StageModeCommitOnEach: + return r.commit(ctx, r.writer, message) + case repository.StageModeCommitAndPushOnEach: + if err := r.commit(ctx, r.writer, message); err != nil { + return err + } + 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) + } +} + func (r *stagedGitRepository) Create(ctx context.Context, path, ref string, data []byte, message string) error { if ref != "" && ref != r.gitConfig.Branch { return errors.New("ref is not supported for staged repository") @@ -78,15 +97,7 @@ func (r *stagedGitRepository) Create(ctx context.Context, path, ref string, data return err } - if err := r.commit(ctx, r.writer, message); err != nil { - return err - } - - if r.opts.PushOnWrites { - return r.Push(ctx) - } - - return nil + return r.handleCommitAndPush(ctx, message) } func (r *stagedGitRepository) blobExists(ctx context.Context, path string) (bool, error) { @@ -116,15 +127,7 @@ func (r *stagedGitRepository) Write(ctx context.Context, path, ref string, data } } - if err := r.commit(ctx, r.writer, message); err != nil { - return err - } - - if r.opts.PushOnWrites { - return r.Push(ctx) - } - - return nil + return r.handleCommitAndPush(ctx, message) } func (r *stagedGitRepository) Update(ctx context.Context, path, ref string, data []byte, message string) error { @@ -140,15 +143,7 @@ func (r *stagedGitRepository) Update(ctx context.Context, path, ref string, data return err } - if err := r.commit(ctx, r.writer, message); err != nil { - return err - } - - if r.opts.PushOnWrites { - return r.Push(ctx) - } - - return nil + return r.handleCommitAndPush(ctx, message) } func (r *stagedGitRepository) Delete(ctx context.Context, path, ref, message string) error { @@ -160,15 +155,7 @@ func (r *stagedGitRepository) Delete(ctx context.Context, path, ref, message str return err } - if err := r.commit(ctx, r.writer, message); err != nil { - return err - } - - if r.opts.PushOnWrites { - return r.Push(ctx) - } - - return nil + return r.handleCommitAndPush(ctx, message) } func (r *stagedGitRepository) Push(ctx context.Context) error { @@ -178,6 +165,16 @@ func (r *stagedGitRepository) Push(ctx context.Context) error { defer cancel() } + if r.opts.Mode == repository.StageModeCommitOnlyOnce { + message := r.opts.CommitOnlyOnceMessage + if message == "" { + message = "Staged changes" + } + if err := r.commit(ctx, r.writer, message); err != nil { + return err + } + } + return r.writer.Push(ctx) } diff --git a/pkg/registry/apis/provisioning/repository/git/staged_test.go b/pkg/registry/apis/provisioning/repository/git/staged_test.go index 9dc502cf8c2..73604e72fa3 100644 --- a/pkg/registry/apis/provisioning/repository/git/staged_test.go +++ b/pkg/registry/apis/provisioning/repository/git/staged_test.go @@ -3,6 +3,7 @@ package git import ( "context" "errors" + "strings" "testing" "time" @@ -32,7 +33,7 @@ func TestNewStagedGitRepository(t *testing.T) { mockClient.NewStagedWriterReturns(mockWriter, nil) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, wantError: nil, }, @@ -47,7 +48,7 @@ func TestNewStagedGitRepository(t *testing.T) { mockClient.NewStagedWriterReturns(mockWriter, nil) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, wantError: nil, }, @@ -62,8 +63,39 @@ func TestNewStagedGitRepository(t *testing.T) { mockClient.NewStagedWriterReturns(mockWriter, nil) }, opts: repository.StageOptions{ - PushOnWrites: false, - Timeout: time.Second * 5, + Mode: repository.StageModeCommitOnEach, + Timeout: time.Second * 5, + }, + wantError: nil, + }, + { + name: "succeeds with CommitOnlyOnce option", + setupMock: func(mockClient *mocks.FakeClient) { + mockClient.GetRefReturns(nanogit.Ref{ + Name: "refs/heads/main", + Hash: hash.Hash{1, 2, 3}, + }, nil) + mockWriter := &mocks.FakeStagedWriter{} + mockClient.NewStagedWriterReturns(mockWriter, nil) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnlyOnce, + CommitOnlyOnceMessage: "Custom commit message", + }, + wantError: nil, + }, + { + name: "succeeds with CommitAndPushOnEach option", + setupMock: func(mockClient *mocks.FakeClient) { + mockClient.GetRefReturns(nanogit.Ref{ + Name: "refs/heads/main", + Hash: hash.Hash{1, 2, 3}, + }, nil) + mockWriter := &mocks.FakeStagedWriter{} + mockClient.NewStagedWriterReturns(mockWriter, nil) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitAndPushOnEach, }, wantError: nil, }, @@ -73,7 +105,7 @@ func TestNewStagedGitRepository(t *testing.T) { mockClient.GetRefReturns(nanogit.Ref{}, errors.New("ref not found")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, wantError: errors.New("ref not found"), }, @@ -87,7 +119,7 @@ func TestNewStagedGitRepository(t *testing.T) { mockClient.NewStagedWriterReturns(nil, errors.New("failed to create writer")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, wantError: errors.New("build staged writer: failed to create writer"), }, @@ -121,8 +153,9 @@ func TestNewStagedGitRepository(t *testing.T) { // Compare opts fields individually since function pointers can't be compared directly actualOpts := stagedRepo.(*stagedGitRepository).opts - require.Equal(t, tt.opts.PushOnWrites, actualOpts.PushOnWrites) + require.Equal(t, tt.opts.Mode, actualOpts.Mode) require.Equal(t, tt.opts.Timeout, actualOpts.Timeout) + require.Equal(t, tt.opts.CommitOnlyOnceMessage, actualOpts.CommitOnlyOnceMessage) } }) } @@ -277,7 +310,7 @@ func TestStagedGitRepository_Create(t *testing.T) { mockWriter.CommitReturns(&nanogit.Commit{}, nil) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -294,7 +327,7 @@ func TestStagedGitRepository_Create(t *testing.T) { mockWriter.PushReturns(nil) }, opts: repository.StageOptions{ - PushOnWrites: true, + Mode: repository.StageModeCommitAndPushOnEach, }, path: "test.yaml", ref: "main", @@ -309,7 +342,7 @@ func TestStagedGitRepository_Create(t *testing.T) { // No setup needed as error occurs before writer calls }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "feature-branch", @@ -323,7 +356,7 @@ func TestStagedGitRepository_Create(t *testing.T) { mockWriter.CreateBlobReturns(hash.Hash{}, errors.New("create blob failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -338,7 +371,7 @@ func TestStagedGitRepository_Create(t *testing.T) { mockWriter.CommitReturns(&nanogit.Commit{}, errors.New("commit failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -354,7 +387,7 @@ func TestStagedGitRepository_Create(t *testing.T) { mockWriter.PushReturns(errors.New("push failed")) }, opts: repository.StageOptions{ - PushOnWrites: true, + Mode: repository.StageModeCommitAndPushOnEach, }, path: "test.yaml", ref: "", @@ -363,6 +396,21 @@ func TestStagedGitRepository_Create(t *testing.T) { wantError: errors.New("push failed"), expectPush: true, // Push is still called even though it fails }, + { + name: "succeeds with CommitOnlyOnce - no immediate commit", + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CreateBlobReturns(hash.Hash{1, 2, 3}, nil) + }, + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnlyOnce, + }, + path: "test.yaml", + ref: "", + data: []byte("content"), + message: "Create test file", + wantError: nil, + expectPush: false, + }, } for _, tt := range tests { @@ -386,6 +434,28 @@ func TestStagedGitRepository_Create(t *testing.T) { } else if tt.wantError == nil { require.Equal(t, 0, mockWriter.PushCallCount()) } + + // Verify commit behavior based on StageMode + switch tt.opts.Mode { + case repository.StageModeCommitOnlyOnce: + require.Equal(t, 0, mockWriter.CommitCallCount(), "No commits should be made when StageModeCommitOnlyOnce is used") + case repository.StageModeCommitOnEach, repository.StageModeCommitAndPushOnEach: + if tt.wantError == nil || strings.Contains(tt.wantError.Error(), "push failed") { + require.Equal(t, 1, mockWriter.CommitCallCount(), "One commit should be made when using commit modes (even if push fails)") + } else if tt.wantError != nil && strings.Contains(tt.wantError.Error(), "commit") { + // Commit failed, so it should have been attempted but failed + require.Equal(t, 1, mockWriter.CommitCallCount(), "Commit should be attempted even if it fails") + } else if tt.wantError != nil { + require.Equal(t, 0, mockWriter.CommitCallCount(), "No commits should be made when error occurs before commit") + } + default: + // Default behavior (backward compatibility) + if tt.wantError == nil || strings.Contains(tt.wantError.Error(), "push failed") { + require.Equal(t, 1, mockWriter.CommitCallCount(), "One commit should be made with default mode (even if push fails)") + } else if tt.wantError != nil { + require.Equal(t, 0, mockWriter.CommitCallCount(), "No commits should be made when error occurs before commit") + } + } }) } } @@ -411,7 +481,7 @@ func TestStagedGitRepository_Write(t *testing.T) { mockWriter.CommitReturns(&nanogit.Commit{}, nil) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -430,7 +500,7 @@ func TestStagedGitRepository_Write(t *testing.T) { mockWriter.PushReturns(nil) }, opts: repository.StageOptions{ - PushOnWrites: true, + Mode: repository.StageModeCommitAndPushOnEach, }, path: "test.yaml", ref: "main", @@ -446,7 +516,7 @@ func TestStagedGitRepository_Write(t *testing.T) { // No setup needed as error occurs before writer calls }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "feature-branch", @@ -460,7 +530,7 @@ func TestStagedGitRepository_Write(t *testing.T) { mockWriter.BlobExistsReturns(false, errors.New("blob exists check failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -475,7 +545,7 @@ func TestStagedGitRepository_Write(t *testing.T) { mockWriter.CreateBlobReturns(hash.Hash{}, errors.New("create failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -490,7 +560,7 @@ func TestStagedGitRepository_Write(t *testing.T) { mockWriter.UpdateBlobReturns(hash.Hash{}, errors.New("update failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -506,7 +576,7 @@ func TestStagedGitRepository_Write(t *testing.T) { mockWriter.CommitReturns(&nanogit.Commit{}, errors.New("commit failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -560,7 +630,7 @@ func TestStagedGitRepository_Update(t *testing.T) { mockWriter.CommitReturns(&nanogit.Commit{}, nil) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -577,7 +647,7 @@ func TestStagedGitRepository_Update(t *testing.T) { mockWriter.PushReturns(nil) }, opts: repository.StageOptions{ - PushOnWrites: true, + Mode: repository.StageModeCommitAndPushOnEach, }, path: "test.yaml", ref: "main", @@ -592,7 +662,7 @@ func TestStagedGitRepository_Update(t *testing.T) { // No setup needed as error occurs before writer calls }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "feature-branch", @@ -606,7 +676,7 @@ func TestStagedGitRepository_Update(t *testing.T) { // No setup needed as error occurs before writer calls }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "directory/", ref: "", @@ -620,7 +690,7 @@ func TestStagedGitRepository_Update(t *testing.T) { mockWriter.UpdateBlobReturns(hash.Hash{}, errors.New("update blob failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -635,7 +705,7 @@ func TestStagedGitRepository_Update(t *testing.T) { mockWriter.CommitReturns(&nanogit.Commit{}, errors.New("commit failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -688,7 +758,7 @@ func TestStagedGitRepository_Delete(t *testing.T) { mockWriter.CommitReturns(&nanogit.Commit{}, nil) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -704,7 +774,7 @@ func TestStagedGitRepository_Delete(t *testing.T) { mockWriter.PushReturns(nil) }, opts: repository.StageOptions{ - PushOnWrites: true, + Mode: repository.StageModeCommitAndPushOnEach, }, path: "testdir/", ref: "main", @@ -718,7 +788,7 @@ func TestStagedGitRepository_Delete(t *testing.T) { // No setup needed as error occurs before writer calls }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "feature-branch", @@ -731,7 +801,7 @@ func TestStagedGitRepository_Delete(t *testing.T) { mockWriter.DeleteBlobReturns(hash.Hash{}, errors.New("delete blob failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -745,7 +815,7 @@ func TestStagedGitRepository_Delete(t *testing.T) { mockWriter.CommitReturns(&nanogit.Commit{}, errors.New("commit failed")) }, opts: repository.StageOptions{ - PushOnWrites: false, + Mode: repository.StageModeCommitOnEach, }, path: "test.yaml", ref: "", @@ -781,42 +851,96 @@ func TestStagedGitRepository_Delete(t *testing.T) { func TestStagedGitRepository_Push(t *testing.T) { tests := []struct { - name string - setupMock func(*mocks.FakeStagedWriter) - wantError error - expectCalls int + name string + opts repository.StageOptions + setupMock func(*mocks.FakeStagedWriter) + wantError error + expectPushCalls int + expectCommitCalls int }{ { - name: "succeeds with empty ref", + name: "succeeds with normal push", + opts: repository.StageOptions{}, setupMock: func(mockWriter *mocks.FakeStagedWriter) { mockWriter.PushReturns(nil) }, - wantError: nil, - expectCalls: 1, - }, - { - name: "succeeds with matching ref", - setupMock: func(mockWriter *mocks.FakeStagedWriter) { - mockWriter.PushReturns(nil) - }, - wantError: nil, - expectCalls: 1, + wantError: nil, + expectPushCalls: 1, + expectCommitCalls: 0, }, { name: "succeeds with timeout", + opts: repository.StageOptions{ + Timeout: time.Second * 5, + }, setupMock: func(mockWriter *mocks.FakeStagedWriter) { mockWriter.PushReturns(nil) }, - wantError: nil, - expectCalls: 1, + wantError: nil, + expectPushCalls: 1, + expectCommitCalls: 0, + }, + { + name: "succeeds with CommitOnlyOnce and default message", + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnlyOnce, + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + mockWriter.PushReturns(nil) + }, + wantError: nil, + expectPushCalls: 1, + expectCommitCalls: 1, + }, + { + name: "succeeds with CommitOnlyOnce and custom message", + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnlyOnce, + CommitOnlyOnceMessage: "Custom commit message", + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + mockWriter.PushReturns(nil) + }, + wantError: nil, + expectPushCalls: 1, + expectCommitCalls: 1, + }, + { + name: "fails with commit error when CommitOnlyOnce", + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnlyOnce, + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CommitReturns(&nanogit.Commit{}, errors.New("commit failed")) + }, + wantError: errors.New("commit changes: commit failed"), + expectPushCalls: 0, + expectCommitCalls: 1, }, { name: "fails with push error", + opts: repository.StageOptions{}, setupMock: func(mockWriter *mocks.FakeStagedWriter) { mockWriter.PushReturns(errors.New("push failed")) }, - wantError: errors.New("push failed"), - expectCalls: 1, + wantError: errors.New("push failed"), + expectPushCalls: 1, + expectCommitCalls: 0, + }, + { + name: "fails with push error after successful commit", + opts: repository.StageOptions{ + Mode: repository.StageModeCommitOnlyOnce, + }, + setupMock: func(mockWriter *mocks.FakeStagedWriter) { + mockWriter.CommitReturns(&nanogit.Commit{}, nil) + mockWriter.PushReturns(errors.New("push failed")) + }, + wantError: errors.New("push failed"), + expectPushCalls: 1, + expectCommitCalls: 1, }, } @@ -825,7 +949,7 @@ func TestStagedGitRepository_Push(t *testing.T) { mockWriter := &mocks.FakeStagedWriter{} tt.setupMock(mockWriter) - stagedRepo := createTestStagedRepositoryWithWriter(mockWriter, repository.StageOptions{}) + stagedRepo := createTestStagedRepositoryWithWriter(mockWriter, tt.opts) err := stagedRepo.Push(context.Background()) @@ -835,7 +959,18 @@ func TestStagedGitRepository_Push(t *testing.T) { require.NoError(t, err) } - require.Equal(t, tt.expectCalls, mockWriter.PushCallCount()) + require.Equal(t, tt.expectPushCalls, mockWriter.PushCallCount()) + require.Equal(t, tt.expectCommitCalls, mockWriter.CommitCallCount()) + + // Verify commit message when CommitOnlyOnce is used + if tt.opts.Mode == repository.StageModeCommitOnlyOnce && tt.expectCommitCalls > 0 && tt.wantError == nil { + _, actualMessage, _, _ := mockWriter.CommitArgsForCall(0) + expectedMessage := tt.opts.CommitOnlyOnceMessage + if expectedMessage == "" { + expectedMessage = "Staged changes" + } + require.Equal(t, expectedMessage, actualMessage) + } }) } } diff --git a/pkg/registry/apis/provisioning/repository/github/repository_test.go b/pkg/registry/apis/provisioning/repository/github/repository_test.go index 2301156529e..960b80e42d0 100644 --- a/pkg/registry/apis/provisioning/repository/github/repository_test.go +++ b/pkg/registry/apis/provisioning/repository/github/repository_test.go @@ -985,8 +985,8 @@ func TestGitHubRepositoryDelegation(t *testing.T) { mockGitRepo := git.NewMockGitRepository(t) mockStagedRepo := repository.NewMockStagedRepository(t) opts := repository.StageOptions{ - PushOnWrites: true, - Timeout: 10 * time.Second, + Mode: repository.StageModeCommitOnEach, + Timeout: 10 * time.Second, } mockGitRepo.On("Stage", ctx, opts).Return(mockStagedRepo, nil) diff --git a/pkg/registry/apis/provisioning/repository/staged.go b/pkg/registry/apis/provisioning/repository/staged.go index 6eb0aed357a..6649b085d5c 100644 --- a/pkg/registry/apis/provisioning/repository/staged.go +++ b/pkg/registry/apis/provisioning/repository/staged.go @@ -10,11 +10,25 @@ import ( "github.com/grafana/nanogit" ) +// StageMode defines the staging and commit behavior +type StageMode int + +const ( + // StageModeCommitOnEach commits each file operation individually (default) + StageModeCommitOnEach StageMode = iota + // StageModeCommitOnlyOnce stages all changes and commits them all at once on push + StageModeCommitOnlyOnce + // StageModeCommitAndPushOnEach commits and pushes each file operation individually + StageModeCommitAndPushOnEach +) + type StageOptions struct { - // Push on every write - PushOnWrites bool + // Mode defines the staging and commit behavior + Mode StageMode // Maximum time allowed for clone operation in seconds (0 means no limit) Timeout time.Duration + // Commit message to use when Mode is StageModeCommitOnlyOnce + CommitOnlyOnceMessage string } //go:generate mockery --name StageableRepository --structname MockStageableRepository --inpackage --filename stageable_repository_mock.go --with-expecter 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 76fe0149441..4f1beaa5e67 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -2636,6 +2636,10 @@ "description": "The source folder (or empty) to export", "type": "string" }, + "message": { + "description": "Message to use when committing the changes in a single commit", + "type": "string" + }, "path": { "description": "Prefix in target file system", "type": "string" @@ -3156,6 +3160,10 @@ "history": { "description": "Preserve history (if possible)", "type": "boolean" + }, + "message": { + "description": "Message to use when committing the changes in a single commit", + "type": "string" } } }, diff --git a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts index 7122a2e268b..0ecc569f2cb 100644 --- a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts +++ b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts @@ -752,6 +752,8 @@ export type ObjectMeta = { export type MigrateJobOptions = { /** Preserve history (if possible) */ history?: boolean; + /** Message to use when committing the changes in a single commit */ + message?: string; }; export type PullRequestJobOptions = { /** The specific commit hash that triggered this notice */ @@ -772,6 +774,8 @@ export type ExportJobOptions = { branch?: string; /** The source folder (or empty) to export */ folder?: string; + /** Message to use when committing the changes in a single commit */ + message?: string; /** Prefix in target file system */ path?: string; };