Chore/Folders: reduce direct use of settings.Cfg (#110657)
This commit is contained in:
@@ -22,7 +22,6 @@ func LegacyCreateCommandToUnstructured(cmd *folder.CreateFolderCommand) (*unstru
|
||||
"spec": map[string]any{
|
||||
"title": cmd.Title,
|
||||
"description": cmd.Description,
|
||||
"version": 1,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
@@ -20,9 +20,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol"
|
||||
"github.com/grafana/grafana/pkg/services/apiserver/endpoints/request"
|
||||
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/services/org"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
|
||||
var (
|
||||
@@ -37,12 +35,13 @@ var (
|
||||
)
|
||||
|
||||
type folderStorage struct {
|
||||
tableConverter rest.TableConvertor
|
||||
cfg *setting.Cfg
|
||||
features featuremgmt.FeatureToggles
|
||||
// Wrapped storage
|
||||
store grafanarest.Storage
|
||||
tableConverter rest.TableConvertor
|
||||
|
||||
permissionsOnCreate bool // cfg.RBAC.PermissionsOnCreation("folder")
|
||||
folderPermissionsSvc accesscontrol.FolderPermissionsService
|
||||
acService accesscontrol.Service
|
||||
store grafanarest.Storage
|
||||
}
|
||||
|
||||
func (s *folderStorage) New() runtime.Object {
|
||||
@@ -86,6 +85,11 @@ func (s *folderStorage) Create(ctx context.Context,
|
||||
return nil, &statusErr
|
||||
}
|
||||
|
||||
// When cfg.RBAC.PermissionsOnCreation("folder") is not enabled
|
||||
if !s.permissionsOnCreate {
|
||||
return obj, err
|
||||
}
|
||||
|
||||
info, err := request.NamespaceInfoFrom(ctx, true)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -154,10 +158,6 @@ func (s *folderStorage) DeleteCollection(ctx context.Context, deleteValidation r
|
||||
}
|
||||
|
||||
func (s *folderStorage) setDefaultFolderPermissions(ctx context.Context, orgID int64, user identity.Requester, uid string, parentUID string) error {
|
||||
if !s.cfg.RBAC.PermissionsOnCreation("folder") {
|
||||
return nil
|
||||
}
|
||||
|
||||
var permissions []accesscontrol.SetResourcePermissionCommand
|
||||
|
||||
if user.IsIdentityType(claims.TypeUser, claims.TypeServiceAccount) {
|
||||
|
||||
@@ -7,14 +7,13 @@ import (
|
||||
"github.com/stretchr/testify/mock"
|
||||
"github.com/stretchr/testify/require"
|
||||
"gopkg.in/ini.v1"
|
||||
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/apiserver/pkg/endpoints/request"
|
||||
"k8s.io/apiserver/pkg/registry/rest"
|
||||
|
||||
folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1"
|
||||
"github.com/grafana/grafana/pkg/apimachinery/identity"
|
||||
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
|
||||
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
|
||||
@@ -46,12 +45,15 @@ func TestSetDefaultPermissionsWhenCreatingFolder(t *testing.T) {
|
||||
tempCfg, err := setting.NewCfgFromINIFile(f)
|
||||
require.NoError(t, err)
|
||||
cfg.RBAC = tempCfg.RBAC
|
||||
store := grafanarest.NewMockStorage(t)
|
||||
store.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
|
||||
Return(&folders.Folder{}, nil).Maybe() // we don't really care
|
||||
|
||||
fs := folderStorage{
|
||||
folderPermissionsSvc: folderPermService,
|
||||
acService: actest.FakeService{},
|
||||
store: &fakeStorage{},
|
||||
cfg: cfg,
|
||||
store: store,
|
||||
permissionsOnCreate: cfg.RBAC.PermissionsOnCreation("folder"),
|
||||
}
|
||||
obj := &folders.Folder{}
|
||||
|
||||
@@ -73,63 +75,3 @@ func TestSetDefaultPermissionsWhenCreatingFolder(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
var (
|
||||
_ rest.Scoper = (*fakeStorage)(nil)
|
||||
_ rest.SingularNameProvider = (*fakeStorage)(nil)
|
||||
_ rest.Getter = (*fakeStorage)(nil)
|
||||
_ rest.Lister = (*fakeStorage)(nil)
|
||||
_ rest.Storage = (*fakeStorage)(nil)
|
||||
_ rest.Creater = (*fakeStorage)(nil)
|
||||
_ rest.Updater = (*fakeStorage)(nil)
|
||||
_ rest.GracefulDeleter = (*fakeStorage)(nil)
|
||||
)
|
||||
|
||||
type fakeStorage struct{}
|
||||
|
||||
func (s *fakeStorage) New() runtime.Object {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *fakeStorage) Destroy() {}
|
||||
|
||||
func (s *fakeStorage) NamespaceScoped() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
func (s *fakeStorage) GetSingularName() string {
|
||||
return ""
|
||||
}
|
||||
|
||||
func (s *fakeStorage) NewList() runtime.Object {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *fakeStorage) ConvertToTable(ctx context.Context, object runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (s *fakeStorage) List(ctx context.Context, options *internalversion.ListOptions) (runtime.Object, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (s *fakeStorage) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (s *fakeStorage) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
|
||||
return obj, nil
|
||||
}
|
||||
|
||||
func (s *fakeStorage) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc,
|
||||
updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
|
||||
return nil, false, nil
|
||||
}
|
||||
|
||||
func (s *fakeStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
|
||||
return nil, false, nil
|
||||
}
|
||||
|
||||
func (s *fakeStorage) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
@@ -3,7 +3,6 @@ package folders
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
@@ -17,10 +16,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/apimachinery/utils"
|
||||
"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/util"
|
||||
)
|
||||
|
||||
var (
|
||||
@@ -38,8 +34,6 @@ type legacyStorage struct {
|
||||
service folder.Service
|
||||
namespacer request.NamespaceMapper
|
||||
tableConverter rest.TableConvertor
|
||||
cfg *setting.Cfg
|
||||
features featuremgmt.FeatureToggles
|
||||
}
|
||||
|
||||
func (s *legacyStorage) New() runtime.Object {
|
||||
@@ -177,12 +171,6 @@ func (s *legacyStorage) Create(ctx context.Context,
|
||||
return nil, fmt.Errorf("expected folder?")
|
||||
}
|
||||
|
||||
// Simplify creating unique folder names with
|
||||
if p.GenerateName != "" && strings.Contains(p.Spec.Title, "${RAND}") {
|
||||
rand, _ := util.GetRandomString(10)
|
||||
p.Spec.Title = strings.ReplaceAll(p.Spec.Title, "${RAND}", rand)
|
||||
}
|
||||
|
||||
accessor, err := utils.MetaAccessor(p)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -2,6 +2,7 @@ package folders
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
@@ -10,8 +11,6 @@ import (
|
||||
metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion"
|
||||
"k8s.io/apimachinery/pkg/labels"
|
||||
|
||||
"encoding/base64"
|
||||
|
||||
folderv1 "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1"
|
||||
"github.com/grafana/grafana/pkg/apimachinery/identity"
|
||||
"github.com/grafana/grafana/pkg/apimachinery/utils"
|
||||
|
||||
@@ -55,9 +55,9 @@ type FolderAPIBuilder struct {
|
||||
authorizer authorizer.Authorizer
|
||||
parents parentsGetter
|
||||
|
||||
searcher resourcepb.ResourceIndexClient
|
||||
cfg *setting.Cfg
|
||||
ignoreLegacy bool // skip legacy storage and only use unified storage
|
||||
searcher resourcepb.ResourceIndexClient
|
||||
permissionsOnCreate bool
|
||||
ignoreLegacy bool // skip legacy storage and only use unified storage
|
||||
}
|
||||
|
||||
func RegisterAPIService(cfg *setting.Cfg,
|
||||
@@ -78,7 +78,7 @@ func RegisterAPIService(cfg *setting.Cfg,
|
||||
folderPermissionsSvc: folderPermissionsSvc,
|
||||
acService: acService,
|
||||
ac: accessControl,
|
||||
cfg: cfg,
|
||||
permissionsOnCreate: cfg.RBAC.PermissionsOnCreation("folder"),
|
||||
authorizer: newLegacyAuthorizer(accessControl),
|
||||
searcher: unified,
|
||||
}
|
||||
@@ -153,8 +153,6 @@ func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API
|
||||
service: b.folderSvc,
|
||||
namespacer: b.namespacer,
|
||||
tableConverter: resourceInfo.TableConverter(),
|
||||
features: b.features,
|
||||
cfg: b.cfg,
|
||||
}
|
||||
|
||||
opts.StorageOptsRegister(resourceInfo.GroupResource(), apistore.StorageOptions{
|
||||
@@ -165,8 +163,7 @@ func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API
|
||||
tableConverter: resourceInfo.TableConverter(),
|
||||
folderPermissionsSvc: b.folderPermissionsSvc,
|
||||
acService: b.acService,
|
||||
features: b.features,
|
||||
cfg: b.cfg,
|
||||
permissionsOnCreate: b.permissionsOnCreate,
|
||||
}
|
||||
|
||||
if optsGetter != nil && dualWriteBuilder != nil {
|
||||
|
||||
+1
-1
@@ -3,5 +3,5 @@ kind: Folder
|
||||
metadata:
|
||||
generateName: x # anything is ok here... except yes or true -- they become boolean!
|
||||
spec:
|
||||
title: Generated folder title (${RAND})
|
||||
title: Generated folder title
|
||||
description: A description from here
|
||||
|
||||
Reference in New Issue
Block a user