Skip to content

fix(BA-6897): make pre-constraint RBAC backfill upgrades skip existing rows#12883

Draft
jopemachine wants to merge 2 commits into
fix/BA-6896-rbac-backfill-downgrade-data-lossfrom
fix/BA-6897-rbac-backfill-upgrade-idempotency
Draft

fix(BA-6897): make pre-constraint RBAC backfill upgrades skip existing rows#12883
jopemachine wants to merge 2 commits into
fix/BA-6896-rbac-backfill-downgrade-data-lossfrom
fix/BA-6897-rbac-backfill-upgrade-idempotency

Conversation

@jopemachine

@jopemachine jopemachine commented Jul 15, 2026

Copy link
Copy Markdown
Member

📚 Stacked PRs

Merge in order, bottom-up. Each PR is based on the one above it, so its diff shows only its own layer.

Problem

Six RBAC backfill migrations insert with a bare ON CONFLICT DO NOTHING, but the clause can never fire.

At their point in the chain, permissions (created in 9adcd6f48ba1) carries only:

sa.PrimaryKeyConstraint("id", name=op.f("pk_permissions")),

id defaults to uuid_generate_v4(), so the sole conflict target is a freshly generated UUID. There is no unique constraint on (permission_group_id, entity_type, operation)ix_id_permission_group_id is unique=False. The same holds for permission_groups and object_permissions in the vfolder migration.

3f5c20f7bb07 only adds the permissions unique constraint much later in the chain, and it deletes duplicates before creating it — evidence that duplicates were reachable all along.

This was masked while the downgrades deleted the rows first. With #12881 making them forward-only, a downgrade/upgrade cycle accumulates duplicates until the chain reaches 3f5c20f7bb07 / 7369d1eb7d4a, which dedupe before creating the constraints.

Verified by running the real migration code

These six target the pre-redesign schema (permission_group_id), which no longer exists at head, so they cannot be driven through alembic on a head DB. Instead the tables were recreated exactly as defined at that revision and each migration module's own _migrate_new_entity_type() was invoked three times against a real Postgres connection — the real code path, not a transcription of it.

Row counts after each run:

migration main run 1 → 2 → 3 this PR run 1 → 2 → 3
artifact 11 → 22 → 33 DUPLICATES 11 → 11 → 11 idempotent
artifact_registry 11 → 22 → 33 DUPLICATES 11 → 11 → 11 idempotent
app_config 11 → 22 → 33 DUPLICATES 11 → 11 → 11 idempotent
model_deployment 11 → 22 → 33 DUPLICATES 11 → 11 → 11 idempotent
notification 22 → 44 → 66 DUPLICATES 22 → 22 → 22 idempotent
vfolder 11 → 22 → 33 DUPLICATES 11 → 11 → 11 idempotent

Scope

  • Changed (6): artifact, artifact_registry, app_config, model_deployment, notification, vfolder (vfolder also guards permission_groups + object_permissions).
  • Unchanged (9): session, agent, image, keypair, container_registry, resource_group, vfolder_invitation, model_card, admin_page — their constraint already exists, so their ON CONFLICT genuinely fires (a bare ON CONFLICT DO NOTHING catches any unique violation).
  • association_scopes_entities left alone: it carries uq_scope_id_entity_id at these revisions (82f4d3dea750 recreates it after 430b1631804d drops it), so its bare ON CONFLICT works.

DISTINCT collapses repeats within a single batch. Editing released upgrade() bodies is safe here: a DB already past these revisions never re-runs them.

🤖 Generated with Claude Code

…g rows

Six RBAC backfill migrations insert with a bare ON CONFLICT DO NOTHING.
At their point in the chain the permissions table carries only a primary
key on id, and id defaults to uuid_generate_v4(), so the only conflict
target is a freshly generated UUID and the clause never fires. The same
holds for permission_groups and object_permissions in the vfolder
migration. Re-running any of these upgrades duplicated every row.

This was masked while the downgrades deleted the rows first. With the
downgrades now forward-only, a downgrade/upgrade cycle would accumulate
duplicates until the chain reached the dedupe migrations that run before
the unique constraints are created.

Skip existing rows explicitly instead, and let DISTINCT collapse repeats
within a batch. association_scopes_entities is left alone: it already
carries uq_scope_id_entity_id at these revisions, so its bare ON CONFLICT
works. The nine migrations whose constraints already exist are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size:L 100~500 LoC comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant