From c5d76a8bba764489e17b1c8ff1e7978d47d869be Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Thu, 3 Apr 2025 18:58:05 +0300 Subject: [PATCH] Provisioning: Require a name in the saved resource (#103309) --- pkg/apis/provisioning/v0alpha1/jobs.go | 6 - .../v0alpha1/zz_generated.openapi.go | 18 -- pkg/registry/apis/provisioning/files.go | 4 +- .../apis/provisioning/jobs/export/worker.go | 20 +- .../provisioning/jobs/migrate/resources.go | 5 +- .../apis/provisioning/jobs/migrate/storage.go | 4 +- .../apis/provisioning/jobs/migrate/users.go | 2 +- .../apis/provisioning/jobs/migrate/worker.go | 45 +-- .../apis/provisioning/jobs/sync/worker.go | 2 +- .../apis/provisioning/resources/client.go | 116 +++----- .../provisioning/resources/clients_mock.go | 281 ++++++++++++++++++ .../apis/provisioning/resources/fileformat.go | 2 +- .../apis/provisioning/resources/folders.go | 2 +- .../apis/provisioning/resources/parser.go | 40 ++- .../provisioning/resources/parser_test.go | 76 +++++ .../apis/provisioning/resources/resources.go | 27 +- .../provisioning.grafana.app-v0alpha1.json | 16 - pkg/tests/apis/provisioning/helper_test.go | 2 +- .../apis/provisioning/provisioning_test.go | 135 +++++++-- .../api/clients/provisioning/endpoints.gen.ts | 4 - .../provisioning/Wizard/BootstrapStep.tsx | 60 ++-- .../provisioning/Wizard/MigrateStep.tsx | 2 - public/locales/en-US/grafana.json | 4 +- 23 files changed, 609 insertions(+), 264 deletions(-) create mode 100644 pkg/registry/apis/provisioning/resources/clients_mock.go create mode 100644 pkg/registry/apis/provisioning/resources/parser_test.go diff --git a/pkg/apis/provisioning/v0alpha1/jobs.go b/pkg/apis/provisioning/v0alpha1/jobs.go index 0be95cd64ad..6e26014b38e 100644 --- a/pkg/apis/provisioning/v0alpha1/jobs.go +++ b/pkg/apis/provisioning/v0alpha1/jobs.go @@ -126,17 +126,11 @@ type ExportJobOptions struct { // Prefix in target file system Path string `json:"path,omitempty"` - - // Include the identifier in the exported metadata - Identifier bool `json:"identifier"` } type MigrateJobOptions struct { // Preserve history (if possible) History bool `json:"history,omitempty"` - - // Include the identifier in the exported metadata - Identifier bool `json:"identifier"` } // The job status diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index cf3a050475b..8122e6c71dc 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -117,16 +117,7 @@ func schema_pkg_apis_provisioning_v0alpha1_ExportJobOptions(ref common.Reference Format: "", }, }, - "identifier": { - SchemaProps: spec.SchemaProps{ - Description: "Include the identifier in the exported metadata", - Default: false, - Type: []string{"boolean"}, - Format: "", - }, - }, }, - Required: []string{"identifier"}, }, }, } @@ -918,16 +909,7 @@ func schema_pkg_apis_provisioning_v0alpha1_MigrateJobOptions(ref common.Referenc Format: "", }, }, - "identifier": { - SchemaProps: spec.SchemaProps{ - Description: "Include the identifier in the exported metadata", - Default: false, - Type: []string{"boolean"}, - Format: "", - }, - }, }, - Required: []string{"identifier"}, }, }, } diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index 53be0555ddf..3176d5eec7d 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -130,7 +130,7 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. obj = resource.AsResourceWrapper() code = http.StatusOK if len(resource.Errors) > 0 { - code = http.StatusNotAcceptable + code = http.StatusExpectationFailed } case http.MethodPost: if isDir { @@ -186,7 +186,7 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. } // something failed - if len(obj.Errors) > 0 { + if len(obj.Errors) > 0 && code < 400 { code = http.StatusInternalServerError } diff --git a/pkg/registry/apis/provisioning/jobs/export/worker.go b/pkg/registry/apis/provisioning/jobs/export/worker.go index 9a4a986ecd6..42049e4cb32 100644 --- a/pkg/registry/apis/provisioning/jobs/export/worker.go +++ b/pkg/registry/apis/provisioning/jobs/export/worker.go @@ -7,14 +7,14 @@ import ( "os" "time" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/grafana/grafana-app-sdk/logging" 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" "github.com/grafana/grafana/pkg/storage/legacysql/dualwrite" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/dynamic" ) type ExportWorker struct { @@ -129,14 +129,19 @@ func (r *ExportWorker) Process(ctx context.Context, repo repository.Repository, } resourceManager := resources.NewResourcesManager(rw, folders, parser, clients, nil) - for _, kind := range resources.SupportedResources { - // skip from folders as we do them first + for _, kind := range resources.SupportedProvisioningResources { + // skip from folders as we do them first... so only dashboards if kind == resources.FolderResource { continue } progress.SetMessage(ctx, fmt.Sprintf("reading %s resource", kind.Resource)) - if err := clients.ForEachResource(ctx, kind, func(_ dynamic.ResourceInterface, item *unstructured.Unstructured) error { + client, _, err := clients.ForResource(kind) + if err != nil { + return err + } + + if err := resources.ForEach(ctx, client, func(item *unstructured.Unstructured) error { result := jobs.JobResourceResult{ Name: item.GetName(), Resource: kind.Resource, @@ -145,9 +150,8 @@ func (r *ExportWorker) Process(ctx context.Context, repo repository.Repository, } fileName, err := resourceManager.CreateResourceFileFromObject(ctx, item, resources.WriteOptions{ - Path: options.Path, - Ref: options.Branch, - Identifier: options.Identifier, + Path: options.Path, + Ref: options.Branch, }) if errors.Is(err, resources.ErrAlreadyInRepository) { result.Action = repository.FileActionIgnored diff --git a/pkg/registry/apis/provisioning/jobs/migrate/resources.go b/pkg/registry/apis/provisioning/jobs/migrate/resources.go index 0e01c1b251d..4c43ae06485 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/resources.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/resources.go @@ -69,9 +69,8 @@ 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{ - Path: "", - Ref: "", - Identifier: r.options.Identifier, + Path: "", + Ref: "", }) result := jobs.JobResourceResult{ diff --git a/pkg/registry/apis/provisioning/jobs/migrate/storage.go b/pkg/registry/apis/provisioning/jobs/migrate/storage.go index 798ce384a14..db604d692d3 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/storage.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/storage.go @@ -14,7 +14,7 @@ import ( ) func stopReadingUnifiedStorage(ctx context.Context, dual dualwrite.Service) error { - for _, gr := range resources.SupportedResources { + for _, gr := range resources.SupportedProvisioningResources { status, _ := dual.Status(ctx, gr.GroupResource()) status.ReadUnified = false status.Migrated = 0 @@ -29,7 +29,7 @@ func stopReadingUnifiedStorage(ctx context.Context, dual dualwrite.Service) erro } func wipeUnifiedAndSetMigratedFlag(ctx context.Context, dual dualwrite.Service, namespace string, batch resource.BulkStoreClient) error { - for _, gr := range resources.SupportedResources { + for _, gr := range resources.SupportedProvisioningResources { status, _ := dual.Status(ctx, gr.GroupResource()) if status.ReadUnified { return fmt.Errorf("unexpected state - already using unified storage for: %s", gr) diff --git a/pkg/registry/apis/provisioning/jobs/migrate/users.go b/pkg/registry/apis/provisioning/jobs/migrate/users.go index 2b7b4d09cb2..cb023255a2b 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/users.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/users.go @@ -21,7 +21,7 @@ func loadUsers(ctx context.Context, parser *resources.Parser) (map[string]reposi userInfo := make(map[string]repository.CommitSignature) var count int - err = resources.ForEachResource(ctx, client, func(item *unstructured.Unstructured) error { + err = resources.ForEach(ctx, client, func(item *unstructured.Unstructured) error { count++ if count > maxUsers { return errors.New("too many users") diff --git a/pkg/registry/apis/provisioning/jobs/migrate/worker.go b/pkg/registry/apis/provisioning/jobs/migrate/worker.go index f1e64f19158..07deef86114 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/worker.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/worker.go @@ -10,7 +10,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/dynamic" "github.com/grafana/grafana-app-sdk/logging" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" @@ -172,7 +171,7 @@ func (w *MigrationWorker) migrateFromLegacy(ctx context.Context, rw repository.R progress.SetMessage(ctx, "exporting legacy resources") resourceManager := resources.NewResourcesManager(rw, folders, parser, parser.Clients(), userInfo) - for _, kind := range resources.SupportedResources { + for _, kind := range resources.SupportedProvisioningResources { if kind == resources.FolderResource { continue } @@ -235,9 +234,7 @@ func (w *MigrationWorker) migrateFromAPIServer(ctx context.Context, repo reposit progress.SetMessage(ctx, "exporting unified storage resources") exportJob := provisioning.Job{ Spec: provisioning.JobSpec{ - Push: &provisioning.ExportJobOptions{ - Identifier: options.Identifier, - }, + Push: &provisioning.ExportJobOptions{}, }, } if err := w.exportWorker.Process(ctx, repo, exportJob, progress); err != nil { @@ -260,23 +257,31 @@ func (w *MigrationWorker) migrateFromAPIServer(ctx context.Context, repo reposit return fmt.Errorf("pull resources: %w", err) } - progress.SetMessage(ctx, "removing unprovisioned resources") - return parser.Clients().ForEachUnmanagedResource(ctx, func(client dynamic.ResourceInterface, item *unstructured.Unstructured) error { - result := jobs.JobResourceResult{ - Name: item.GetName(), - Resource: item.GetKind(), - Group: item.GroupVersionKind().Group, - Action: repository.FileActionDeleted, + for _, kind := range resources.SupportedProvisioningResources { + progress.SetMessage(ctx, fmt.Sprintf("removing unprovisioned %s", kind.Resource)) + client, _, err := parser.Clients().ForResource(kind) + if err != nil { + return err } + if err = resources.ForEach(ctx, client, func(item *unstructured.Unstructured) error { + result := jobs.JobResourceResult{ + Name: item.GetName(), + Resource: item.GetKind(), + Group: item.GroupVersionKind().Group, + Action: repository.FileActionDeleted, + } + + if err := client.Delete(ctx, item.GetName(), metav1.DeleteOptions{}); err != nil { + result.Error = fmt.Errorf("failed to delete folder: %w", err) + progress.Record(ctx, result) + return result.Error + } - if err := client.Delete(ctx, item.GetName(), metav1.DeleteOptions{}); err != nil { - result.Error = fmt.Errorf("failed to delete folder: %w", err) progress.Record(ctx, result) - return result.Error + return nil + }); err != nil { + return err } - - progress.Record(ctx, result) - - return nil - }) + } + return nil } diff --git a/pkg/registry/apis/provisioning/jobs/sync/worker.go b/pkg/registry/apis/provisioning/jobs/sync/worker.go index 2def2faa177..6d334ac61ba 100644 --- a/pkg/registry/apis/provisioning/jobs/sync/worker.go +++ b/pkg/registry/apis/provisioning/jobs/sync/worker.go @@ -175,8 +175,8 @@ type syncJob struct { repository repository.Reader progress jobs.JobProgressRecorder lister resources.ResourceLister + clients resources.ResourceClients folders *resources.FolderManager - clients *resources.ResourceClients resourceManager *resources.ResourcesManager } diff --git a/pkg/registry/apis/provisioning/resources/client.go b/pkg/registry/apis/provisioning/resources/client.go index d7d9f2c6918..9b35b86b0ee 100644 --- a/pkg/registry/apis/provisioning/resources/client.go +++ b/pkg/registry/apis/provisioning/resources/client.go @@ -11,22 +11,41 @@ import ( "k8s.io/client-go/dynamic" dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1alpha1" - "github.com/grafana/grafana/pkg/apimachinery/utils" folders "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" iam "github.com/grafana/grafana/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/apiserver/client" ) +var ( + UserResource = iam.UserResourceInfo.GroupVersionResource() + FolderResource = folders.FolderResourceInfo.GroupVersionResource() + DashboardResource = dashboard.DashboardResourceInfo.GroupVersionResource() + + // SupportedProvisioningResources is the list of resources that can fully managed from the UI + SupportedProvisioningResources = []schema.GroupVersionResource{FolderResource, DashboardResource} +) + type ClientFactory struct { configProvider apiserver.RestConfigProvider } +// ResourceClients provides access to clients within a namespace +// +//go:generate mockery --name ResourceClients --structname MockResourceClients --inpackage --filename clients_mock.go --with-expecter +type ResourceClients interface { + ForKind(gvk schema.GroupVersionKind) (dynamic.ResourceInterface, schema.GroupVersionResource, error) + ForResource(gvr schema.GroupVersionResource) (dynamic.ResourceInterface, schema.GroupVersionKind, error) + + Folder() (dynamic.ResourceInterface, error) + User() (dynamic.ResourceInterface, error) +} + func NewClientFactory(configProvider apiserver.RestConfigProvider) *ClientFactory { return &ClientFactory{configProvider} } -func (f *ClientFactory) Clients(ctx context.Context, namespace string) (*ResourceClients, error) { +func (f *ClientFactory) Clients(ctx context.Context, namespace string) (ResourceClients, error) { restConfig, err := f.configProvider.GetRestConfig(ctx) if err != nil { return nil, err @@ -46,7 +65,7 @@ func (f *ClientFactory) Clients(ctx context.Context, namespace string) (*Resourc return nil, err } - return &ResourceClients{ + return &resourceClients{ namespace: namespace, discovery: discovery, dynamic: client, @@ -55,7 +74,7 @@ func (f *ClientFactory) Clients(ctx context.Context, namespace string) (*Resourc }, nil } -type ResourceClients struct { +type resourceClients struct { namespace string dynamic dynamic.Interface @@ -73,7 +92,7 @@ type clientInfo struct { client dynamic.ResourceInterface } -func (c *ResourceClients) ForKind(gvk schema.GroupVersionKind) (dynamic.ResourceInterface, schema.GroupVersionResource, error) { +func (c *resourceClients) ForKind(gvk schema.GroupVersionKind) (dynamic.ResourceInterface, schema.GroupVersionResource, error) { c.mutex.Lock() defer c.mutex.Unlock() @@ -99,7 +118,7 @@ func (c *ResourceClients) ForKind(gvk schema.GroupVersionKind) (dynamic.Resource // ForResource returns a client for a resource. // If the resource has a version, it will be used. // If the resource does not have a version, the preferred version will be used. -func (c *ResourceClients) ForResource(gvr schema.GroupVersionResource) (dynamic.ResourceInterface, schema.GroupVersionKind, error) { +func (c *resourceClients) ForResource(gvr schema.GroupVersionResource) (dynamic.ResourceInterface, schema.GroupVersionKind, error) { c.mutex.Lock() defer c.mutex.Unlock() @@ -145,20 +164,18 @@ func (c *ResourceClients) ForResource(gvr schema.GroupVersionResource) (dynamic. return info.client, info.gvk, nil } -// ForEachResource applies the function to each resource in the discovery client -func (c *ResourceClients) ForEachResource(ctx context.Context, kind schema.GroupVersionResource, fn func(client dynamic.ResourceInterface, item *unstructured.Unstructured) error) error { - client, _, err := c.ForResource(kind) - if err != nil { - return err - } - - return ForEachResource(ctx, client, func(item *unstructured.Unstructured) error { - return fn(client, item) - }) +func (c *resourceClients) Folder() (dynamic.ResourceInterface, error) { + client, _, err := c.ForResource(FolderResource) + return client, err } -// ForEachResource applies the function to each resource in the discovery client -func ForEachResource(ctx context.Context, client dynamic.ResourceInterface, fn func(item *unstructured.Unstructured) error) error { +func (c *resourceClients) User() (dynamic.ResourceInterface, error) { + v, _, err := c.ForResource(UserResource) + return v, err +} + +// ForEach applies the function to each resource returned from the list operation +func ForEach(ctx context.Context, client dynamic.ResourceInterface, fn func(item *unstructured.Unstructured) error) error { var continueToken string for ctx.Err() == nil { list, err := client.List(ctx, metav1.ListOptions{Limit: 100, Continue: continueToken}) @@ -184,66 +201,3 @@ func ForEachResource(ctx context.Context, client dynamic.ResourceInterface, fn f return nil } - -// ForEachUnmanagedResource applies the function to each unprovisioned supported resource -func (c *ResourceClients) ForEachUnmanagedResource(ctx context.Context, fn func(client dynamic.ResourceInterface, item *unstructured.Unstructured) error) error { - return c.ForEachSupportedResource(ctx, func(client dynamic.ResourceInterface, item *unstructured.Unstructured) error { - meta, err := utils.MetaAccessor(item) - if err != nil { - return fmt.Errorf("extract meta accessor: %w", err) - } - - // Skip if managed - _, ok := meta.GetManagerProperties() - if ok { - return nil - } - - return fn(client, item) - }) -} - -// ForEachSupportedResource applies the function to each supported resource -func (c *ResourceClients) ForEachSupportedResource(ctx context.Context, fn func(client dynamic.ResourceInterface, item *unstructured.Unstructured) error) error { - for _, kind := range SupportedResources { - if err := c.ForEachResource(ctx, kind, fn); err != nil { - return err - } - } - return nil -} - -func (c *ResourceClients) Folder() (dynamic.ResourceInterface, error) { - client, _, err := c.ForResource(FolderResource) - return client, err -} - -func (c *ResourceClients) ForEachFolder(ctx context.Context, fn func(client dynamic.ResourceInterface, item *unstructured.Unstructured) error) error { - return c.ForEachResource(ctx, FolderResource, fn) -} - -func (c *ResourceClients) User() (dynamic.ResourceInterface, error) { - v, _, err := c.ForResource(UserResource) - return v, err -} - -var UserResource = schema.GroupVersionResource{ - Group: iam.GROUP, - Version: iam.VERSION, - Resource: iam.UserResourceInfo.GroupResource().Resource, -} - -var FolderResource = schema.GroupVersionResource{ - Group: folders.GROUP, - Version: folders.VERSION, - Resource: folders.RESOURCE, -} - -var DashboardResource = schema.GroupVersionResource{ - Group: dashboard.GROUP, - Version: dashboard.VERSION, - Resource: dashboard.DASHBOARD_RESOURCE, -} - -// SupportedResources is the list of resources that are supported by provisioning -var SupportedResources = []schema.GroupVersionResource{FolderResource, DashboardResource} diff --git a/pkg/registry/apis/provisioning/resources/clients_mock.go b/pkg/registry/apis/provisioning/resources/clients_mock.go new file mode 100644 index 00000000000..dbbd1347edd --- /dev/null +++ b/pkg/registry/apis/provisioning/resources/clients_mock.go @@ -0,0 +1,281 @@ +// Code generated by mockery v2.53.3. DO NOT EDIT. + +package resources + +import ( + mock "github.com/stretchr/testify/mock" + dynamic "k8s.io/client-go/dynamic" + + schema "k8s.io/apimachinery/pkg/runtime/schema" +) + +// MockResourceClients is an autogenerated mock type for the ResourceClients type +type MockResourceClients struct { + mock.Mock +} + +type MockResourceClients_Expecter struct { + mock *mock.Mock +} + +func (_m *MockResourceClients) EXPECT() *MockResourceClients_Expecter { + return &MockResourceClients_Expecter{mock: &_m.Mock} +} + +// Folder provides a mock function with no fields +func (_m *MockResourceClients) Folder() (dynamic.ResourceInterface, error) { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for Folder") + } + + var r0 dynamic.ResourceInterface + var r1 error + if rf, ok := ret.Get(0).(func() (dynamic.ResourceInterface, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() dynamic.ResourceInterface); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(dynamic.ResourceInterface) + } + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockResourceClients_Folder_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Folder' +type MockResourceClients_Folder_Call struct { + *mock.Call +} + +// Folder is a helper method to define mock.On call +func (_e *MockResourceClients_Expecter) Folder() *MockResourceClients_Folder_Call { + return &MockResourceClients_Folder_Call{Call: _e.mock.On("Folder")} +} + +func (_c *MockResourceClients_Folder_Call) Run(run func()) *MockResourceClients_Folder_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *MockResourceClients_Folder_Call) Return(_a0 dynamic.ResourceInterface, _a1 error) *MockResourceClients_Folder_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockResourceClients_Folder_Call) RunAndReturn(run func() (dynamic.ResourceInterface, error)) *MockResourceClients_Folder_Call { + _c.Call.Return(run) + return _c +} + +// ForKind provides a mock function with given fields: gvk +func (_m *MockResourceClients) ForKind(gvk schema.GroupVersionKind) (dynamic.ResourceInterface, schema.GroupVersionResource, error) { + ret := _m.Called(gvk) + + if len(ret) == 0 { + panic("no return value specified for ForKind") + } + + var r0 dynamic.ResourceInterface + var r1 schema.GroupVersionResource + var r2 error + if rf, ok := ret.Get(0).(func(schema.GroupVersionKind) (dynamic.ResourceInterface, schema.GroupVersionResource, error)); ok { + return rf(gvk) + } + if rf, ok := ret.Get(0).(func(schema.GroupVersionKind) dynamic.ResourceInterface); ok { + r0 = rf(gvk) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(dynamic.ResourceInterface) + } + } + + if rf, ok := ret.Get(1).(func(schema.GroupVersionKind) schema.GroupVersionResource); ok { + r1 = rf(gvk) + } else { + r1 = ret.Get(1).(schema.GroupVersionResource) + } + + if rf, ok := ret.Get(2).(func(schema.GroupVersionKind) error); ok { + r2 = rf(gvk) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} + +// MockResourceClients_ForKind_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ForKind' +type MockResourceClients_ForKind_Call struct { + *mock.Call +} + +// ForKind is a helper method to define mock.On call +// - gvk schema.GroupVersionKind +func (_e *MockResourceClients_Expecter) ForKind(gvk interface{}) *MockResourceClients_ForKind_Call { + return &MockResourceClients_ForKind_Call{Call: _e.mock.On("ForKind", gvk)} +} + +func (_c *MockResourceClients_ForKind_Call) Run(run func(gvk schema.GroupVersionKind)) *MockResourceClients_ForKind_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(schema.GroupVersionKind)) + }) + return _c +} + +func (_c *MockResourceClients_ForKind_Call) Return(_a0 dynamic.ResourceInterface, _a1 schema.GroupVersionResource, _a2 error) *MockResourceClients_ForKind_Call { + _c.Call.Return(_a0, _a1, _a2) + return _c +} + +func (_c *MockResourceClients_ForKind_Call) RunAndReturn(run func(schema.GroupVersionKind) (dynamic.ResourceInterface, schema.GroupVersionResource, error)) *MockResourceClients_ForKind_Call { + _c.Call.Return(run) + return _c +} + +// ForResource provides a mock function with given fields: gvr +func (_m *MockResourceClients) ForResource(gvr schema.GroupVersionResource) (dynamic.ResourceInterface, schema.GroupVersionKind, error) { + ret := _m.Called(gvr) + + if len(ret) == 0 { + panic("no return value specified for ForResource") + } + + var r0 dynamic.ResourceInterface + var r1 schema.GroupVersionKind + var r2 error + if rf, ok := ret.Get(0).(func(schema.GroupVersionResource) (dynamic.ResourceInterface, schema.GroupVersionKind, error)); ok { + return rf(gvr) + } + if rf, ok := ret.Get(0).(func(schema.GroupVersionResource) dynamic.ResourceInterface); ok { + r0 = rf(gvr) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(dynamic.ResourceInterface) + } + } + + if rf, ok := ret.Get(1).(func(schema.GroupVersionResource) schema.GroupVersionKind); ok { + r1 = rf(gvr) + } else { + r1 = ret.Get(1).(schema.GroupVersionKind) + } + + if rf, ok := ret.Get(2).(func(schema.GroupVersionResource) error); ok { + r2 = rf(gvr) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} + +// MockResourceClients_ForResource_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ForResource' +type MockResourceClients_ForResource_Call struct { + *mock.Call +} + +// ForResource is a helper method to define mock.On call +// - gvr schema.GroupVersionResource +func (_e *MockResourceClients_Expecter) ForResource(gvr interface{}) *MockResourceClients_ForResource_Call { + return &MockResourceClients_ForResource_Call{Call: _e.mock.On("ForResource", gvr)} +} + +func (_c *MockResourceClients_ForResource_Call) Run(run func(gvr schema.GroupVersionResource)) *MockResourceClients_ForResource_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(schema.GroupVersionResource)) + }) + return _c +} + +func (_c *MockResourceClients_ForResource_Call) Return(_a0 dynamic.ResourceInterface, _a1 schema.GroupVersionKind, _a2 error) *MockResourceClients_ForResource_Call { + _c.Call.Return(_a0, _a1, _a2) + return _c +} + +func (_c *MockResourceClients_ForResource_Call) RunAndReturn(run func(schema.GroupVersionResource) (dynamic.ResourceInterface, schema.GroupVersionKind, error)) *MockResourceClients_ForResource_Call { + _c.Call.Return(run) + return _c +} + +// User provides a mock function with no fields +func (_m *MockResourceClients) User() (dynamic.ResourceInterface, error) { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for User") + } + + var r0 dynamic.ResourceInterface + var r1 error + if rf, ok := ret.Get(0).(func() (dynamic.ResourceInterface, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() dynamic.ResourceInterface); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(dynamic.ResourceInterface) + } + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockResourceClients_User_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'User' +type MockResourceClients_User_Call struct { + *mock.Call +} + +// User is a helper method to define mock.On call +func (_e *MockResourceClients_Expecter) User() *MockResourceClients_User_Call { + return &MockResourceClients_User_Call{Call: _e.mock.On("User")} +} + +func (_c *MockResourceClients_User_Call) Run(run func()) *MockResourceClients_User_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *MockResourceClients_User_Call) Return(_a0 dynamic.ResourceInterface, _a1 error) *MockResourceClients_User_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockResourceClients_User_Call) RunAndReturn(run func() (dynamic.ResourceInterface, error)) *MockResourceClients_User_Call { + _c.Call.Return(run) + return _c +} + +// NewMockResourceClients creates a new instance of MockResourceClients. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockResourceClients(t interface { + mock.TestingT + Cleanup(func()) +}) *MockResourceClients { + mock := &MockResourceClients{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/registry/apis/provisioning/resources/fileformat.go b/pkg/registry/apis/provisioning/resources/fileformat.go index e2805954343..b11795d3ca2 100644 --- a/pkg/registry/apis/provisioning/resources/fileformat.go +++ b/pkg/registry/apis/provisioning/resources/fileformat.go @@ -35,7 +35,7 @@ func ReadClassicResource(ctx context.Context, info *repository.FileInfo) (*unstr return nil, nil, "", err } } else { - return nil, nil, "", fmt.Errorf("yaml not yet implemented") + return nil, nil, "", fmt.Errorf("classic resource must be JSON") } // regular version headers exist diff --git a/pkg/registry/apis/provisioning/resources/folders.go b/pkg/registry/apis/provisioning/resources/folders.go index 976eda9e52f..ad27a1abe5f 100644 --- a/pkg/registry/apis/provisioning/resources/folders.go +++ b/pkg/registry/apis/provisioning/resources/folders.go @@ -174,7 +174,7 @@ func (fm *FolderManager) EnsureTreeExists(ctx context.Context, ref, path string, } func (fm *FolderManager) LoadFromServer(ctx context.Context) error { - return ForEachResource(ctx, fm.client, func(item *unstructured.Unstructured) error { + return ForEach(ctx, fm.client, func(item *unstructured.Unstructured) error { if fm.tree.Count() > maxFolders { return errors.New("too many folders") } diff --git a/pkg/registry/apis/provisioning/resources/parser.go b/pkg/registry/apis/provisioning/resources/parser.go index c38c638a37d..613f3fd9df5 100644 --- a/pkg/registry/apis/provisioning/resources/parser.go +++ b/pkg/registry/apis/provisioning/resources/parser.go @@ -12,6 +12,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/dynamic" "github.com/grafana/grafana-app-sdk/logging" @@ -60,7 +61,7 @@ type Parser struct { urls repository.RepositoryWithURLs // ResourceClients give access to k8s apis - clients *ResourceClients + clients ResourceClients } type ParsedResource struct { @@ -105,9 +106,8 @@ type ParsedResource struct { Errors []error } -// FIXME: eliminate clients from parser - -func (r *Parser) Clients() *ResourceClients { +// FIXME: eliminate clients from parser (but be careful that we can use the same cache/resolved GVK+GVR) +func (r *Parser) Clients() ResourceClients { return r.clients } @@ -168,13 +168,14 @@ func (r *Parser) Parse(ctx context.Context, info *repository.FileInfo, validate Checksum: info.Hash, }) - // Calculate name+folder from the file path - if info.Path != "" { - objName := FileNameFromHashedRepoPath(r.repo.Name, info.Path) - if obj.GetName() == "" { - obj.SetName(objName) // use the name saved in config - } + if obj.GetName() == "" && obj.GetGenerateName() == "" { + parsed.Errors = append(parsed.Errors, + field.Required(field.NewPath("name", "metadata", "name"), + "An explicit name must be saved in the resource (or generateName)")) + } + // Calculate folder identifier from the file path + if info.Path != "" { dirPath := safepath.Dir(info.Path) if dirPath != "" { parsed.Meta.SetFolder(ParseFolder(dirPath, r.repo.Name).ID) @@ -221,6 +222,12 @@ func (r *Parser) Parse(ctx context.Context, info *repository.FileInfo, validate DryRun: []string{"All"}, }) } + + // When the name is missing (and generateName is configured) use the value from DryRun + if obj.GetName() == "" && parsed.DryRunResponse != nil { + obj.SetName(parsed.DryRunResponse.GetName()) + } + if err != nil { parsed.Errors = append(parsed.Errors, err) } @@ -228,12 +235,13 @@ func (r *Parser) Parse(ctx context.Context, info *repository.FileInfo, validate } func (f *ParsedResource) ToSaveBytes() ([]byte, error) { - // TODO? should we use the dryRun (validated) version? - obj := make(map[string]any) - for k, v := range f.Obj.Object { - if k != "metadata" { - obj[k] = v - } + obj := f.Obj.DeepCopy().Object + delete(obj, "status") + name := f.Obj.GetName() + if name == "" { + delete(obj, "metadata") + } else { + obj["metadata"] = map[string]any{"name": name} } switch path.Ext(f.Info.Path) { diff --git a/pkg/registry/apis/provisioning/resources/parser_test.go b/pkg/registry/apis/provisioning/resources/parser_test.go new file mode 100644 index 00000000000..9a03321293b --- /dev/null +++ b/pkg/registry/apis/provisioning/resources/parser_test.go @@ -0,0 +1,76 @@ +package resources + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + dashboardV0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" + dashboardV1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1alpha1" + + "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" +) + +func TestParer(t *testing.T) { + clients := NewMockResourceClients(t) + clients.On("ForKind", dashboardV0.DashboardResourceInfo.GroupVersionKind()). + Return(nil, dashboardV0.DashboardResourceInfo.GroupVersionResource(), nil).Maybe() + clients.On("ForKind", dashboardV1.DashboardResourceInfo.GroupVersionKind()). + Return(nil, dashboardV1.DashboardResourceInfo.GroupVersionResource(), nil).Maybe() + + parser := &Parser{ + repo: v0alpha1.ResourceRepositoryInfo{ + Type: v0alpha1.LocalRepositoryType, + Namespace: "xxx", + Name: "repo", + }, + clients: clients, + } + + t.Run("invalid input", func(t *testing.T) { + _, err := parser.Parse(context.Background(), &repository.FileInfo{ + Data: []byte("hello"), // not a real resource + }, false) + require.Error(t, err) + require.Equal(t, "classic resource must be JSON", err.Error()) + }) + + t.Run("dashboard parsing (with and without name)", func(t *testing.T) { + dash, err := parser.Parse(context.Background(), &repository.FileInfo{ + Data: []byte(`apiVersion: dashboard.grafana.app/v0alpha1 +kind: Dashboard +metadata: + name: test-v0 +spec: + title: Test dashboard +`), + }, false) + require.NoError(t, err) + require.Equal(t, "test-v0", dash.Obj.GetName()) + require.Equal(t, "dashboard.grafana.app", dash.GVK.Group) + require.Equal(t, "v0alpha1", dash.GVK.Version) + require.Equal(t, "dashboard.grafana.app", dash.GVR.Group) + require.Equal(t, "v0alpha1", dash.GVR.Version) + + // Now try again without a name + dash, err = parser.Parse(context.Background(), &repository.FileInfo{ + Data: []byte(`apiVersion: dashboard.grafana.app/v1alpha1 +kind: Dashboard +spec: + title: Test dashboard +`), + }, false) + require.NoError(t, err) // parsed, but has internal error + require.NotEmpty(t, dash.Errors) + + // Read the name from classic grafana format + dash, err = parser.Parse(context.Background(), &repository.FileInfo{ + Data: []byte(`{ "uid": "test", "schemaVersion": 30, "panels": [], "tags": [] }`), + }, false) + require.NoError(t, err) + require.Equal(t, v0alpha1.ClassicDashboard, dash.Classic) + require.Equal(t, "test", dash.Obj.GetName()) + }) +} diff --git a/pkg/registry/apis/provisioning/resources/resources.go b/pkg/registry/apis/provisioning/resources/resources.go index b1899a4865d..b6a14f9f8fb 100644 --- a/pkg/registry/apis/provisioning/resources/resources.go +++ b/pkg/registry/apis/provisioning/resources/resources.go @@ -7,21 +7,22 @@ import ( "errors" "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" ) var ErrAlreadyInRepository = errors.New("already in repository") type WriteOptions struct { - Identifier bool - Path string - Ref string + Path string + Ref string } type resourceID struct { @@ -34,12 +35,12 @@ type ResourcesManager struct { repo repository.ReaderWriter folders *FolderManager parser *Parser - clients *ResourceClients + clients ResourceClients userInfo map[string]repository.CommitSignature resourcesLookup map[resourceID]string // the path with this k8s name } -func NewResourcesManager(repo repository.ReaderWriter, folders *FolderManager, parser *Parser, clients *ResourceClients, userInfo map[string]repository.CommitSignature) *ResourcesManager { +func NewResourcesManager(repo repository.ReaderWriter, folders *FolderManager, parser *Parser, clients ResourceClients, userInfo map[string]repository.CommitSignature) *ResourcesManager { return &ResourcesManager{ repo: repo, folders: folders, @@ -75,6 +76,11 @@ func (r *ResourcesManager) CreateResourceFileFromObject(ctx context.Context, obj ctx = r.withAuthorSignature(ctx, meta) name := meta.GetName() + if name == "" { + return "", field.Required(field.NewPath("name", "metadata", "name"), + "An explicit name must be saved in the resource") + } + manager, _ := meta.GetManagerProperties() // TODO: how we should handle this? if manager.Identity == r.repo.Config().GetName() { @@ -101,9 +107,8 @@ func (r *ResourcesManager) CreateResourceFileFromObject(ctx context.Context, obj // Clear the metadata delete(obj.Object, "metadata") - if options.Identifier { - meta.SetName(name) // keep the identifier in the metadata - } + // Always write the identifier + meta.SetName(name) body, err := json.MarshalIndent(obj.Object, "", " ") if err != nil { 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 a48c9e80902..c74aaedf061 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -2474,9 +2474,6 @@ }, "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.ExportJobOptions": { "type": "object", - "required": [ - "identifier" - ], "properties": { "branch": { "description": "Target branch for export (only git)", @@ -2486,11 +2483,6 @@ "description": "The source folder (or empty) to export", "type": "string" }, - "identifier": { - "description": "Include the identifier in the exported metadata", - "type": "boolean", - "default": false - }, "path": { "description": "Prefix in target file system", "type": "string" @@ -2986,18 +2978,10 @@ }, "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.MigrateJobOptions": { "type": "object", - "required": [ - "identifier" - ], "properties": { "history": { "description": "Preserve history (if possible)", "type": "boolean" - }, - "identifier": { - "description": "Include the identifier in the exported metadata", - "type": "boolean", - "default": false } } }, diff --git a/pkg/tests/apis/provisioning/helper_test.go b/pkg/tests/apis/provisioning/helper_test.go index e46d3fdfc04..cf2426119b0 100644 --- a/pkg/tests/apis/provisioning/helper_test.go +++ b/pkg/tests/apis/provisioning/helper_test.go @@ -109,7 +109,7 @@ func (h *provisioningTestHelper) AwaitJobSuccess(t *testing.T, ctx context.Conte state := mustNestedString(result.Object, "status", "state") require.Equal(t, string(provisioning.JobStateSuccess), state, "historic job '%s' was not successful", job.GetName()) - }, time.Second*5, time.Millisecond*20) { + }, time.Second*10, time.Millisecond*25) { // We also want to add the job details to the error when it fails. job, err := h.Jobs.Resource.Get(ctx, job.GetName(), metav1.GetOptions{}) if err != nil { diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index 2f1ca0c1ec4..94939fc5fd0 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -2,6 +2,7 @@ package provisioning import ( "context" + "encoding/json" "net/http" "os" "path/filepath" @@ -16,6 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/grafana/grafana/pkg/apimachinery/utils" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/infra/usagestats" @@ -196,7 +198,7 @@ func TestIntegrationProvisioning_CreatingGitHubRepository(t *testing.T) { assert.Contains(t, names, "WZ7AhQiVz", "should contain dashboard2.yaml's contents") } -func TestIntegrationProvisioning_SafePathUsages(t *testing.T) { +func TestIntegrationProvisioning_RunLocalRepository(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } @@ -204,41 +206,111 @@ func TestIntegrationProvisioning_SafePathUsages(t *testing.T) { helper := runGrafana(t) ctx := context.Background() - const repo = "local-safe-path-usages" + const allPanels = "n1jR8vnnz" + const repo = "local-local-examples" + const targetPath = "all-panels.json" + // Set up the repository. localTmp := helper.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{"Name": repo}) - _, err := helper.Repositories.Resource.Create(ctx, localTmp, metav1.CreateOptions{}) + obj, err := helper.Repositories.Resource.Create(ctx, localTmp, metav1.CreateOptions{}) require.NoError(t, err) + name, _, _ := unstructured.NestedString(obj.Object, "metadata", "name") + require.Equal(t, repo, name, "wrote the expected name") - // Write a file - result := helper.AdminREST.Post(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("files", "all-panels.json"). - Body(helper.LoadFile("testdata/all-panels.json")). - SetHeader("Content-Type", "application/json"). - Do(ctx) - require.NoError(t, result.Error(), "expecting to be able to create file") + // Write a file -- this will create it *both* in the local file system, and in grafana + t.Run("write all panels", func(t *testing.T) { + code := 0 + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", targetPath). + Body(helper.LoadFile("testdata/all-panels.json")). + SetHeader("Content-Type", "application/json"). + Do(ctx).StatusCode(&code) + require.NoError(t, result.Error(), "expecting to be able to create file") + wrapper := &provisioning.ResourceWrapper{} + raw, err := result.Raw() + require.NoError(t, err) + err = json.Unmarshal(raw, wrapper) + require.NoError(t, err) + require.Equal(t, 200, code, "expected 200 response") + require.Equal(t, provisioning.ClassicDashboard, wrapper.Resource.Type.Classic) + name, _, _ := unstructured.NestedString(wrapper.Resource.File.Object, "metadata", "name") + require.Equal(t, allPanels, name, "name from classic UID") + name, _, _ = unstructured.NestedString(wrapper.Resource.Upsert.Object, "metadata", "name") + require.Equal(t, allPanels, name, "save the name from the request") - // Write a file with a bad path - result = helper.AdminREST.Post(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("files", "test", "..", "..", "all-panels.json"). - Body(helper.LoadFile("testdata/all-panels.json")). - SetHeader("Content-Type", "application/json"). - Do(ctx) - require.Error(t, result.Error(), "invalid path should return error") + // Get the file from the grafana database + obj, err := helper.Dashboards.Resource.Get(ctx, allPanels, metav1.GetOptions{}) + require.NoError(t, err, "the value should be saved in grafana") + val, _, _ := unstructured.NestedString(obj.Object, "metadata", "annotations", utils.AnnoKeyManagerKind) + require.Equal(t, string(utils.ManagerKindRepo), val, "should have repo annotations") + val, _, _ = unstructured.NestedString(obj.Object, "metadata", "annotations", utils.AnnoKeyManagerIdentity) + require.Equal(t, repo, val, "should have repo annotations") - // Read a file - _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "all-panels.json") - require.NoError(t, err, "valid path should be fine") + // Read the file we wrote + obj, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", targetPath) + require.NoError(t, err, "read value") + name, _, _ = unstructured.NestedString(obj.Object, "resource", "file", "metadata", "name") + require.Equal(t, allPanels, name, "read the name out of the saved file") + }) - // Read a file with a bad path - _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "../../all-panels.json") - require.Error(t, err, "invalid path should not be fine") + t.Run("fail using invalid paths", func(t *testing.T) { + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", "test", "..", "..", "all-panels.json"). // UNSAFE PATH + Body(helper.LoadFile("testdata/all-panels.json")). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.Error(t, result.Error(), "invalid path should return error") + + // Read a file with a bad path + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "../../all-panels.json") + require.Error(t, err, "invalid path should error") + }) + + t.Run("require name or generateName", func(t *testing.T) { + code := 0 + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", "example.json"). + Body([]byte(`apiVersion: dashboard.grafana.app/v0alpha1 +kind: Dashboard +spec: + title: Test dashboard +`)).Do(ctx).StatusCode(&code) + require.Error(t, result.Error(), "missing name") + + result = helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", "example.json"). + Body([]byte(`apiVersion: dashboard.grafana.app/v0alpha1 +kind: Dashboard +metadata: + generateName: prefix- +spec: + title: Test dashboard +`)).Do(ctx).StatusCode(&code) + require.NoError(t, result.Error(), "should create name") + require.Equal(t, 200, code, "expect OK result") + + raw, err := result.Raw() + require.NoError(t, err) + + obj := &unstructured.Unstructured{} + err = json.Unmarshal(raw, obj) + require.NoError(t, err) + + name, _, _ = unstructured.NestedString(obj.Object, "resource", "upsert", "metadata", "name") + require.True(t, strings.HasPrefix(name, "prefix-"), "should generate name") + }) } func TestIntegrationProvisioning_ImportAllPanelsFromLocalRepository(t *testing.T) { @@ -319,9 +391,8 @@ func TestProvisioning_ExportUnifiedToRepository(t *testing.T) { SetHeader("Content-Type", "application/json"). Body(asJSON(&provisioning.JobSpec{ Push: &provisioning.ExportJobOptions{ - Folder: "", // export entire instance - Path: "", // no prefix necessary for testing - Identifier: true, // doesn't _really_ matter, but handy for debugging. + Folder: "", // export entire instance + Path: "", // no prefix necessary for testing }, })). Do(ctx) diff --git a/public/app/api/clients/provisioning/endpoints.gen.ts b/public/app/api/clients/provisioning/endpoints.gen.ts index d198a13454d..004fb55d01e 100644 --- a/public/app/api/clients/provisioning/endpoints.gen.ts +++ b/public/app/api/clients/provisioning/endpoints.gen.ts @@ -727,8 +727,6 @@ export type ObjectMeta = { export type MigrateJobOptions = { /** Preserve history (if possible) */ history?: boolean; - /** Include the identifier in the exported metadata */ - identifier: boolean; }; export type PullRequestJobOptions = { hash?: string; @@ -748,8 +746,6 @@ export type ExportJobOptions = { branch?: string; /** The source folder (or empty) to export */ folder?: string; - /** Include the identifier in the exported metadata */ - identifier: boolean; /** Prefix in target file system */ path?: string; }; diff --git a/public/app/features/provisioning/Wizard/BootstrapStep.tsx b/public/app/features/provisioning/Wizard/BootstrapStep.tsx index 37b0b05b53b..3d65ece14bc 100644 --- a/public/app/features/provisioning/Wizard/BootstrapStep.tsx +++ b/public/app/features/provisioning/Wizard/BootstrapStep.tsx @@ -84,6 +84,9 @@ export function BootstrapStep({ onOptionSelect, settingsData, repoName }: Props) ); } + // Show the history selection + const canIncludeHistory = repoType === 'github' && settingsData?.legacyStorage; + return ( @@ -160,42 +163,29 @@ export function BootstrapStep({ onOptionSelect, settingsData, repoName }: Props) /> )} -
- - - - - Include identifiers - - - - + {canIncludeHistory && ( +
+ + {canIncludeHistory && ( + + + + Include history + + + + + + )} - {repoType === 'github' && settingsData?.legacyStorage && ( - - - - Include history - - - - - - )} - -
+
+ )} )} diff --git a/public/app/features/provisioning/Wizard/MigrateStep.tsx b/public/app/features/provisioning/Wizard/MigrateStep.tsx index 4d212b6207c..3c950a5ba6f 100644 --- a/public/app/features/provisioning/Wizard/MigrateStep.tsx +++ b/public/app/features/provisioning/Wizard/MigrateStep.tsx @@ -15,7 +15,6 @@ export interface MigrateStepProps { export function MigrateStep({ onStepUpdate }: MigrateStepProps) { const [createJob] = useCreateRepositoryJobsMutation(); const { watch } = useFormContext(); - const identifier = watch('migrate.identifier'); const history = watch('migrate.history'); const startMigration = async (repositoryName: string) => { @@ -23,7 +22,6 @@ export function MigrateStep({ onStepUpdate }: MigrateStepProps) { name: repositoryName, jobSpec: { migrate: { - identifier, history, }, }, diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 1fe2a3cfa87..fd7ab959871 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -5110,7 +5110,6 @@ "files-count_other": "{{count}} files", "grafana": "Grafana", "include-history": "Include history", - "include-identifiers": "Include identifiers", "label-display-name": "Display name", "label-migrate-options": "Migrate options", "placeholder-my-repository-connection": "My repository connection", @@ -5119,8 +5118,7 @@ "text-loading-resource-information": "Loading resource information...", "title-files-exist-in-the-target": "Files exist in the target", "title-note": "Note", - "tooltip-include-history": "Include complete dashboard version history", - "tooltip-include-identifiers": "Include unique identifiers for each dashboard to maintain references" + "tooltip-include-history": "Include complete dashboard version history" }, "check-repository": { "check": "Check"