opensource/grafana
1
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-08-02 18:23:50 +08:00
Code Issues Packages Projects Releases Wiki Activity

Use searcher GetStats to count for folders and dashboards under a folder to be deleted (#97708)

Browse Source 
* get stats from index

* fix logging

* fix logging

* actually counting

* Use searcher to check for resources referencing a folder

* Add tests when deleting a folder

* Lint

* merge/fix test

* will delegate to SQL

* get stats from index

* fix logging

* actually counting

* merge/fix test

* will delegate to SQL

* Uncomment test

* Add tests for checking stats before deleting a folder

* Change base branch to main

* Lint

* [REVIEW] remove logs

* Check for type assertion success

---------

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
This commit is contained in:
Leonor Oliveira
2024-12-12 11:53:01 +01:00
committed by GitHub
parent afd0699c85
commit f710573b37
3 changed files with 145 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
pkg/registry/apis/folders/mocks.go
View File

@ -3,7 +3,9 @@ package folders
import (
"context"
"github.com/grafana/grafana/pkg/storage/unified/resource"
"github.com/stretchr/testify/mock"
"google.golang.org/grpc"
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
@ -15,6 +17,11 @@ type storageMock struct {
rest.Storage
}
type searcherMock struct {
*mock.Mock
resource.ResourceIndexClient
}
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)
@ -67,3 +74,8 @@ func (m storageMock) Update(ctx context.Context, name string, objInfo rest.Updat
}
return args.Get(0).(runtime.Object), args.Bool(1), args.Error(2)
}
func (s searcherMock) GetStats(ctx context.Context, req *resource.ResourceStatsRequest, opts ...grpc.CallOption) (*resource.ResourceStatsResponse, error) {
args := s.Called(ctx, req)
return args.Get(0).(*resource.ResourceStatsResponse), args.Error(1)
}

63
pkg/registry/apis/folders/register.go
View File

@ -17,11 +17,13 @@ import (
common "k8s.io/kube-openapi/pkg/common"
"k8s.io/kube-openapi/pkg/spec3"
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/storage/unified/resource"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
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"
@ -29,7 +31,6 @@ import (
"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/resource"
)
var _ builder.APIGroupBuilder = (*FolderAPIBuilder)(nil)
@ -127,9 +128,7 @@ func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API
storage[resourceInfo.StoragePath()] = legacyStore
storage[resourceInfo.StoragePath("parents")] = &subParentsREST{b.folderSvc}
storage[resourceInfo.StoragePath("access")] = &subAccessREST{b.folderSvc}
storage[resourceInfo.StoragePath("count")] = &subCountREST{
searcher: b.searcher,
}
storage[resourceInfo.StoragePath("count")] = &subCountREST{searcher: b.searcher}
// enable dual writer
if optsGetter != nil && dualWriteBuilder != nil {
@ -241,12 +240,6 @@ var folderValidationRules = struct {
func (b *FolderAPIBuilder) Validate(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) error {
id := a.GetName()
for _, invalidName := range folderValidationRules.invalidNames {
if id == invalidName {
return dashboards.ErrFolderInvalidUID
}
}
obj := a.GetObject()
if obj == nil || a.GetOperation() == admission.Connect {
return nil // This is normal for sub-resource
@ -256,7 +249,55 @@ func (b *FolderAPIBuilder) Validate(ctx context.Context, a admission.Attributes,
if !ok {
return fmt.Errorf("obj is not v0alpha1.Folder")
}
verb := a.GetOperation()
switch verb {
case admission.Create:
return b.validateOnCreate(ctx, id, obj)
case admission.Delete:
return b.validateOnDelete(ctx, f)
case admission.Update:
return nil
case admission.Connect:
return nil
}
return nil
}
func (b *FolderAPIBuilder) validateOnDelete(ctx context.Context, f *v0alpha1.Folder) error {
resp, err := b.searcher.GetStats(ctx, &resource.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
}
}
f, ok := obj.(*v0alpha1.Folder)
if !ok {
return fmt.Errorf("obj is not v0alpha1.Folder")
}
if f.Spec.Title == "" {
return dashboards.ErrFolderTitleEmpty
}

83
pkg/registry/apis/folders/register_test.go
View File

@ -15,8 +15,10 @@ import (
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/folder/foldertest"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/storage/unified/resource"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authorization/authorizer"
)
@ -210,7 +212,7 @@ func TestFolderAPIBuilder_getAuthorizerFunc(t *testing.T) {
}
}
func TestFolderAPIBuilder_Validate(t *testing.T) {
func TestFolderAPIBuilder_Validate_Create(t *testing.T) {
type input struct {
obj *v0alpha1.Folder
annotations map[string]string
@ -316,7 +318,7 @@ func TestFolderAPIBuilder_Validate(t *testing.T) {
tt.input.name,
v0alpha1.SchemeGroupVersion.WithResource("folders"),
"",
"create",
"CREATE",
nil,
true,
&user.SignedInUser{},
@ -331,3 +333,80 @@ func TestFolderAPIBuilder_Validate(t *testing.T) {
})
}
}
func TestFolderAPIBuilder_Validate_Delete(t *testing.T) {
tests := []struct {
name string
statsResponse *resource.ResourceStatsResponse_Stats
wantErr bool
}{
{
name: "should allow deletion when folder is empty",
statsResponse: &resource.ResourceStatsResponse_Stats{Count: 0},
},
{
name: "should return folder not empty when the folder is not empty",
statsResponse: &resource.ResourceStatsResponse_Stats{Count: 2},
wantErr: true,
},
}
s := (grafanarest.Storage)(nil)
m := &mock.Mock{}
us := storageMock{m, s}
sm := searcherMock{Mock: m}
obj := &v0alpha1.Folder{
Spec: v0alpha1.Spec{
Title: "foo",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: "stacks-123",
Name: "valid-name",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var setupFn = func(m *mock.Mock, stats *resource.ResourceStatsResponse_Stats) {
m.On("GetStats", mock.Anything, &resource.ResourceStatsRequest{Namespace: obj.Namespace, Folder: obj.Name}).Return(
&resource.ResourceStatsResponse{Stats: []*resource.ResourceStatsResponse_Stats{stats}},
nil,
).Once()
}
setupFn(m, tt.statsResponse)
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()),
searcher: sm,
}
err := b.Validate(context.Background(), admission.NewAttributesRecord(
obj,
nil,
v0alpha1.SchemeGroupVersion.WithKind("folder"),
obj.Namespace,
obj.Name,
v0alpha1.SchemeGroupVersion.WithResource("folders"),
"",
"DELETE",
nil,
true,
&user.SignedInUser{},
),
nil)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
})
}
}