From 9736898b465a111c395b3c9ba62b4e5d7da1b371 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 16 Dec 2025 18:25:36 +0300 Subject: [PATCH] checksum --- .../pkg/repository/git/git_repository_mock.go | 51 +++--------------- .../pkg/repository/git/repository.go | 34 +++++++----- .../pkg/repository/git/repository_test.go | 26 +++++----- .../provisioning/pkg/repository/git/staged.go | 42 +++++++++------ .../pkg/repository/git/staged_test.go | 9 ++-- .../github/github_repository_mock.go | 51 +++--------------- .../pkg/repository/github/repository_test.go | 6 +-- .../pkg/repository/local/local.go | 52 ++++++++++++------- .../pkg/repository/local/local_test.go | 6 +-- .../provisioning/pkg/repository/repository.go | 6 +-- .../pkg/repository/repository_mock.go | 51 +++--------------- .../pkg/repository/staged_repository_mock.go | 51 +++--------------- .../provisioning/jobs/delete/worker_test.go | 7 +-- .../apis/provisioning/resources/dualwriter.go | 13 +++-- .../apis/provisioning/resources/folders.go | 2 +- .../apis/provisioning/resources/resources.go | 2 +- 16 files changed, 145 insertions(+), 264 deletions(-) diff --git a/apps/provisioning/pkg/repository/git/git_repository_mock.go b/apps/provisioning/pkg/repository/git/git_repository_mock.go index 2fa800a4d7a..9deccb058bf 100644 --- a/apps/provisioning/pkg/repository/git/git_repository_mock.go +++ b/apps/provisioning/pkg/repository/git/git_repository_mock.go @@ -179,21 +179,8 @@ func (_c *MockGitRepository_Config_Call) RunAndReturn(run func() *v0alpha1.Repos } // Create provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockGitRepository) Create(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Create") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockGitRepository) Create(ctx context.Context, path string, ref string, data []byte, message string) (*repository.FileInfo, error) { + return nil, nil } // MockGitRepository_Create_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Create' @@ -945,21 +932,8 @@ func (_c *MockGitRepository_URL_Call) RunAndReturn(run func() string) *MockGitRe } // Update provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockGitRepository) Update(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Update") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockGitRepository) Update(ctx context.Context, path string, ref string, data []byte, message string) (*repository.FileInfo, error) { + return nil, nil } // MockGitRepository_Update_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Update' @@ -1042,21 +1016,8 @@ func (_c *MockGitRepository_Validate_Call) RunAndReturn(run func() field.ErrorLi } // Write provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockGitRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Write") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockGitRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) (*repository.FileInfo, error) { + return nil, nil } // MockGitRepository_Write_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Write' diff --git a/apps/provisioning/pkg/repository/git/repository.go b/apps/provisioning/pkg/repository/git/repository.go index 82d5ca6bd1d..e459a8b35cc 100644 --- a/apps/provisioning/pkg/repository/git/repository.go +++ b/apps/provisioning/pkg/repository/git/repository.go @@ -316,26 +316,27 @@ func (r *gitRepository) ReadTree(ctx context.Context, ref string) ([]repository. return entries, nil } -func (r *gitRepository) Create(ctx context.Context, path, ref string, data []byte, comment string) error { +func (r *gitRepository) Create(ctx context.Context, path, ref string, data []byte, comment string) (*repository.FileInfo, error) { if ref == "" { ref = r.gitConfig.Branch } ctx, _ = r.withGitContext(ctx, ref) branchRef, err := r.ensureBranchExists(ctx, ref) if err != nil { - return err + return nil, err } writer, err := r.client.NewStagedWriter(ctx, branchRef) if err != nil { - return fmt.Errorf("create staged writer: %w", err) + return nil, fmt.Errorf("create staged writer: %w", err) } if err := r.create(ctx, path, data, writer); err != nil { - return err + return nil, err } - return r.commitAndPush(ctx, writer, comment) + // ???? or can we hash directly? + return r.Read(ctx, path, ref) } func (r *gitRepository) create(ctx context.Context, path string, data []byte, writer nanogit.StagedWriter) error { @@ -361,7 +362,7 @@ func (r *gitRepository) create(ctx context.Context, path string, data []byte, wr return nil } -func (r *gitRepository) Update(ctx context.Context, path, ref string, data []byte, comment string) error { +func (r *gitRepository) Update(ctx context.Context, path, ref string, data []byte, comment string) (*repository.FileInfo, error) { if ref == "" { ref = r.gitConfig.Branch } @@ -369,24 +370,29 @@ func (r *gitRepository) Update(ctx context.Context, path, ref string, data []byt // Check if trying to update a directory if safepath.IsDir(path) { - return apierrors.NewBadRequest("cannot update a directory") + return nil, apierrors.NewBadRequest("cannot update a directory") } branchRef, err := r.ensureBranchExists(ctx, ref) if err != nil { - return err + return nil, err } // Create a staged writer writer, err := r.client.NewStagedWriter(ctx, branchRef) if err != nil { - return fmt.Errorf("create staged writer: %w", err) + return nil, fmt.Errorf("create staged writer: %w", err) } if err := r.update(ctx, path, data, writer); err != nil { - return err + return nil, err } - return r.commitAndPush(ctx, writer, comment) + if err := r.commitAndPush(ctx, writer, comment); err != nil { + return nil, err + } + + // ???? or can we hash directly? + return r.Read(ctx, path, ref) } func (r *gitRepository) update(ctx context.Context, path string, data []byte, writer nanogit.StagedWriter) error { @@ -407,7 +413,7 @@ func (r *gitRepository) update(ctx context.Context, path string, data []byte, wr return nil } -func (r *gitRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) error { +func (r *gitRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) (*repository.FileInfo, error) { if ref == "" { ref = r.gitConfig.Branch } @@ -415,12 +421,12 @@ func (r *gitRepository) Write(ctx context.Context, path string, ref string, data ctx, _ = r.withGitContext(ctx, ref) info, err := r.Read(ctx, path, ref) if err != nil && !(errors.Is(err, repository.ErrFileNotFound)) { - return fmt.Errorf("check if file exists before writing: %w", err) + return nil, fmt.Errorf("check if file exists before writing: %w", err) } if err == nil { // If the value already exists and is the same, we don't need to do anything if bytes.Equal(info.Data, data) { - return nil + return info, nil } return r.Update(ctx, path, ref, data, message) } diff --git a/apps/provisioning/pkg/repository/git/repository_test.go b/apps/provisioning/pkg/repository/git/repository_test.go index 4958ab372ab..9e3348880e3 100644 --- a/apps/provisioning/pkg/repository/git/repository_test.go +++ b/apps/provisioning/pkg/repository/git/repository_test.go @@ -939,7 +939,7 @@ func TestGitRepository_Create(t *testing.T) { }, } - err := gitRepo.Create(context.Background(), tt.path, tt.ref, tt.data, tt.comment) + _, err := gitRepo.Create(context.Background(), tt.path, tt.ref, tt.data, tt.comment) if tt.wantError { require.Error(t, err) @@ -1042,7 +1042,7 @@ func TestGitRepository_Update(t *testing.T) { }, } - err := gitRepo.Update(context.Background(), tt.path, tt.ref, tt.data, tt.comment) + _, err := gitRepo.Update(context.Background(), tt.path, tt.ref, tt.data, tt.comment) if tt.wantError { require.Error(t, err) @@ -1461,7 +1461,7 @@ func TestGitRepository_Write(t *testing.T) { }, } - err := gitRepo.Write(context.Background(), tt.path, tt.ref, tt.data, tt.message) + _, err := gitRepo.Write(context.Background(), tt.path, tt.ref, tt.data, tt.message) if tt.wantError { require.Error(t, err) @@ -2273,7 +2273,7 @@ func TestGitRepository_EdgeCases(t *testing.T) { }, } - err := gitRepo.Create(context.Background(), "newdir/", "main", []byte("data"), "comment") + _, err := gitRepo.Create(context.Background(), "newdir/", "main", []byte("data"), "comment") // This should fail because we're providing data for a directory require.Error(t, err) @@ -2288,7 +2288,7 @@ func TestGitRepository_EdgeCases(t *testing.T) { }, } - err := gitRepo.Update(context.Background(), "directory/", "main", []byte("data"), "comment") + _, err := gitRepo.Update(context.Background(), "directory/", "main", []byte("data"), "comment") require.Error(t, err) require.Contains(t, err.Error(), "cannot update a directory") @@ -2311,7 +2311,7 @@ func TestGitRepository_EdgeCases(t *testing.T) { }, } - err := gitRepo.Write(context.Background(), "test.yaml", "main", []byte("data"), "message") + _, err := gitRepo.Write(context.Background(), "test.yaml", "main", []byte("data"), "message") require.Error(t, err) require.Contains(t, err.Error(), "check if file exists before writing") @@ -3008,7 +3008,7 @@ func TestGitRepository_Create_ErrorConditions(t *testing.T) { }, } - err := gitRepo.Create(context.Background(), "test.yaml", "main", []byte("content"), "comment") + _, err := gitRepo.Create(context.Background(), "test.yaml", "main", []byte("content"), "comment") if tt.wantError { require.Error(t, err) @@ -3073,7 +3073,7 @@ func TestGitRepository_Update_ErrorConditions(t *testing.T) { }, } - err := gitRepo.Update(context.Background(), "test.yaml", "main", []byte("content"), "comment") + _, err := gitRepo.Update(context.Background(), "test.yaml", "main", []byte("content"), "comment") if tt.wantError { require.Error(t, err) @@ -3266,9 +3266,9 @@ func TestGitRepository_EmptyRefHandling(t *testing.T) { var err error switch tt.method { case "Create": - err = gitRepo.Create(context.Background(), "test.yaml", "", []byte("content"), "comment") + _, err = gitRepo.Create(context.Background(), "test.yaml", "", []byte("content"), "comment") case "Update": - err = gitRepo.Update(context.Background(), "test.yaml", "", []byte("content"), "comment") + _, err = gitRepo.Update(context.Background(), "test.yaml", "", []byte("content"), "comment") case "Delete": err = gitRepo.Delete(context.Background(), "test.yaml", "", "comment") } @@ -3428,7 +3428,7 @@ func TestGitRepository_Update_EnsureBranchExistsError(t *testing.T) { }, } - err := gitRepo.Update(context.Background(), "test.yaml", "feature", []byte("content"), "comment") + _, err := gitRepo.Update(context.Background(), "test.yaml", "feature", []byte("content"), "comment") require.Error(t, err) require.Contains(t, err.Error(), "branch error") @@ -3451,7 +3451,7 @@ func TestGitRepository_Create_EnsureBranchExistsError(t *testing.T) { }, } - err := gitRepo.Create(context.Background(), "test.yaml", "feature", []byte("content"), "comment") + _, err := gitRepo.Create(context.Background(), "test.yaml", "feature", []byte("content"), "comment") require.Error(t, err) require.Contains(t, err.Error(), "branch error") @@ -3538,7 +3538,7 @@ func TestGitRepository_Write_DefaultRef(t *testing.T) { } // Test Write with empty ref to trigger default branch usage - err := gitRepo.Write(context.Background(), "test.yaml", "", []byte("content"), "message") + _, err := gitRepo.Write(context.Background(), "test.yaml", "", []byte("content"), "message") require.NoError(t, err) } diff --git a/apps/provisioning/pkg/repository/git/staged.go b/apps/provisioning/pkg/repository/git/staged.go index 15d8ef0bb93..201c3350c96 100644 --- a/apps/provisioning/pkg/repository/git/staged.go +++ b/apps/provisioning/pkg/repository/git/staged.go @@ -121,16 +121,20 @@ func (r *stagedGitRepository) handleCommitAndPush(ctx context.Context, message s } } -func (r *stagedGitRepository) Create(ctx context.Context, path, ref string, data []byte, message string) error { +func (r *stagedGitRepository) Create(ctx context.Context, path, ref string, data []byte, message string) (*repository.FileInfo, error) { if !r.isRefSupported(ref) { - return errors.New("ref is not supported for staged repository") + return nil, errors.New("ref is not supported for staged repository") } if err := r.create(ctx, path, data, r.writer); err != nil { - return err + return nil, err } - return r.handleCommitAndPush(ctx, message) + if err := r.handleCommitAndPush(ctx, message); err != nil { + return nil, err + } + + return r.Read(ctx, path, ref) // ??? or } func (r *stagedGitRepository) blobExists(ctx context.Context, path string) (bool, error) { @@ -140,43 +144,51 @@ func (r *stagedGitRepository) blobExists(ctx context.Context, path string) (bool return r.writer.BlobExists(ctx, path) } -func (r *stagedGitRepository) Write(ctx context.Context, path, ref string, data []byte, message string) error { +func (r *stagedGitRepository) Write(ctx context.Context, path, ref string, data []byte, message string) (*repository.FileInfo, error) { if !r.isRefSupported(ref) { - return errors.New("ref is not supported for staged repository") + return nil, errors.New("ref is not supported for staged repository") } exists, err := r.blobExists(ctx, path) if err != nil { - return fmt.Errorf("check if file exists: %w", err) + return nil, fmt.Errorf("check if file exists: %w", err) } if exists { if err := r.update(ctx, path, data, r.writer); err != nil { - return err + return nil, err } } else { if err := r.create(ctx, path, data, r.writer); err != nil { - return err + return nil, err } } - return r.handleCommitAndPush(ctx, message) + if err := r.handleCommitAndPush(ctx, message); err != nil { + return nil, err + } + + return r.Read(ctx, path, ref) // ??? or } -func (r *stagedGitRepository) Update(ctx context.Context, path, ref string, data []byte, message string) error { +func (r *stagedGitRepository) Update(ctx context.Context, path, ref string, data []byte, message string) (*repository.FileInfo, error) { if !r.isRefSupported(ref) { - return errors.New("ref is not supported for staged repository") + return nil, errors.New("ref is not supported for staged repository") } if safepath.IsDir(path) { - return errors.New("cannot update a directory in a staged repository") + return nil, errors.New("cannot update a directory in a staged repository") } if err := r.update(ctx, path, data, r.writer); err != nil { - return err + return nil, err } - return r.handleCommitAndPush(ctx, message) + if err := r.handleCommitAndPush(ctx, message); err != nil { + return nil, err + } + + return r.Read(ctx, path, ref) // ??? or } func (r *stagedGitRepository) Delete(ctx context.Context, path, ref, message string) error { diff --git a/apps/provisioning/pkg/repository/git/staged_test.go b/apps/provisioning/pkg/repository/git/staged_test.go index ff6f8beddbd..5353d698523 100644 --- a/apps/provisioning/pkg/repository/git/staged_test.go +++ b/apps/provisioning/pkg/repository/git/staged_test.go @@ -8,12 +8,13 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/apps/provisioning/pkg/repository" "github.com/grafana/nanogit" "github.com/grafana/nanogit/mocks" "github.com/grafana/nanogit/protocol/hash" - "github.com/stretchr/testify/require" ) func TestNewStagedGitRepository(t *testing.T) { @@ -515,7 +516,7 @@ func TestStagedGitRepository_Create(t *testing.T) { stagedRepo := createTestStagedRepositoryWithWriter(mockWriter, tt.opts) - err := stagedRepo.Create(context.Background(), tt.path, tt.ref, tt.data, tt.message) + _, err := stagedRepo.Create(context.Background(), tt.path, tt.ref, tt.data, tt.message) if tt.wantError != nil { require.EqualError(t, err, tt.wantError.Error()) @@ -688,7 +689,7 @@ func TestStagedGitRepository_Write(t *testing.T) { stagedRepo := createTestStagedRepositoryWithWriter(mockWriter, tt.opts) - err := stagedRepo.Write(context.Background(), tt.path, tt.ref, tt.data, tt.message) + _, err := stagedRepo.Write(context.Background(), tt.path, tt.ref, tt.data, tt.message) if tt.wantError != nil { require.EqualError(t, err, tt.wantError.Error()) @@ -817,7 +818,7 @@ func TestStagedGitRepository_Update(t *testing.T) { stagedRepo := createTestStagedRepositoryWithWriter(mockWriter, tt.opts) - err := stagedRepo.Update(context.Background(), tt.path, tt.ref, tt.data, tt.message) + _, err := stagedRepo.Update(context.Background(), tt.path, tt.ref, tt.data, tt.message) if tt.wantError != nil { require.EqualError(t, err, tt.wantError.Error()) diff --git a/apps/provisioning/pkg/repository/github/github_repository_mock.go b/apps/provisioning/pkg/repository/github/github_repository_mock.go index ca4a2c07361..1cecc7eae63 100644 --- a/apps/provisioning/pkg/repository/github/github_repository_mock.go +++ b/apps/provisioning/pkg/repository/github/github_repository_mock.go @@ -181,21 +181,8 @@ func (_c *MockGithubRepository_Config_Call) RunAndReturn(run func() *v0alpha1.Re } // Create provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockGithubRepository) Create(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Create") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockGithubRepository) Create(ctx context.Context, path string, ref string, data []byte, message string) (*repository.FileInfo, error) { + return nil, nil } // MockGithubRepository_Create_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Create' @@ -948,21 +935,8 @@ func (_c *MockGithubRepository_Test_Call) RunAndReturn(run func(context.Context) } // Update provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockGithubRepository) Update(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Update") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockGithubRepository) Update(ctx context.Context, path string, ref string, data []byte, message string) (*repository.FileInfo, error) { + return nil, nil } // MockGithubRepository_Update_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Update' @@ -1045,21 +1019,8 @@ func (_c *MockGithubRepository_Validate_Call) RunAndReturn(run func() field.Erro } // Write provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockGithubRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Write") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockGithubRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) (*repository.FileInfo, error) { + return nil, nil } // MockGithubRepository_Write_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Write' diff --git a/apps/provisioning/pkg/repository/github/repository_test.go b/apps/provisioning/pkg/repository/github/repository_test.go index 23843f1a7fa..9f776f16862 100644 --- a/apps/provisioning/pkg/repository/github/repository_test.go +++ b/apps/provisioning/pkg/repository/github/repository_test.go @@ -987,7 +987,7 @@ func TestGitHubRepositoryDelegation(t *testing.T) { GitRepository: mockGitRepo, } - err := repo.Create(ctx, "new-file.yaml", "main", data, "Create new file") + _, err := repo.Create(ctx, "new-file.yaml", "main", data, "Create new file") require.NoError(t, err) mockGitRepo.AssertExpectations(t) }) @@ -1002,7 +1002,7 @@ func TestGitHubRepositoryDelegation(t *testing.T) { GitRepository: mockGitRepo, } - err := repo.Update(ctx, "existing-file.yaml", "main", data, "Update file") + _, err := repo.Update(ctx, "existing-file.yaml", "main", data, "Update file") require.NoError(t, err) mockGitRepo.AssertExpectations(t) }) @@ -1017,7 +1017,7 @@ func TestGitHubRepositoryDelegation(t *testing.T) { GitRepository: mockGitRepo, } - err := repo.Write(ctx, "file.yaml", "main", data, "Write file") + _, err := repo.Write(ctx, "file.yaml", "main", data, "Write file") require.NoError(t, err) mockGitRepo.AssertExpectations(t) }) diff --git a/apps/provisioning/pkg/repository/local/local.go b/apps/provisioning/pkg/repository/local/local.go index 4e2482326fd..e9d8e20edbc 100644 --- a/apps/provisioning/pkg/repository/local/local.go +++ b/apps/provisioning/pkg/repository/local/local.go @@ -288,75 +288,87 @@ func (r *localRepository) calculateFileHash(path string) (string, int64, error) return hex.EncodeToString(hasher.Sum(nil)), size, nil } -func (r *localRepository) Create(ctx context.Context, filepath string, ref string, data []byte, comment string) error { +func (r *localRepository) Create(ctx context.Context, filepath string, ref string, data []byte, comment string) (*repository.FileInfo, error) { if err := r.validateRequest(ref); err != nil { - return err + return nil, err } fpath := safepath.Join(r.path, filepath) _, err := os.Stat(fpath) if !errors.Is(err, os.ErrNotExist) { if err != nil { - return apierrors.NewInternalError(fmt.Errorf("failed to check if file exists: %w", err)) + return nil, apierrors.NewInternalError(fmt.Errorf("failed to check if file exists: %w", err)) } - return apierrors.NewAlreadyExists(schema.GroupResource{}, filepath) + return nil, apierrors.NewAlreadyExists(schema.GroupResource{}, filepath) } if safepath.IsDir(fpath) { if data != nil { - return apierrors.NewBadRequest("data cannot be provided for a directory") + return nil, apierrors.NewBadRequest("data cannot be provided for a directory") } if err := os.MkdirAll(fpath, 0700); err != nil { - return apierrors.NewInternalError(fmt.Errorf("failed to create path: %w", err)) + return nil, apierrors.NewInternalError(fmt.Errorf("failed to create path: %w", err)) } - return nil + return nil, nil } if err := os.MkdirAll(path.Dir(fpath), 0700); err != nil { - return apierrors.NewInternalError(fmt.Errorf("failed to create path: %w", err)) + return nil, apierrors.NewInternalError(fmt.Errorf("failed to create path: %w", err)) } - return os.WriteFile(fpath, data, 0600) + if err := os.WriteFile(fpath, data, 0600); err != nil { + return nil, err + } + + return r.Read(ctx, filepath, ref) } -func (r *localRepository) Update(ctx context.Context, path string, ref string, data []byte, comment string) error { +func (r *localRepository) Update(ctx context.Context, path string, ref string, data []byte, comment string) (*repository.FileInfo, error) { if err := r.validateRequest(ref); err != nil { - return err + return nil, err } path = safepath.Join(r.path, path) if safepath.IsDir(path) { - return apierrors.NewBadRequest("cannot update a directory") + return nil, apierrors.NewBadRequest("cannot update a directory") } f, err := os.Stat(path) if err != nil && errors.Is(err, os.ErrNotExist) { - return repository.ErrFileNotFound + return nil, repository.ErrFileNotFound } if f.IsDir() { - return apierrors.NewBadRequest("path exists but it is a directory") + return nil, apierrors.NewBadRequest("path exists but it is a directory") } - return os.WriteFile(path, data, 0600) + if err := os.WriteFile(path, data, 0600); err != nil { + return nil, err + } + + return r.Read(ctx, path, ref) } -func (r *localRepository) Write(ctx context.Context, fpath, ref string, data []byte, comment string) error { +func (r *localRepository) Write(ctx context.Context, fpath, ref string, data []byte, comment string) (*repository.FileInfo, error) { if err := r.validateRequest(ref); err != nil { - return err + return nil, err } fpath = safepath.Join(r.path, fpath) if safepath.IsDir(fpath) { - return os.MkdirAll(fpath, 0700) + return nil, os.MkdirAll(fpath, 0700) } if err := os.MkdirAll(path.Dir(fpath), 0700); err != nil { - return apierrors.NewInternalError(fmt.Errorf("failed to create path: %w", err)) + return nil, apierrors.NewInternalError(fmt.Errorf("failed to create path: %w", err)) } - return os.WriteFile(fpath, data, 0600) + if err := os.WriteFile(fpath, data, 0600); err != nil { + return nil, err + } + + return r.Read(ctx, fpath, ref) } func (r *localRepository) Delete(ctx context.Context, path string, ref string, comment string) error { diff --git a/apps/provisioning/pkg/repository/local/local_test.go b/apps/provisioning/pkg/repository/local/local_test.go index 9e4507f3493..1a4b4866587 100644 --- a/apps/provisioning/pkg/repository/local/local_test.go +++ b/apps/provisioning/pkg/repository/local/local_test.go @@ -776,7 +776,7 @@ func TestLocalRepository_Update(t *testing.T) { _, repo := tc.setup(t) // Execute the update operation - err := repo.Update(context.Background(), tc.path, tc.ref, tc.data, tc.comment) + _, err := repo.Update(context.Background(), tc.path, tc.ref, tc.data, tc.comment) // Verify results if tc.expectedErr != nil { @@ -939,7 +939,7 @@ func TestLocalRepository_Write(t *testing.T) { _, repo := tc.setup(t) // Execute the write operation - err := repo.Write(context.Background(), tc.path, tc.ref, tc.data, tc.comment) + _, err := repo.Write(context.Background(), tc.path, tc.ref, tc.data, tc.comment) // Verify results if tc.expectedErr != nil { @@ -1139,7 +1139,7 @@ func TestLocalRepository_Create(t *testing.T) { _, repo := tc.setup(t) // Execute the create operation - err := repo.Create(context.Background(), tc.path, tc.ref, tc.data, tc.comment) + _, err := repo.Create(context.Background(), tc.path, tc.ref, tc.data, tc.comment) // Verify results if tc.expectedErr != nil { diff --git a/apps/provisioning/pkg/repository/repository.go b/apps/provisioning/pkg/repository/repository.go index 6ac356fc6c5..83ba93f1e8f 100644 --- a/apps/provisioning/pkg/repository/repository.go +++ b/apps/provisioning/pkg/repository/repository.go @@ -102,15 +102,15 @@ type Writer interface { // Write a file to the repository. // The data has already been validated and is ready for save - Create(ctx context.Context, path, ref string, data []byte, message string) error + Create(ctx context.Context, path, ref string, data []byte, message string) (*FileInfo, error) // Update a file in the remote repository // The data has already been validated and is ready for save - Update(ctx context.Context, path, ref string, data []byte, message string) error + Update(ctx context.Context, path, ref string, data []byte, message string) (*FileInfo, error) // Write a file to the repository. // Functionally the same as Read then Create or Update, but more efficient depending on the backend - Write(ctx context.Context, path, ref string, data []byte, message string) error + Write(ctx context.Context, path, ref string, data []byte, message string) (*FileInfo, error) // Delete a file in the remote repository Delete(ctx context.Context, path, ref, message string) error diff --git a/apps/provisioning/pkg/repository/repository_mock.go b/apps/provisioning/pkg/repository/repository_mock.go index e78a7467884..26ca3357719 100644 --- a/apps/provisioning/pkg/repository/repository_mock.go +++ b/apps/provisioning/pkg/repository/repository_mock.go @@ -39,21 +39,8 @@ func (_m *MockRepository) Config() *v0alpha1.Repository { } // Create provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockRepository) Create(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Create") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockRepository) Create(ctx context.Context, path string, ref string, data []byte, message string) (*FileInfo, error) { + return nil, nil } // Delete provides a mock function with given fields: ctx, path, ref, message @@ -195,21 +182,8 @@ func (_m *MockRepository) Test(ctx context.Context) (*v0alpha1.TestResults, erro } // Update provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockRepository) Update(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Update") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockRepository) Update(ctx context.Context, path string, ref string, data []byte, message string) (*FileInfo, error) { + return nil, nil } // Validate provides a mock function with no fields @@ -263,21 +237,8 @@ func (_m *MockRepository) Webhook(ctx context.Context, req *http.Request) (*v0al } // Write provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Write") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) (*FileInfo, error) { + return nil, nil } // Move provides a mock function with given fields: ctx, oldPath, newPath, ref, message diff --git a/apps/provisioning/pkg/repository/staged_repository_mock.go b/apps/provisioning/pkg/repository/staged_repository_mock.go index bd5b8785326..7f726c3c6ef 100644 --- a/apps/provisioning/pkg/repository/staged_repository_mock.go +++ b/apps/provisioning/pkg/repository/staged_repository_mock.go @@ -72,21 +72,8 @@ func (_c *MockStagedRepository_Config_Call) RunAndReturn(run func() *v0alpha1.Re } // Create provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockStagedRepository) Create(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Create") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockStagedRepository) Create(ctx context.Context, path string, ref string, data []byte, message string) (*FileInfo, error) { + return nil, nil } // MockStagedRepository_Create_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Create' @@ -490,21 +477,8 @@ func (_c *MockStagedRepository_Test_Call) RunAndReturn(run func(context.Context) } // Update provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockStagedRepository) Update(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Update") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockStagedRepository) Update(ctx context.Context, path string, ref string, data []byte, message string) (*FileInfo, error) { + return nil, nil } // MockStagedRepository_Update_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Update' @@ -587,21 +561,8 @@ func (_c *MockStagedRepository_Validate_Call) RunAndReturn(run func() field.Erro } // Write provides a mock function with given fields: ctx, path, ref, data, message -func (_m *MockStagedRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) error { - ret := _m.Called(ctx, path, ref, data, message) - - if len(ret) == 0 { - panic("no return value specified for Write") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, []byte, string) error); ok { - r0 = rf(ctx, path, ref, data, message) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *MockStagedRepository) Write(ctx context.Context, path string, ref string, data []byte, message string) (*FileInfo, error) { + return nil, nil } // MockStagedRepository_Write_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Write' diff --git a/pkg/registry/apis/provisioning/jobs/delete/worker_test.go b/pkg/registry/apis/provisioning/jobs/delete/worker_test.go index 07cc855f194..0b575ea639e 100644 --- a/pkg/registry/apis/provisioning/jobs/delete/worker_test.go +++ b/pkg/registry/apis/provisioning/jobs/delete/worker_test.go @@ -10,13 +10,14 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + v0alpha1 "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/apps/provisioning/pkg/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" - "github.com/prometheus/client_golang/prometheus" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" ) type mockReaderWriter struct { diff --git a/pkg/registry/apis/provisioning/resources/dualwriter.go b/pkg/registry/apis/provisioning/resources/dualwriter.go index 8ffcee696e8..ddc597db475 100644 --- a/pkg/registry/apis/provisioning/resources/dualwriter.go +++ b/pkg/registry/apis/provisioning/resources/dualwriter.go @@ -157,7 +157,7 @@ func (r *DualReadWriter) CreateFolder(ctx context.Context, opts DualWriteOptions } // Now actually create the folder - if err := r.repo.Create(ctx, opts.Path, opts.Ref, nil, opts.Message); err != nil { + if _, err := r.repo.Create(ctx, opts.Path, opts.Ref, nil, opts.Message); err != nil { return nil, fmt.Errorf("failed to create folder: %w", err) } @@ -263,10 +263,11 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts D } // Create or update + var after *repository.FileInfo if create { - err = r.repo.Create(ctx, opts.Path, opts.Ref, data, opts.Message) + after, err = r.repo.Create(ctx, opts.Path, opts.Ref, data, opts.Message) } else { - err = r.repo.Update(ctx, opts.Path, opts.Ref, data, opts.Message) + after, err = r.repo.Update(ctx, opts.Path, opts.Ref, data, opts.Message) } if err != nil { return nil, err // raw error is useful @@ -277,6 +278,10 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts D // FIXME: to make sure if behaves in the same way as in sync, we should // we should refactor the code to use the same function. if r.shouldUpdateGrafanaDB(opts, parsed) { + parsed.Meta.SetSourceProperties(utils.SourceProperties{ + Path: after.Path, + Checksum: after.Hash, + }) if _, err := r.folders.EnsureFolderPathExist(ctx, opts.Path); err != nil { return nil, fmt.Errorf("ensure folder path exists: %w", err) } @@ -445,7 +450,7 @@ func (r *DualReadWriter) moveFile(ctx context.Context, opts DualWriteOptions) (* if err = r.repo.Delete(ctx, opts.OriginalPath, opts.Ref, opts.Message); err != nil { return nil, fmt.Errorf("delete original file in repository: %w", err) } - if err = r.repo.Create(ctx, opts.Path, opts.Ref, data, opts.Message); err != nil { + if _, err = r.repo.Create(ctx, opts.Path, opts.Ref, data, opts.Message); err != nil { return nil, fmt.Errorf("create moved file with new content in repository: %w", err) } } else { diff --git a/pkg/registry/apis/provisioning/resources/folders.go b/pkg/registry/apis/provisioning/resources/folders.go index b74ab67976a..145a6c7844c 100644 --- a/pkg/registry/apis/provisioning/resources/folders.go +++ b/pkg/registry/apis/provisioning/resources/folders.go @@ -201,7 +201,7 @@ func (fm *FolderManager) EnsureFolderTreeExists(ctx context.Context, ref, path s } msg := fmt.Sprintf("Add folder %s", p) - if err := fm.repo.Create(ctx, p, ref, nil, msg); err != nil { + if _, err := fm.repo.Create(ctx, p, ref, nil, msg); err != nil { return fn(folder, true, fmt.Errorf("write folder in repo: %w", err)) } // Add it to the existing tree diff --git a/pkg/registry/apis/provisioning/resources/resources.go b/pkg/registry/apis/provisioning/resources/resources.go index 4954e6b51b6..36ad3439d02 100644 --- a/pkg/registry/apis/provisioning/resources/resources.go +++ b/pkg/registry/apis/provisioning/resources/resources.go @@ -205,7 +205,7 @@ func (r *ResourcesManager) WriteResourceFileFromObject(ctx context.Context, obj return "", err } - err = r.repo.Write(ctx, fileName, options.Ref, body, commitMessage) + _, err = r.repo.Write(ctx, fileName, options.Ref, body, commitMessage) if err != nil { return "", fmt.Errorf("failed to write file: %s, %w", fileName, err) }