Skip to content

Commit 4a1c750

Browse files
committed
Fix org_id inconsistency in baseline migration
- Remove server_default="0" from api_key.org_id (id=0 is not a valid FK) - Seed an "Uncategorized" catch-all organization (arange: 0.0.0.0/0 ::/0) during migration to serve as a valid placeholder for existing rows - Backfill NULL org_id on all affected tables (flowspec4, flowspec6, RTBH, api_key, machine_api_key) with the catch-all org's real id - Update migrate_v0x_to_v1.py to look up the catch-all org by name instead of filtering on the hardcoded org_id=0 - Add migration tests: catchall org created, correct arange, no NULL org_id remains after upgrade from real 2019 backup
1 parent 2033d8d commit 4a1c750

File tree

3 files changed

+88
-13
lines changed

3 files changed

+88
-13
lines changed

flowapp/migrations/versions/001_baseline.py

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,12 +254,7 @@ def upgrade():
254254
if not _column_exists("api_key", "org_id"):
255255
op.add_column(
256256
"api_key",
257-
sa.Column(
258-
"org_id",
259-
sa.Integer(),
260-
nullable=True,
261-
server_default="0",
262-
),
257+
sa.Column("org_id", sa.Integer(), nullable=True),
263258
)
264259

265260
if not _table_exists("machine_api_key"):
@@ -505,6 +500,40 @@ def upgrade():
505500
],
506501
)
507502

503+
# Ensure the catch-all organization exists.
504+
# Used as a temporary placeholder for rules/keys that have no org_id yet
505+
# (added in v1.0). Run migrate_v0x_to_v1.py afterwards to reassign them.
506+
conn = op.get_bind()
507+
catchall_name = "Uncategorized"
508+
result = conn.execute(
509+
sa.text("SELECT id FROM organization WHERE name = :name"),
510+
{"name": catchall_name},
511+
)
512+
catchall_row = result.fetchone()
513+
if catchall_row is None:
514+
conn.execute(
515+
sa.text(
516+
"INSERT INTO organization (name, arange, limit_flowspec4, limit_flowspec6, limit_rtbh)"
517+
" VALUES (:name, :arange, 0, 0, 0)"
518+
),
519+
{"name": catchall_name, "arange": "0.0.0.0/0 ::/0"},
520+
)
521+
result = conn.execute(
522+
sa.text("SELECT id FROM organization WHERE name = :name"),
523+
{"name": catchall_name},
524+
)
525+
catchall_row = result.fetchone()
526+
catchall_id = catchall_row[0]
527+
528+
# Backfill NULL org_id on existing rows with the catch-all org id.
529+
# migrate_v0x_to_v1.py will reassign these to the real organization.
530+
for tbl in ("flowspec4", "flowspec6", "RTBH", "api_key", "machine_api_key"):
531+
if _table_exists(tbl) and _column_exists(tbl, "org_id"):
532+
conn.execute(
533+
sa.text(f"UPDATE {tbl} SET org_id = :cid WHERE org_id IS NULL"), # noqa: S608
534+
{"cid": catchall_id},
535+
)
536+
508537
if _seed_communities and not _table_has_data("community"):
509538
op.bulk_insert(
510539
community_table,

scripts/migrate_v0x_to_v1.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@
1717
from os import environ
1818

1919
from flowapp import create_app, db
20-
from flowapp.models import Flowspec4, Flowspec6, RTBH, ApiKey, MachineApiKey
20+
from flowapp.models import Flowspec4, Flowspec6, RTBH, ApiKey, MachineApiKey, Organization
2121
from sqlalchemy import text
2222

2323
import config
2424

25+
CATCHALL_ORG_NAME = "Uncategorized"
26+
2527

2628
def migrate_org_data():
2729
exafs_env = environ.get("EXAFS_ENV", "Production").lower()
@@ -43,23 +45,28 @@ def migrate_org_data():
4345
"""
4446
)
4547
try:
46-
result = db.session.execute(update_statement)
48+
db.session.execute(update_statement)
4749
db.session.commit()
48-
print(f" Updated organization limits.")
50+
print(" Updated organization limits.")
4951
except Exception as e:
5052
db.session.rollback()
5153
print(f" Error updating organizations: {e}")
5254
return
5355

54-
# Step 2: Assign rules with org_id=0 to user's organization
55-
print("\nAssigning rules with org_id=0 to user organizations...")
56+
# Step 2: Assign rules belonging to the catch-all org to the user's real organization
57+
catchall = Organization.query.filter_by(name=CATCHALL_ORG_NAME).first()
58+
if catchall is None:
59+
print(f"\nNo '{CATCHALL_ORG_NAME}' organization found — nothing to reassign.")
60+
return
61+
62+
print(f"\nAssigning rules with org_id={catchall.id} ('{CATCHALL_ORG_NAME}') to user organizations...")
5663
models = [Flowspec4, Flowspec6, RTBH, ApiKey, MachineApiKey]
5764
users_with_multiple_orgs = {}
5865
total_updated = 0
5966

6067
for model in models:
6168
model_name = model.__name__
62-
data_records = model.query.filter(model.org_id == 0).all()
69+
data_records = model.query.filter(model.org_id == catchall.id).all()
6370

6471
if not data_records:
6572
print(f" {model_name}: no records with org_id=0")
@@ -98,6 +105,8 @@ def migrate_org_data():
98105
print("\nPlease manually assign org_id for rules belonging to these users.")
99106
else:
100107
print("\nAll records assigned successfully.")
108+
print(f"\nYou may now delete the '{CATCHALL_ORG_NAME}' organization")
109+
print("from the admin interface if no records remain assigned to it.")
101110

102111

103112
if __name__ == "__main__":

tests/test_migration.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -986,12 +986,29 @@ def test_all_expected_columns(self, migration_db):
986986
app, db_uri = migration_db
987987
_run_migration(app)
988988
for table_name, expected_cols in EXPECTED_COLUMNS.items():
989-
actual_cols = _get_columns(db_uri,table_name)
989+
actual_cols = _get_columns(db_uri, table_name)
990990
for col in expected_cols:
991991
assert col in actual_cols, (
992992
f"Missing column {table_name}.{col}"
993993
)
994994

995+
def test_creates_catchall_organization(self, migration_db):
996+
app, db_uri = migration_db
997+
_run_migration(app)
998+
count = _query_scalar(
999+
db_uri, "SELECT COUNT(*) FROM organization WHERE name = 'Uncategorized'"
1000+
)
1001+
assert count == 1
1002+
1003+
def test_catchall_organization_covers_all_addresses(self, migration_db):
1004+
app, db_uri = migration_db
1005+
_run_migration(app)
1006+
arange = _query_scalar(
1007+
db_uri, "SELECT arange FROM organization WHERE name = 'Uncategorized'"
1008+
)
1009+
assert "0.0.0.0/0" in arange
1010+
assert "::/0" in arange
1011+
9951012

9961013
class TestIdempotent:
9971014
"""Test that running migration twice doesn't fail."""
@@ -1418,6 +1435,26 @@ def test_alembic_version_updated(self, migration_db):
14181435
db_uri, "SELECT version_num FROM alembic_version"
14191436
) == "001_baseline"
14201437

