diff --git a/pkg/registry/apis/folders/parents.go b/pkg/registry/apis/folders/parents.go new file mode 100644 index 00000000000..61d694b3e36 --- /dev/null +++ b/pkg/registry/apis/folders/parents.go @@ -0,0 +1,88 @@ +package folders + +import ( + "context" + "fmt" + "slices" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apiserver/pkg/registry/rest" + + folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" + "github.com/grafana/grafana/pkg/apimachinery/utils" + folderLegacy "github.com/grafana/grafana/pkg/services/folder" +) + +type parentsGetter = func(ctx context.Context, folder *folders.Folder) (*folders.FolderInfoList, error) + +func newParentsGetter(getter rest.Getter, maxDepth int) parentsGetter { + return func(ctx context.Context, folder *folders.Folder) (*folders.FolderInfoList, error) { + info := &folders.FolderInfoList{ + Items: []folders.FolderInfo{}, + } + id := folder.Name + if id == folderLegacy.GeneralFolderUID || id == folderLegacy.SharedWithMeFolderUID { + info.Items = []folders.FolderInfo{{ + Name: folder.Name, + Title: folder.Spec.Title, + }} + return info, nil + } + + found := make(map[string]bool) + found[folder.Name] = true + var err error + + for folder != nil { + meta, _ := utils.MetaAccessor(folder) + item := folders.FolderInfo{ + Name: folder.Name, + Title: folder.Spec.Title, + Parent: meta.GetFolder(), + } + if folder.Spec.Description != nil { + item.Description = *folder.Spec.Description + } + info.Items = append(info.Items, item) + if item.Parent == "" { + break + } + + if found[item.Parent] { + return nil, fmt.Errorf("cyclic folder references found: %s", item.Parent) + } + + obj, e2 := getter.Get(ctx, item.Parent, &metav1.GetOptions{}) + if e2 != nil { + info.Items = append(info.Items, folders.FolderInfo{ + Name: item.Parent, + Detached: true, + Description: e2.Error(), + }) + break + } + + parentFolder, ok := obj.(*folders.Folder) + if !ok { + info.Items = append(info.Items, folders.FolderInfo{ + Name: item.Parent, + Detached: true, + Description: fmt.Sprintf("expected folder, found: %T", obj), + }) + break + } + + if len(info.Items) >= maxDepth { + err = folderLegacy.ErrMaximumDepthReached + break + } + + found[parentFolder.Name] = true + folder = parentFolder + } + + // Start from the root + slices.Reverse(info.Items) + return info, err + } +} diff --git a/pkg/registry/apis/folders/parents_test.go b/pkg/registry/apis/folders/parents_test.go new file mode 100644 index 00000000000..9ecc84e4954 --- /dev/null +++ b/pkg/registry/apis/folders/parents_test.go @@ -0,0 +1,187 @@ +package folders + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" + "github.com/grafana/grafana/pkg/apimachinery/utils" + grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" +) + +func TestParents(t *testing.T) { + type input struct { + name string + folder string + } + tests := []struct { + name string + request struct { + name string + folder string + } + getter map[string]*folders.Folder + setupFn func(*mock.Mock) // called after the getter is registered + expected *folders.FolderInfoList + expectedErr string + maxDepth int // defaults to 5 unless set + }{ + { + name: "no parents", + request: input{ + name: "test", + }, + expected: &folders.FolderInfoList{Items: []folders.FolderInfo{ + {Name: "test"}, + }}, + }, + { + name: "has a parent", + request: input{ + name: "test", + folder: "parent", + }, + expected: &folders.FolderInfoList{Items: []folders.FolderInfo{ + {Name: "test", Parent: "parent"}, + {Name: "parent"}, + }}, + }, + { + name: "general has no parent", + request: input{ + name: "general", + }, + getter: map[string]*folders.Folder{}, + expected: &folders.FolderInfoList{Items: []folders.FolderInfo{ + {Name: "general"}, + }}, + }, + { + name: "error in parent", + request: input{ + name: "test", + folder: "parent", // NOTE that parent is not found + }, + setupFn: func(m *mock.Mock) { + var nothing *folders.Folder // needs to be an object + m.On("Get", context.TODO(), "parent", &metav1.GetOptions{}).Return( + nothing, fmt.Errorf("custom error message")).Once() + }, + expected: &folders.FolderInfoList{Items: []folders.FolderInfo{ + {Name: "test", Parent: "parent"}, + {Name: "parent", Detached: true, Description: "custom error message"}, + }}, + }, + { + name: "parent is not a folder", + request: input{ + name: "test", + folder: "parent", // not a folder + }, + setupFn: func(m *mock.Mock) { + m.On("Get", context.TODO(), "parent", &metav1.GetOptions{}).Return( + &unstructured.Unstructured{}, // not a folder + nil).Once() + }, + expected: &folders.FolderInfoList{Items: []folders.FolderInfo{ + {Name: "test", Parent: "parent"}, + {Name: "parent", Detached: true, Description: "expected folder, found: *unstructured.Unstructured"}, + }}, + }, + { + name: "avoid cycles", + request: input{ + name: "test", + folder: "test", + }, + setupFn: func(m *mock.Mock) { + m.On("Get", context.TODO(), "test", &metav1.GetOptions{}).Return( + &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.AnnoKeyFolder: "test", // invalid! this will cycle + }, + }, + }, nil).Times(2) + }, + expectedErr: "cyclic folder references found", + }, + { + name: "too deep", + request: input{ + name: "test", + folder: "p1", + }, + maxDepth: 3, + expectedErr: "[folder.maximum-depth-reached]", + expected: &folders.FolderInfoList{Items: []folders.FolderInfo{ + {Name: "test", Parent: "p1"}, + {Name: "p1", Parent: "p2"}, + {Name: "p2", Parent: "p3"}, + {Name: "p3", Parent: "p4"}, // should not try calling p4 + }}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := (grafanarest.Storage)(nil) + m := &mock.Mock{} + if tt.getter == nil && tt.setupFn == nil { + // Default to filling the getter with expected results + for _, item := range tt.expected.Items { + m.On("Get", context.TODO(), item.Name, &metav1.GetOptions{}).Return( + &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: item.Name, + Annotations: map[string]string{ + utils.AnnoKeyFolder: item.Parent, + }, + }, + Spec: folders.FolderSpec{ + Title: item.Title, + Description: &item.Description, + }, + }, nil) // we don't care how often they are called + } + } else { + for k, v := range tt.getter { + v.Name = k // set the name + m.On("Get", context.TODO(), k, &metav1.GetOptions{}).Return(v, nil).Once() + } + if tt.setupFn != nil { + tt.setupFn(m) + } + } + + gm := storageMock{m, s} + maxDepth := tt.maxDepth + if maxDepth == 0 { + maxDepth = 5 + } + + getter := newParentsGetter(gm, maxDepth) + parents, err := getter(context.TODO(), &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.request.name, + Annotations: map[string]string{ + utils.AnnoKeyFolder: tt.request.folder, + }}, + }) + if tt.expectedErr == "" { + require.NoError(t, err) + require.NotNil(t, parents) + require.ElementsMatch(t, tt.expected.Items, parents.Items) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), tt.expectedErr) + } + }) + } +} diff --git a/pkg/registry/apis/folders/register.go b/pkg/registry/apis/folders/register.go index 397cbcb94a5..51a0d6d82dc 100644 --- a/pkg/registry/apis/folders/register.go +++ b/pkg/registry/apis/folders/register.go @@ -18,23 +18,19 @@ import ( "k8s.io/kube-openapi/pkg/spec3" authtypes "github.com/grafana/authlib/types" - folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" "github.com/grafana/grafana/pkg/apimachinery/identity" - "github.com/grafana/grafana/pkg/apimachinery/utils" grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apiserver/builder" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" - "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/apistore" "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/resourcepb" - "github.com/grafana/grafana/pkg/util" ) var _ builder.APIGroupBuilder = (*FolderAPIBuilder)(nil) @@ -57,6 +53,7 @@ type FolderAPIBuilder struct { storage grafanarest.Storage authorizer authorizer.Authorizer + parents parentsGetter searcher resourcepb.ResourceIndexClient cfg *setting.Cfg @@ -187,8 +184,10 @@ func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API } storage[resourceInfo.StoragePath()] = folderStore + b.parents = newParentsGetter(folderStore, folderValidationRules.maxDepth) // used for validation storage[resourceInfo.StoragePath("parents")] = &subParentsREST{ - getter: storage[resourceInfo.StoragePath()].(rest.Getter), // Get the parents + getter: folderStore, + parents: b.parents, } storage[resourceInfo.StoragePath("counts")] = &subCountREST{searcher: b.searcher} storage[resourceInfo.StoragePath("access")] = &subAccessREST{b.folderSvc, b.ac} @@ -222,11 +221,9 @@ func (b *FolderAPIBuilder) GetAuthorizer() authorizer.Authorizer { } var folderValidationRules = struct { - maxDepth int - invalidNames []string + maxDepth int }{ - maxDepth: 5, - invalidNames: []string{"general"}, + maxDepth: 5, // why different than folder.MaxNestedFolderDepth?? (4) } func (b *FolderAPIBuilder) Mutate(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) error { @@ -244,7 +241,6 @@ func (b *FolderAPIBuilder) Mutate(ctx context.Context, a admission.Attributes, _ } func (b *FolderAPIBuilder) Validate(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) error { - id := a.GetName() obj := a.GetObject() if obj == nil || a.GetOperation() == admission.Connect { return nil // This is normal for sub-resource @@ -254,150 +250,19 @@ func (b *FolderAPIBuilder) Validate(ctx context.Context, a admission.Attributes, if !ok { return fmt.Errorf("obj is not folders.Folder") } - verb := a.GetOperation() - switch verb { + switch a.GetOperation() { case admission.Create: - return b.validateOnCreate(ctx, id, obj) + return validateOnCreate(ctx, f, b.parents, folderValidationRules.maxDepth) case admission.Delete: - return b.validateOnDelete(ctx, f) + return validateOnDelete(ctx, f, b.searcher) case admission.Update: - old := a.GetOldObject() - if old == nil { - return fmt.Errorf("old object is nil") + old, ok := a.GetOldObject().(*folders.Folder) + if !ok { + return fmt.Errorf("obj is not folders.Folder") } - return b.validateOnUpdate(ctx, obj, old) - case admission.Connect: + return validateOnUpdate(ctx, f, old, b.storage, b.parents, folderValidationRules.maxDepth) + default: return nil } - return nil -} - -func (b *FolderAPIBuilder) validateOnDelete(ctx context.Context, f *folders.Folder) error { - resp, err := b.searcher.GetStats(ctx, &resourcepb.ResourceStatsRequest{Namespace: f.Namespace, Folder: f.Name}) - if err != nil { - return err - } - - if resp != nil && resp.Error != nil { - return fmt.Errorf("could not verify if folder is empty: %v", resp.Error) - } - - if resp.Stats == nil { - return fmt.Errorf("could not verify if folder is empty: %v", resp.Error) - } - - for _, v := range resp.Stats { - if v.Count > 0 { - return folder.ErrFolderNotEmpty - } - } - - return nil -} - -func (b *FolderAPIBuilder) validateOnCreate(ctx context.Context, id string, obj runtime.Object) error { - for _, invalidName := range folderValidationRules.invalidNames { - if id == invalidName { - return dashboards.ErrFolderInvalidUID - } - } - - if !util.IsValidShortUID(id) { - return dashboards.ErrDashboardInvalidUid - } - - if util.IsShortUIDTooLong(id) { - return dashboards.ErrDashboardUidTooLong - } - - f, ok := obj.(*folders.Folder) - if !ok { - return fmt.Errorf("obj is not folders.Folder") - } - if f.Spec.Title == "" { - return dashboards.ErrFolderTitleEmpty - } - - if f.Name == getParent(obj) { - return folder.ErrFolderCannotBeParentOfItself - } - - _, err := b.checkFolderMaxDepth(ctx, obj) - if err != nil { - return err - } - - return err -} - -func getParent(o runtime.Object) string { - meta, err := utils.MetaAccessor(o) - if err != nil { - return "" - } - return meta.GetFolder() -} - -func (b *FolderAPIBuilder) checkFolderMaxDepth(ctx context.Context, obj runtime.Object) ([]string, error) { - var parents = []string{} - for i := 0; i < folderValidationRules.maxDepth; i++ { - parent := getParent(obj) - if parent == "" { - break - } - parents = append(parents, parent) - if i+1 == folderValidationRules.maxDepth { - return parents, folder.ErrMaximumDepthReached - } - - parentObj, err := b.storage.Get(ctx, parent, &metav1.GetOptions{}) - if err != nil { - return parents, err - } - obj = parentObj - } - return parents, nil -} - -func (b *FolderAPIBuilder) validateOnUpdate(ctx context.Context, obj, old runtime.Object) error { - f, ok := obj.(*folders.Folder) - if !ok { - return fmt.Errorf("obj is not folders.Folder") - } - - fOld, ok := old.(*folders.Folder) - if !ok { - return fmt.Errorf("obj is not folders.Folder") - } - var newParent = getParent(obj) - if newParent != getParent(fOld) { - // it's a move operation - return b.validateMove(ctx, obj, newParent) - } - // it's a spec update - if f.Spec.Title == "" { - return dashboards.ErrFolderTitleEmpty - } - return nil -} - -func (b *FolderAPIBuilder) validateMove(ctx context.Context, obj runtime.Object, newParent string) error { - // folder cannot be moved to a k6 folder - if newParent == accesscontrol.K6FolderUID { - return fmt.Errorf("k6 project may not be moved") - } - - //FIXME: until we have a way to represent the tree, we can only - // look at folder parents to check how deep the new folder tree will be - parents, err := b.checkFolderMaxDepth(ctx, obj) - if err != nil { - return err - } - - // if by moving a folder we exceed the max depth, return an error - if len(parents)+1 >= folderValidationRules.maxDepth { - return folder.ErrMaximumDepthReached - } - return nil } diff --git a/pkg/registry/apis/folders/register_test.go b/pkg/registry/apis/folders/register_test.go index f4ab009ec10..1c23d761393 100644 --- a/pkg/registry/apis/folders/register_test.go +++ b/pkg/registry/apis/folders/register_test.go @@ -27,23 +27,6 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) { name string } - initialMaxDepth := folderValidationRules.maxDepth - folderValidationRules.maxDepth = 2 - defer func() { folderValidationRules.maxDepth = initialMaxDepth }() - deepFolder := &folders.Folder{ - Spec: folders.FolderSpec{ - Title: "foo", - }, - } - deepFolder.Name = "valid-parent" - deepFolder.Annotations = map[string]string{"grafana.app/folder": "valid-grandparent"} - parentFolder := &folders.Folder{ - Spec: folders.FolderSpec{ - Title: "foo-grandparent", - }, - } - deepFolder.Name = "valid-grandparent" - tests := []struct { name string input input @@ -58,7 +41,7 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) { Title: "foo", }, }, - name: folderValidationRules.invalidNames[0], + name: "general", }, err: dashboards.ErrFolderInvalidUID, }, @@ -81,16 +64,24 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) { Title: "foo", }, }, - annotations: map[string]string{"grafana.app/folder": "valid-parent"}, + annotations: map[string]string{"grafana.app/folder": "p1"}, // already max depth name: "valid-name", }, setupFn: func(m *mock.Mock) { - m.On("Get", mock.Anything, "valid-parent", mock.Anything).Return( - deepFolder, - nil) - m.On("Get", mock.Anything, "valid-grandparent", mock.Anything).Return( - parentFolder, - nil) + m.On("Get", mock.Anything, "p1", mock.Anything).Return( + &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + Annotations: map[string]string{"grafana.app/folder": "p2"}, + }, + }, nil) + m.On("Get", mock.Anything, "p2", mock.Anything).Return( + &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p2", + Annotations: map[string]string{"grafana.app/folder": "p3"}, + }, + }, nil) }, err: folder.ErrMaximumDepthReached, }, @@ -121,20 +112,21 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) { }, } - s := (grafanarest.Storage)(nil) - m := &mock.Mock{} - us := storageMock{m, s} - - b := &FolderAPIBuilder{ - gv: resourceInfo.GroupVersion(), - features: nil, - namespacer: func(_ int64) string { return "123" }, - folderSvc: foldertest.NewFakeService(), - storage: us, - } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + s := (grafanarest.Storage)(nil) + m := &mock.Mock{} + us := storageMock{m, s} + + b := &FolderAPIBuilder{ + gv: resourceInfo.GroupVersion(), + features: nil, + namespacer: func(_ int64) string { return "123" }, + folderSvc: foldertest.NewFakeService(), + storage: us, + parents: newParentsGetter(us, 2), // Max Depth of 2 + } + tt.input.obj.Name = tt.input.name tt.input.obj.Annotations = tt.input.annotations @@ -243,14 +235,6 @@ func TestFolderAPIBuilder_Validate_Delete(t *testing.T) { } func TestFolderAPIBuilder_Validate_Update(t *testing.T) { - var circularObj = &folders.Folder{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "stacks-123", - Name: "new-parent", - Annotations: map[string]string{"grafana.app/folder": "new-parent"}, - }, - } - tests := []struct { name string updatedObj *folders.Folder @@ -333,7 +317,7 @@ func TestFolderAPIBuilder_Validate_Update(t *testing.T) { wantErr: true, }, { - name: "should not allow moving to a folder that is too deep", + name: "should not allow moving to a folder that will become too deep", updatedObj: &folders.Folder{ Spec: folders.FolderSpec{ Title: "foo", @@ -346,18 +330,31 @@ func TestFolderAPIBuilder_Validate_Update(t *testing.T) { }, setupFn: func(m *mock.Mock) { m.On("Get", mock.Anything, "new-parent", mock.Anything).Return( - circularObj, - nil) + &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + Annotations: map[string]string{"grafana.app/folder": "p2"}, + }, + }, nil) + m.On("Get", mock.Anything, "p2", mock.Anything).Return( + &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p2", + Annotations: map[string]string{"grafana.app/folder": "p3"}, + }, + }, nil) + m.On("Get", mock.Anything, "p3", mock.Anything).Return( + &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p3", + Annotations: map[string]string{"grafana.app/folder": "p4"}, + }, + }, nil) }, wantErr: true, }, } - s := (grafanarest.Storage)(nil) - m := &mock.Mock{} - us := storageMock{m, s} - sm := searcherMock{Mock: m} - obj := &folders.Folder{ Spec: folders.FolderSpec{ Title: "foo", @@ -370,10 +367,15 @@ func TestFolderAPIBuilder_Validate_Update(t *testing.T) { } for _, tt := range tests { - if tt.setupFn != nil { - tt.setupFn(m) - } t.Run(tt.name, func(t *testing.T) { + s := (grafanarest.Storage)(nil) + m := &mock.Mock{} + us := storageMock{m, s} + sm := searcherMock{Mock: m} + if tt.setupFn != nil { + tt.setupFn(m) + } + b := &FolderAPIBuilder{ gv: resourceInfo.GroupVersion(), features: nil, @@ -381,6 +383,7 @@ func TestFolderAPIBuilder_Validate_Update(t *testing.T) { folderSvc: foldertest.NewFakeService(), storage: us, searcher: sm, + parents: newParentsGetter(us, 2), // Max Depth of 2 } err := b.Validate(context.Background(), admission.NewAttributesRecord( @@ -465,20 +468,21 @@ func TestFolderAPIBuilder_Mutate_Create(t *testing.T) { wantErr: true, }, } - s := (grafanarest.Storage)(nil) - m := &mock.Mock{} - us := storageMock{m, s} - sm := searcherMock{Mock: m} - b := &FolderAPIBuilder{ - gv: resourceInfo.GroupVersion(), - features: nil, - namespacer: func(_ int64) string { return "123" }, - folderSvc: foldertest.NewFakeService(), - storage: us, - searcher: sm, - } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + s := (grafanarest.Storage)(nil) + m := &mock.Mock{} + us := storageMock{m, s} + sm := searcherMock{Mock: m} + b := &FolderAPIBuilder{ + gv: resourceInfo.GroupVersion(), + features: nil, + namespacer: func(_ int64) string { return "123" }, + folderSvc: foldertest.NewFakeService(), + storage: us, + searcher: sm, + parents: newParentsGetter(us, 2), // Max Depth of 2 + } admAttr := admission.NewAttributesRecord( tt.input, nil, @@ -587,6 +591,7 @@ func TestFolderAPIBuilder_Mutate_Update(t *testing.T) { folderSvc: foldertest.NewFakeService(), storage: us, searcher: sm, + parents: newParentsGetter(us, 2), // Max Depth of 2 } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/registry/apis/folders/sub_children.go b/pkg/registry/apis/folders/sub_children.go index 92f84093ffd..13dccda8d73 100644 --- a/pkg/registry/apis/folders/sub_children.go +++ b/pkg/registry/apis/folders/sub_children.go @@ -10,6 +10,8 @@ import ( "k8s.io/apiserver/pkg/registry/rest" folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" + "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/services/folder" ) type subChildrenREST struct { @@ -19,9 +21,6 @@ type subChildrenREST struct { var _ = rest.Connecter(&subChildrenREST{}) var _ = rest.StorageMetadata(&subChildrenREST{}) -// RootFolderName Hardcoded magic const to get root folders without parent. -var RootFolderName = "general" - func (r *subChildrenREST) New() runtime.Object { return &folders.FolderList{} } @@ -46,7 +45,10 @@ func (r *subChildrenREST) NewConnectOptions() (runtime.Object, bool, string) { } func (r *subChildrenREST) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { - obj, err := r.lister.List(ctx, &internalversion.ListOptions{}) + obj, err := r.lister.List(ctx, &internalversion.ListOptions{ + Limit: 500, + // TODO, field selector + }) if err != nil { return nil, err } @@ -54,16 +56,23 @@ func (r *subChildrenREST) Connect(ctx context.Context, name string, opts runtime if !ok { return nil, fmt.Errorf("could not list folders") } + if allFolders.Continue != "" { + return nil, fmt.Errorf("found too many folders to process") + } + + if name == folder.GeneralFolderUID { + name = "" // general is empty + } return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { children := &folders.FolderList{} - parentName := "" - - if name != RootFolderName { - parentName = name - } for _, folder := range allFolders.Items { - if parentName == getParent(&folder) { + v, err := utils.MetaAccessor(folder) + if err != nil { + continue + } + + if name == v.GetFolder() { children.Items = append(children.Items, folder) } } diff --git a/pkg/registry/apis/folders/sub_parents.go b/pkg/registry/apis/folders/sub_parents.go index 53abaa59e87..220921570d7 100644 --- a/pkg/registry/apis/folders/sub_parents.go +++ b/pkg/registry/apis/folders/sub_parents.go @@ -4,20 +4,19 @@ import ( "context" "fmt" "net/http" - "slices" - - "github.com/grafana/grafana/pkg/services/folder" - "k8s.io/apiserver/pkg/storage" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" + "k8s.io/apiserver/pkg/storage" folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" + "github.com/grafana/grafana/pkg/services/folder" ) type subParentsREST struct { - getter rest.Getter + getter rest.Getter + parents parentsGetter } var _ = rest.Connecter(&subParentsREST{}) @@ -71,53 +70,11 @@ func (r *subParentsREST) Connect(ctx context.Context, name string, opts runtime. return } - info := r.parents(ctx, folderObj) - // Start from the root - slices.Reverse(info.Items) + info, err := r.parents(ctx, folderObj) + if err != nil { + responder.Error(err) + return + } responder.Object(http.StatusOK, info) }), nil } - -func (r *subParentsREST) parents(ctx context.Context, folder *folders.Folder) *folders.FolderInfoList { - info := &folders.FolderInfoList{ - Items: []folders.FolderInfo{}, - } - for folder != nil { - parent := getParent(folder) - descr := "" - if folder.Spec.Description != nil { - descr = *folder.Spec.Description - } - info.Items = append(info.Items, folders.FolderInfo{ - Name: folder.Name, - Title: folder.Spec.Title, - Description: descr, - Parent: parent, - }) - if parent == "" { - break - } - - obj, err := r.getter.Get(ctx, parent, &metav1.GetOptions{}) - if err != nil { - info.Items = append(info.Items, folders.FolderInfo{ - Name: parent, - Detached: true, - Description: err.Error(), - }) - break - } - - parentFolder, ok := obj.(*folders.Folder) - if !ok { - info.Items = append(info.Items, folders.FolderInfo{ - Name: parent, - Detached: true, - Description: fmt.Sprintf("expected folder, found: %T", obj), - }) - break - } - folder = parentFolder - } - return info -} diff --git a/pkg/registry/apis/folders/sub_parents_test.go b/pkg/registry/apis/folders/sub_parents_test.go deleted file mode 100644 index 8d184cc8980..00000000000 --- a/pkg/registry/apis/folders/sub_parents_test.go +++ /dev/null @@ -1,78 +0,0 @@ -package folders - -import ( - "context" - "testing" - - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" - grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" -) - -func TestSubParent(t *testing.T) { - tests := []struct { - name string - input *folders.Folder - expected *folders.FolderInfoList - setuFn func(*mock.Mock) - }{ - { - name: "no parents", - input: &folders.Folder{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Annotations: map[string]string{}, - }, - Spec: folders.FolderSpec{ - Title: "some tittle", - }, - }, - expected: &folders.FolderInfoList{Items: []folders.FolderInfo{{Name: "test", Title: "some tittle"}}}, - }, - { - name: "has a parent", - input: &folders.Folder{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Annotations: map[string]string{"grafana.app/folder": "parent-test"}, - }, - Spec: folders.FolderSpec{ - Title: "some tittle", - }, - }, - setuFn: func(m *mock.Mock) { - m.On("Get", context.TODO(), "parent-test", &metav1.GetOptions{}).Return(&folders.Folder{ - ObjectMeta: metav1.ObjectMeta{ - Name: "parent-test", - Annotations: map[string]string{}, - }, - Spec: folders.FolderSpec{ - Title: "some other tittle", - }, - }, nil).Once() - }, - expected: &folders.FolderInfoList{Items: []folders.FolderInfo{ - {Name: "test", Title: "some tittle", Parent: "parent-test"}, - {Name: "parent-test", Title: "some other tittle"}}, - }}, - } - s := (grafanarest.Storage)(nil) - m := &mock.Mock{} - gm := storageMock{m, s} - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - r := &subParentsREST{ - getter: gm, - } - if tt.setuFn != nil { - tt.setuFn(m) - } - parents := r.parents(context.TODO(), tt.input) - require.Equal(t, tt.expected, parents) - }) - } -} diff --git a/pkg/registry/apis/folders/validate.go b/pkg/registry/apis/folders/validate.go new file mode 100644 index 00000000000..1874a90f57e --- /dev/null +++ b/pkg/registry/apis/folders/validate.go @@ -0,0 +1,147 @@ +package folders + +import ( + "context" + "fmt" + "slices" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apiserver/pkg/registry/rest" + + folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" + "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/folder" + "github.com/grafana/grafana/pkg/storage/unified/resourcepb" + "github.com/grafana/grafana/pkg/util" +) + +func validateOnCreate(ctx context.Context, f *folders.Folder, getter parentsGetter, maxDepth int) error { + id := f.Name + + if slices.Contains([]string{ + folder.GeneralFolderUID, + folder.SharedWithMeFolderUID, + }, id) { + return dashboards.ErrFolderInvalidUID + } + + meta, err := utils.MetaAccessor(f) + if err != nil { + return fmt.Errorf("unable to read metadata from object: %w", err) + } + + if !util.IsValidShortUID(id) { + return dashboards.ErrDashboardInvalidUid + } + + if util.IsShortUIDTooLong(id) { + return dashboards.ErrDashboardUidTooLong + } + + if f.Spec.Title == "" { + return dashboards.ErrFolderTitleEmpty + } + + parentName := meta.GetFolder() + if parentName == "" { + return nil // OK, we do not need to validate the tree + } + + if parentName == f.Name { + return folder.ErrFolderCannotBeParentOfItself + } + + parents, err := getter(ctx, f) + if err != nil { + return fmt.Errorf("unable to create folder inside parent: %w", err) + } + + // Can not create a folder that will be too deep + if len(parents.Items)+1 > maxDepth { + return fmt.Errorf("folder max depth exceeded, max depth is %d", maxDepth) + } + + return nil +} + +func validateOnUpdate(ctx context.Context, + obj *folders.Folder, + old *folders.Folder, + getter rest.Getter, + parents parentsGetter, + maxDepth int, +) error { + folderObj, err := utils.MetaAccessor(obj) + if err != nil { + return err + } + oldFolder, err := utils.MetaAccessor(old) + if err != nil { + return err + } + + if obj.Spec.Title == "" { + return dashboards.ErrFolderTitleEmpty + } + + if folderObj.GetFolder() == oldFolder.GetFolder() { + return nil + } + + // Validate the move operation + newParent := folderObj.GetFolder() + + // folder cannot be moved to a k6 folder + if newParent == accesscontrol.K6FolderUID { + return fmt.Errorf("k6 project may not be moved") + } + + parentObj, err := getter.Get(ctx, newParent, &metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("move target not found %w", err) + } + parent, ok := parentObj.(*folders.Folder) + if !ok { + return fmt.Errorf("expected folder, found %T", parentObj) + } + + //FIXME: until we have a way to represent the tree, we can only + // look at folder parents to check how deep the new folder tree will be + info, err := parents(ctx, parent) + if err != nil { + return err + } + + // if by moving a folder we exceed the max depth, return an error + if len(info.Items)+1 >= maxDepth { + return folder.ErrMaximumDepthReached + } + return nil +} + +func validateOnDelete(ctx context.Context, + f *folders.Folder, + searcher resourcepb.ResourceIndexClient, +) error { + resp, err := searcher.GetStats(ctx, &resourcepb.ResourceStatsRequest{Namespace: f.Namespace, Folder: f.Name}) + if err != nil { + return err + } + + if resp != nil && resp.Error != nil { + return fmt.Errorf("could not verify if folder is empty: %v", resp.Error) + } + + if resp.Stats == nil { + return fmt.Errorf("could not verify if folder is empty: %v", resp.Error) + } + + for _, v := range resp.Stats { + if v.Count > 0 { + return folder.ErrFolderNotEmpty + } + } + return nil +} diff --git a/pkg/registry/apis/folders/validate_test.go b/pkg/registry/apis/folders/validate_test.go new file mode 100644 index 00000000000..30b523cb260 --- /dev/null +++ b/pkg/registry/apis/folders/validate_test.go @@ -0,0 +1,347 @@ +package folders + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" + "github.com/grafana/grafana/pkg/apimachinery/utils" + grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" + "github.com/grafana/grafana/pkg/storage/unified/resourcepb" +) + +func TestValidateCreate(t *testing.T) { + tests := []struct { + name string + folder *folders.Folder + getter *folders.FolderInfoList + getterError error + expectedErr string + maxDepth int // defaults to 5 unless set + }{{ + name: "ok", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + Annotations: map[string]string{"grafana.app/folder": "p2"}, + }, + Spec: folders.FolderSpec{ + Title: "some title", + }, + }, + getter: &folders.FolderInfoList{ + Items: []folders.FolderInfo{ + {Name: "p2", Parent: "p3"}, + {Name: "p3"}, + }, + }, + }, { + name: "reserved name", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "general", // can not name something with general + }, + }, + expectedErr: "invalid uid for folder provided", + }, { + name: "too long", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "a0123456789012345678901234567890123456789", // longer than 40 + }, + }, + expectedErr: "uid too long, max 40 characters", + }, { + name: "bad name", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hello world", // not a-z|0-9, + }, + }, + expectedErr: "uid contains illegal characters", + }, { + name: "can not be a parent of yourself", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + Annotations: map[string]string{"grafana.app/folder": "p1"}, + }, + Spec: folders.FolderSpec{ + Title: "some title", + }, + }, + expectedErr: "folder cannot be parent of itself", + }, { + name: "can not create a tree that is too deep", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + Annotations: map[string]string{"grafana.app/folder": "p2"}, + }, + Spec: folders.FolderSpec{ + Title: "some title", + }, + }, + getter: &folders.FolderInfoList{ + Items: []folders.FolderInfo{ + {Name: "p2", Parent: "p3"}, + {Name: "p3"}, + }, + }, + maxDepth: 2, // will become 3 + expectedErr: "folder max depth exceeded", + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + maxDepth := tt.maxDepth + if maxDepth == 0 { + maxDepth = 5 + } + err := validateOnCreate(context.Background(), tt.folder, + func(ctx context.Context, folder *folders.Folder) (*folders.FolderInfoList, error) { + return tt.getter, tt.getterError + }, maxDepth) + + if tt.expectedErr == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), tt.expectedErr) + } + }) + } +} + +func TestValidateUpdate(t *testing.T) { + tests := []struct { + name string + folder *folders.Folder + old *folders.Folder + parents *folders.FolderInfoList + parentsError error + expectedErr string + maxDepth int // defaults to 5 unless set + }{{ + name: "change title", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nnn", + }, + Spec: folders.FolderSpec{ + Title: "changed", + }, + }, + old: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nnn", + }, + Spec: folders.FolderSpec{ + Title: "old title", + }, + }, + }, { + name: "error to move into k6 folder", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nnn", + Annotations: map[string]string{ + utils.AnnoKeyFolder: "k6-app", + }, + }, + Spec: folders.FolderSpec{ + Title: "changed", + }, + }, + old: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nnn", + }, + Spec: folders.FolderSpec{ + Title: "old title", + }, + }, + expectedErr: "k6 project may not be moved", + }, { + name: "error when moving too deep", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Annotations: map[string]string{ + utils.AnnoKeyFolder: "p1", + }, + }, + Spec: folders.FolderSpec{ + Title: "changed", + }, + }, + old: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: folders.FolderSpec{ + Title: "old title", + }, + }, + parents: &folders.FolderInfoList{ + Items: []folders.FolderInfo{ + {Name: "p1", Parent: "p2"}, + {Name: "p2", Parent: "p3"}, + {Name: "p3"}, + }, + }, + maxDepth: 2, // will become 3 + expectedErr: "[folder.maximum-depth-reached]", + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + maxDepth := tt.maxDepth + if maxDepth == 0 { + maxDepth = 5 + } + s := (grafanarest.Storage)(nil) + m := &mock.Mock{} + if tt.parents != nil { + for _, v := range tt.parents.Items { + m.On("Get", context.Background(), v.Name, &metav1.GetOptions{}).Return(&folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: v.Name, + }, Spec: folders.FolderSpec{ + Title: v.Title, + }, + }, nil) + } + } + + err := validateOnUpdate(context.Background(), tt.folder, tt.old, storageMock{m, s}, + func(ctx context.Context, folder *folders.Folder) (*folders.FolderInfoList, error) { + return tt.parents, tt.parentsError + }, maxDepth) + + if tt.expectedErr == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), tt.expectedErr) + } + }) + } +} + +func TestValidateDelete(t *testing.T) { + tests := []struct { + name string + folder *folders.Folder + searcher *mockSearchClient + expectedErr string + }{{ + name: "simple delete", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nnn", + }, + }, + searcher: &mockSearchClient{ + stats: &resourcepb.ResourceStatsResponse{ + // Empty stats + Stats: []*resourcepb.ResourceStatsResponse_Stats{}, + }, + }, + }, { + name: "stats error", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nnn", + }, + }, + searcher: &mockSearchClient{ + stats: &resourcepb.ResourceStatsResponse{}, + }, + expectedErr: "could not verify if folder is empty", + }, { + name: "stats error", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nnn", + }, + }, + searcher: &mockSearchClient{ + statsErr: fmt.Errorf("error running stats"), + }, + expectedErr: "error running stats", + }, { + name: "stats error", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nnn", + }, + }, + searcher: &mockSearchClient{ + stats: &resourcepb.ResourceStatsResponse{ + Error: &resourcepb.ErrorResult{ + Reason: "error", + }, + }, + }, + expectedErr: "could not verify if folder is empty", + }, { + name: "folder not empty", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nnn", + }, + }, + searcher: &mockSearchClient{ + stats: &resourcepb.ResourceStatsResponse{ + Stats: []*resourcepb.ResourceStatsResponse_Stats{ + { + Group: "folders.grafana.app", + Resource: "folders", + Count: 10, // not empty + }, + }, + }, + }, + expectedErr: "[folder.not-empty]", + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateOnDelete(context.Background(), tt.folder, tt.searcher) + + if tt.expectedErr == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), tt.expectedErr) + } + }) + } +} + +var ( + _ = resourcepb.ResourceIndexClient(&mockSearchClient{}) +) + +type mockSearchClient struct { + stats *resourcepb.ResourceStatsResponse + statsErr error + + search *resourcepb.ResourceSearchResponse + searchErr error +} + +// GetStats implements resourcepb.ResourceIndexClient. +func (m *mockSearchClient) GetStats(ctx context.Context, in *resourcepb.ResourceStatsRequest, opts ...grpc.CallOption) (*resourcepb.ResourceStatsResponse, error) { + return m.stats, m.statsErr +} + +// Search implements resourcepb.ResourceIndexClient. +func (m *mockSearchClient) Search(ctx context.Context, in *resourcepb.ResourceSearchRequest, opts ...grpc.CallOption) (*resourcepb.ResourceSearchResponse, error) { + return m.search, m.searchErr +}