fix(backup): Remove HybridCloud keys from Relocation models (#59579)

In PR #59160, we added some new `Relocation*` models. These are meant to
live on the `files` connection, which does not currently support region
tombstones. Since this support would be pretty tricky to add, we've
decided to just use regular integers to represent users, ditching
HybridCloudForeignKey for this model.

Closes getsentry/sentry#59578

fixtures/backup/model_dependencies/detailed.json

@@ -4991,20 +4991,11 @@
   },
   "sentry.relocation": {
     "dangling": false,
-    "foreign_keys": {
-      "creator": {
-        "kind": "HybridCloudForeignKey",
-        "model": "sentry.user",
-        "nullable": true
-      },
-      "owner": {
-        "kind": "HybridCloudForeignKey",
-        "model": "sentry.user",
-        "nullable": true
-      }
-    },
+    "foreign_keys": {},
     "model": "sentry.relocation",
-    "relocation_dependencies": [],
+    "relocation_dependencies": [
+      "sentry.user"
+    ],
     "relocation_scope": "Excluded",
     "silos": [
       "Region"

fixtures/backup/model_dependencies/flat.json

@@ -694,9 +694,7 @@
     "sentry.environment",
     "sentry.project"
   ],
-  "sentry.relocation": [
-    "sentry.user"
-  ],
+  "sentry.relocation": [],
   "sentry.relocationfile": [
     "sentry.file",
     "sentry.relocation"

migrations_lockfile.txt

@@ -9,5 +9,5 @@ feedback: 0003_feedback_add_env
 hybridcloud: 0008_add_externalactorreplica
 nodestore: 0002_nodestore_no_dictfield
 replays: 0003_add_size_to_recording_segment
-sentry: 0590_add_metadata_to_sentry_app
+sentry: 0592_delete_relocation_hybrid_cloud_foreign_keys
 social_auth: 0002_default_auto_field

src/sentry/api/endpoints/relocation.py

@@ -77,7 +77,7 @@ class RelocationEndpoint(Endpoint):
 
         # Quickly check that this `owner` does not have more than one active `Relocation` in flight.
         if Relocation.objects.filter(
-            owner=owner.id, status=Relocation.Status.IN_PROGRESS.value
+            owner_id=owner.id, status=Relocation.Status.IN_PROGRESS.value
         ).exists():
             return Response({"detail": ERR_DUPLICATE_RELOCATION}, status=409)
 
@@ -91,8 +91,8 @@ class RelocationEndpoint(Endpoint):
             using=(router.db_for_write(Relocation), router.db_for_write(RelocationFile))
         ):
             relocation: Relocation = Relocation.objects.create(
-                creator=request.user.id,
-                owner=owner.id,
+                creator_id=request.user.id,
+                owner_id=owner.id,
                 want_org_slugs=org_slugs,
                 step=Relocation.Step.UPLOADING.value,
             )

src/sentry/migrations/0591_remove_relocation_hybrid_cloud_foreign_keys.py

@@ -0,0 +1,56 @@
+# Generated by Django 3.2.23 on 2023-11-07 22:35
+
+from django.db import migrations
+
+import sentry.db.models.fields.bounded
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, this should only be used for operations where it's safe to run the migration
+    # after your code has deployed. So this should not be used for most operations that alter the
+    # schema of a table.
+    # Here are some things that make sense to mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that they can
+    #   be monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = False
+
+    dependencies = [
+        ("sentry", "0590_add_metadata_to_sentry_app"),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            database_operations=[
+                migrations.RunSQL(
+                    """
+                    ALTER TABLE "sentry_relocation" ADD COLUMN "creator_id" bigint NOT NULL;
+                    ALTER TABLE "sentry_relocation" ADD COLUMN "owner_id" bigint NOT NULL;
+                    """,
+                    reverse_sql="""
+                    ALTER TABLE "sentry_relocation" DROP COLUMN "creator_id";
+                    ALTER TABLE "sentry_relocation" DROP COLUMN "owner_id";
+                    """,
+                    hints={"tables": ["sentry_relocation"]},
+                )
+            ],
+            state_operations=[
+                migrations.RemoveField(model_name="relocation", name="creator"),
+                migrations.RemoveField(model_name="relocation", name="owner"),
+                migrations.AddField(
+                    model_name="relocation",
+                    name="creator_id",
+                    field=sentry.db.models.fields.bounded.BoundedBigIntegerField(),
+                ),
+                migrations.AddField(
+                    model_name="relocation",
+                    name="owner_id",
+                    field=sentry.db.models.fields.bounded.BoundedBigIntegerField(),
+                ),
+            ],
+        )
+    ]

src/sentry/migrations/0592_delete_relocation_hybrid_cloud_foreign_keys.py

