mirror of
https://github.com/grafana/grafana.git
synced 2025-02-25 18:55:37 -06:00
Validate if we are creating a folder more than 5 levels deep (#95579)
* Add getter to FolderAPIBuilder so that we can access it in admission * Remove deprecated return * Fix test * Update pkg/registry/apis/folders/register_test.go Co-authored-by: maicon <maiconscosta@gmail.com> * Fix maxNested folder test * Remove log --------- Co-authored-by: maicon <maiconscosta@gmail.com>
This commit is contained in:
Leonor Oliveira
GitHub
parent
841a87c978
commit
b1157dbd7a
69
pkg/registry/apis/folders/mocks.go
Normal file
69
pkg/registry/apis/folders/mocks.go
Normal file
@ -0,0 +1,69 @@
|
||||
package folders
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/stretchr/testify/mock"
|
||||
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/apiserver/pkg/registry/rest"
|
||||
)
|
||||
|
||||
type storageMock struct {
|
||||
*mock.Mock
|
||||
rest.Storage
|
||||
}
|
||||
|
||||
func (m storageMock) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) {
|
||||
args := m.Called(ctx, name, options)
|
||||
return args.Get(0).(runtime.Object), args.Error(1)
|
||||
}
|
||||
|
||||
func (m storageMock) ConvertToTable(ctx context.Context, obj runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) {
|
||||
args := m.Called(ctx, obj, tableOptions)
|
||||
return args.Get(0).(*metav1.Table), args.Error(1)
|
||||
}
|
||||
|
||||
func (m storageMock) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
|
||||
args := m.Called(ctx, obj, createValidation, options)
|
||||
return args.Get(0).(runtime.Object), args.Error(1)
|
||||
}
|
||||
|
||||
func (m storageMock) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
|
||||
args := m.Called(ctx, name, deleteValidation, options)
|
||||
return args.Get(0).(runtime.Object), args.Bool(1), args.Error(2)
|
||||
}
|
||||
|
||||
func (m storageMock) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) {
|
||||
args := m.Called(ctx, deleteValidation, options, listOptions)
|
||||
return args.Get(0).(runtime.Object), args.Error(1)
|
||||
}
|
||||
|
||||
func (m storageMock) GetSingularName() string {
|
||||
args := m.Called()
|
||||
return args.String(0)
|
||||
}
|
||||
|
||||
func (m storageMock) List(ctx context.Context, options *internalversion.ListOptions) (runtime.Object, error) {
|
||||
args := m.Called(ctx, options)
|
||||
return args.Get(0).(runtime.Object), args.Error(1)
|
||||
}
|
||||
|
||||
func (m storageMock) NamespaceScoped() bool {
|
||||
args := m.Called()
|
||||
return args.Bool(0)
|
||||
}
|
||||
|
||||
func (m storageMock) NewList() runtime.Object {
|
||||
args := m.Called()
|
||||
return args.Get(0).(runtime.Object)
|
||||
}
|
||||
|
||||
func (m storageMock) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
|
||||
args := m.Called(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options)
|
||||
if name == "object-fail" {
|
||||
return nil, false, args.Error(2)
|
||||
}
|
||||
return args.Get(0).(runtime.Object), args.Bool(1), args.Error(2)
|
||||
}
|
||||
@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
|
||||
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
@ -36,20 +37,13 @@ var resourceInfo = v0alpha1.FolderResourceInfo
|
||||
var errNoUser = errors.New("valid user is required")
|
||||
var errNoResource = errors.New("resource name is required")
|
||||
|
||||
var folderValidationRules = struct {
|
||||
maxDepth int
|
||||
invalidNames []string
|
||||
}{
|
||||
maxDepth: 4,
|
||||
invalidNames: []string{"general"},
|
||||
}
|
||||
|
||||
// This is used just so wire has something unique to return
|
||||
type FolderAPIBuilder struct {
|
||||
gv schema.GroupVersion
|
||||
features featuremgmt.FeatureToggles
|
||||
namespacer request.NamespaceMapper
|
||||
folderSvc folder.Service
|
||||
storage grafanarest.Storage
|
||||
accessControl accesscontrol.AccessControl
|
||||
}
|
||||
|
||||
@ -138,6 +132,7 @@ func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API
|
||||
}
|
||||
|
||||
apiGroupInfo.VersionedResourcesStorageMap[v0alpha1.VERSION] = storage
|
||||
b.storage = storage[resourceInfo.StoragePath()].(grafanarest.Storage)
|
||||
return nil
|
||||
}
|
||||
|
||||
@ -225,8 +220,15 @@ func authorizerFunc(ctx context.Context, attr authorizer.Attributes) (*authorize
|
||||
return &authorizerParams{evaluator: eval, user: user}, nil
|
||||
}
|
||||
|
||||
var folderValidationRules = struct {
|
||||
maxDepth int
|
||||
invalidNames []string
|
||||
}{
|
||||
maxDepth: 5,
|
||||
invalidNames: []string{"general"},
|
||||
}
|
||||
|
||||
func (b *FolderAPIBuilder) Validate(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) error {
|
||||
// name cannot be called "general"
|
||||
id := a.GetName()
|
||||
for _, invalidName := range folderValidationRules.invalidNames {
|
||||
if id == invalidName {
|
||||
@ -236,18 +238,20 @@ func (b *FolderAPIBuilder) Validate(ctx context.Context, a admission.Attributes,
|
||||
|
||||
obj := a.GetObject()
|
||||
|
||||
for i := 0; i <= folderValidationRules.maxDepth+1; i++ {
|
||||
for i := 1; i <= folderValidationRules.maxDepth; i++ {
|
||||
parent := getParent(obj)
|
||||
if parent == "" {
|
||||
break
|
||||
}
|
||||
if i == folderValidationRules.maxDepth+1 {
|
||||
if i == folderValidationRules.maxDepth {
|
||||
return folder.ErrMaximumDepthReached
|
||||
}
|
||||
|
||||
// TODO: investigate the best way to get a folder by name, considering validate is called before the storage layer
|
||||
// parent, err := getFolderByNameSomehow()
|
||||
// obj = parent
|
||||
parentObj, err := b.storage.Get(ctx, parent, &metav1.GetOptions{})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
obj = parentObj
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@ -7,12 +7,15 @@ import (
|
||||
"github.com/grafana/grafana/pkg/apimachinery/identity"
|
||||
"github.com/grafana/grafana/pkg/apimachinery/utils"
|
||||
"github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
|
||||
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
|
||||
"github.com/grafana/grafana/pkg/services/authz/zanzana"
|
||||
"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/services/folder/foldertest"
|
||||
"github.com/grafana/grafana/pkg/services/user"
|
||||
"github.com/stretchr/testify/mock"
|
||||
"github.com/stretchr/testify/require"
|
||||
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
|
||||
"k8s.io/apiserver/pkg/admission"
|
||||
@ -126,10 +129,11 @@ func TestFolderAPIBuilder_Validate(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input input
|
||||
setupFn func(*mock.Mock)
|
||||
err error
|
||||
}{
|
||||
{
|
||||
name: "should return error when nameis invalid",
|
||||
name: "should return error when name is invalid",
|
||||
input: input{
|
||||
obj: &unstructured.Unstructured{
|
||||
Object: map[string]interface{}{
|
||||
@ -140,17 +144,58 @@ func TestFolderAPIBuilder_Validate(t *testing.T) {
|
||||
},
|
||||
err: dashboards.ErrFolderInvalidUID,
|
||||
},
|
||||
{
|
||||
name: "should return no error if every validation passes",
|
||||
input: input{
|
||||
obj: &unstructured.Unstructured{
|
||||
Object: map[string]interface{}{
|
||||
"meta": map[string]interface{}{"name": "valid-name"},
|
||||
},
|
||||
},
|
||||
name: "valid-name",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "should return error when creating a nested folder higher than max depth",
|
||||
input: input{
|
||||
obj: &unstructured.Unstructured{
|
||||
Object: map[string]any{
|
||||
"metadata": map[string]any{"name": "valid-name", "annotations": map[string]any{"grafana.app/folder": "valid-name"}},
|
||||
},
|
||||
},
|
||||
name: "valid-name",
|
||||
},
|
||||
setupFn: func(m *mock.Mock) {
|
||||
m.On("Get", mock.Anything, "valid-name", mock.Anything).Return(
|
||||
&unstructured.Unstructured{
|
||||
Object: map[string]any{
|
||||
"metadata": map[string]any{"name": "valid-name", "annotations": map[string]any{"grafana.app/folder": "valid-name"}},
|
||||
},
|
||||
}, nil)
|
||||
},
|
||||
err: folder.ErrMaximumDepthReached,
|
||||
},
|
||||
}
|
||||
|
||||
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,
|
||||
accessControl: acimpl.ProvideAccessControl(featuremgmt.WithFeatures("nestedFolders"), zanzana.NewNoopClient()),
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if tt.setupFn != nil {
|
||||
tt.setupFn(m)
|
||||
}
|
||||
|
||||
err := b.Validate(context.Background(), admission.NewAttributesRecord(
|
||||
tt.input.obj,
|
||||
nil,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user