Support delete endpoint for folders (#96573)

* Support delete endpoint for folders

* Include authorizer

* Add test for delete verb

* Add delete command to delete options

* Pass query string to context to admission

* Dont support nested folder deletion for now

* Skip test if feature flag is present

* Add test case

* Remove comment

* Only rely on the storage type config to run alerting tests

* Dont change legacy subpath

* Remove unised function

* Add test case when an editor can delete alert rules

* Lint

pkg/api/folder.go

@@ -52,7 +52,6 @@ func (hs *HTTPServer) registerFolderAPI(apiRoute routing.RouteRegister, authoriz
 			folderUidRoute.Get("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, uidScope)), routing.Wrap(hs.GetFolderByUID))
 			folderUidRoute.Put("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, uidScope)), routing.Wrap(hs.UpdateFolder))
 			folderUidRoute.Post("/move", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, uidScope)), routing.Wrap(hs.MoveFolder))
-			folderUidRoute.Delete("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersDelete, uidScope)), routing.Wrap(hs.DeleteFolder))
 			folderUidRoute.Get("/counts", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, uidScope)), routing.Wrap(hs.GetFolderDescendantCounts))
 
 			folderUidRoute.Group("/permissions", func(folderPermissionRoute routing.RouteRegister) {
@@ -65,16 +64,21 @@ func (hs *HTTPServer) registerFolderAPI(apiRoute routing.RouteRegister, authoriz
 			handler := newFolderK8sHandler(hs)
 			folderRoute.Post("/", handler.createFolder)
 			folderRoute.Get("/", handler.getFolders)
+			folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) {
+				folderUidRoute.Delete("/", handler.deleteFolder)
+			})
 		} else {
 			folderRoute.Post("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)), routing.Wrap(hs.CreateFolder))
 			folderRoute.Get("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead)), routing.Wrap(hs.GetFolders))
+			folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) {
+				folderUidRoute.Delete("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersDelete, uidScope)), routing.Wrap(hs.DeleteFolder))
+			})
 		}
 		// Only adding support for some routes with the k8s handler for now. Include the rest here.
 		if false {
 			handler := newFolderK8sHandler(hs)
 			folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) {
 				folderUidRoute.Get("/", handler.getFolder)
-				folderUidRoute.Delete("/", handler.deleteFolder)
 				folderUidRoute.Put("/:uid", handler.updateFolder)
 			})
 		}

pkg/registry/apis/folders/register.go

@@ -3,6 +3,7 @@ package folders
 import (
 	"context"
 	"errors"
+	"slices"
 
 	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
 	"github.com/prometheus/client_golang/prometheus"
@@ -187,9 +188,10 @@ func (b *FolderAPIBuilder) GetAuthorizer() authorizer.Authorizer {
 }
 
 func authorizerFunc(ctx context.Context, attr authorizer.Attributes) (*authorizerParams, error) {
+	allowedVerbs := []string{utils.VerbCreate, utils.VerbDelete, utils.VerbList}
 	verb := attr.GetVerb()
 	name := attr.GetName()
-	if (!attr.IsResourceRequest()) || (name == "" && verb != utils.VerbCreate && verb != utils.VerbList) {
+	if (!attr.IsResourceRequest()) || (name == "" && verb != utils.VerbCreate && slices.Contains(allowedVerbs, verb)) {
 		return nil, errNoResource
 	}
 

pkg/registry/apis/folders/register_test.go

@@ -103,18 +103,36 @@ func TestFolderAPIBuilder_getAuthorizerFunc(t *testing.T) {
 					OrgID:  orgID,
 					Name:   "123",
 					Permissions: map[int64]map[string][]string{
-						orgID: {dashboards.ActionFoldersRead: {dashboards.ScopeFoldersAll}},
+						orgID: {},
 					},
 				},
 				verb: string(utils.VerbList),
 			},
 			expect: expect{
 				eval:  "folders:read",
+				allow: false,
+			},
+		},
+		{
+			name: "user with delete permissions should be able to delete a folder",
+			input: input{
+				user: &user.SignedInUser{
+					UserID: 1,
+					OrgID:  orgID,
+					Name:   "123",
+					Permissions: map[int64]map[string][]string{
+						orgID: {dashboards.ActionFoldersDelete: {dashboards.ScopeFoldersAll}, dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll}},
+					},
+				},
+				verb: string(utils.VerbDelete),
+			},
+			expect: expect{
+				eval:  "folders:delete",
 				allow: true,
 			},
 		},
 		{
-			name: "user without read permissions should not be able to list folders",
+			name: "user without delete permissions should NOT be able to delete a folder",
 			input: input{
 				user: &user.SignedInUser{
 					UserID: 1,
@@ -124,10 +142,10 @@ func TestFolderAPIBuilder_getAuthorizerFunc(t *testing.T) {
 						orgID: {},
 					},
 				},
-				verb: string(utils.VerbList),
+				verb: string(utils.VerbDelete),
 			},
 			expect: expect{
-				eval:  "folders:read",
+				eval:  "folders:delete",
 				allow: false,
 			},
 		},

pkg/tests/api/alerting/api_alertmanager_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/grafana/grafana/pkg/expr"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/tracing"
+	"github.com/grafana/grafana/pkg/services/apiserver/options"
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 	ngstore "github.com/grafana/grafana/pkg/services/ngalert/store"
@@ -694,15 +695,18 @@ func TestIntegrationRulerAccess(t *testing.T) {
 func TestIntegrationDeleteFolderWithRules(t *testing.T) {
 	testinfra.SQLiteIntegrationTest(t)
 
-	// Setup Grafana and its Database
-	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
+	opts := testinfra.GrafanaOpts{
 		DisableLegacyAlerting: true,
 		EnableUnifiedAlerting: true,
 		EnableQuota:           true,
 		DisableAnonymous:      true,
 		ViewersCanEdit:        true,
 		AppModeProduction:     true,
-	})
+		APIServerStorageType:  options.StorageTypeLegacy,
+	}
+
+	// Setup Grafana and its Database
+	dir, path := testinfra.CreateGrafDir(t, opts)
 
 	grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, path)
 
@@ -725,8 +729,7 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) {
 
 	createRule(t, apiClient, "default")
 
-	// First, let's have an editor create a rule within the folder/namespace.
-	{
+	t.Run("editor create a rule within the folder/namespace", func(t *testing.T) {
 		u := fmt.Sprintf("http://editor:editor@%s/api/ruler/grafana/api/v1/rules", grafanaListedAddr)
 		// nolint:gosec
 		resp, err := http.Get(u)
@@ -804,10 +807,8 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) {
 				]
 			}`, namespaceUID)
 		assert.JSONEq(t, expectedGetRulesResponseBody, string(b))
-	}
-
-	// Next, the editor can not delete the folder because it contains Grafana 8 alerts.
-	{
+	})
+	t.Run("editor can not delete the folder because it contains Grafana 8 alerts", func(t *testing.T) {
 		u := fmt.Sprintf("http://editor:editor@%s/api/folders/%s", grafanaListedAddr, namespaceUID)
 		req, err := http.NewRequest(http.MethodDelete, u, nil)
 		require.NoError(t, err)
@@ -825,10 +826,8 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) {
 		err = json.Unmarshal(b, &errutilErr)
 		require.NoError(t, err)
 		assert.Equal(t, "Folder cannot be deleted: folder is not empty", errutilErr.Message)
-	}
-
-	// Next, the editor can delete the folder if forceDeleteRules is true.
-	{
+	})
+	t.Run("editor can delete the folder if forceDeleteRules is true", func(t *testing.T) {
 		u := fmt.Sprintf("http://editor:editor@%s/api/folders/%s?forceDeleteRules=true", grafanaListedAddr, namespaceUID)
 		req, err := http.NewRequest(http.MethodDelete, u, nil)
 		require.NoError(t, err)
@@ -842,10 +841,8 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) {
 		_, err = io.ReadAll(resp.Body)
 		require.NoError(t, err)
 		require.Equal(t, 200, resp.StatusCode)
-	}
-
-	// Finally, we ensure the rules were deleted.
-	{
+	})
+	t.Run("editor can delete rules", func(t *testing.T) {
 		u := fmt.Sprintf("http://editor:editor@%s/api/ruler/grafana/api/v1/rules", grafanaListedAddr)
 		// nolint:gosec
 		resp, err := http.Get(u)
@@ -859,7 +856,8 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) {
 
 		assert.Equal(t, 200, resp.StatusCode)
 		assert.JSONEq(t, "{}", string(b))
-	}
+	})
+	// TODO(@leonorfmartins): write tests for uni store when we are able to support it
 }
 
 func TestIntegrationAlertRuleCRUD(t *testing.T) {

pkg/tests/apis/folder/folders_test.go

@@ -440,7 +440,7 @@ func doFolderTests(t *testing.T, helper *apis.K8sTestHelper) *apis.K8sTestHelper
 		}
 		slices.Sort(uids) // make list compare stable
 
-		// Check all playlists
+		// Check all folders
 		for _, uid := range uids {
 			getFromBothAPIs(t, helper, client, uid, nil)
 		}
@@ -461,17 +461,21 @@ func doFolderTests(t *testing.T, helper *apis.K8sTestHelper) *apis.K8sTestHelper
 		require.Equal(t, first.GetUID(), updated.GetUID())
 		require.Equal(t, "Test folder (replaced from k8s; 1 item; PUT)", title)
 		require.Equal(t, "New description", description)
+
 		// #TODO figure out why this breaks just for MySQL integration tests
 		// require.Less(t, first.GetResourceVersion(), updated.GetResourceVersion())
 
 		// ensure that we get 4 items when listing via k8s
 		l, err := client.Resource.List(context.Background(), metav1.ListOptions{})
 		require.NoError(t, err)
-		require.NotNil(t, l)
 		folders, err := meta.ExtractList(l)
 		require.NoError(t, err)
 		require.NotNil(t, folders)
 		require.Equal(t, len(folders), 4)
+
+		// delete test
+		errDelete := client.Resource.Delete(context.Background(), first.GetName(), metav1.DeleteOptions{})
+		require.NoError(t, errDelete)
 	})
 	return helper
 }