Skip to content

Commit 106cfb0

Browse files
jopemachineclaude
andcommitted
feat(BA-6628): cascade fragment deletion from the allow list
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 1ad86c1 commit 106cfb0

12 files changed

Lines changed: 171 additions & 149 deletions

File tree

proposals/BEP-1052-scoped-app-config-redesign.md

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,13 @@ Three scopes cover the use cases (`public` for the pre-login shell):
7272
joined to `app_config_allow_list` only for each fragment's `rank` — an
7373
indexed `(config_name, scope_type)` join. No permission or policy is
7474
evaluated at read time.
75-
- **Allow-list = the write gate for every fragment.** `app_config_allow_list`
75+
- **Allow-list = the write gate and the merge order.** `app_config_allow_list`
7676
holds **one record per `(config_name, scope_type)`**; a fragment at
77-
that scope may be created/updated/purged **only if** the record exists
78-
— through the admin mutations and the regular ones alike. What sets
79-
admins apart is that they alone manage the allow-list (and the
80-
`app_config_definitions`) itself. It governs **writes only** — never reads.
77+
that scope may be created **only if** the record exists — through the
78+
admin mutations and the regular ones alike. Fragments reference their
79+
entry by FK (`ON DELETE CASCADE`), so removing an entry removes its
80+
fragments. What sets admins apart is that they alone manage the
81+
allow-list (and the `app_config_definitions`) itself.
8182
- **`rank` lives on the allow-list entry.** Merge priority is an
8283
admin-owned policy: it sits on the (admin-managed) allow-list entry,
8384
not on the fragment — a fragment owner editing their own fragment can
@@ -110,12 +111,13 @@ Keyed by the natural composite `(scope_type, scope_id, config_name)`
110111
- `scope_type``public | domain | user`.
111112
- `scope_id` — the scope's identifier (see convention below).
112113
- `config_name` — FK → `app_config_definitions.config_name`.
114+
- `(config_name, scope_type)` — composite FK →
115+
`app_config_allow_list` with `ON DELETE CASCADE`: a fragment exists
116+
only while its allow-list entry does, and carries no rank of its own —
117+
its merge priority is the entry's `rank` (see §2).
113118
- `config` — schema-less JSON payload.
114119
- `created_at` / `updated_at`.
115120

116-
The fragment carries no rank of its own — its merge priority is its
117-
allow-list entry's `rank` (see §2).
118-
119121
### `app_config_allow_list` — the per-`(config_name, scope_type)` write gate
120122

121123
One row per `(config_name, scope_type)` (unique) — a normalized,
@@ -135,7 +137,8 @@ advance.
135137

136138
A row's **presence** is the write grant, and its `rank` is the merge
137139
order of its fragments. It gates **both** write paths; admins, unlike
138-
users, may also create/update/purge the allow-list rows themselves.
140+
users, may also create/purge the allow-list rows themselves — and
141+
purging one cascades to its fragments.
139142

140143
### Scope-ID convention
141144

@@ -147,20 +150,22 @@ users, may also create/update/purge the allow-list rows themselves.
147150

148151
### Integrity
149152

150-
- **Every** fragment write (admin or regular) requires (a) a registered
153+
- **Every** fragment create (admin or regular) requires (a) a registered
151154
`config_name` (FK to `app_config_definitions`) and (b) an
152155
`app_config_allow_list` row for the write's `(config_name,
153-
scope_type)`. The service layer rejects per-row when either is missing.
156+
scope_type)` — enforced both by the write gate (domain error) and by
157+
the composite FK. Updates and purges of an existing fragment need no
158+
gate: the FK guarantees the entry exists while the fragment does.
154159
- A regular (non-admin) mutation is further restricted to the caller's
155160
own `user` row; admin mutations may target any scope (still gated by
156161
the allow-list) and are the only writes that may touch the allow-list
157162
and `app_config_definitions`.
158163
- `app_config_definitions` purge is rejected while any fragment or allow-list
159164
entry still references the `config_name` (`ON DELETE NO ACTION`).
160-
- `app_config_allow_list` purge **revokes future writes** at that
161-
`(config_name, scope_type)`. Because reads never consult the
162-
allow-list, **existing fragments are untouched and keep merging** — to
163-
actually drop a value, an admin purges the fragments themselves.
165+
- `app_config_allow_list` purge **revokes the grant and drops its data**:
166+
the fragments at that `(config_name, scope_type)` are removed by the
167+
`ON DELETE CASCADE` FK, so a revoked value disappears from the merge
168+
in the same statement — no orphaned fragments to clean up separately.
164169

165170
<a id="write-model"></a>
166171
### Write model
@@ -196,9 +201,8 @@ boundary.
196201

197202
To promote a fixed value to user-customizable, the admin adds a single
198203
`(config_name, user)` grant — no data migration. To lock it back down,
199-
the admin removes the grant **and** purges any existing `user` fragments
200-
(removing the grant alone only blocks new writes; reads still merge what
201-
is already stored).
204+
the admin removes the grant: the cascade drops the existing `user`
205+
fragments with it, so the admin value applies again immediately.
202206

203207
---
204208

@@ -300,12 +304,11 @@ likewise `null` — clients fall back to their built-in defaults.
300304
user)` grant; users then create/update/purge their own copy. No data
301305
migration.
302306
- **Admin locks a value back down** — remove the `(config_name, user)`
303-
grant **and** purge existing `user` fragments (the grant gates writes,
304-
not reads).
307+
grant; the cascade drops the existing `user` fragments with it.
305308
- **Admin reorders contributions** — set the allow-list entries'
306309
`rank`s (per `(config_name, scope_type)`, not per fragment).
307-
- **Admin retires a config name** — purge the fragments, then the
308-
allow-list entries, then the `app_config_definitions` row (purge is rejected
310+
- **Admin retires a config name** — purge the allow-list entries (their
311+
fragments cascade), then the `app_config_definitions` row (purge is rejected
309312
while references remain).
310313
- **Admin audit** — cross-scope fragment search and cross-user merged
311314
search for support.
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
"""cascade fragment deletion from app_config_allow_list
2+
3+
Add a composite FK from ``app_config_fragments (config_name, scope_type)`` to
4+
``app_config_allow_list`` with ``ON DELETE CASCADE`` (BEP-1052): removing an
5+
allow-list entry removes every fragment written under it, so a revoked value
6+
disappears from the merge in the same statement. Fragments whose allow-list
7+
entry was already removed (previously kept and still merged) are deleted before
8+
the FK is added — the same outcome the cascade would have produced.
9+
10+
Revision ID: a560420476b6
11+
Revises: 66d0f891ed20
12+
Create Date: 2026-07-03
13+
14+
"""
15+
16+
import sqlalchemy as sa
17+
from alembic import op
18+
19+
# revision identifiers, used by Alembic.
20+
revision = "a560420476b6"
21+
down_revision = "66d0f891ed20"
22+
# Part of: NEXT_RELEASE_VERSION
23+
branch_labels = None
24+
depends_on = None
25+
26+
27+
def upgrade() -> None:
28+
# Fragments without an allow-list entry are unrepresentable once the FK exists;
29+
# remove them as the cascade would have when the entry was purged.
30+
op.execute(
31+
sa.text(
32+
"""
33+
DELETE FROM app_config_fragments f
34+
WHERE NOT EXISTS (
35+
SELECT 1
36+
FROM app_config_allow_list al
37+
WHERE al.config_name = f.config_name
38+
AND al.scope_type = f.scope_type
39+
)
40+
"""
41+
)
42+
)
43+
op.create_foreign_key(
44+
"fk_app_config_fragments_config_name_scope_type",
45+
"app_config_fragments",
46+
"app_config_allow_list",
47+
["config_name", "scope_type"],
48+
["config_name", "scope_type"],
49+
ondelete="CASCADE",
50+
)
51+
52+
53+
def downgrade() -> None:
54+
op.drop_constraint(
55+
"fk_app_config_fragments_config_name_scope_type",
56+
"app_config_fragments",
57+
type_="foreignkey",
58+
)

src/ai/backend/manager/models/app_config_allow_list/row.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@
1818
class AppConfigAllowListRow(Base): # type: ignore[misc]
1919
"""Permission to write config fragments for one config name at one scope type.
2020
21-
A config fragment may be created, updated, or purged only when a matching row
22-
exists here; without one, the write is rejected. ``rank`` is the merge priority
23-
the entry's fragments carry (low → high; higher wins) — admin-owned here so
24-
fragment owners cannot re-order the merge. There is at most one row per
21+
A config fragment may be created only when a matching row exists here; without
22+
one, the write is rejected. Fragments reference this table by
23+
``(config_name, scope_type)`` with ``ON DELETE CASCADE``, so removing an entry
24+
also removes every fragment written under it. ``rank`` is the merge priority the
25+
entry's fragments carry (low → high; higher wins) — admin-owned here so fragment
26+
owners cannot re-order the merge. There is at most one row per
2527
``(config_name, scope_type)``, and only admins create or remove these rows.
2628
"""
2729

src/ai/backend/manager/models/app_config_fragment/row.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
class AppConfigFragmentRow(Base): # type: ignore[misc]
2121
"""One scoped app config fragment — a single JSON document at ``(config_name, scope_type, scope_id)``.
2222
23-
The fragment carries no merge priority of its own — its rank is the ``rank`` of
24-
the allow-list entry for its ``(config_name, scope_type)``.
23+
A fragment hangs off its ``app_config_allow_list`` entry (FK on
24+
``(config_name, scope_type)``, ``ON DELETE CASCADE``): it exists only while the
25+
entry does, and its merge priority is the entry's ``rank`` — the fragment row
26+
carries no rank of its own.
2527
"""
2628

2729
__tablename__ = "app_config_fragments"
@@ -32,6 +34,12 @@ class AppConfigFragmentRow(Base): # type: ignore[misc]
3234
"scope_id",
3335
name="uq_app_config_fragments_config_name_scope_type_scope_id",
3436
),
37+
sa.ForeignKeyConstraint(
38+
["config_name", "scope_type"],
39+
["app_config_allow_list.config_name", "app_config_allow_list.scope_type"],
40+
name="fk_app_config_fragments_config_name_scope_type",
41+
ondelete="CASCADE",
42+
),
3543
)
3644

3745
id: Mapped[AppConfigFragmentID] = mapped_column(

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

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ async def create(
7373
only_if: ExistsQuerier[AppConfigAllowListRow],
7474
) -> AppConfigFragmentData:
7575
# ``only_if`` (built by the caller) and the write run in one transaction, so the gate
76-
# check and the write commit atomically — no check-then-write race.
76+
# check and the write commit atomically — no check-then-write race. The FK to the
77+
# allow-list would also reject the insert, but the gate surfaces the domain error.
7778
async with self._ops.write_ops() as w:
7879
if not await w.exists(only_if):
7980
raise AppConfigFragmentWriteNotAllowed(
@@ -92,36 +93,20 @@ async def get_by_id(self, fragment_id: AppConfigFragmentID) -> AppConfigFragment
9293
return result.row.to_data()
9394

9495
@app_config_fragment_db_source_resilience.apply()
95-
async def update(
96-
self,
97-
updater: Updater[AppConfigFragmentRow],
98-
only_if: ExistsQuerier[AppConfigAllowListRow],
99-
) -> AppConfigFragmentData:
100-
# Gate first, then write — both in one transaction so the check and the write commit
101-
# atomically. A missing fragment surfaces as the update returning None below.
96+
async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragmentData:
97+
# No write-gate here: the FK to the allow-list guarantees a fragment row exists
98+
# only while its ``(config_name, scope_type)`` entry does, so an existing
99+
# fragment is always writable at its own scope.
102100
async with self._ops.write_ops() as w:
103-
if not await w.exists(only_if):
104-
raise AppConfigFragmentWriteNotAllowed(
105-
f"Writing app config fragment {updater.pk_value} is not allowed."
106-
)
107101
result = await w.update(updater)
108102
if result is None:
109103
raise AppConfigFragmentNotFound(f"App config fragment {updater.pk_value} not found")
110104
return result.row.to_data()
111105

112106
@app_config_fragment_db_source_resilience.apply()
113-
async def purge(
114-
self,
115-
purger: Purger[AppConfigFragmentRow],
116-
only_if: ExistsQuerier[AppConfigAllowListRow],
117-
) -> AppConfigFragmentData:
118-
# Gate first, then write — both in one transaction so the check and the write commit
119-
# atomically. A missing fragment surfaces as the purge returning None below.
107+
async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData:
108+
# No write-gate here — see ``update``.
120109
async with self._ops.write_ops() as w:
121-
if not await w.exists(only_if):
122-
raise AppConfigFragmentWriteNotAllowed(
123-
f"Writing app config fragment {purger.pk_value} is not allowed."
124-
)
125110
result = await w.purge(purger)
126111
if result is None:
127112
raise AppConfigFragmentNotFound(f"App config fragment {purger.pk_value} not found")

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

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,12 @@ async def get_by_id(self, fragment_id: AppConfigFragmentID) -> AppConfigFragment
6767
return await self._db_source.get_by_id(fragment_id)
6868

6969
@app_config_fragment_repository_resilience.apply()
70-
async def update(
71-
self,
72-
updater: Updater[AppConfigFragmentRow],
73-
only_if: ExistsQuerier[AppConfigAllowListRow],
74-
) -> AppConfigFragmentData:
75-
return await self._db_source.update(updater, only_if)
70+
async def update(self, updater: Updater[AppConfigFragmentRow]) -> AppConfigFragmentData:
71+
return await self._db_source.update(updater)
7672

7773
@app_config_fragment_repository_resilience.apply()
78-
async def purge(
79-
self,
80-
purger: Purger[AppConfigFragmentRow],
81-
only_if: ExistsQuerier[AppConfigAllowListRow],
82-
) -> AppConfigFragmentData:
83-
return await self._db_source.purge(purger, only_if)
74+
async def purge(self, purger: Purger[AppConfigFragmentRow]) -> AppConfigFragmentData:
75+
return await self._db_source.purge(purger)
8476

8577
@app_config_fragment_repository_resilience.apply()
8678
async def admin_search(self, querier: BatchQuerier) -> AppConfigFragmentSearchResult:

src/ai/backend/manager/services/app_config_fragment/actions/purge.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@
77
from ai.backend.manager.actions.types import ActionOperationType
88
from ai.backend.manager.data.app_config_fragment.types import AppConfigFragmentData
99
from ai.backend.manager.data.permission.types import RBACElementRef
10-
from ai.backend.manager.models.app_config_allow_list.row import AppConfigAllowListRow
1110
from ai.backend.manager.models.app_config_fragment.row import AppConfigFragmentRow
12-
from ai.backend.manager.repositories.base import ExistsQuerier, Purger
11+
from ai.backend.manager.repositories.base import Purger
1312
from ai.backend.manager.services.app_config_fragment.actions.base import (
1413
AppConfigFragmentSingleEntityAction,
1514
AppConfigFragmentSingleEntityActionResult,
@@ -20,13 +19,13 @@
2019
class PurgeAppConfigFragmentAction(AppConfigFragmentSingleEntityAction):
2120
"""Purge a fragment — not admin-only.
2221
23-
Gated by the allow-list write-gate (``only_if``) against the fragment's
24-
``(config_name, scope_type)``, so an allow-listed user may purge their own
25-
``user``-scope fragment.
22+
No allow-list gate is needed — a fragment row exists only while its
23+
``(config_name, scope_type)`` allow-list entry does (FK with cascade), so an
24+
existing fragment is always removable at its own scope. Purging the allow-list
25+
entry itself cascades to its fragments without going through this action.
2626
"""
2727

2828
purger: Purger[AppConfigFragmentRow]
29-
only_if: ExistsQuerier[AppConfigAllowListRow]
3029

3130
@override
3231
@classmethod

src/ai/backend/manager/services/app_config_fragment/actions/update.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@
77
from ai.backend.manager.actions.types import ActionOperationType
88
from ai.backend.manager.data.app_config_fragment.types import AppConfigFragmentData
99
from ai.backend.manager.data.permission.types import RBACElementRef
10-
from ai.backend.manager.models.app_config_allow_list.row import AppConfigAllowListRow
1110
from ai.backend.manager.models.app_config_fragment.row import AppConfigFragmentRow
12-
from ai.backend.manager.repositories.base import ExistsQuerier, Updater
11+
from ai.backend.manager.repositories.base import Updater
1312
from ai.backend.manager.services.app_config_fragment.actions.base import (
1413
AppConfigFragmentSingleEntityAction,
1514
AppConfigFragmentSingleEntityActionResult,
@@ -20,13 +19,13 @@
2019
class UpdateAppConfigFragmentAction(AppConfigFragmentSingleEntityAction):
2120
"""Update a fragment's ``config`` (replaced wholesale).
2221
23-
Not admin-only: an allow-listed user may update their own ``user``-scope fragment. The
24-
allow-list write-gate (``only_if``) authorizes the write against the fragment's
25-
``(config_name, scope_type)``.
22+
Not admin-only: a user may update their own ``user``-scope fragment. No allow-list
23+
gate is needed — a fragment row exists only while its ``(config_name, scope_type)``
24+
allow-list entry does (FK with cascade), so an existing fragment is always writable
25+
at its own scope.
2626
"""
2727

2828
updater: Updater[AppConfigFragmentRow]
29-
only_if: ExistsQuerier[AppConfigAllowListRow]
3029

3130
@override
3231
@classmethod

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@
3434
class AppConfigFragmentService:
3535
"""Write/read paths for app config fragments (not admin-only).
3636
37-
``create`` / ``update`` / ``purge`` are gated by the allow-list write-gate in the
38-
repository: the gate check and the write run in a single transaction, so a fragment is
39-
only written or removed when an ``app_config_allow_list`` row exists for its
40-
``(config_name, scope_type)``. Because an allow-list row requires a registered
41-
``config_name`` (FK), this also enforces registration. An allow-listed user may
42-
therefore manage their own ``user``-scope fragment without admin privileges.
37+
``create`` is gated by the allow-list write-gate in the repository: the gate check
38+
and the write run in a single transaction, so a fragment is only written when an
39+
``app_config_allow_list`` row exists for its ``(config_name, scope_type)``. Because
40+
an allow-list row requires a registered ``config_name`` (FK), this also enforces
41+
registration. ``update`` / ``purge`` need no gate — the fragments' FK to the
42+
allow-list (``ON DELETE CASCADE``) guarantees an existing fragment's entry exists.
43+
An allow-listed user may therefore manage their own ``user``-scope fragment without
44+
admin privileges.
4345
"""
4446

4547
_repository: AppConfigFragmentRepository
@@ -85,11 +87,11 @@ async def scoped_search(
8587
async def update(
8688
self, action: UpdateAppConfigFragmentAction
8789
) -> UpdateAppConfigFragmentActionResult:
88-
data = await self._repository.update(action.updater, action.only_if)
90+
data = await self._repository.update(action.updater)
8991
return UpdateAppConfigFragmentActionResult(fragment=data)
9092

9193
async def purge(
9294
self, action: PurgeAppConfigFragmentAction
9395
) -> PurgeAppConfigFragmentActionResult:
94-
data = await self._repository.purge(action.purger, action.only_if)
96+
data = await self._repository.purge(action.purger)
9597
return PurgeAppConfigFragmentActionResult(fragment=data)

0 commit comments

Comments
 (0)