public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Folders: Improve folder parent testing (#110439)

Browse Source
This commit is contained in:
Ryan McKinley
2025-09-04 10:50:31 +00:00
committed by GitHub
parent 7d630ec3b1
commit 145a50be4d
9 changed files with 884 additions and 357 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/registry/apis/folders/parents.go
+88
View File
@@ -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
}
}
pkg/registry/apis/folders/parents_test.go
+187
View File
@@ -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)
}
})
}
}
pkg/registry/apis/folders/register.go
+14 -149
View File
@@ -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
}
pkg/registry/apis/folders/register_test.go
+73 -68
View File
@@ -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) {
pkg/registry/apis/folders/sub_children.go
+19 -10
View File
@@ -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)
}
}
pkg/registry/apis/folders/sub_parents.go
+9 -52
View File
@@ -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
}
pkg/registry/apis/folders/sub_parents_test.go
-78
View File
@@ -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)
})
}
}
pkg/registry/apis/folders/validate.go
+147
View File
@@ -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
}
pkg/registry/apis/folders/validate_test.go
+347
View File
@@ -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
}