Skip to content

feat(BA-5765): add RBAC-enforced VFolder purge mutation#11165

Draft
fregataa wants to merge 1 commit intomainfrom
feature/BA-5765-vfolder-purge-rbac
Draft

feat(BA-5765): add RBAC-enforced VFolder purge mutation#11165
fregataa wants to merge 1 commit intomainfrom
feature/BA-5765-vfolder-purge-rbac

Conversation

@fregataa
Copy link
Copy Markdown
Member

Summary

  • Add PurgeVFolderV2RBACAction (SingleEntityActionProcessor + single_entity_rbac_validators).
  • Service purge_v2_rbac()get_by_id + delete_vfolders_forever + storage removal, no manual user/host checks.
  • Adapter purge() routes to RBAC processor; bulk_purge() stays on legacy path.

Test plan

  • pants fmt/fix/lint/check — green
  • Component test: TestPurgeVFolderRBAC (regular user 403) via SDK

Resolves BA-5765

Copilot AI review requested due to automatic review settings April 16, 2026 15:11
@github-actions github-actions bot added size:L 100~500 LoC comp:manager Related to Manager component labels Apr 16, 2026
@fregataa fregataa force-pushed the feature/BA-5765-vfolder-purge-rbac branch from 5812862 to 66164d9 Compare April 16, 2026 15:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an RBAC-enforced purge path for the v2 VFolder REST mutation, shifting authorization to the SingleEntityActionProcessor/RBAC validators and introducing a dedicated service method for the RBAC flow.

Changes:

  • Introduce PurgeVFolderV2RBACAction (+ result) and wire it into VFolderProcessors via SingleEntityActionProcessor.
  • Add VFolderService.purge_v2_rbac() and route the v2 adapter purge() call through the RBAC processor.
  • Add a component test covering the “regular user denied” scenario and a towncrier fragment.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/component/vfolder_v2/test_vfolder_mutation.py New component test exercising the v2 purge endpoint via SDK (currently denial-only).
src/ai/backend/manager/services/vfolder/services/vfolder.py Adds purge_v2_rbac() service implementation used by the RBAC processor.
src/ai/backend/manager/services/vfolder/processors/vfolder.py Registers the new RBAC purge action + processor and advertises it in supported_actions().
src/ai/backend/manager/services/vfolder/actions/vfolder_in_project.py New RBAC purge action/result definitions for single-entity validation.
src/ai/backend/manager/api/adapters/vfolder.py Routes purge() to the new RBAC processor (bulk purge remains legacy).
changes/BA-5765.feature.md Towncrier fragment documenting the new RBAC-enforced purge mutation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/component/vfolder_v2/test_vfolder_mutation.py
Comment thread tests/component/vfolder_v2/test_vfolder_mutation.py
Comment thread src/ai/backend/manager/services/vfolder/services/vfolder.py
Comment thread src/ai/backend/manager/services/vfolder/actions/vfolder_v2_rbac.py
@fregataa fregataa force-pushed the feature/BA-5765-vfolder-purge-rbac branch from 66164d9 to 219f0ac Compare April 16, 2026 16:05
@fregataa fregataa requested a review from a team April 17, 2026 01:44
@jopemachine
Copy link
Copy Markdown
Member

jopemachine commented Apr 17, 2026

Is there a reason why bulk_purge is not replaced with the purge_v2_rbac?
If bulk_purge is migrated to the RBAC version, there would be no remaining usages of purge_v2, so it seems safe to replace it.

Add PurgeVFolderV2RBACAction (SingleEntityActionProcessor + RBAC).
Adapter purge() routes to RBAC path; bulk_purge() stays on legacy.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@fregataa fregataa force-pushed the feature/BA-5765-vfolder-purge-rbac branch from 219f0ac to 6046cdf Compare April 19, 2026 09:42
me = current_user()
if me is None:
raise UnreachableError("User context is not available")
action = PurgeVFolderV2Action(user_id=me.user_id, vfolder_id=vfolder_id)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PurgeVFolderV2Action seems like it might become a dangling reference. If we need to move forward with this, please clean up PurgeVFolderV2Action.

@fregataa fregataa marked this pull request as draft April 20, 2026 05:03
@fregataa
Copy link
Copy Markdown
Member Author

Is there a reason why bulk_purge is not replaced with the purge_v2_rbac? If bulk_purge is migrated to the RBAC version, there would be no remaining usages of purge_v2, so it seems safe to replace it.

Because batch RBAC validation is not ready yet. I target this job to support purge single VFolder only and I will impl batch purge with RBAC in the future. Check #11186 @jopemachine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants