From d6dbc0a4213eff4a16140f164cbece99d6fa8cf7 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Mon, 21 Apr 2025 21:10:37 +0300 Subject: [PATCH] Provisioning: Migrate use history only with github (#104219) --- .../provisioning/jobs/export/resources.go | 7 +- .../jobs/export/resources_test.go | 23 ++--- .../migrate/{folders.go => legacy_folders.go} | 0 ...folders_test.go => legacy_folders_test.go} | 0 .../{resources.go => legacy_resources.go} | 19 ++++- ...urces_test.go => legacy_resources_test.go} | 83 +++++++++++++++---- .../apis/provisioning/jobs/migrate/worker.go | 10 +++ .../provisioning/jobs/migrate/worker_test.go | 44 ++++++++++ .../apis/provisioning/repository/local.go | 2 +- .../apis/provisioning/resources/repository.go | 7 +- .../resources/repository_resources_mock.go | 22 ++--- .../apis/provisioning/resources/resources.go | 28 +++---- .../provisioning/Wizard/SynchronizeStep.tsx | 7 +- 13 files changed, 191 insertions(+), 61 deletions(-) rename pkg/registry/apis/provisioning/jobs/migrate/{folders.go => legacy_folders.go} (100%) rename pkg/registry/apis/provisioning/jobs/migrate/{folders_test.go => legacy_folders_test.go} (100%) rename pkg/registry/apis/provisioning/jobs/migrate/{resources.go => legacy_resources.go} (91%) rename pkg/registry/apis/provisioning/jobs/migrate/{resources_test.go => legacy_resources_test.go} (90%) diff --git a/pkg/registry/apis/provisioning/jobs/export/resources.go b/pkg/registry/apis/provisioning/jobs/export/resources.go index 7d57a2b1039..a401615c427 100644 --- a/pkg/registry/apis/provisioning/jobs/export/resources.go +++ b/pkg/registry/apis/provisioning/jobs/export/resources.go @@ -5,12 +5,13 @@ import ( "errors" "fmt" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/dynamic" + provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/dynamic" ) func ExportResources(ctx context.Context, options provisioning.ExportJobOptions, clients resources.ResourceClients, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder) error { @@ -37,7 +38,7 @@ func ExportResources(ctx context.Context, options provisioning.ExportJobOptions, func exportResource(ctx context.Context, options provisioning.ExportJobOptions, client dynamic.ResourceInterface, repositoryResources resources.RepositoryResources, progress jobs.JobProgressRecorder) error { return resources.ForEach(ctx, client, func(item *unstructured.Unstructured) error { - fileName, err := repositoryResources.CreateResourceFileFromObject(ctx, item, resources.WriteOptions{ + fileName, err := repositoryResources.WriteResourceFileFromObject(ctx, item, resources.WriteOptions{ Path: options.Path, Ref: options.Branch, }) diff --git a/pkg/registry/apis/provisioning/jobs/export/resources_test.go b/pkg/registry/apis/provisioning/jobs/export/resources_test.go index a8346ad1429..d1a75443316 100644 --- a/pkg/registry/apis/provisioning/jobs/export/resources_test.go +++ b/pkg/registry/apis/provisioning/jobs/export/resources_test.go @@ -5,10 +5,6 @@ import ( "fmt" "testing" - v0alpha1 "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" mock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -17,6 +13,11 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" dynamicfake "k8s.io/client-go/dynamic/fake" k8testing "k8s.io/client-go/testing" + + provisioningV0 "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" ) func TestExportResources(t *testing.T) { @@ -78,11 +79,11 @@ func TestExportResources(t *testing.T) { Ref: "feature/branch", } - repoResources.On("CreateResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { + repoResources.On("WriteResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { return obj.GetName() == "dashboard-1" }), options).Return("dashboard-1.json", nil) - repoResources.On("CreateResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { + repoResources.On("WriteResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { return obj.GetName() == "dashboard-2" }), options).Return("dashboard-2.json", nil) }, @@ -165,11 +166,11 @@ func TestExportResources(t *testing.T) { Ref: "feature/branch", } - repoResources.On("CreateResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { + repoResources.On("WriteResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { return obj.GetName() == "dashboard-1" }), options).Return("", fmt.Errorf("failed to export dashboard")) - repoResources.On("CreateResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { + repoResources.On("WriteResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { return obj.GetName() == "dashboard-2" }), options).Return("dashboard-2.json", nil) }, @@ -211,7 +212,7 @@ func TestExportResources(t *testing.T) { Ref: "feature/branch", } - repoResources.On("CreateResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { + repoResources.On("WriteResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { return obj.GetName() == "dashboard-1" }), options).Return("", fmt.Errorf("failed to export dashboard")) }, @@ -254,7 +255,7 @@ func TestExportResources(t *testing.T) { } // Return true to indicate the file already exists, and provide the updated path - repoResources.On("CreateResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { + repoResources.On("WriteResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { return obj.GetName() == "existing-dashboard" }), options).Return("", resources.ErrAlreadyInRepository) }, @@ -291,7 +292,7 @@ func TestExportResources(t *testing.T) { repoResources := resources.NewMockRepositoryResources(t) tt.setupResources(repoResources, resourceClients, fakeDynamicClient, listGVK) - options := v0alpha1.ExportJobOptions{ + options := provisioningV0.ExportJobOptions{ Path: "grafana", Branch: "feature/branch", } diff --git a/pkg/registry/apis/provisioning/jobs/migrate/folders.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_folders.go similarity index 100% rename from pkg/registry/apis/provisioning/jobs/migrate/folders.go rename to pkg/registry/apis/provisioning/jobs/migrate/legacy_folders.go diff --git a/pkg/registry/apis/provisioning/jobs/migrate/folders_test.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_folders_test.go similarity index 100% rename from pkg/registry/apis/provisioning/jobs/migrate/folders_test.go rename to pkg/registry/apis/provisioning/jobs/migrate/legacy_folders_test.go diff --git a/pkg/registry/apis/provisioning/jobs/migrate/resources.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go similarity index 91% rename from pkg/registry/apis/provisioning/jobs/migrate/resources.go rename to pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go index 43cb3375f77..0a34cecb2f7 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/resources.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go @@ -108,6 +108,7 @@ type legacyResourceResourceMigrator struct { options provisioning.MigrateJobOptions resources resources.RepositoryResources signer signature.Signer + history map[string]string // UID >> file path } func NewLegacyResourceMigrator( @@ -120,6 +121,10 @@ func NewLegacyResourceMigrator( kind schema.GroupResource, signer signature.Signer, ) *legacyResourceResourceMigrator { + var history map[string]string + if options.History { + history = make(map[string]string) + } return &legacyResourceResourceMigrator{ legacy: legacy, parser: parser, @@ -129,6 +134,7 @@ func NewLegacyResourceMigrator( kind: kind, resources: resources, signer: signer, + history: history, } } @@ -165,11 +171,22 @@ func (r *legacyResourceResourceMigrator) Write(ctx context.Context, key *resourc // TODO: this seems to be same logic as the export job // TODO: we should use a kind safe manager here - fileName, err := r.resources.CreateResourceFileFromObject(ctx, parsed.Obj, resources.WriteOptions{ + fileName, err := r.resources.WriteResourceFileFromObject(ctx, parsed.Obj, resources.WriteOptions{ Path: "", Ref: "", }) + // When replaying history, the path to the file may change over time + // This happens when the title or folder change + if r.history != nil { + name := parsed.Meta.GetName() + previous := r.history[name] + if previous != "" && previous != fileName { + _, _, err = r.resources.RemoveResourceFromFile(ctx, previous, "") + } + r.history[name] = fileName + } + result := jobs.JobResourceResult{ Name: parsed.Meta.GetName(), Resource: r.kind.Resource, diff --git a/pkg/registry/apis/provisioning/jobs/migrate/resources_test.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go similarity index 90% rename from pkg/registry/apis/provisioning/jobs/migrate/resources_test.go rename to pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go index e48c2567575..e73930bd718 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/resources_test.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go @@ -290,8 +290,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { t.Run("records error when create resource file fails", func(t *testing.T) { mockParser := resources.NewMockParser(t) obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ + Object: map[string]any{ + "metadata": map[string]any{ "name": "test", }, }, @@ -306,7 +306,7 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { }, nil) mockRepoResources := resources.NewMockRepositoryResources(t) - mockRepoResources.On("CreateResourceFileFromObject", mock.Anything, mock.Anything, mock.Anything). + mockRepoResources.On("WriteResourceFileFromObject", mock.Anything, mock.Anything, mock.Anything). Return("", errors.New("create file error")) progress := jobs.NewMockJobProgressRecorder(t) @@ -340,8 +340,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { t.Run("should fail when signer fails", func(t *testing.T) { mockParser := resources.NewMockParser(t) obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ + Object: map[string]any{ + "metadata": map[string]any{ "name": "test", }, }, @@ -383,8 +383,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { t.Run("should successfully add author signature", func(t *testing.T) { mockParser := resources.NewMockParser(t) obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ + Object: map[string]any{ + "metadata": map[string]any{ "name": "test", }, }, @@ -407,7 +407,7 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { Return(signedCtx, nil) mockRepoResources := resources.NewMockRepositoryResources(t) - mockRepoResources.On("CreateResourceFileFromObject", signedCtx, mock.Anything, mock.Anything). + mockRepoResources.On("WriteResourceFileFromObject", signedCtx, mock.Anything, mock.Anything). Return("test/path", nil) progress := jobs.NewMockJobProgressRecorder(t) @@ -439,11 +439,66 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { progress.AssertExpectations(t) }) + t.Run("should maintain history", func(t *testing.T) { + progress := jobs.NewMockJobProgressRecorder(t) + progress.On("Record", mock.Anything, mock.Anything).Return() + progress.On("TooManyErrors").Return(nil) + + mockParser := resources.NewMockParser(t) + obj := &unstructured.Unstructured{ + Object: map[string]any{ + "metadata": map[string]any{ + "name": "test", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) + mockParser.On("Parse", mock.Anything, mock.Anything). + Return(&resources.ParsedResource{ + Meta: meta, + Obj: obj, + }, nil) + + mockRepoResources := resources.NewMockRepositoryResources(t) + writeResourceFileFromObject := mockRepoResources.On("WriteResourceFileFromObject", mock.Anything, mock.Anything, mock.Anything) + + migrator := NewLegacyResourceMigrator( + nil, + mockParser, + mockRepoResources, + progress, + provisioning.MigrateJobOptions{ + History: true, + }, + "test-namespace", + schema.GroupResource{Group: "test.grafana.app", Resource: "tests"}, + signature.NewGrafanaSigner(), + ) + + writeResourceFileFromObject.Return("aaaa.json", nil) + err := migrator.Write(context.Background(), &resource.ResourceKey{}, []byte("")) + require.NoError(t, err) + require.Equal(t, "aaaa.json", migrator.history["test"], "kept track of the old files") + + // Change the result file name + writeResourceFileFromObject.Return("bbbb.json", nil) + mockRepoResources.On("RemoveResourceFromFile", mock.Anything, "aaaa.json", ""). + Return("", schema.GroupVersionKind{}, nil).Once() + + err = migrator.Write(context.Background(), &resource.ResourceKey{}, []byte("")) + require.NoError(t, err) + require.Equal(t, "bbbb.json", migrator.history["test"], "kept track of the old files") + + mockParser.AssertExpectations(t) + mockRepoResources.AssertExpectations(t) + progress.AssertExpectations(t) + }) + t.Run("should successfully write resource", func(t *testing.T) { mockParser := resources.NewMockParser(t) obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ + Object: map[string]any{ + "metadata": map[string]any{ "name": "test", }, }, @@ -471,7 +526,7 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { }, nil) mockRepoResources := resources.NewMockRepositoryResources(t) - mockRepoResources.On("CreateResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { + mockRepoResources.On("WriteResourceFileFromObject", mock.Anything, mock.MatchedBy(func(obj *unstructured.Unstructured) bool { if obj == nil { return false } @@ -524,8 +579,8 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { t.Run("should fail when too many errors", func(t *testing.T) { mockParser := resources.NewMockParser(t) obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ + Object: map[string]any{ + "metadata": map[string]any{ "name": "test", }, }, @@ -540,7 +595,7 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { }, nil) mockRepoResources := resources.NewMockRepositoryResources(t) - mockRepoResources.On("CreateResourceFileFromObject", mock.Anything, mock.Anything, resources.WriteOptions{}). + mockRepoResources.On("WriteResourceFileFromObject", mock.Anything, mock.Anything, resources.WriteOptions{}). Return("test/path", nil) progress := jobs.NewMockJobProgressRecorder(t) diff --git a/pkg/registry/apis/provisioning/jobs/migrate/worker.go b/pkg/registry/apis/provisioning/jobs/migrate/worker.go index 6b28955ad49..736ecfc81b4 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/worker.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/worker.go @@ -50,9 +50,19 @@ func (w *MigrationWorker) Process(ctx context.Context, repo repository.Repositor return errors.New("migration job submitted targeting repository that is not a ReaderWriter") } + if options.History { + if repo.Config().Spec.Type != provisioning.GitHubRepositoryType { + return errors.New("history is only supported for github repositories") + } + } + if dualwrite.IsReadingLegacyDashboardsAndFolders(ctx, w.storageStatus) { return w.legacyMigrator.Migrate(ctx, rw, *options, progress) } + if options.History { + return errors.New("history is not yet supported in unified storage") + } + return w.unifiedMigrator.Migrate(ctx, rw, *options, progress) } diff --git a/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go b/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go index 2b6ac3e08a2..1524487bf75 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go @@ -68,6 +68,50 @@ func TestMigrationWorker_ProcessNotReaderWriter(t *testing.T) { require.EqualError(t, err, "migration job submitted targeting repository that is not a ReaderWriter") } +func TestMigrationWorker_WithHistory(t *testing.T) { + fakeDualwrite := dualwrite.NewMockService(t) + fakeDualwrite.On("ReadFromUnified", mock.Anything, mock.Anything). + Maybe().Return(true, nil) // using unified storage + + worker := NewMigrationWorker(nil, nil, fakeDualwrite) + job := provisioning.Job{ + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMigrate, + Migrate: &provisioning.MigrateJobOptions{ + History: true, + }, + }, + } + + t.Run("fail local", func(t *testing.T) { + progressRecorder := jobs.NewMockJobProgressRecorder(t) + progressRecorder.On("SetTotal", mock.Anything, 10).Return() + progressRecorder.On("Strict").Return() + + repo := repository.NewLocal(&provisioning.Repository{}, nil) + err := worker.Process(context.Background(), repo, job, progressRecorder) + require.EqualError(t, err, "history is only supported for github repositories") + }) + + t.Run("fail unified", func(t *testing.T) { + progressRecorder := jobs.NewMockJobProgressRecorder(t) + progressRecorder.On("SetTotal", mock.Anything, 10).Return() + progressRecorder.On("Strict").Return() + + repo := repository.NewMockRepository(t) + repo.On("Config").Return(&provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "empty", // not valid + }, + }, + }) + err := worker.Process(context.Background(), repo, job, progressRecorder) + require.EqualError(t, err, "history is not yet supported in unified storage") + }) +} + func TestMigrationWorker_Process(t *testing.T) { tests := []struct { name string diff --git a/pkg/registry/apis/provisioning/repository/local.go b/pkg/registry/apis/provisioning/repository/local.go index 5ee716ffaf5..fc7bd18c99f 100644 --- a/pkg/registry/apis/provisioning/repository/local.go +++ b/pkg/registry/apis/provisioning/repository/local.go @@ -145,7 +145,7 @@ func (r *localRepository) Validate() field.ErrorList { // Test implements provisioning.Repository. // NOTE: Validate has been called (and passed) before this function should be called func (r *localRepository) Test(ctx context.Context) (*provisioning.TestResults, error) { - path := field.NewPath("spec", "localhost", "path") + path := field.NewPath("spec", "local", "path") if r.config.Spec.Local.Path == "" { return fromFieldError(field.Required(path, "no path is configured")), nil } diff --git a/pkg/registry/apis/provisioning/resources/repository.go b/pkg/registry/apis/provisioning/resources/repository.go index 87957f78023..44bfe42f74e 100644 --- a/pkg/registry/apis/provisioning/resources/repository.go +++ b/pkg/registry/apis/provisioning/resources/repository.go @@ -4,10 +4,11 @@ import ( "context" "fmt" - provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + + provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" ) //go:generate mockery --name RepositoryResourcesFactory --structname MockRepositoryResourcesFactory --inpackage --filename repository_resources_factory_mock.go --with-expecter @@ -23,7 +24,7 @@ type RepositoryResources interface { EnsureFolderExists(ctx context.Context, folder Folder, parentID string) error EnsureFolderTreeExists(ctx context.Context, ref, path string, tree FolderTree, fn func(folder Folder, created bool, err error) error) error // File from Resource - CreateResourceFileFromObject(ctx context.Context, obj *unstructured.Unstructured, options WriteOptions) (string, error) + WriteResourceFileFromObject(ctx context.Context, obj *unstructured.Unstructured, options WriteOptions) (string, error) // Resource from file WriteResourceFromFile(ctx context.Context, path, ref string) (string, schema.GroupVersionKind, error) RemoveResourceFromFile(ctx context.Context, path, ref string) (string, schema.GroupVersionKind, error) diff --git a/pkg/registry/apis/provisioning/resources/repository_resources_mock.go b/pkg/registry/apis/provisioning/resources/repository_resources_mock.go index cadf9476da8..6245188528e 100644 --- a/pkg/registry/apis/provisioning/resources/repository_resources_mock.go +++ b/pkg/registry/apis/provisioning/resources/repository_resources_mock.go @@ -26,12 +26,12 @@ func (_m *MockRepositoryResources) EXPECT() *MockRepositoryResources_Expecter { return &MockRepositoryResources_Expecter{mock: &_m.Mock} } -// CreateResourceFileFromObject provides a mock function with given fields: ctx, obj, options -func (_m *MockRepositoryResources) CreateResourceFileFromObject(ctx context.Context, obj *unstructured.Unstructured, options WriteOptions) (string, error) { +// WriteResourceFileFromObject provides a mock function with given fields: ctx, obj, options +func (_m *MockRepositoryResources) WriteResourceFileFromObject(ctx context.Context, obj *unstructured.Unstructured, options WriteOptions) (string, error) { ret := _m.Called(ctx, obj, options) if len(ret) == 0 { - panic("no return value specified for CreateResourceFileFromObject") + panic("no return value specified for WriteResourceFileFromObject") } var r0 string @@ -54,32 +54,32 @@ func (_m *MockRepositoryResources) CreateResourceFileFromObject(ctx context.Cont return r0, r1 } -// MockRepositoryResources_CreateResourceFileFromObject_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreateResourceFileFromObject' -type MockRepositoryResources_CreateResourceFileFromObject_Call struct { +// MockRepositoryResources_WriteResourceFileFromObject_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WriteResourceFileFromObject' +type MockRepositoryResources_WriteResourceFileFromObject_Call struct { *mock.Call } -// CreateResourceFileFromObject is a helper method to define mock.On call +// WriteResourceFileFromObject is a helper method to define mock.On call // - ctx context.Context // - obj *unstructured.Unstructured // - options WriteOptions -func (_e *MockRepositoryResources_Expecter) CreateResourceFileFromObject(ctx interface{}, obj interface{}, options interface{}) *MockRepositoryResources_CreateResourceFileFromObject_Call { - return &MockRepositoryResources_CreateResourceFileFromObject_Call{Call: _e.mock.On("CreateResourceFileFromObject", ctx, obj, options)} +func (_e *MockRepositoryResources_Expecter) WriteResourceFileFromObject(ctx interface{}, obj interface{}, options interface{}) *MockRepositoryResources_WriteResourceFileFromObject_Call { + return &MockRepositoryResources_WriteResourceFileFromObject_Call{Call: _e.mock.On("WriteResourceFileFromObject", ctx, obj, options)} } -func (_c *MockRepositoryResources_CreateResourceFileFromObject_Call) Run(run func(ctx context.Context, obj *unstructured.Unstructured, options WriteOptions)) *MockRepositoryResources_CreateResourceFileFromObject_Call { +func (_c *MockRepositoryResources_WriteResourceFileFromObject_Call) Run(run func(ctx context.Context, obj *unstructured.Unstructured, options WriteOptions)) *MockRepositoryResources_WriteResourceFileFromObject_Call { _c.Call.Run(func(args mock.Arguments) { run(args[0].(context.Context), args[1].(*unstructured.Unstructured), args[2].(WriteOptions)) }) return _c } -func (_c *MockRepositoryResources_CreateResourceFileFromObject_Call) Return(_a0 string, _a1 error) *MockRepositoryResources_CreateResourceFileFromObject_Call { +func (_c *MockRepositoryResources_WriteResourceFileFromObject_Call) Return(_a0 string, _a1 error) *MockRepositoryResources_WriteResourceFileFromObject_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *MockRepositoryResources_CreateResourceFileFromObject_Call) RunAndReturn(run func(context.Context, *unstructured.Unstructured, WriteOptions) (string, error)) *MockRepositoryResources_CreateResourceFileFromObject_Call { +func (_c *MockRepositoryResources_WriteResourceFileFromObject_Call) RunAndReturn(run func(context.Context, *unstructured.Unstructured, WriteOptions) (string, error)) *MockRepositoryResources_WriteResourceFileFromObject_Call { _c.Call.Return(run) return _c } diff --git a/pkg/registry/apis/provisioning/resources/resources.go b/pkg/registry/apis/provisioning/resources/resources.go index 7f819999b90..0acbbc4c36a 100644 --- a/pkg/registry/apis/provisioning/resources/resources.go +++ b/pkg/registry/apis/provisioning/resources/resources.go @@ -3,7 +3,6 @@ package resources import ( "bytes" "context" - "encoding/json" "errors" "fmt" "slices" @@ -54,7 +53,7 @@ func NewResourcesManager(repo repository.ReaderWriter, folders *FolderManager, p } // CreateResource writes an object to the repository -func (r *ResourcesManager) CreateResourceFileFromObject(ctx context.Context, obj *unstructured.Unstructured, options WriteOptions) (string, error) { +func (r *ResourcesManager) WriteResourceFileFromObject(ctx context.Context, obj *unstructured.Unstructured, options WriteOptions) (string, error) { if err := ctx.Err(); err != nil { return "", fmt.Errorf("context error: %w", err) } @@ -81,7 +80,7 @@ func (r *ResourcesManager) CreateResourceFileFromObject(ctx context.Context, obj } manager, _ := meta.GetManagerProperties() - // TODO: how we should handle this? + // TODO: how should we handle this? if manager.Identity == r.repo.Config().GetName() { // If it's already in the repository, we don't need to write it return "", ErrAlreadyInRepository @@ -100,17 +99,6 @@ func (r *ResourcesManager) CreateResourceFileFromObject(ctx context.Context, obj return "", fmt.Errorf("folder not found in tree: %s", folder) } - // Clear the metadata - delete(obj.Object, "metadata") - - // Always write the identifier - meta.SetName(name) - - body, err := json.MarshalIndent(obj.Object, "", " ") - if err != nil { - return "", fmt.Errorf("failed to marshal dashboard: %w", err) - } - fileName := slugify.Slugify(title) + ".json" if fid.Path != "" { fileName = safepath.Join(fid.Path, fileName) @@ -119,6 +107,18 @@ func (r *ResourcesManager) CreateResourceFileFromObject(ctx context.Context, obj fileName = safepath.Join(options.Path, fileName) } + parsed := ParsedResource{ + Info: &repository.FileInfo{ + Path: fileName, + Ref: options.Ref, + }, + Obj: obj, + } + body, err := parsed.ToSaveBytes() + if err != nil { + return "", err + } + err = r.repo.Write(ctx, fileName, options.Ref, body, commitMessage) if err != nil { return "", fmt.Errorf("failed to write file: %w", err) diff --git a/public/app/features/provisioning/Wizard/SynchronizeStep.tsx b/public/app/features/provisioning/Wizard/SynchronizeStep.tsx index 1c0888c04d4..5b73b936592 100644 --- a/public/app/features/provisioning/Wizard/SynchronizeStep.tsx +++ b/public/app/features/provisioning/Wizard/SynchronizeStep.tsx @@ -17,7 +17,8 @@ export interface SynchronizeStepProps { export function SynchronizeStep({ onStepStatusUpdate, requiresMigration }: SynchronizeStepProps) { const [createJob] = useCreateRepositoryJobsMutation(); const { getValues, register, watch } = useFormContext(); - const target = watch('repository.sync.target'); + const repoType = watch('repository.type'); + const supportsHistory = requiresMigration && repoType === 'github'; const [job, setJob] = useState(); const startSynchronization = async () => { @@ -35,7 +36,7 @@ export function SynchronizeStep({ onStepStatusUpdate, requiresMigration }: Synch const jobSpec = requiresMigration ? { migrate: { - history, + history: history && supportsHistory, }, } : { @@ -111,7 +112,7 @@ export function SynchronizeStep({ onStepStatusUpdate, requiresMigration }: Synch - {requiresMigration && target !== 'folder' && ( + {supportsHistory && ( <> Synchronization options