From 64fdea2954b17d7bdba5d068f2a4fd84930add16 Mon Sep 17 00:00:00 2001 From: StylusFrost Date: Sun, 31 May 2026 22:16:43 +0200 Subject: [PATCH] feat(api): store provider as varchar and drop the enum type - Promote the synced shadow column into provider, dropping the enum - Rebuild the partial unique index concurrently after the swap - Keep provider input validation at the serializer layer --- .../0096_provider_enum_to_varchar_contract.py | 54 ++++++++++++++++++ .../0097_provider_unique_index_concurrent.py | 28 +++++++++ api/src/backend/api/models.py | 13 ++--- api/src/backend/api/tests/test_models.py | 57 ------------------- api/src/backend/api/v1/serializers.py | 6 ++ 5 files changed, 92 insertions(+), 66 deletions(-) create mode 100644 api/src/backend/api/migrations/0096_provider_enum_to_varchar_contract.py create mode 100644 api/src/backend/api/migrations/0097_provider_unique_index_concurrent.py diff --git a/api/src/backend/api/migrations/0096_provider_enum_to_varchar_contract.py b/api/src/backend/api/migrations/0096_provider_enum_to_varchar_contract.py new file mode 100644 index 0000000000..0f8bc9a6cf --- /dev/null +++ b/api/src/backend/api/migrations/0096_provider_enum_to_varchar_contract.py @@ -0,0 +1,54 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + """Contract step of the zero-downtime migration of Provider.provider from a + native PostgreSQL enum to varchar. + + The shadow column added in 0094 has been kept in sync by the trigger and + backfilled in 0095, so it now holds the value for every row. This migration + promotes it into place: drop the trigger and the enum column, rename the + shadow column to `provider`, and drop the orphaned enum type. The column + name is preserved throughout, and varchar accepts the same string values + the enum held, so app instances running the previous release keep working + against the swapped column. + + The drop/rename runs in this migration's transaction so `provider` never + disappears for readers. The partial unique index is dropped here and + rebuilt concurrently in the next migration to avoid a long write lock. + """ + + dependencies = [ + ("api", "0095_backfill_provider_str"), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.RemoveField( + model_name="provider", + name="provider_str", + ), + migrations.AlterField( + model_name="provider", + name="provider", + field=models.CharField(default="aws", max_length=50), + ), + ], + database_operations=[ + migrations.RunSQL( + sql=( + "DROP TRIGGER IF EXISTS providers_sync_provider_str ON providers;\n" + "DROP FUNCTION IF EXISTS sync_provider_str();\n" + "DROP INDEX IF EXISTS unique_provider_uids;\n" + "ALTER TABLE providers DROP COLUMN provider;\n" + "ALTER TABLE providers RENAME COLUMN provider_str TO provider;\n" + "ALTER TABLE providers ALTER COLUMN provider SET DEFAULT 'aws';\n" + "ALTER TABLE providers ALTER COLUMN provider SET NOT NULL;\n" + "DROP TYPE provider;" + ), + reverse_sql=migrations.RunSQL.noop, + ), + ], + ), + ] diff --git a/api/src/backend/api/migrations/0097_provider_unique_index_concurrent.py b/api/src/backend/api/migrations/0097_provider_unique_index_concurrent.py new file mode 100644 index 0000000000..94fdef6e4c --- /dev/null +++ b/api/src/backend/api/migrations/0097_provider_unique_index_concurrent.py @@ -0,0 +1,28 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + """Rebuild the partial unique index on `providers` after the enum-to-varchar + contract in 0096 dropped it along with the old enum column. + + Built with CREATE INDEX CONCURRENTLY (hence `atomic = False`) so the rebuild + holds no long write lock on a large table. The index keeps the name and + predicate Django expects for the existing `unique_provider_uids` constraint, + which stays in the model state untouched, so no state operation is needed. + """ + + atomic = False + + dependencies = [ + ("api", "0096_provider_enum_to_varchar_contract"), + ] + + operations = [ + migrations.RunSQL( + sql=( + "CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS unique_provider_uids " + "ON providers (tenant_id, provider, uid) WHERE NOT is_deleted;" + ), + reverse_sql="DROP INDEX CONCURRENTLY IF EXISTS unique_provider_uids;", + ), + ] diff --git a/api/src/backend/api/models.py b/api/src/backend/api/models.py index ecbaa07bc1..4c85044f9d 100644 --- a/api/src/backend/api/models.py +++ b/api/src/backend/api/models.py @@ -40,7 +40,6 @@ from api.db_utils import ( InvitationStateEnumField, MemberRoleEnumField, ProcessorTypeEnumField, - ProviderEnumField, ProviderSecretTypeEnumField, ScanTriggerEnumField, SeverityEnumField, @@ -482,14 +481,10 @@ class Provider(RowLevelSecurityProtectedModel): inserted_at = models.DateTimeField(auto_now_add=True, editable=False) updated_at = models.DateTimeField(auto_now=True, editable=False) is_deleted = models.BooleanField(default=False) - provider = ProviderEnumField( - choices=ProviderChoices.choices, default=ProviderChoices.AWS - ) - # Transitional shadow column for the zero-downtime migration of `provider` - # from a native PostgreSQL enum to varchar. Kept in sync with `provider` by - # a DB trigger; a later migration drops the enum column and renames this one - # to take its place. - provider_str = models.CharField(max_length=50, null=True, blank=True) + # Stored as a plain varchar: the SDK is the source of truth for which + # providers are valid, so the column accepts any provider name without a + # database-level enum to keep in sync. + provider = models.CharField(max_length=50, default=ProviderChoices.AWS) uid = models.CharField( "Unique identifier for the provider, set by the provider", max_length=250, diff --git a/api/src/backend/api/tests/test_models.py b/api/src/backend/api/tests/test_models.py index 89433dd40c..b8b7f61dd1 100644 --- a/api/src/backend/api/tests/test_models.py +++ b/api/src/backend/api/tests/test_models.py @@ -525,60 +525,3 @@ class TestTenantComplianceSummaryModel: assert summary1.id != summary2.id assert summary1.requirements_passed != summary2.requirements_passed - - -@pytest.mark.django_db -class TestProviderShadowColumn: - """The provider_str shadow column is kept in sync with the provider enum - column by a database trigger, on both INSERT and UPDATE.""" - - def test_provider_str_populated_on_insert(self, providers_fixture): - for provider in providers_fixture: - provider.refresh_from_db() - assert provider.provider_str == provider.provider - - def test_provider_str_synced_on_update(self, providers_fixture): - provider = providers_fixture[0] - provider.alias = "renamed-alias" - provider.save() - provider.refresh_from_db() - assert provider.provider_str == provider.provider - - def test_backfill_populates_pre_trigger_rows(self, providers_fixture): - """Rows created before the sync trigger existed have a NULL shadow - column; the backfill copies `provider::text` into them in batches.""" - from django.db import connection - - from tasks.jobs.backfill import backfill_provider_str - - tenant_id = str(providers_fixture[0].tenant_id) - - # Simulate rows that predate the trigger: bypass triggers for the - # session, null the shadow column, then restore normal replication so - # subsequent writes resync as in production. - with connection.cursor() as cursor: - cursor.execute("SET session_replication_role = replica") - cursor.execute("UPDATE providers SET provider_str = NULL") - cursor.execute("SET session_replication_role = origin") - - for provider in providers_fixture: - provider.refresh_from_db() - assert provider.provider_str is None - - result = backfill_provider_str(tenant_id, batch_size=2) - - assert result["updated"] == len(providers_fixture) - for provider in providers_fixture: - provider.refresh_from_db() - assert provider.provider_str == provider.provider - - def test_backfill_is_idempotent(self, providers_fixture): - """Re-running the backfill touches no rows once the column is populated - (the trigger already keeps it in sync), so it is safe to retry.""" - from tasks.jobs.backfill import backfill_provider_str - - tenant_id = str(providers_fixture[0].tenant_id) - - result = backfill_provider_str(tenant_id) - - assert result["updated"] == 0 diff --git a/api/src/backend/api/v1/serializers.py b/api/src/backend/api/v1/serializers.py index 03d2fecad2..d085edea0c 100644 --- a/api/src/backend/api/v1/serializers.py +++ b/api/src/backend/api/v1/serializers.py @@ -940,6 +940,12 @@ class ProviderIncludeSerializer(RLSSerializer): class ProviderCreateSerializer(RLSSerializer, BaseWriteSerializer): + # Declared explicitly so provider validation stays at the serializer layer: + # the model column is now a plain varchar with no choices, so without this + # an unknown provider would slip through to Provider.clean() instead of + # being rejected here with `invalid_choice`. + provider = ProviderEnumSerializerField() + class Meta: model = Provider fields = [