-
Notifications
You must be signed in to change notification settings - Fork 165
feat(BA-2839): RBAC entity creator, purger and inviter #7581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces RBAC (Role-Based Access Control) entity management functionality, adding three new modules for creating, purging, and inviting entities with RBAC-aware operations. The implementation provides specialized repository patterns for managing entity lifecycle with automatic handling of RBAC associations, permissions, and scope relationships.
Key changes:
- New RBAC entity creator with automatic scope-entity association creation
- New RBAC entity purger with cascading cleanup of related permissions and associations
- New RBAC entity inviter for granting permissions to system roles
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
src/ai/backend/manager/repositories/base/rbac_entity/creator.py |
New RBAC-aware creator that extends basic insert operations with automatic AssociationScopesEntities row creation |
src/ai/backend/manager/repositories/base/rbac_entity/purger.py |
New RBAC-aware purger that deletes entities and cascades to clean up object permissions, permission groups, and scope associations |
src/ai/backend/manager/repositories/base/rbac_entity/inviter.py |
New functionality to grant object permissions to system roles within a scope |
src/ai/backend/manager/repositories/base/creator.py |
Modified to add RBACEntityRow interface (appears to be unintentional breaking change) |
src/ai/backend/manager/repositories/base/rbac_entity/__init__.py |
Empty init file for new rbac_entity module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stmt = sa.delete(table).where(pk_columns[0] == purger.pk_value).returning(*table.columns) | ||
|
|
||
| result = await db_sess.execute(stmt) | ||
| row_data = result.fetchone() | ||
|
|
||
| if row_data is None: | ||
| return None | ||
|
|
||
| deleted_row: TRow = row_class(**dict(row_data._mapping)) | ||
| return PurgerResult(row=deleted_row) |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The execute_purger function duplicates the core deletion logic from the base purger (base/purger.py lines 68-86) including table access, PK column extraction, the DELETE statement with RETURNING, and result processing. This significant code duplication makes the codebase harder to maintain. Consider refactoring to reuse the base purger's deletion logic and add RBAC-specific cleanup as a pre-deletion hook or wrapper.
src/ai/backend/manager/repositories/base/rbac_entity/inviter.py
Outdated
Show resolved
Hide resolved
src/ai/backend/manager/repositories/base/rbac_entity/inviter.py
Outdated
Show resolved
Hide resolved
src/ai/backend/manager/repositories/base/rbac_entity/inviter.py
Outdated
Show resolved
Hide resolved
| row: TEntityRow | ||
|
|
||
|
|
||
| async def execute_creator( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since execute_creator and execute_purger are already existing names, how about using different names?
| scope_id = creator.scope_id | ||
| object_id = row.parsed_object_id() | ||
| db_sess.add( | ||
| AssociationScopesEntitiesRow( | ||
| scope_type=scope_id.scope_type, | ||
| scope_id=scope_id.scope_id, | ||
| entity_type=object_id.entity_type, | ||
| entity_id=object_id.entity_id, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it okay if this part isn't flushed?
| db_sess.add( | ||
| AssociationScopesEntitiesRow( | ||
| scope_type=scope_id.scope_type, | ||
| scope_id=scope_id.scope_id, | ||
| entity_type=object_id.entity_type, | ||
| entity_id=object_id.entity_id, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. Doesn't need to be flushed(or commit)?
| class RBACEntityRow(ABC): | ||
| @abstractmethod | ||
| def parsed_scope_id(self) -> ScopeId: | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this duplicate what's defined in rbac_entity/creator.py? (If both are needed, it seems we should use different names.)
HyeockJinKim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking of inheriting the existing CreatorSpec and implementing the abstractmethod and then defining an additional abstractmethod.
9552aac to
4af738e
Compare
…BAC entity operations - Rename Inviter class to Granter for more general terminology - Add field-level support for Creator and Purger operations - Extract common insert utilities to utils.py (insert_on_conflict_do_nothing, bulk_insert_on_conflict_do_nothing) - Refactor Creator/Purger to extend base classes - Add entity scope permission group handling in Granter
- Add test_creator.py for RBACCreator entity and field creation tests - Add test_granter.py for RBACGranter permission assignment tests - Add test_purger.py for RBACPurger entity deletion and cleanup tests
- Fix _sa_instance_state error in utils.py by filtering ORM internal attributes - Fix permission group deletion logic to preserve groups when other entities exist in same scope - Fix duplicate role rows by using unique() in _get_related_roles query - Remove unnecessary logging in creator.py
- Replace N+1 queries with batch entity-scope lookup - Add _get_entity_scopes() for single-query scope retrieval - Extract helper functions for permission_group deletion logic - Add test for multi-role scenario with shared entity
…tion - Add scope_association_rows relationship to ObjectPermissionRow - Use selectinload chain to load scope associations with object permissions - Remove _get_entity_scopes() and _all_object_permission_entities_in_roles() - Reduce purger queries from 2 to 1
d9dd148 to
1c326cc
Compare
resolves #6417 (BA-2839)
Summary
Implement RBAC Entity Repository Pattern with three core components: Creator, Granter, and Purger.
This pattern provides a consistent way to manage RBAC-related records throughout the lifecycle of RBAC entities (vfolder, endpoint, etc.).
Components
1. Creator
Automatically creates required RBAC records when an RBAC entity is created.
flowchart TD A[execute_creator] --> B[Insert main entity Row] B --> C{Has field_id?} C -->|Yes| D[Insert EntityFieldRow] C -->|No| E[Insert AssociationScopesEntitiesRow] D --> F[Return CreatorResult] E --> FRecords Created:
entity_fieldsassociation_scopes_entities2. Granter
Grants object-level permissions on an RBAC entity to another scope's system roles.
Use Case: When user A invites user B to a VFolder, user B's system role gets object permissions for that VFolder.
flowchart TD A[execute_granter] --> B[Find system roles for target scope] B --> C[Ensure PermissionGroupRow for entity's original scope] C --> D[Add ObjectPermissionRow for each role x operation] D --> E[Add AssociationScopesEntitiesRow for target scope] E --> F[Flush]Records Created:
permission_groupsobject_permissionsassociation_scopes_entities3. Purger
Cleans up related RBAC records when an RBAC entity is deleted.
flowchart TD A[execute_purger] --> B{Has field_id?} B -->|Yes| C[Delete EntityFieldRow] B -->|No| D[Delete RBAC entity records] D --> E[Get related roles with selectinload] E --> F[Identify ObjectPermissionRows to delete] E --> G[Identify PermissionGroupRows to delete] F --> H[Delete ObjectPermissionRows] G --> I[Delete PermissionGroupRows] H --> J[Delete AssociationScopesEntitiesRows] I --> J C --> K[Delete main entity Row] J --> K K --> L[Return PurgerResult]PermissionGroup Deletion Condition:
A PermissionGroup is deleted only when:
Query Optimization:
ObjectPermissionRow.scope_association_rows) for single-query loadingRoleRow -> ObjectPermissionRow -> AssociationScopesEntitiesRowArchitecture
erDiagram MainEntity ||--o| AssociationScopesEntities : "entity-scoped" MainEntity ||--o| EntityField : "field-scoped" Role ||--o{ ObjectPermission : has Role ||--o{ PermissionGroup : has ObjectPermission }o--|| MainEntity : references ObjectPermission ||--o{ AssociationScopesEntities : "via composite join" PermissionGroup ||--o{ Permission : containsTest Coverage
tests/unit/manager/repositories/base/rbac_entity/test_creator.pytests/unit/manager/repositories/base/rbac_entity/test_granter.pytests/unit/manager/repositories/base/rbac_entity/test_purger.pyChecklist: (if applicable)
ai.backend.testdocsdirectory