Skip to content

refactor(BA-4521): replace ScopeType/EntityType with RBACElementType in RBAC layers#9999

Closed
fregataa wants to merge 2 commits into
mainfrom
feat/BA-4521-unify-rbac-element-type
Closed

refactor(BA-4521): replace ScopeType/EntityType with RBACElementType in RBAC layers#9999
fregataa wants to merge 2 commits into
mainfrom
feat/BA-4521-unify-rbac-element-type

Conversation

@fregataa
Copy link
Copy Markdown
Member

Summary

  • Replace ScopeType and RBAC EntityType references with the unified RBACElementType enum across permission controller (BA-4525), domain repositories/services (BA-4526), and REST API/DTO layers (BA-4612)
  • Use bridge methods (to_scope_type(), to_entity_type(), to_element()) at DB boundaries where ORM columns still store the old types
  • Handle GLOBAL scope as None since it has no RBACElementType equivalent — handler returns empty result for GLOBAL scope searches

Test plan

  • Verify RBAC permission CRUD operations via REST API
  • Verify scope search endpoints handle DOMAIN, PROJECT, USER, and GLOBAL correctly
  • Verify entity search endpoints return correct results
  • Verify vfolder creation/listing with scope types
  • Run pants check --changed-since=origin/main in CI

Resolves BA-4525, BA-4526, BA-4612

…in RBAC layers

Replace ScopeType and RBAC EntityType references with the unified
RBACElementType enum across permission controller, domain repositories,
REST API, and DTO layers. Bridge methods (to_scope_type/to_entity_type)
are used at DB boundaries where ORM columns still store the old types.
GLOBAL scope is handled as None since it has no RBACElementType equivalent.

Covers BA-4525, BA-4526, BA-4612.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 12, 2026 14:12
@github-actions github-actions Bot added size:L 100~500 LoC comp:manager Related to Manager component comp:common Related to Common component labels Mar 12, 2026
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

This PR replaces ScopeType and EntityType references with the unified RBACElementType enum across the RBAC permission controller, domain repositories/services, and REST API/DTO layers. Bridge methods (to_scope_type(), to_entity_type(), to_element()) are used at DB boundaries where ORM columns still store the old types.

Changes:

  • Replace ScopeType/EntityType with RBACElementType in action dataclasses, creator/updater specs, query conditions, and API DTOs
  • Handle GLOBAL scope as None since it has no RBACElementType equivalent, returning empty results for GLOBAL scope searches
  • Use bridge methods at ORM boundaries to convert between type systems

Reviewed changes

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

