Skip to content

fix: ensure to return delete requests only for the requested tanent - #18589

Merged
sandeepsukhani merged 5 commits into
grafana:mainfrom
tsaikat:fix/18585
Jul 26, 2025
Merged

fix: ensure to return delete requests only for the requested tanent#18589
sandeepsukhani merged 5 commits into
grafana:mainfrom
tsaikat:fix/18585

Conversation

@tsaikat

@tsaikat tsaikat commented Jul 25, 2025

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
When api used to query delete requests for a tanent/userID, this we returning delete requests for all tanent with similar prefix in their name. For example when getting delete request for tanent-1 we also endup getting delete requests for tenant-12 because their prefix match.
In this PR, This will filter out the irrelevant delete requests and only return the one that is exact match with the provided tenant/userID. (Test included)

Which issue(s) this PR fixes:
Fixes #18585

@tsaikat
tsaikat requested a review from a team as a code owner July 25, 2025 13:57
@CLAassistant

CLAassistant commented Jul 25, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@sandeepsukhani sandeepsukhani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing the issue! I have left a minor comment to improve the fix a bit.
Can you please take care of it?

Comment on lines +214 to +221
filteredDeleteGroups := []DeleteRequest{}
for _, dr := range deleteGroups {
if dr.UserID == userID {
filteredDeleteGroups = append(filteredDeleteGroups, dr)
}
}

deleteRequests := mergeDeletes(filteredDeleteGroups)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid doing this by using []byte(userID + ":") for RangeValuePrefix on line 208. This would also let us fetch only the required records from the database.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. done!

@sandeepsukhani
sandeepsukhani enabled auto-merge (squash) July 26, 2025 06:21
@sandeepsukhani
sandeepsukhani merged commit cdb190b into grafana:main Jul 26, 2025
65 checks passed
@tsaikat
tsaikat deleted the fix/18585 branch July 26, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants