Skip to content

Commit 1851f3b

Browse files
jopemachineclaude
andcommitted
refactor: model the fragment scoped search as a scope action
`ScopedSearchAppConfigFragmentAction` was a `BaseBulkAction[SearchableActionTarget]` holding a list of domain/user targets that the repository OR-combined into one query. A search is not a bulk mutation over entities: it acts at one RBAC scope and returns whatever lives under it. `BaseBulkAction` exists to authorize an operation per entity across a batch, which is why `bulk_update` / `bulk_purge` carry the bulk RBAC validator — the scoped search carried none at all, so once an API reaches it any authenticated caller could read any domain's or user's fragments by naming them in the request. Model it the way the sibling `CreateAppConfigFragmentAction` already models a write: one `BaseScopeAction` at the single scope named by the request, deriving its RBAC facets through `AppConfigScopeType.to_rbac_*`, wired through `ScopeActionProcessor` with the scope RBAC validator. The fragment row is `(scope_type, scope_id)`, so one scope expresses the query exactly: `DomainAppConfigFragmentSearchScope` and `UserAppConfigFragmentSearchScope` collapse into `AppConfigFragmentSearchScope`, and `scoped_search(querier, scope)` takes a single scope. Searching several scopes in one request is gone — it was the only thing the list bought, and it cannot be authorized as a single scope action. Nothing consumed the removed `queried_refs` / `element_refs()` on the result. Tests are parametrized over the scope instead of one near-identical test per kind; the OR-across-scopes case is dropped with the capability. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ef72c3f commit 1851f3b

8 files changed

Lines changed: 167 additions & 190 deletions

File tree