1438+
def test_org_id_backfilled_with_catchall(self, migration_db):
1439+
"""Existing rules with no org_id should be assigned the catchall org after migration."""
1440+
_, db_uri = self._setup_and_migrate(migration_db)
1441+
catchall_id = _query_scalar(
1442+
db_uri, "SELECT id FROM organization WHERE name = 'Uncategorized'"
1443+
)
1444+
assert catchall_id is not None
1445+
# No NULLs anywhere
1446+
for table in ("flowspec4", "flowspec6", "RTBH", "api_key", "machine_api_key"):
1447+
null_count = _query_scalar(
1448+
db_uri, f"SELECT COUNT(*) FROM {table} WHERE org_id IS NULL"
1449+
)
1450+
assert null_count == 0, f"NULL org_id found in {table} after migration"
1451+
# Tables that have rows in the 2019 backup should point to catchall
1452+
for table in ("flowspec4",):
1453+
catchall_count = _query_scalar(
1454+
db_uri, f"SELECT COUNT(*) FROM {table} WHERE org_id = {catchall_id}"
1455+
)
1456+
assert catchall_count > 0, f"Expected rows in {table} to be assigned to catchall org"
1457+
14211458
def test_fails_without_clearing_old_revision(self, migration_db):
14221459
"""Migration should fail if old alembic_version is not cleared first."""
14231460
app, db_uri = migration_db

0 commit comments

Comments
 (0)