Skip to content

Commit bd2ac5b

Browse files
fregataaclaude
fregataa
and
claude
committed
fix(BA-4525): Update callers of changed interfaces for type consistency
Replace ScopeType/EntityType with RBACElementType at all call sites where permission controller interfaces now expect the unified enum. Update test files to use RBACElementType for scope and entity type parameters, and remove unused GLOBAL scope references from tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1f9ff1a commit bd2ac5b

8 files changed

Lines changed: 134 additions & 128 deletions

File tree

‎src/ai/backend/manager/api/adapters/rbac.py‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -689,9 +689,9 @@ async def create_permission(self, input: CreatePermissionInputDTO) -> Permission
689689
creator: Creator[PermissionRow] = Creator(
690690
spec=PermissionCreatorSpec(
691691
role_id=input.role_id,
692-
scope_type=RBACElementType(input.scope_type).to_scope_type(),
692+
scope_type=RBACElementType(input.scope_type),
693693
scope_id=input.scope_id,
694-
entity_type=RBACElementType(input.entity_type).to_entity_type(),
694+
entity_type=RBACElementType(input.entity_type),
695695
operation=InternalOperationType(input.operation),
696696
)
697697
)
@@ -706,7 +706,7 @@ async def update_permission(self, input: UpdatePermissionInputDTO) -> Permission
706706
"""Update an existing scoped permission."""
707707
spec = PermissionUpdaterSpec(
708708
scope_type=(
709-
OptionalState.update(RBACElementType(input.scope_type).to_scope_type())
709+
OptionalState.update(RBACElementType(input.scope_type))
710710
if input.scope_type is not None
711711
else OptionalState.nop()
712712
),
@@ -716,7 +716,7 @@ async def update_permission(self, input: UpdatePermissionInputDTO) -> Permission
716716
else OptionalState.nop()
717717
),
718718
entity_type=(
719-
OptionalState.update(RBACElementType(input.entity_type).to_entity_type())
719+
OptionalState.update(RBACElementType(input.entity_type))
720720
if input.entity_type is not None
721721
else OptionalState.nop()
722722
),
@@ -877,11 +877,13 @@ def _convert_permission_filter(self, f: PermissionFilterDTO) -> list[QueryCondit
877877
if f.role_id is not None:
878878
conditions.append(ScopedPermissionConditions.by_role_id(f.role_id))
879879
if f.scope_type is not None:
880-
scope_type = RBACElementType(f.scope_type).to_scope_type()
881-
conditions.append(ScopedPermissionConditions.by_scope_type(scope_type))
880+
conditions.append(
881+
ScopedPermissionConditions.by_scope_type(RBACElementType(f.scope_type))
882+
)
882883
if f.entity_type is not None:
883-
entity_type = RBACElementType(f.entity_type).to_entity_type()
884-
conditions.append(ScopedPermissionConditions.by_entity_type(entity_type))
884+
conditions.append(
885+
ScopedPermissionConditions.by_entity_type(RBACElementType(f.entity_type))
886+
)
885887
if f.AND:
886888
for sub in f.AND:
887889
conditions.extend(self._convert_permission_filter(sub))
@@ -1111,8 +1113,7 @@ def _convert_assignment_orders(orders: list[RoleAssignmentOrderByDTO]) -> list[Q
11111113
def _convert_entity_filter(self, f: EntityFilterDTO) -> list[QueryCondition]:
11121114
conditions: list[QueryCondition] = []
11131115
if f.entity_type is not None:
1114-
entity_type = RBACElementType(f.entity_type).to_entity_type()
1115-
conditions.append(EntityScopeConditions.by_entity_type(entity_type))
1116+
conditions.append(EntityScopeConditions.by_entity_type(RBACElementType(f.entity_type)))
11161117
if f.entity_id is not None:
11171118
condition = self.convert_string_filter(
11181119
f.entity_id,

‎tests/component/rbac/test_rbac_permission.py‎

Lines changed: 59 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@
55

66
import pytest
77

8-
from ai.backend.common.data.permission.types import GLOBAL_SCOPE_ID, OperationType, ScopeType
8+
from ai.backend.common.data.permission.types import (
9+
OperationType,
10+
RBACElementType,
11+
ScopeType,
12+
)
913
from ai.backend.manager.data.permission.id import ObjectId, ScopeId
1014
from ai.backend.manager.data.permission.object_permission import ObjectPermissionData
1115
from ai.backend.manager.data.permission.permission import PermissionData
@@ -71,14 +75,15 @@ async def test_create_basic_permission(
7175
self,
7276
permission_controller_processors: PermissionControllerProcessors,
7377
target_role: Any,
78+
domain_fixture: str,
7479
) -> None:
7580
"""S-CREATE-1: Create basic permission with valid params → PermissionData returned."""
7681
creator = Creator(
7782
spec=PermissionCreatorSpec(
7883
role_id=target_role.role.id,
79-
scope_type=ScopeType.GLOBAL,
80-
scope_id=GLOBAL_SCOPE_ID,
81-
entity_type=EntityType.SESSION,
84+
scope_type=RBACElementType.DOMAIN,
85+
scope_id=domain_fixture,
86+
entity_type=RBACElementType.SESSION,
8287
operation=OperationType.READ,
8388
)
8489
)
@@ -88,7 +93,7 @@ async def test_create_basic_permission(
8893

8994
assert isinstance(result.data, PermissionData)
9095
assert result.data.role_id == target_role.role.id
91-
assert result.data.scope_type == ScopeType.GLOBAL
96+
assert result.data.scope_type == ScopeType.DOMAIN
9297
assert result.data.entity_type == EntityType.SESSION
9398
assert result.data.operation == OperationType.READ
9499

@@ -101,12 +106,18 @@ async def test_create_permissions_with_various_combinations(
101106
self,
102107
permission_controller_processors: PermissionControllerProcessors,
103108
target_role: Any,
109+
domain_fixture: str,
104110
) -> None:
105111
"""S-CREATE-2: Create permissions with various scope/entity/operation combinations."""
106-
combos: list[tuple[ScopeType, str, EntityType, OperationType]] = [
107-
(ScopeType.GLOBAL, GLOBAL_SCOPE_ID, EntityType.SESSION, OperationType.READ),
108-
(ScopeType.GLOBAL, GLOBAL_SCOPE_ID, EntityType.IMAGE, OperationType.UPDATE),
109-
(ScopeType.GLOBAL, GLOBAL_SCOPE_ID, EntityType.VFOLDER, OperationType.SOFT_DELETE),
112+
combos: list[tuple[RBACElementType, str, RBACElementType, OperationType]] = [
113+
(RBACElementType.DOMAIN, domain_fixture, RBACElementType.SESSION, OperationType.READ),
114+
(RBACElementType.DOMAIN, domain_fixture, RBACElementType.IMAGE, OperationType.UPDATE),
115+
(
116+
RBACElementType.DOMAIN,
117+
domain_fixture,
118+
RBACElementType.VFOLDER,
119+
OperationType.SOFT_DELETE,
120+
),
110121
]
111122
created_ids: list[uuid.UUID] = []
112123

@@ -124,7 +135,7 @@ async def test_create_permissions_with_various_combinations(
124135
)
125136
)
126137
)
127-
assert result.data.entity_type == entity_type
138+
assert result.data.entity_type == entity_type.to_entity_type()
128139
assert result.data.operation == operation
129140
assert result.data.role_id == target_role.role.id
130141
created_ids.append(result.data.id)
@@ -139,13 +150,14 @@ async def test_create_duplicate_permission_raises_unique_constraint(
139150
self,
140151
permission_controller_processors: PermissionControllerProcessors,
141152
target_role: Any,
153+
domain_fixture: str,
142154
) -> None:
143155
"""F-BIZ-4: Create duplicate permission → unique constraint error."""
144156
spec = PermissionCreatorSpec(
145157
role_id=target_role.role.id,
146-
scope_type=ScopeType.GLOBAL,
147-
scope_id=GLOBAL_SCOPE_ID,
148-
entity_type=EntityType.VFOLDER,
158+
scope_type=RBACElementType.DOMAIN,
159+
scope_id=domain_fixture,
160+
entity_type=RBACElementType.VFOLDER,
149161
operation=OperationType.READ,
150162
)
151163

@@ -172,16 +184,17 @@ async def test_delete_existing_permission(
172184
self,
173185
permission_controller_processors: PermissionControllerProcessors,
174186
target_role: Any,
187+
domain_fixture: str,
175188
) -> None:
176189
"""S-DELETE-1: Delete existing permission → deletion response."""
177190
create_result = await permission_controller_processors.create_permission.wait_for_complete(
178191
CreatePermissionAction(
179192
creator=Creator(
180193
spec=PermissionCreatorSpec(
181194
role_id=target_role.role.id,
182-
scope_type=ScopeType.GLOBAL,
183-
scope_id=GLOBAL_SCOPE_ID,
184-
entity_type=EntityType.SESSION,
195+
scope_type=RBACElementType.DOMAIN,
196+
scope_id=domain_fixture,
197+
entity_type=RBACElementType.SESSION,
185198
operation=OperationType.HARD_DELETE,
186199
)
187200
)
@@ -200,16 +213,17 @@ async def test_deleted_permission_no_longer_exists(
200213
self,
201214
permission_controller_processors: PermissionControllerProcessors,
202215
target_role: Any,
216+
domain_fixture: str,
203217
) -> None:
204218
"""S-DELETE-2: Verify deleted permission no longer exists."""
205219
create_result = await permission_controller_processors.create_permission.wait_for_complete(
206220
CreatePermissionAction(
207221
creator=Creator(
208222
spec=PermissionCreatorSpec(
209223
role_id=target_role.role.id,
210-
scope_type=ScopeType.GLOBAL,
211-
scope_id=GLOBAL_SCOPE_ID,
212-
entity_type=EntityType.IMAGE,
224+
scope_type=RBACElementType.DOMAIN,
225+
scope_id=domain_fixture,
226+
entity_type=RBACElementType.IMAGE,
213227
operation=OperationType.SOFT_DELETE,
214228
)
215229
)
@@ -255,7 +269,7 @@ async def test_create_object_permission(
255269
creator=Creator(
256270
spec=ObjectPermissionCreatorSpec(
257271
role_id=target_role.role.id,
258-
entity_type=EntityType.SESSION,
272+
entity_type=RBACElementType.SESSION,
259273
entity_id=entity_id,
260274
operation=OperationType.READ,
261275
)
@@ -291,7 +305,7 @@ async def test_create_multiple_object_permissions_for_same_role(
291305
creator=Creator(
292306
spec=ObjectPermissionCreatorSpec(
293307
role_id=target_role.role.id,
294-
entity_type=EntityType.VFOLDER,
308+
entity_type=RBACElementType.VFOLDER,
295309
entity_id=entity_id,
296310
operation=OperationType.READ,
297311
)
@@ -320,7 +334,7 @@ async def test_create_object_permission_with_nonexistent_role_succeeds(
320334
creator=Creator(
321335
spec=ObjectPermissionCreatorSpec(
322336
role_id=uuid.uuid4(), # non-existent role — no FK constraint enforced
323-
entity_type=EntityType.SESSION,
337+
entity_type=RBACElementType.SESSION,
324338
entity_id=str(uuid.uuid4()),
325339
operation=OperationType.READ,
326340
)
@@ -352,7 +366,7 @@ async def test_delete_object_permission(
352366
creator=Creator(
353367
spec=ObjectPermissionCreatorSpec(
354368
role_id=target_role.role.id,
355-
entity_type=EntityType.IMAGE,
369+
entity_type=RBACElementType.IMAGE,
356370
entity_id=entity_id,
357371
operation=OperationType.READ,
358372
)
@@ -394,6 +408,7 @@ async def test_user_with_matching_object_permission_returns_true(
394408
permission_repo: PermissionControllerRepository,
395409
role_factory: RoleFactory,
396410
admin_user_fixture: Any,
411+
domain_fixture: str,
397412
) -> None:
398413
"""S-ENTITY-1: User with matching role+ObjectPermission → True."""
399414
role = await role_factory()
@@ -407,9 +422,9 @@ async def test_user_with_matching_object_permission_returns_true(
407422
creator=Creator(
408423
spec=PermissionCreatorSpec(
409424
role_id=role_id,
410-
scope_type=ScopeType.GLOBAL,
411-
scope_id=GLOBAL_SCOPE_ID,
412-
entity_type=EntityType.SESSION,
425+
scope_type=RBACElementType.DOMAIN,
426+
scope_id=domain_fixture,
427+
entity_type=RBACElementType.SESSION,
413428
operation=OperationType.READ,
414429
)
415430
)
@@ -422,7 +437,7 @@ async def test_user_with_matching_object_permission_returns_true(
422437
creator=Creator(
423438
spec=ObjectPermissionCreatorSpec(
424439
role_id=role_id,
425-
entity_type=EntityType.SESSION,
440+
entity_type=RBACElementType.SESSION,
426441
entity_id=entity_id,
427442
operation=OperationType.READ,
428443
)
@@ -467,6 +482,7 @@ async def test_user_without_matching_object_permission_returns_false(
467482
permission_repo: PermissionControllerRepository,
468483
role_factory: RoleFactory,
469484
admin_user_fixture: Any,
485+
domain_fixture: str,
470486
) -> None:
471487
"""S-ENTITY-2: User without matching ObjectPermission → False."""
472488
role = await role_factory()
@@ -481,9 +497,9 @@ async def test_user_without_matching_object_permission_returns_false(
481497
creator=Creator(
482498
spec=PermissionCreatorSpec(
483499
role_id=role_id,
484-
scope_type=ScopeType.GLOBAL,
485-
scope_id=GLOBAL_SCOPE_ID,
486-
entity_type=EntityType.SESSION,
500+
scope_type=RBACElementType.DOMAIN,
501+
scope_id=domain_fixture,
502+
entity_type=RBACElementType.SESSION,
487503
operation=OperationType.READ,
488504
)
489505
)
@@ -496,7 +512,7 @@ async def test_user_without_matching_object_permission_returns_false(
496512
creator=Creator(
497513
spec=ObjectPermissionCreatorSpec(
498514
role_id=role_id,
499-
entity_type=EntityType.SESSION,
515+
entity_type=RBACElementType.SESSION,
500516
entity_id=other_entity_id,
501517
operation=OperationType.READ,
502518
)
@@ -560,6 +576,7 @@ async def test_user_with_permission_in_scope_returns_true(
560576
permission_repo: PermissionControllerRepository,
561577
role_factory: RoleFactory,
562578
admin_user_fixture: Any,
579+
domain_fixture: str,
563580
) -> None:
564581
"""S-SCOPE-1: User has permission in target scope → True."""
565582
role = await role_factory()
@@ -571,9 +588,9 @@ async def test_user_with_permission_in_scope_returns_true(
571588
creator=Creator(
572589
spec=PermissionCreatorSpec(
573590
role_id=role_id,
574-
scope_type=ScopeType.GLOBAL,
575-
scope_id=GLOBAL_SCOPE_ID,
576-
entity_type=EntityType.SESSION,
591+
scope_type=RBACElementType.DOMAIN,
592+
scope_id=domain_fixture,
593+
entity_type=RBACElementType.SESSION,
577594
operation=OperationType.READ,
578595
)
579596
)
@@ -589,7 +606,7 @@ async def test_user_with_permission_in_scope_returns_true(
589606
ScopePermissionCheckInput(
590607
user_id=user_id,
591608
target_entity_type=EntityType.SESSION,
592-
target_scope_id=ScopeId(scope_type=ScopeType.GLOBAL, scope_id=GLOBAL_SCOPE_ID),
609+
target_scope_id=ScopeId(scope_type=ScopeType.DOMAIN, scope_id=domain_fixture),
593610
operation=OperationType.READ,
594611
)
595612
)
@@ -607,13 +624,14 @@ async def test_user_with_permission_in_scope_returns_true(
607624
async def test_user_without_permission_in_scope_returns_false(
608625
self,
609626
permission_repo: PermissionControllerRepository,
627+
domain_fixture: str,
610628
) -> None:
611629
"""S-SCOPE-2: User lacks permission in target scope → False."""
612630
has_perm = await permission_repo.check_permission_in_scope(
613631
ScopePermissionCheckInput(
614632
user_id=uuid.uuid4(), # random user with no roles
615633
target_entity_type=EntityType.SESSION,
616-
target_scope_id=ScopeId(scope_type=ScopeType.GLOBAL, scope_id=GLOBAL_SCOPE_ID),
634+
target_scope_id=ScopeId(scope_type=ScopeType.DOMAIN, scope_id=domain_fixture),
617635
operation=OperationType.READ,
618636
)
619637
)
@@ -630,6 +648,7 @@ async def test_batch_check_returns_correct_mapping(
630648
permission_repo: PermissionControllerRepository,
631649
role_factory: RoleFactory,
632650
admin_user_fixture: Any,
651+
domain_fixture: str,
633652
) -> None:
634653
"""S-BATCH-1: Batch check returns correct per-object bool mapping."""
635654
role = await role_factory()
@@ -645,9 +664,9 @@ async def test_batch_check_returns_correct_mapping(
645664
creator=Creator(
646665
spec=PermissionCreatorSpec(
647666
role_id=role_id,
648-
scope_type=ScopeType.GLOBAL,
649-
scope_id=GLOBAL_SCOPE_ID,
650-
entity_type=EntityType.SESSION,
667+
scope_type=RBACElementType.DOMAIN,
668+
scope_id=domain_fixture,
669+
entity_type=RBACElementType.SESSION,
651670
operation=OperationType.READ,
652671
)
653672
)
@@ -660,7 +679,7 @@ async def test_batch_check_returns_correct_mapping(
660679
creator=Creator(
661680
spec=ObjectPermissionCreatorSpec(
662681
role_id=role_id,
663-
entity_type=EntityType.SESSION,
682+
entity_type=RBACElementType.SESSION,
664683
entity_id=entity_id_with_perm,
665684
operation=OperationType.READ,
666685
)

‎tests/unit/manager/api/rbac/test_scope_handlers.py‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import pytest
1414

1515
from ai.backend.common.api_handlers import BodyParam, PathParam
16-
from ai.backend.common.data.permission.types import ScopeType
16+
from ai.backend.common.data.permission.types import RBACElementType, ScopeType
1717
from ai.backend.common.dto.manager.rbac.path import SearchScopesPathParam
1818
from ai.backend.common.dto.manager.rbac.request import SearchScopesRequest
1919
from ai.backend.manager.api.rest.rbac.handler import RBACHandler
@@ -65,8 +65,11 @@ def make_test_user_ctx() -> UserContext:
6565
class TestGetScopeTypesHandler:
6666
"""Tests for get_scope_types handler."""
6767

68-
# Constants
69-
EXPECTED_SCOPE_TYPES_COUNT = len(ScopeType)
68+
# Count only RBACElementType members convertible to ScopeType
69+
# (the handler filters out non-convertible element types)
70+
EXPECTED_SCOPE_TYPES_COUNT = len([
71+
et for et in RBACElementType if et.value in {st.value for st in ScopeType}
72+
])
7073

7174
@pytest.fixture
7275
def mock_permission_controller(self) -> MagicMock:
@@ -83,7 +86,7 @@ async def test_get_scope_types_returns_scope_types(
8386
"""Test get_scope_types returns all scope types for superadmin."""
8487
handler = make_test_handler(mock_permission_controller)
8588
ctx = make_test_superadmin_ctx()
86-
action_result = GetScopeTypesActionResult(scope_types=list(ScopeType))
89+
action_result = GetScopeTypesActionResult(element_types=list(RBACElementType))
8790
mock_permission_controller.get_scope_types.wait_for_complete.return_value = action_result
8891

8992
response = await handler.get_scope_types(ctx=ctx)

0 commit comments

Comments
 (0)