@@ -0,0 +1,42 @@
+# Generated by Django 3.2.23 on 2023-11-07 22:35
+
+from django.db import migrations
+
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, this should only be used for operations where it's safe to run the migration
+    # after your code has deployed. So this should not be used for most operations that alter the
+    # schema of a table.
+    # Here are some things that make sense to mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that they can
+    #   be monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = False
+
+    dependencies = [
+        ("sentry", "0591_remove_relocation_hybrid_cloud_foreign_keys"),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            database_operations=[
+                migrations.RunSQL(
+                    """
+                    ALTER TABLE "sentry_relocation" DROP COLUMN "creator";
+                    ALTER TABLE "sentry_relocation" DROP COLUMN "owner";
+                    """,
+                    reverse_sql="""
+                    ALTER TABLE "sentry_relocation" ADD COLUMN "creator" int NULL;
+                    ALTER TABLE "sentry_relocation" ADD COLUMN "owner" int NULL;
+                    """,
+                    hints={"tables": ["sentry_relocation"]},
+                )
+            ],
+            state_operations=[],
+        )
+    ]

src/sentry/models/relocation.py

@@ -7,10 +7,9 @@ from uuid import uuid4
 from django.db import models
 
 from sentry.backup.scopes import RelocationScope
-from sentry.db.models import region_silo_only_model
+from sentry.db.models import BoundedBigIntegerField, region_silo_only_model
 from sentry.db.models.base import DefaultFieldsModel, sane_repr
 from sentry.db.models.fields.foreignkey import FlexibleForeignKey
-from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
 from sentry.db.models.fields.uuid import UUIDField
 
 logger = logging.getLogger(__name__)
@@ -28,6 +27,7 @@ class Relocation(DefaultFieldsModel):
     """
 
     __relocation_scope__ = RelocationScope.Excluded
+    __relocation_dependencies__ = {"sentry.User"}
 
     # The last stage this relocation reached. If the `Status` is `IN_PROGRESS`, the relocation is
     # still active; otherwise, this can be considered the terminal stage.
@@ -66,22 +66,14 @@ class Relocation(DefaultFieldsModel):
 
     # The user that requested this relocation - if the request was made by an admin on behalf of a
     # user, this will be different from `owner`. Otherwise, they are identical.
-    #
-    # This is left NULL because we want to retain the ability to audit and rollback a `Relocation`
-    # even in the (unlikely) event of an admin account being deleted, but that `NULL` should never
-    # be set at creation time.
-    creator = HybridCloudForeignKey("sentry.User", null=True, on_delete="SET_NULL")
+    creator_id = BoundedBigIntegerField()
 
     # The user that will be marked as the `owner` of this relocation. This is subtly different from
     # the `creator` - anyone with superuser privileges can create a new relocation, but it must
     # always be assigned to some user who will be responsible for it (ex: will become a global admin
     # for all newly imported orgs, will receive emails with status updates as the relocation
     # progresses, etc) over its lifetime and once it is completed.
-    #
-    # This is left NULL because we want to retain the ability to audit and rollback a `Relocation`
-    # even in the event of the owner's account being deleted, but that `NULL` should never be set at
-    # creation time.
-    owner = HybridCloudForeignKey("sentry.User", null=True, on_delete="SET_NULL")
+    owner_id = BoundedBigIntegerField()
 
     # Unique ID for this import attempt. All assembled files in the remote filestore will be in a
     # directory named after this UUID.

tests/sentry/api/endpoints/test_relocation.py

@@ -75,15 +75,15 @@ class RelocationCreateTest(APITestCase):
 
     def test_success_relocation_for_same_owner_already_completed(self):
         Relocation.objects.create(
-            creator=self.superuser.id,
-            owner=self.owner.id,
+            creator_id=self.superuser.id,
+            owner_id=self.owner.id,
             want_org_slugs=["not-relevant-to-this-test"],
             step=Relocation.Step.COMPLETED.value,
             status=Relocation.Status.FAILURE.value,
         )
         Relocation.objects.create(
-            creator=self.superuser.id,
-            owner=self.owner.id,
+            creator_id=self.superuser.id,
+            owner_id=self.owner.id,
             want_org_slugs=["not-relevant-to-this-test"],
             step=Relocation.Step.COMPLETED.value,
             status=Relocation.Status.SUCCESS.value,
@@ -231,8 +231,8 @@ class RelocationCreateTest(APITestCase):
 
     def test_fail_relocation_for_same_owner_already_in_progress(self):
         Relocation.objects.create(
-            creator=self.superuser.id,
-            owner=self.owner.id,
+            creator_id=self.superuser.id,
+            owner_id=self.owner.id,
             want_org_slugs=["not-relevant-to-this-test"],
             step=Relocation.Step.UPLOADING.value,
         )

tests/sentry/tasks/test_relocation.py

@@ -83,8 +83,8 @@ class RelocationTaskTestCase(TestCase):
         )
         self.login_as(user=self.superuser, superuser=True)
         self.relocation: Relocation = Relocation.objects.create(
-            creator=self.superuser.id,
-            owner=self.owner.id,
+            creator_id=self.superuser.id,
+            owner_id=self.owner.id,
             want_org_slugs=["testing"],
             step=Relocation.Step.UPLOADING.value,
         )

tests/sentry/utils/test_relocation.py

@@ -22,8 +22,8 @@ class RelocationUtilsTestCase(TestCase):
             "superuser", is_superuser=True, is_staff=True, is_active=True
         )
         self.relocation: Relocation = Relocation.objects.create(
-            creator=self.superuser.id,
-            owner=self.owner.id,
+            creator_id=self.superuser.id,
+            owner_id=self.owner.id,
             want_org_slugs=["testing"],
             step=Relocation.Step.UPLOADING.value,
         )