src/ai/backend/manager/repositories/app_config_fragment/db_source/db_source.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,12 +197,12 @@ async def admin_search(self, querier: BatchQuerier) -> AppConfigFragmentSearchRe
197197
async def scoped_search(
198198
self,
199199
querier: BatchQuerier,
200-
scopes: Sequence[SearchScope],
200+
scope: SearchScope,
201201
) -> AppConfigFragmentSearchResult:
202-
"""Scoped path: query fragments restricted to ``scopes`` (combined with OR)."""
202+
"""Scoped path: query the fragments written at ``scope``."""
203203
async with self._rbac_ops_provider.read_ops() as r:
204204
result = await r.batch_query_with_scopes(
205-
sa.select(AppConfigFragmentRow), querier, scopes
205+
sa.select(AppConfigFragmentRow), querier, [scope]
206206
)
207207
return AppConfigFragmentSearchResult(
208208
items=[row.AppConfigFragmentRow.to_data() for row in result.rows],

src/ai/backend/manager/repositories/app_config_fragment/repository.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ async def admin_search(self, querier: BatchQuerier) -> AppConfigFragmentSearchRe
8585

8686
@app_config_fragment_repository_resilience.apply()
8787
async def scoped_search(
88-
self, querier: BatchQuerier, scopes: Sequence[SearchScope]
88+
self, querier: BatchQuerier, scope: SearchScope
8989
) -> AppConfigFragmentSearchResult:
90-
return await self._db_source.scoped_search(querier, scopes)
90+
return await self._db_source.scoped_search(querier, scope)
9191

9292
@app_config_fragment_repository_resilience.apply()
9393
async def bulk_update(

src/ai/backend/manager/repositories/app_config_fragment/types.py

Lines changed: 15 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,17 @@
99
import sqlalchemy as sa
1010

1111
from ai.backend.common.data.app_config.types import AppConfigScopeType
12+
from ai.backend.common.identifier.app_config import AppConfigScopeID
1213
from ai.backend.common.identifier.domain import DomainID
1314
from ai.backend.common.identifier.user import UserID
1415
from ai.backend.manager.models.app_config_fragment.row import AppConfigFragmentRow
1516
from ai.backend.manager.models.clauses import QueryCondition
1617
from ai.backend.manager.models.scopes import ExistenceCheck, SearchScope
1718

1819
__all__ = (
20+
"AppConfigFragmentSearchScope",
1921
"AppConfigScopeArguments",
2022
"ResolvedAppConfigScope",
21-
"DomainAppConfigFragmentSearchScope",
22-
"UserAppConfigFragmentSearchScope",
2323
)
2424

2525

@@ -46,48 +46,28 @@ class ResolvedAppConfigScope:
4646

4747

4848
@dataclass(frozen=True)
49-
class DomainAppConfigFragmentSearchScope(SearchScope):
50-
"""Fragments written at one domain scope (``scope_type == domain``, ``scope_id == domain_id``).
49+
class AppConfigFragmentSearchScope(SearchScope):
50+
"""The fragments written at one scope, matching the row's ``(scope_type, scope_id)``.
5151
52-
One scope = one item of a scoped fragment query; the repository combines multiple
53-
scopes with ``OR``. ``existence_checks`` is empty by ``SearchableActionTarget``
54-
convention — RBAC already gates scope reachability.
52+
``existence_checks`` is empty: RBAC already gates whether the caller can reach the
53+
scope, so a missing owner is indistinguishable from an unreachable one.
5554
"""
5655

57-
domain_id: DomainID
58-
59-
@override
60-
def to_condition(self) -> QueryCondition:
61-
domain_id = self.domain_id
62-
63-
def inner() -> sa.sql.expression.ColumnElement[bool]:
64-
return sa.and_(
65-
AppConfigFragmentRow.scope_type == AppConfigScopeType.DOMAIN,
66-
AppConfigFragmentRow.scope_id == domain_id,
67-
)
68-
69-
return inner
70-
71-
@property
72-
@override
73-
def existence_checks(self) -> Sequence[ExistenceCheck[Any]]:
74-
return ()
75-
76-
77-
@dataclass(frozen=True)
78-
class UserAppConfigFragmentSearchScope(SearchScope):
79-
"""Fragments written at one user scope (``scope_type == user``, ``scope_id == user_id``)."""
80-
81-
user_id: UserID
56+
scope_type: AppConfigScopeType
57+
scope_id: AppConfigScopeID | None
58+
"""The scope owner — ``None`` only for ``public``, which has no owner."""
8259

8360
@override
8461
def to_condition(self) -> QueryCondition:
85-
user_id = self.user_id
62+
scope_type = self.scope_type
63+
scope_id = self.scope_id
8664

8765
def inner() -> sa.sql.expression.ColumnElement[bool]:
8866
return sa.and_(
89-
AppConfigFragmentRow.scope_type == AppConfigScopeType.USER,
90-
AppConfigFragmentRow.scope_id == user_id,
67+
AppConfigFragmentRow.scope_type == scope_type,
68+
AppConfigFragmentRow.scope_id.is_(None)
69+
if scope_id is None
70+
else AppConfigFragmentRow.scope_id == scope_id,
9171
)
9272

9373
return inner
Lines changed: 35 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,91 +1,68 @@
1-
"""Scoped app config fragment search action and its searchable targets."""
2-
31
from __future__ import annotations
42

5-
from collections.abc import Sequence
63
from dataclasses import dataclass
74
from typing import override
85

9-
from ai.backend.common.data.permission.types import EntityType, RBACElementType
10-
from ai.backend.common.identifier.domain import DomainID
11-
from ai.backend.common.identifier.user import UserID
12-
from ai.backend.manager.actions.action.bulk import BaseBulkAction, BaseBulkActionResult
13-
from ai.backend.manager.actions.action.types import SearchableActionTarget
6+
from ai.backend.common.data.permission.types import RBACElementType, ScopeType
147
from ai.backend.manager.actions.types import ActionOperationType
158
from ai.backend.manager.data.app_config_fragment.types import AppConfigFragmentData
169
from ai.backend.manager.data.permission.types import RBACElementRef
17-
from ai.backend.manager.models.scopes import SearchScope
1810
from ai.backend.manager.repositories.app_config_fragment.types import (
19-
DomainAppConfigFragmentSearchScope,
20-
UserAppConfigFragmentSearchScope,
11+
AppConfigFragmentSearchScope,
2112
)
2213
from ai.backend.manager.repositories.base import BatchQuerier
23-
24-
25-
@dataclass(frozen=True)
26-
class DomainAppConfigFragmentTarget(SearchableActionTarget):
27-
"""Scope item keyed by a domain — the fragments written at that domain scope."""
28-
29-
domain_id: DomainID
30-
31-
@override
32-
def to_rbac_element_ref(self) -> RBACElementRef:
33-
return RBACElementRef(element_type=RBACElementType.DOMAIN, element_id=str(self.domain_id))
34-
35-
@override
36-
def to_search_scope(self) -> SearchScope:
37-
return DomainAppConfigFragmentSearchScope(domain_id=self.domain_id)
38-
39-
40-
@dataclass(frozen=True)
41-
class UserAppConfigFragmentTarget(SearchableActionTarget):
42-
"""Scope item keyed by a user — the fragments written at that user scope."""
43-
44-
user_id: UserID
45-
46-
@override
47-
def to_rbac_element_ref(self) -> RBACElementRef:
48-
return RBACElementRef(element_type=RBACElementType.USER, element_id=str(self.user_id))
49-
50-
@override
51-
def to_search_scope(self) -> SearchScope:
52-
return UserAppConfigFragmentSearchScope(user_id=self.user_id)
14+
from ai.backend.manager.services.app_config_fragment.actions.base import (
15+
AppConfigFragmentScopeAction,
16+
AppConfigFragmentScopeActionResult,
17+
)
5318

5419

5520
@dataclass
56-
class ScopedSearchAppConfigFragmentAction(BaseBulkAction[SearchableActionTarget]):
57-
"""Scoped path: search the fragments under the given domain/user scope targets."""
21+
class ScopedSearchAppConfigFragmentAction(AppConfigFragmentScopeAction):
22+
"""Search the fragments written at one scope.
23+
24+
Acts at the RBAC scope named by ``scope``, so the scope RBAC validator authorizes the
25+
read the same way it authorizes a write at that scope in
26+
:class:`CreateAppConfigFragmentAction`.
27+
"""
5828

59-
items: list[SearchableActionTarget]
29+
scope: AppConfigFragmentSearchScope
6030
querier: BatchQuerier
6131

6232
@override
63-
def entity_id(self) -> str | None:
64-
return None
33+
@classmethod
34+
def operation_type(cls) -> ActionOperationType:
35+
return ActionOperationType.SEARCH
6536

6637
@override
67-
@classmethod
68-
def entity_type(cls) -> EntityType:
69-
return EntityType.APP_CONFIG_FRAGMENT
38+
def scope_type(self) -> ScopeType:
39+
return self.scope.scope_type.to_rbac_scope_type()
7040

7141
@override
72-
@classmethod
73-
def operation_type(cls) -> ActionOperationType:
74-
return ActionOperationType.SEARCH
42+
def scope_id(self) -> str:
43+
return self.scope.scope_type.to_rbac_scope_id(self.scope.scope_id)
7544

7645
@override
77-
def targets(self) -> Sequence[SearchableActionTarget]:
78-
return list(self.items)
46+
def target_element(self) -> RBACElementRef:
47+
element = self.scope.scope_type.to_rbac_element_type()
48+
if element is None:
49+
return RBACElementRef(RBACElementType.APP_CONFIG_FRAGMENT, "")
50+
# A non-null element type means domain or user scope, which always carries an owner.
51+
return RBACElementRef(element, str(self.scope.scope_id))
7952

8053

8154
@dataclass
82-
class ScopedSearchAppConfigFragmentActionResult(BaseBulkActionResult):
55+
class ScopedSearchAppConfigFragmentActionResult(AppConfigFragmentScopeActionResult):
56+
scope: AppConfigFragmentSearchScope
8357
data: list[AppConfigFragmentData]
8458
total_count: int
8559
has_next_page: bool
8660
has_previous_page: bool
87-
queried_refs: list[RBACElementRef]
8861

8962
@override
90-
def element_refs(self) -> list[RBACElementRef]:
91-
return list(self.queried_refs)
63+
def scope_type(self) -> ScopeType:
64+
return self.scope.scope_type.to_rbac_scope_type()
65+
66+
@override
67+
def scope_id(self) -> str:
68+
return self.scope.scope_type.to_rbac_scope_id(self.scope.scope_id)

src/ai/backend/manager/services/app_config_fragment/processors.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class AppConfigFragmentProcessors(AbstractProcessorPackage):
5151
admin_search: ScopeActionProcessor[
5252
AdminSearchAppConfigFragmentAction, AdminSearchAppConfigFragmentActionResult
5353
]
54-
scoped_search: BulkActionProcessor[
54+
scoped_search: ScopeActionProcessor[
5555
ScopedSearchAppConfigFragmentAction, ScopedSearchAppConfigFragmentActionResult
5656
]
5757
update: SingleEntityActionProcessor[
@@ -84,9 +84,12 @@ def __init__(
8484
service.get, action_monitors, validators=[validators.rbac.single_entity]
8585
)
8686
# admin_search is system-wide (all scopes) — gated by superadmin_required at the API
87-
# layer, so it carries no per-scope RBAC validator. Non-admins use scoped_search.
87+
# layer, so it carries no per-scope RBAC validator. Non-admins use scoped_search,
88+
# which is validated at the scope it names.
8889
self.admin_search = ScopeActionProcessor(service.admin_search, action_monitors)
89-
self.scoped_search = BulkActionProcessor(service.scoped_search, monitors=action_monitors)
90+
self.scoped_search = ScopeActionProcessor(
91+
service.scoped_search, action_monitors, validators=[validators.rbac.scope]
92+
)
9093
self.update = SingleEntityActionProcessor(
9194
service.update, action_monitors, validators=[validators.rbac.single_entity]
9295
)

src/ai/backend/manager/services/app_config_fragment/service.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,13 @@ async def admin_search(
7878
async def scoped_search(
7979
self, action: ScopedSearchAppConfigFragmentAction
8080
) -> ScopedSearchAppConfigFragmentActionResult:
81-
targets = list(action.targets())
82-
scopes = [t.to_search_scope() for t in targets]
83-
result = await self._repository.scoped_search(action.querier, scopes)
81+
result = await self._repository.scoped_search(action.querier, action.scope)
8482
return ScopedSearchAppConfigFragmentActionResult(
83+
scope=action.scope,
8584
data=result.items,
8685
total_count=result.total_count,
8786
has_next_page=result.has_next_page,
8887
has_previous_page=result.has_previous_page,
89-
queried_refs=[t.to_rbac_element_ref() for t in targets],
9088
)
9189

9290
async def update(

0 commit comments

Comments
 (0)