Show a summary per file
File Description
src/ai/backend/common/dto/manager/rbac/path.py Replace ScopeType/EntityType with RBACElementType in path params; GLOBAL → None
src/ai/backend/common/dto/manager/rbac/request.py Replace ScopeType/EntityType with RBACElementType in request models
src/ai/backend/common/dto/manager/rbac/response.py Replace ScopeType/EntityType with RBACElementType in response DTOs
src/ai/backend/manager/api/rest/rbac/handler.py Handle None scope_type for GLOBAL, update field references
src/ai/backend/manager/api/rest/rbac/scope_adapter.py Accept `RBACElementType
src/ai/backend/manager/api/rest/rbac/entity_adapter.py Accept RBACElementType params, convert at query boundary
src/ai/backend/manager/api/rest/rbac/permission_adapter.py Convert request types to RBACElementType for creator spec
src/ai/backend/manager/api/rest/rbac/object_permission_adapter.py Convert request entity_type for object permission creator
src/ai/backend/manager/api/rest/vfolder/handler.py Use RBACElementType for scope type in vfolder create/list
src/ai/backend/manager/api/gql/rbac/types/permission.py Simplify double conversions to single .to_element()
src/ai/backend/manager/api/gql/rbac/types/entity.py Simplify entity type conversion
src/ai/backend/manager/services/permission_contoller/service.py Use RBACElementType in service methods
src/ai/backend/manager/services/permission_contoller/actions/*.py Replace action field types
src/ai/backend/manager/services/vfolder/actions/base.py Replace ScopeType with RBACElementType in vfolder actions
src/ai/backend/manager/services/vfolder/services/vfolder.py Access _scope_type directly
src/ai/backend/manager/repositories/permission_controller/*.py Replace types in creators, updaters, options, role_manager, db_source
src/ai/backend/manager/repositories/vfolder/repository.py Use bridge methods at DB boundary
src/ai/backend/manager/repositories/session/creators.py Update comment to reference RBACElementType
src/ai/backend/manager/data/user/types.py Return RBACElementType keys from entity_operations()
src/ai/backend/manager/data/group/types.py Return RBACElementType keys from entity_operations()
src/ai/backend/manager/data/domain/types.py Return RBACElementType keys from entity_operations()

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

Comment on lines +48 to +50
EntityScopeConditions.by_scope_type(scope_type.to_scope_type()),
EntityScopeConditions.by_scope_id(scope_id),
EntityScopeConditions.by_entity_type(entity_type),
EntityScopeConditions.by_entity_type(entity_type.to_entity_type()),
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Double conversion bug: EntityScopeConditions.by_scope_type() now accepts RBACElementType and internally calls .to_scope_type(). But here you're calling .to_scope_type() first, passing a ScopeType to a function that expects RBACElementType. This will cause .to_scope_type() to be called on a ScopeType object, which doesn't have that method. Same issue with .to_entity_type() on the next line. Pass the RBACElementType values directly without conversion.

Copilot uses AI. Check for mistakes.
Comment on lines +57 to 65
"""Path parameter for searching scopes.

scope_type: ScopeType = Field(
description="Scope types", examples=["domain", "project", "user", "global"]
``scope_type`` is ``None`` when the caller requests the GLOBAL scope,
which has no ``RBACElementType`` equivalent.
"""

scope_type: RBACElementType | None = Field(
description="Scope types", examples=["domain", "project", "user"]
)
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Breaking API change: scope_type is typed as RBACElementType | None, but if a client sends "global" in the URL path, Pydantic will raise a validation error because "global" is not a valid RBACElementType value and path parameters won't naturally become None. The None case can only be reached if the path parameter is somehow omitted, which typically isn't how path parameters work. You likely need a custom validator or a separate union type that maps "global" to None.

Copilot uses AI. Check for mistakes.
Comment on lines +46 to +48
scope_type=request.scope_type.to_element(),
scope_id=request.scope_id,
entity_type=request.entity_type,
entity_type=request.entity_type.to_element(),
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

request.scope_type and request.entity_type are now RBACElementType (per the updated CreatePermissionRequest), but RBACElementType does not have a to_element() method — that method exists on ScopeType and EntityType. Since these are already RBACElementType, pass them directly without calling .to_element().

Copilot uses AI. Check for mistakes.
spec=ObjectPermissionCreatorSpec(
role_id=request.role_id,
entity_type=request.entity_type,
entity_type=request.entity_type.to_element(),
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

request.entity_type is already RBACElementType (per the updated CreateObjectPermissionRequest), but RBACElementType does not have a to_element() method. Pass it directly without calling .to_element().

Suggested change
entity_type=request.entity_type.to_element(),
entity_type=request.entity_type,

Copilot uses AI. Check for mistakes.
) -> BatchQuerier:
"""Build a BatchQuerier based on scope type.

``scope_type`` is ``None`` for the GLOBAL scope, which has no
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be possible to add GLOBAL as an enum value?
I think it doesn’t seem good to treat scope_type as GLOBAL when it is None or an empty string.

@fregataa fregataa added this to the 26.4 milestone Mar 19, 2026
@fregataa
Copy link
Copy Markdown
Member Author

Splitting into separate PRs per issue: BA-4525 and BA-4526. BA-4612 deferred.

@fregataa fregataa closed this Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants