Folder API: optionally force deleting Grafana 8 alerts when deleting a folder (or error) (#36427)

* Folder API: Add an optional query parameter for allowing deleting a  folder containing rules

* Update frontend

- Set forceDeleteRules=true when frontend deletes a folder
- Improve modal text

* Update docs

* Apply suggestions from code review

Co-authored-by: gotjosh <josue@grafana.com>
Co-authored-by: Nathan Rodman <nathanrodman@gmail.com>
Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
This commit is contained in:
Sofia Papagiannaki 2021-07-22 12:27:13 +03:00 committed by GitHub
parent ced42e9439
commit b96dd1877c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 164 additions and 27 deletions

View File

@ -237,7 +237,9 @@ Content-Length: 97
`DELETE /api/folders/:uid`
Deletes an existing folder identified by uid together with all dashboards stored in the folder, if any. This operation cannot be reverted.
Deletes an existing folder identified by UID along with all dashboards (and their alerts) stored in the folder. This operation cannot be reverted.
If [Grafana 8 Alerts]({{< relref "../alerting/unified-alerting/_index.md" >}}) are enabled, you can set an optional query parameter `forceDeleteRules=false` so that requests will fail with 400 (Bad Request) error if the folder contains any Grafana 8 Alerts. However, if this parameter is set to `true` then it will delete any Grafana 8 Alerts under this folder.
**Example Request**:
@ -265,6 +267,7 @@ Status Codes:
- **200** Deleted
- **401** Unauthorized
- **400** Bad Request
- **403** Access Denied
- **404** Folder not found

View File

@ -96,7 +96,7 @@ func (hs *HTTPServer) DeleteFolder(c *models.ReqContext) response.Response { //
return ToFolderErrorResponse(err)
}
f, err := s.DeleteFolder(c.Params(":uid"))
f, err := s.DeleteFolder(c.Params(":uid"), c.QueryBool("forceDeleteRules"))
if err != nil {
return ToFolderErrorResponse(err)
}
@ -149,7 +149,8 @@ func ToFolderErrorResponse(err error) response.Response {
if errors.Is(err, models.ErrFolderTitleEmpty) ||
errors.Is(err, models.ErrDashboardTypeMismatch) ||
errors.Is(err, models.ErrDashboardInvalidUid) ||
errors.Is(err, models.ErrDashboardUidTooLong) {
errors.Is(err, models.ErrDashboardUidTooLong) ||
errors.Is(err, models.ErrFolderContainsAlertRules) {
return response.Error(400, err.Error(), nil)
}

View File

@ -238,7 +238,7 @@ func (s *fakeFolderService) UpdateFolder(existingUID string, cmd *models.UpdateF
return s.UpdateFolderError
}
func (s *fakeFolderService) DeleteFolder(uid string) (*models.Folder, error) {
func (s *fakeFolderService) DeleteFolder(uid string, forceDeleteRules bool) (*models.Folder, error) {
s.DeletedFolderUids = append(s.DeletedFolderUids, uid)
return s.DeleteFolderResult, s.DeleteFolderError
}

View File

@ -365,6 +365,7 @@ type DashboardProvisioning struct {
type DeleteDashboardCommand struct {
Id int64
OrgId int64
ForceDeleteFolderRules bool
}
type DeleteOrphanedProvisionedDashboardsCommand struct {

View File

@ -15,6 +15,7 @@ var (
ErrFolderSameNameExists = errors.New("a folder or dashboard in the general folder with the same name already exists")
ErrFolderFailedGenerateUniqueUid = errors.New("failed to generate unique folder ID")
ErrFolderAccessDenied = errors.New("access denied to folder")
ErrFolderContainsAlertRules = errors.New("folder contains alert rules")
)
type Folder struct {

View File

@ -19,7 +19,7 @@ type FolderService interface {
GetFolderByTitle(title string) (*models.Folder, error)
CreateFolder(title, uid string) (*models.Folder, error)
UpdateFolder(uid string, cmd *models.UpdateFolderCommand) error
DeleteFolder(uid string) (*models.Folder, error)
DeleteFolder(uid string, forceDeleteRules bool) (*models.Folder, error)
MakeUserAdmin(orgID int64, userID, folderID int64, setViewAndEditPermissions bool) error
}
@ -192,7 +192,7 @@ func (dr *dashboardServiceImpl) UpdateFolder(existingUid string, cmd *models.Upd
return nil
}
func (dr *dashboardServiceImpl) DeleteFolder(uid string) (*models.Folder, error) {
func (dr *dashboardServiceImpl) DeleteFolder(uid string, forceDeleteRules bool) (*models.Folder, error) {
query := models.GetDashboardQuery{OrgId: dr.orgId, Uid: uid}
dashFolder, err := getFolder(query)
if err != nil {
@ -207,7 +207,7 @@ func (dr *dashboardServiceImpl) DeleteFolder(uid string) (*models.Folder, error)
return nil, models.ErrFolderAccessDenied
}
deleteCmd := models.DeleteDashboardCommand{OrgId: dr.orgId, Id: dashFolder.Id}
deleteCmd := models.DeleteDashboardCommand{OrgId: dr.orgId, Id: dashFolder.Id, ForceDeleteFolderRules: forceDeleteRules}
if err := bus.Dispatch(&deleteCmd); err != nil {
return nil, toFolderError(err)
}

View File

@ -67,7 +67,7 @@ func TestFolderService(t *testing.T) {
})
t.Run("When deleting folder by uid should return access denied error", func(t *testing.T) {
_, err := service.DeleteFolder("uid")
_, err := service.DeleteFolder("uid", false)
require.Error(t, err)
require.Equal(t, err, models.ErrFolderAccessDenied)
})
@ -121,7 +121,7 @@ func TestFolderService(t *testing.T) {
})
t.Run("When deleting folder by uid should not return access denied error", func(t *testing.T) {
_, err := service.DeleteFolder("uid")
_, err := service.DeleteFolder("uid", false)
require.NoError(t, err)
})

View File

@ -476,19 +476,30 @@ func deleteDashboard(cmd *models.DeleteDashboardCommand, sess *DBSession) error
}
}
// clean ngalert tables
ngalertDeletes := []string{
var existingRuleID int64
exists, err := sess.Table("alert_rule").Where("namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)", dashboard.Id).Cols("id").Get(&existingRuleID)
if err != nil {
return err
}
if exists {
if !cmd.ForceDeleteFolderRules {
return fmt.Errorf("folder cannot be deleted: %w", models.ErrFolderContainsAlertRules)
}
// Delete all rules under this folder.
deleteNGAlertsByFolder := []string{
"DELETE FROM alert_rule WHERE namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)",
"DELETE FROM alert_rule_version WHERE rule_namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)",
}
for _, sql := range ngalertDeletes {
for _, sql := range deleteNGAlertsByFolder {
_, err := sess.Exec(sql, dashboard.Id)
if err != nil {
return err
}
}
}
}
if err := deleteAlertDefinition(dashboard.Id, sess); err != nil {
return err

View File

@ -4,6 +4,8 @@ package sqlstore
import (
"context"
"encoding/json"
"errors"
"fmt"
"testing"
"time"
@ -29,6 +31,7 @@ func TestDashboardDataAccess(t *testing.T) {
savedDash := insertTestDashboard(t, sqlStore, "test dash 23", 1, savedFolder.Id, false, "prod", "webapp")
insertTestDashboard(t, sqlStore, "test dash 45", 1, savedFolder.Id, false, "prod")
savedDash2 := insertTestDashboard(t, sqlStore, "test dash 67", 1, 0, false, "prod")
insertTestRule(t, sqlStore, savedFolder.OrgId, savedFolder.Uid)
Convey("Should return dashboard model", func() {
So(savedDash.Title, ShouldEqual, "test dash 23")
@ -204,8 +207,14 @@ func TestDashboardDataAccess(t *testing.T) {
So(err, ShouldBeNil)
})
Convey("Should be able to delete a dashboard folder and its children", func() {
deleteCmd := &models.DeleteDashboardCommand{Id: savedFolder.Id}
Convey("Should be not able to delete a dashboard if force delete rules is disabled", func() {
deleteCmd := &models.DeleteDashboardCommand{Id: savedFolder.Id, ForceDeleteFolderRules: false}
err := DeleteDashboard(deleteCmd)
So(errors.Is(err, models.ErrFolderContainsAlertRules), ShouldBeTrue)
})
Convey("Should be able to delete a dashboard folder and its children if force delete rules is enabled", func() {
deleteCmd := &models.DeleteDashboardCommand{Id: savedFolder.Id, ForceDeleteFolderRules: true}
err := DeleteDashboard(deleteCmd)
So(err, ShouldBeNil)
@ -219,6 +228,20 @@ func TestDashboardDataAccess(t *testing.T) {
So(err, ShouldBeNil)
So(len(query.Result), ShouldEqual, 0)
sqlStore.WithDbSession(context.Background(), func(sess *DBSession) error {
var existingRuleID int64
exists, err := sess.Table("alert_rule").Where("namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)", savedFolder.Id).Cols("id").Get(&existingRuleID)
require.NoError(t, err)
So(exists, ShouldBeFalse)
var existingRuleVersionID int64
exists, err = sess.Table("alert_rule_version").Where("rule_namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)", savedFolder.Id).Cols("id").Get(&existingRuleVersionID)
require.NoError(t, err)
So(exists, ShouldBeFalse)
return nil
})
})
Convey("Should return error if no dashboard is found for update when dashboard id is greater than zero", func() {
@ -460,6 +483,84 @@ func insertTestDashboard(t *testing.T, sqlStore *SQLStore, title string, orgId i
return dash
}
func insertTestRule(t *testing.T, sqlStore *SQLStore, foderOrgID int64, folderUID string) {
sqlStore.WithDbSession(context.Background(), func(sess *DBSession) error {
type alertQuery struct {
RefID string
DatasourceUID string
Model json.RawMessage
}
type alertRule struct {
ID int64 `xorm:"pk autoincr 'id'"`
OrgID int64 `xorm:"org_id"`
Title string
Updated time.Time
UID string `xorm:"uid"`
NamespaceUID string `xorm:"namespace_uid"`
RuleGroup string
Condition string
Data []alertQuery
}
rule := alertRule{
OrgID: foderOrgID,
NamespaceUID: folderUID,
UID: "rule",
RuleGroup: "rulegroup",
Updated: time.Now(),
Condition: "A",
Data: []alertQuery{
{
RefID: "A",
DatasourceUID: "-100",
Model: json.RawMessage(`{
"type": "math",
"expression": "2 + 3 > 1"
}`),
},
},
}
_, err := sess.Insert(&rule)
require.NoError(t, err)
type alertRuleVersion struct {
ID int64 `xorm:"pk autoincr 'id'"`
RuleOrgID int64 `xorm:"rule_org_id"`
RuleUID string `xorm:"rule_uid"`
RuleNamespaceUID string `xorm:"rule_namespace_uid"`
RuleGroup string
ParentVersion int64
RestoredFrom int64
Version int64
Created time.Time
Title string
Condition string
Data []alertQuery
IntervalSeconds int64
}
ruleVersion := alertRuleVersion{
RuleOrgID: rule.OrgID,
RuleUID: rule.UID,
RuleNamespaceUID: rule.NamespaceUID,
RuleGroup: rule.RuleGroup,
Created: rule.Updated,
Condition: rule.Condition,
Data: rule.Data,
ParentVersion: 0,
RestoredFrom: 0,
Version: 1,
IntervalSeconds: 60,
}
_, err = sess.Insert(&ruleVersion)
require.NoError(t, err)
return err
})
}
func insertTestDashboardForPlugin(t *testing.T, sqlStore *SQLStore, title string, orgId int64,
folderId int64, isFolder bool, pluginId string) *models.Dashboard {
t.Helper()

View File

@ -748,7 +748,7 @@ func TestDeleteFolderWithRules(t *testing.T) {
assert.JSONEq(t, expectedGetRulesResponseBody, string(b))
}
// Next, the editor can delete the folder.
// Next, the editor can not delete the folder because it contains Grafana 8 alerts.
{
u := fmt.Sprintf("http://editor:editor@%s/api/folders/%s", grafanaListedAddr, namespaceUID)
req, err := http.NewRequest(http.MethodDelete, u, nil)
@ -762,6 +762,24 @@ func TestDeleteFolderWithRules(t *testing.T) {
})
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
require.JSONEq(t, `{"message":"folder cannot be deleted: folder contains alert rules"}`, string(b))
}
// Next, the editor can delete the folder if forceDeleteRules is true.
{
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)
client := &http.Client{}
resp, err := client.Do(req)
require.NoError(t, err)
t.Cleanup(func() {
err := resp.Body.Close()
require.NoError(t, err)
})
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
require.Equal(t, 200, resp.StatusCode)
require.JSONEq(t, `{"id":1,"message":"Folder default deleted","title":"default"}`, string(b))
}

View File

@ -66,10 +66,11 @@ export class FolderSettingsPage extends PureComponent<Props, State> {
evt.stopPropagation();
evt.preventDefault();
const confirmationText = `Do you want to delete this folder and all its dashboards and alerts?`;
appEvents.publish(
new ShowConfirmModalEvent({
title: 'Delete',
text: `Do you want to delete this folder and all its dashboards?`,
text: confirmationText,
icon: 'trash-alt',
yesText: 'Delete',
onConfirm: () => {

View File

@ -31,7 +31,7 @@ export function saveFolder(folder: FolderState): ThunkResult<void> {
export function deleteFolder(uid: string): ThunkResult<void> {
return async (dispatch) => {
await backendSrv.delete(`/api/folders/${uid}`);
await backendSrv.delete(`/api/folders/${uid}?forceDeleteRules=true`);
locationService.push('/dashboards');
};
}

View File

@ -213,7 +213,7 @@ export function saveDashboard(options: SaveDashboardOptions) {
function deleteFolder(uid: string, showSuccessAlert: boolean) {
return getBackendSrv().request({
method: 'DELETE',
url: `/api/folders/${uid}`,
url: `/api/folders/${uid}?forceDeleteRules=true`,
showSuccessAlert: showSuccessAlert === true,
});
}

View File

@ -30,9 +30,9 @@ export const ConfirmDeleteModal: FC<Props> = ({ results, onDeleteItems, isOpen,
if (folderCount > 0 && dashCount > 0) {
text += `selected folder${folderEnding} and dashboard${dashEnding}?\n`;
subtitle = `All dashboards of the selected folder${folderEnding} will also be deleted`;
subtitle = `All dashboards and alerts of the selected folder${folderEnding} will also be deleted`;
} else if (folderCount > 0) {
text += `selected folder${folderEnding} and all its dashboards?`;
text += `selected folder${folderEnding} and all their dashboards and alerts?`;
} else {
text += `selected dashboard${dashEnding}?`;
}