Alerting: Fix deleting rules in a folder with matching UID in another organization

[papagian]

What is this feature?

The deleteFolderAlertRules() function that is declared inside dashboard service has become obsolete since #67259 (alert rules get deleted here using DeleteInFolder()) but it was still got called.
Unfortunately, this function doesn't take into account the org_id. As a result:

• trying to delete a folder without alerts with matching folder UID in another organization (with alerts) wrongly fails with folder cannot be deleted: folder contains alert rules
• deleting a folder using forceDeleteRules=true would wrongly delete alert rules with matching folder (namespace_uid) UID in another organization (with alerts)

Why do we need this feature?

It fixes the above issues by removing the obsolete call.

Who is this feature for?

[Add information on what kind of user the feature is for.]

Which issue(s) does this PR fix?:

Fixes #

Special notes for your reviewer:

Please check that:

• It works as expected from a user's perspective.
• If this is a pre-GA feature, it is behind a feature toggle.
• The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

[grobinson-grafana]

LGTM from a code perspective. Do you need help testing this?

[grobinson-grafana]

Should Errorf("") have a message here?

[yuri-tceretian]

Not necessary because Errorf argument changes the private message and log messages. Whereas the public message gets returned to the user here

grafana/pkg/api/response/response.go

Lines 272 to 274 in e4d1fdc

	resp := JSON(grafanaErr.Reason.Status().HTTPStatus(), grafanaErr.Public())
	resp.errMessage = string(grafanaErr.Reason.Status())
	resp.err = grafanaErr

and in a few other places.

Anyway, I think we can return just folder.ErrFolderContainsAlertRules because errutil.Base implements the Error interface.

As a side note, I think we should add method Error to the errutil.Base so we can get just a plain simple errutil.Error and avoid controversial Errorf("").

[yuri-tceretian]

I wonder if it would be better to have msgID more generic "folder.non-empty". It's simpler to change the public message than msgId.

[papagian]

fixed in a869362

[yuri-tceretian]

The change looks good. However, there is one missing piece in this flow: the service should validate that user has permissions to delete alert rules in this folder, i.e. action alert.rules:delete. We can add it to the folder service for now and later move to the service that will be wrapper for the DbStore.

[papagian]

I think I have addressed all the review suggestions: you can check once again

[yuri-tceretian]

Tested and everything works fine. The only problem is that the 403 response does not have much info about why user access was denied but I think we can address it later.

LGTM!

[grafana-delivery-bot]

The backport to v10.1.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-78258-to-v10.1.x origin/v10.1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 6d4625ad52b2e2ad3b2b88e394f69e8fde83c04b

When the conflicts are resolved, stage and commit the changes:

git add . && git cherry-pick --continue

If you have the GitHub CLI installed:

# Push the branch to GitHub:
git push --set-upstream origin backport-78258-to-v10.1.x
# Create the PR body template
PR_BODY=$(gh pr view 78258 --json body --template 'Backport 6d4625ad52b2e2ad3b2b88e394f69e8fde83c04b from #78258{{ "\n\n---\n\n" }}{{ index . "body" }}')
# Create the PR on GitHub
echo "${PR_BODY}" | gh pr create --title "[v10.1.x] Alerting: Fix deleting rules in a folder with matching UID in another organization" --body-file - --label "type/bug" --label "area/backend" --label "add to changelog" --label "backport" --base v10.1.x --milestone 10.1.x --web

Or, if you don't have the GitHub CLI installed (we recommend you install it!):

# Push the branch to GitHub:
git push --set-upstream origin backport-78258-to-v10.1.x

# Create a pull request where the `base` branch is `v10.1.x` and the `compare`/`head` branch is `backport-78258-to-v10.1.x`.

# Remove the local backport branch
git switch main
git branch -D backport-78258-to-v10.1.x

[grafana-delivery-bot]

The backport to v10.2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-78258-to-v10.2.x origin/v10.2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 6d4625ad52b2e2ad3b2b88e394f69e8fde83c04b

When the conflicts are resolved, stage and commit the changes:

git add . && git cherry-pick --continue

If you have the GitHub CLI installed:

# Push the branch to GitHub:
git push --set-upstream origin backport-78258-to-v10.2.x
# Create the PR body template
PR_BODY=$(gh pr view 78258 --json body --template 'Backport 6d4625ad52b2e2ad3b2b88e394f69e8fde83c04b from #78258{{ "\n\n---\n\n" }}{{ index . "body" }}')
# Create the PR on GitHub
echo "${PR_BODY}" | gh pr create --title "[v10.2.x] Alerting: Fix deleting rules in a folder with matching UID in another organization" --body-file - --label "type/bug" --label "area/backend" --label "add to changelog" --label "backport" --base v10.2.x --milestone 10.2.x --web

Or, if you don't have the GitHub CLI installed (we recommend you install it!):

# Push the branch to GitHub:
git push --set-upstream origin backport-78258-to-v10.2.x

# Create a pull request where the `base` branch is `v10.2.x` and the `compare`/`head` branch is `backport-78258-to-v10.2.x`.

# Remove the local backport branch
git switch main
git branch -D backport-78258-to-v10.2.x