fix: only validate allowed descendants for folder deletion (#113440)

This commit is contained in:
Mustafa Sencer Özcan
2025-11-05 14:04:43 +01:00
committed by GitHub
parent d9d227ec4d
commit b97fb638ad
3 changed files with 188 additions and 14 deletions
+75 -7
View File
@@ -183,17 +183,85 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) {
func TestFolderAPIBuilder_Validate_Delete(t *testing.T) {
tests := []struct {
name string
statsResponse *resourcepb.ResourceStatsResponse_Stats
statsResponse []*resourcepb.ResourceStatsResponse_Stats
wantErr bool
}{
{
name: "should allow deletion when folder is empty",
statsResponse: &resourcepb.ResourceStatsResponse_Stats{Count: 0},
name: "should allow deletion when folder is empty",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{
{Count: 0, Resource: "dashboards"},
},
},
{
name: "should return folder not empty when the folder is not empty",
statsResponse: &resourcepb.ResourceStatsResponse_Stats{Count: 2},
wantErr: true,
name: "should return folder not empty when folder contains dashboards",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{
{Count: 2, Resource: "dashboards", Group: "dashboard.grafana.app"},
},
wantErr: true,
},
{
name: "should return folder not empty when folder contains alertrules",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{
{Count: 3, Resource: "alertrules", Group: "alerting.grafana.app"},
},
wantErr: true,
},
{
name: "should return folder not empty when folder contains library_elements",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{
{Count: 1, Resource: "library_elements", Group: "library.grafana.app"},
},
wantErr: true,
},
{
name: "should return folder not empty when folder contains folders",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{
{Count: 2, Resource: "folders", Group: "folders.grafana.app"},
},
wantErr: true,
},
{
name: "should return folder not empty when folder has mixed resources with validated types",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{
{Count: 10, Resource: "folders", Group: "folders.grafana.app"},
{Count: 2, Resource: "dashboards", Group: "dashboard.grafana.app"},
{Count: 5, Resource: "playlists", Group: "playlist.grafana.app"},
},
wantErr: true,
},
{
name: "should return folder not empty when folder has multiple validated resource types",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{
{Count: 1, Resource: "dashboards", Group: "dashboard.grafana.app"},
{Count: 2, Resource: "alertrules", Group: "alerting.grafana.app"},
{Count: 1, Resource: "library_elements", Group: "library.grafana.app"},
{Count: 1, Resource: "folders", Group: "folders.grafana.app"},
},
wantErr: true,
},
{
name: "should allow deletion when all validated resource types are empty",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{
{Count: 0, Resource: "dashboards", Group: "dashboard.grafana.app"},
{Count: 0, Resource: "alertrules", Group: "alerting.grafana.app"},
{Count: 0, Resource: "library_elements", Group: "library.grafana.app"},
{Count: 0, Resource: "folders", Group: "folders.grafana.app"},
{Count: 10, Resource: "playlists", Group: "playlist.grafana.app"},
},
wantErr: false,
},
{
name: "should allow deletion when folder only contains non-validated resource types",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{
{Count: 5, Resource: "playlists", Group: "playlist.grafana.app"},
{Count: 3, Resource: "other", Group: "other.grafana.app"},
},
wantErr: false,
},
{
name: "should allow deletion when stats array is empty",
statsResponse: []*resourcepb.ResourceStatsResponse_Stats{},
wantErr: false,
},
}
@@ -212,7 +280,7 @@ func TestFolderAPIBuilder_Validate_Delete(t *testing.T) {
us := grafanarest.NewMockStorage(t)
sm := resource.NewMockResourceClient(t)
sm.On("GetStats", mock.Anything, &resourcepb.ResourceStatsRequest{Namespace: obj.Namespace, Folder: obj.Name}).Return(
&resourcepb.ResourceStatsResponse{Stats: []*resourcepb.ResourceStatsResponse_Stats{tt.statsResponse}},
&resourcepb.ResourceStatsResponse{Stats: tt.statsResponse},
nil,
).Once()
+4 -2
View File
@@ -144,9 +144,11 @@ func validateOnDelete(ctx context.Context,
return fmt.Errorf("could not verify if folder is empty: %v", resp.Error)
}
allowedResourceTypes := []string{"alertrules", "dashboards", "library_elements", "folders"}
for _, v := range resp.Stats {
if v.Count > 0 {
return folder.ErrFolderNotEmpty.Errorf("folder is not empty, contains %d %s.%s", v.Count, v.Group, v.Resource)
if slices.Contains(allowedResourceTypes, v.Resource) && v.Count > 0 {
return folder.ErrFolderNotEmpty.Errorf("folder is not empty, contains %d %s", v.Count, v.Resource)
}
}
return nil
+109 -5
View File
@@ -320,7 +320,7 @@ func TestValidateDelete(t *testing.T) {
},
},
}, {
name: "stats error",
name: "stats error - nil stats",
folder: &folders.Folder{
ObjectMeta: metav1.ObjectMeta{
Name: "nnn",
@@ -331,7 +331,7 @@ func TestValidateDelete(t *testing.T) {
},
expectedErr: "could not verify if folder is empty",
}, {
name: "stats error",
name: "stats error - search error",
folder: &folders.Folder{
ObjectMeta: metav1.ObjectMeta{
Name: "nnn",
@@ -342,7 +342,7 @@ func TestValidateDelete(t *testing.T) {
},
expectedErr: "error running stats",
}, {
name: "stats error",
name: "stats error - error result",
folder: &folders.Folder{
ObjectMeta: metav1.ObjectMeta{
Name: "nnn",
@@ -357,7 +357,64 @@ func TestValidateDelete(t *testing.T) {
},
expectedErr: "could not verify if folder is empty",
}, {
name: "folder not empty",
name: "folder not empty - contains dashboards",
folder: &folders.Folder{
ObjectMeta: metav1.ObjectMeta{
Name: "nnn",
},
},
searcher: &mockSearchClient{
stats: &resourcepb.ResourceStatsResponse{
Stats: []*resourcepb.ResourceStatsResponse_Stats{
{
Group: "dashboard.grafana.app",
Resource: "dashboards",
Count: 10, // not empty
},
},
},
},
expectedErr: "[folder.not-empty]",
}, {
name: "folder not empty - contains alertrules",
folder: &folders.Folder{
ObjectMeta: metav1.ObjectMeta{
Name: "nnn",
},
},
searcher: &mockSearchClient{
stats: &resourcepb.ResourceStatsResponse{
Stats: []*resourcepb.ResourceStatsResponse_Stats{
{
Group: "alerting.grafana.app",
Resource: "alertrules",
Count: 5, // not empty
},
},
},
},
expectedErr: "[folder.not-empty]",
}, {
name: "folder not empty - contains library_elements",
folder: &folders.Folder{
ObjectMeta: metav1.ObjectMeta{
Name: "nnn",
},
},
searcher: &mockSearchClient{
stats: &resourcepb.ResourceStatsResponse{
Stats: []*resourcepb.ResourceStatsResponse_Stats{
{
Group: "library.grafana.app",
Resource: "library_elements",
Count: 3, // not empty
},
},
},
},
expectedErr: "[folder.not-empty]",
}, {
name: "folder not empty - contains folders",
folder: &folders.Folder{
ObjectMeta: metav1.ObjectMeta{
Name: "nnn",
@@ -369,7 +426,54 @@ func TestValidateDelete(t *testing.T) {
{
Group: "folders.grafana.app",
Resource: "folders",
Count: 10, // not empty
Count: 2, // not empty
},
},
},
},
expectedErr: "[folder.not-empty]",
}, {
name: "folder can be deleted when it only contains non-validated resource types",
folder: &folders.Folder{
ObjectMeta: metav1.ObjectMeta{
Name: "nnn",
},
},
searcher: &mockSearchClient{
stats: &resourcepb.ResourceStatsResponse{
Stats: []*resourcepb.ResourceStatsResponse_Stats{
{
Group: "playlist.grafana.app",
Resource: "playlists",
Count: 10, // has content but not a validated resource type
},
{
Group: "other.grafana.app",
Resource: "other",
Count: 5, // has content but not a validated resource type
},
},
},
},
}, {
name: "folder not empty - mixed resources with validated types",
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, // now validated
},
{
Group: "dashboard.grafana.app",
Resource: "dashboards",
Count: 2, // validated and has content
},
},
},