diff --git a/fixtures/backup/model_dependencies/detailed.json b/fixtures/backup/model_dependencies/detailed.json index bac1277cecd0f0..8f8d466a24094f 100644 --- a/fixtures/backup/model_dependencies/detailed.json +++ b/fixtures/backup/model_dependencies/detailed.json @@ -6100,7 +6100,7 @@ }, "model": "sentry.userip", "relocation_dependencies": [], - "relocation_scope": "User", + "relocation_scope": "Global", "silos": [ "Control" ], diff --git a/src/sentry/models/userip.py b/src/sentry/models/userip.py index 93cb008da08008..8d8df03f079eba 100644 --- a/src/sentry/models/userip.py +++ b/src/sentry/models/userip.py @@ -17,7 +17,12 @@ @control_silo_only_model class UserIP(Model): - __relocation_scope__ = RelocationScope.User + # There is an absolutely massive number of `UserIP` models in any sufficiently long-lived + # install of Sentry. So while it would probably make semantic sense to have this be + # `RelocationScope.User`, only someone interested in backing up every bit of data could want + # this (we certainly don't need it on prod for relocation). Thus, this gets moved into the + # `Global` scope instead. + __relocation_scope__ = RelocationScope.Global __relocation_custom_ordinal__ = ["user", "ip_address"] user = FlexibleForeignKey(settings.AUTH_USER_MODEL) diff --git a/tests/sentry/backup/test_exports.py b/tests/sentry/backup/test_exports.py index 55dbd1689f8d31..28ef6f8a2433e6 100644 --- a/tests/sentry/backup/test_exports.py +++ b/tests/sentry/backup/test_exports.py @@ -15,7 +15,6 @@ from sentry.models.orgauthtoken import OrgAuthToken from sentry.models.user import User from sentry.models.useremail import UserEmail -from sentry.models.userip import UserIP from sentry.models.userpermission import UserPermission from sentry.models.userrole import UserRole, UserRoleUser from sentry.testutils.helpers.backups import ( @@ -162,11 +161,10 @@ def test_export_filter_users(self): data = self.export(tmp_dir, scope=ExportScope.User, filter_by={"user_2"}) # Count users, but also count a random model naively derived from just `User` alone, - # like `UserIP`. Because `Email` and `UserEmail` have some automagic going on that + # like `UserEmail`. Because `Email` and `UserEmail` have some automagic going on that # causes them to be created when a `User` is, we explicitly check to ensure that they # are behaving correctly as well. assert self.count(data, User) == 1 - assert self.count(data, UserIP) == 1 assert self.count(data, UserEmail) == 1 assert self.count(data, Email) == 1 @@ -187,7 +185,6 @@ def test_export_filter_users_shared_email(self): ) assert self.count(data, User) == 3 - assert self.count(data, UserIP) == 3 assert self.count(data, UserEmail) == 3 assert self.count(data, Email) == 2 @@ -231,7 +228,6 @@ def test_export_filter_orgs_single(self): assert not self.exists(data, Organization, "slug", "org-c") assert self.count(data, User) == 4 - assert self.count(data, UserIP) == 4 assert self.count(data, UserEmail) == 4 assert self.count(data, Email) == 3 # Lower due to `shared@example.com` @@ -268,7 +264,6 @@ def test_export_filter_orgs_multiple(self): assert self.exists(data, Organization, "slug", "org-c") assert self.count(data, User) == 5 - assert self.count(data, UserIP) == 5 assert self.count(data, UserEmail) == 5 assert self.count(data, Email) == 3 # Lower due to `shared@example.com` diff --git a/tests/sentry/backup/test_imports.py b/tests/sentry/backup/test_imports.py index 325c98d02ee518..380b3d2038e553 100644 --- a/tests/sentry/backup/test_imports.py +++ b/tests/sentry/backup/test_imports.py @@ -387,42 +387,6 @@ def test_bad_invalid_user(self): assert err.value.context.get_kind() == RpcImportErrorKind.ValidationError assert err.value.context.on.model == "sentry.user" - @patch("sentry.models.userip.geo_by_addr") - def test_good_regional_user_ip_in_user_scope(self, mock_geo_by_addr): - mock_geo_by_addr.return_value = { - "country_code": "US", - "region": "CA", - "subdivision": "San Francisco", - } - - with tempfile.TemporaryDirectory() as tmp_dir: - tmp_path = Path(tmp_dir).joinpath(f"{self._testMethodName}.json") - with open(tmp_path, "w+") as tmp_file: - models = self.json_of_exhaustive_user_with_minimum_privileges() - - # Modify the UserIP to be in California, USA. - for model in models: - if model["model"] == "sentry.userip": - model["fields"]["ip_address"] = "8.8.8.8" - json.dump(models, tmp_file) - - with open(tmp_path, "rb") as tmp_file: - import_in_user_scope(tmp_file, printer=NOOP_PRINTER) - - with assume_test_silo_mode(SiloMode.CONTROL): - assert UserIP.objects.count() == 1 - assert UserIP.objects.filter(ip_address="8.8.8.8").exists() - assert UserIP.objects.filter(country_code="US").exists() - assert UserIP.objects.filter(region_code="CA").exists() - - # Unlike global scope, this time must be reset. - assert UserIP.objects.filter( - last_seen__gt=datetime(2023, 7, 1, 0, 0, tzinfo=UTC) - ).exists() - assert UserIP.objects.filter( - first_seen__gt=datetime(2023, 7, 1, 0, 0, tzinfo=UTC) - ).exists() - @patch("sentry.models.userip.geo_by_addr") def test_good_regional_user_ip_in_global_scope(self, mock_geo_by_addr): mock_geo_by_addr.return_value = { @@ -551,7 +515,7 @@ def test_bad_invalid_user_ip(self): with open(tmp_path, "rb") as tmp_file: with pytest.raises(ImportingError) as err: - import_in_user_scope(tmp_file, printer=NOOP_PRINTER) + import_in_global_scope(tmp_file, printer=NOOP_PRINTER) assert err.value.context.get_kind() == RpcImportErrorKind.ValidationError assert err.value.context.on.model == "sentry.userip" @@ -1119,11 +1083,10 @@ def test_import_filter_users(self): with assume_test_silo_mode(SiloMode.CONTROL): # Count users, but also count a random model naively derived from just `User` alone, - # like `UserIP`. Because `Email` and `UserEmail` have some automagic going on that + # like `UserEmail`. Because `Email` and `UserEmail` have some automagic going on that # causes them to be created when a `User` is, we explicitly check to ensure that they # are behaving correctly as well. assert User.objects.count() == 1 - assert UserIP.objects.count() == 1 assert UserEmail.objects.count() == 1 assert Email.objects.count() == 1 @@ -1133,12 +1096,6 @@ def test_import_filter_users(self): ).count() == 1 ) - assert ( - ControlImportChunk.objects.filter( - model="sentry.userip", min_ordinal=1, max_ordinal=1 - ).count() - == 1 - ) assert ( ControlImportChunk.objects.filter( model="sentry.useremail", min_ordinal=1, max_ordinal=1 @@ -1170,7 +1127,6 @@ def test_export_filter_users_shared_email(self): with assume_test_silo_mode(SiloMode.CONTROL): assert User.objects.count() == 3 - assert UserIP.objects.count() == 3 assert UserEmail.objects.count() == 3 assert Email.objects.count() == 2 # Lower due to shared emails @@ -1180,12 +1136,6 @@ def test_export_filter_users_shared_email(self): ).count() == 1 ) - assert ( - ControlImportChunk.objects.filter( - model="sentry.userip", min_ordinal=1, max_ordinal=3 - ).count() - == 1 - ) assert ( ControlImportChunk.objects.filter( model="sentry.useremail", min_ordinal=1, max_ordinal=3 @@ -1215,7 +1165,6 @@ def test_import_filter_users_empty(self): with assume_test_silo_mode(SiloMode.CONTROL): assert User.objects.count() == 0 - assert UserIP.objects.count() == 0 assert UserEmail.objects.count() == 0 assert Email.objects.count() == 0 @@ -1251,7 +1200,6 @@ def test_import_filter_orgs_single(self): assert OrgAuthToken.objects.count() == 1 assert User.objects.count() == 4 - assert UserIP.objects.count() == 4 assert UserEmail.objects.count() == 4 assert Email.objects.count() == 3 # Lower due to `shared@example.com` @@ -1302,7 +1250,6 @@ def test_import_filter_orgs_multiple(self): ) assert User.objects.count() == 5 - assert UserIP.objects.count() == 5 assert UserEmail.objects.count() == 5 assert Email.objects.count() == 3 # Lower due to `shared@example.com` @@ -1335,7 +1282,6 @@ def test_import_filter_orgs_empty(self): assert OrgAuthToken.objects.count() == 0 assert User.objects.count() == 0 - assert UserIP.objects.count() == 0 assert UserEmail.objects.count() == 0 assert Email.objects.count() == 0 @@ -1853,7 +1799,7 @@ def test_colliding_configs_overwrite_configs_disabled_in_config_scope( with open(tmp_path, "rb") as tmp_file: verify_models_in_output(expected_models, json.load(tmp_file)) - @expect_models(COLLISION_TESTED, Email, User, UserEmail, UserIP) + @expect_models(COLLISION_TESTED, Email, User, UserEmail) def test_colliding_user_with_merging_enabled_in_user_scope( self, expected_models: list[type[Model]] ): @@ -1872,7 +1818,6 @@ def test_colliding_user_with_merging_enabled_in_user_scope( with assume_test_silo_mode(SiloMode.CONTROL): assert User.objects.count() == 1 assert UserEmail.objects.count() == 1 # Keep only original when merging. - assert UserIP.objects.count() == 1 # Keep only original when merging. assert Authenticator.objects.count() == 1 assert Email.objects.count() == 2 @@ -1890,16 +1835,15 @@ def test_colliding_user_with_merging_enabled_in_user_scope( assert UserEmail.objects.filter(email__icontains="existing@").exists() assert not UserEmail.objects.filter(email__icontains="importing@").exists() - # Incoming `UserEmail`s, `UserPermissions`, and `UserIP`s for imported users are - # completely scrubbed when merging is enabled. + # Incoming `UserEmail`s and `UserPermissions` for imported users are completely + # scrubbed when merging is enabled. assert not ControlImportChunk.objects.filter(model="sentry.useremail").exists() - assert not ControlImportChunk.objects.filter(model="sentry.userip").exists() assert not ControlImportChunk.objects.filter(model="sentry.userpermission").exists() with open(tmp_path, "rb") as tmp_file: verify_models_in_output(expected_models, json.load(tmp_file)) - @expect_models(COLLISION_TESTED, Email, User, UserEmail, UserIP) + @expect_models(COLLISION_TESTED, Email, User, UserEmail) def test_colliding_user_with_merging_disabled_in_user_scope( self, expected_models: list[type[Model]] ): @@ -1917,7 +1861,6 @@ def test_colliding_user_with_merging_disabled_in_user_scope( with assume_test_silo_mode(SiloMode.CONTROL): assert User.objects.count() == 2 - assert UserIP.objects.count() == 2 assert UserEmail.objects.count() == 2 assert Authenticator.objects.count() == 1 # Only imported in global scope assert Email.objects.count() == 2 @@ -1945,9 +1888,7 @@ def test_colliding_user_with_merging_disabled_in_user_scope( with open(tmp_path, "rb") as tmp_file: verify_models_in_output(expected_models, json.load(tmp_file)) - @expect_models( - COLLISION_TESTED, Email, Organization, OrganizationMember, User, UserEmail, UserIP - ) + @expect_models(COLLISION_TESTED, Email, Organization, OrganizationMember, User, UserEmail) def test_colliding_user_with_merging_enabled_in_organization_scope( self, expected_models: list[type[Model]] ): @@ -1970,7 +1911,6 @@ def test_colliding_user_with_merging_enabled_in_organization_scope( assert User.objects.count() == 1 assert UserEmail.objects.count() == 1 # Keep only original when merging. - assert UserIP.objects.count() == 1 # Keep only original when merging. assert Authenticator.objects.count() == 1 # Only imported in global scope assert Email.objects.count() == 2 @@ -1989,10 +1929,9 @@ def test_colliding_user_with_merging_enabled_in_organization_scope( assert UserEmail.objects.filter(email__icontains="existing@").exists() assert not UserEmail.objects.filter(email__icontains="importing@").exists() - # Incoming `UserEmail`s, `UserPermissions`, and `UserIP`s for imported users are - # completely dropped when merging is enabled. + # Incoming `UserEmail`s, and `UserPermissions` for imported users are completely + # dropped when merging is enabled. assert not ControlImportChunk.objects.filter(model="sentry.useremail").exists() - assert not ControlImportChunk.objects.filter(model="sentry.userip").exists() assert not ControlImportChunk.objects.filter(model="sentry.userpermission").exists() assert Organization.objects.count() == 2 @@ -2022,9 +1961,7 @@ def test_colliding_user_with_merging_enabled_in_organization_scope( with open(tmp_path, "rb") as tmp_file: verify_models_in_output(expected_models, json.load(tmp_file)) - @expect_models( - COLLISION_TESTED, Email, Organization, OrganizationMember, User, UserEmail, UserIP - ) + @expect_models(COLLISION_TESTED, Email, Organization, OrganizationMember, User, UserEmail) def test_colliding_user_with_merging_disabled_in_organization_scope( self, expected_models: list[type[Model]] ): @@ -2047,7 +1984,6 @@ def test_colliding_user_with_merging_disabled_in_organization_scope( imported_user = User.objects.get(username__icontains="owner-") assert User.objects.count() == 2 - assert UserIP.objects.count() == 2 assert UserEmail.objects.count() == 2 assert Authenticator.objects.count() == 1 # Only imported in global scope assert Email.objects.count() == 2 @@ -2103,7 +2039,7 @@ def test_colliding_user_with_merging_disabled_in_organization_scope( with open(tmp_path, "rb") as tmp_file: verify_models_in_output(expected_models, json.load(tmp_file)) - @expect_models(COLLISION_TESTED, Email, User, UserEmail, UserIP, UserPermission) + @expect_models(COLLISION_TESTED, Email, User, UserEmail, UserPermission) def test_colliding_user_with_merging_enabled_in_config_scope( self, expected_models: list[type[Model]] ): @@ -2124,7 +2060,6 @@ def test_colliding_user_with_merging_enabled_in_config_scope( with assume_test_silo_mode(SiloMode.CONTROL): assert User.objects.count() == 1 assert UserEmail.objects.count() == 1 # Keep only original when merging. - assert UserIP.objects.count() == 1 # Keep only original when merging. assert UserPermission.objects.count() == 1 # Keep only original when merging. assert Authenticator.objects.count() == 1 assert Email.objects.count() == 2 @@ -2144,16 +2079,15 @@ def test_colliding_user_with_merging_enabled_in_config_scope( assert UserEmail.objects.filter(email__icontains="existing@").exists() assert not UserEmail.objects.filter(email__icontains="importing@").exists() - # Incoming `UserEmail`s, `UserPermissions`, and `UserIP`s for imported users are - # completely dropped when merging is enabled. + # Incoming `UserEmail`s, and `UserPermissions` for imported users are completely + # dropped when merging is enabled. assert not ControlImportChunk.objects.filter(model="sentry.useremail").exists() - assert not ControlImportChunk.objects.filter(model="sentry.userip").exists() assert not ControlImportChunk.objects.filter(model="sentry.userpermission").exists() with open(tmp_path, "rb") as tmp_file: verify_models_in_output(expected_models, json.load(tmp_file)) - @expect_models(COLLISION_TESTED, Email, User, UserEmail, UserIP, UserPermission) + @expect_models(COLLISION_TESTED, Email, User, UserEmail, UserPermission) def test_colliding_user_with_merging_disabled_in_config_scope( self, expected_models: list[type[Model]] ): @@ -2173,7 +2107,6 @@ def test_colliding_user_with_merging_disabled_in_config_scope( with assume_test_silo_mode(SiloMode.CONTROL): assert User.objects.count() == 2 - assert UserIP.objects.count() == 2 assert UserEmail.objects.count() == 2 assert UserPermission.objects.count() == 2 assert Authenticator.objects.count() == 1 # Only imported in global scope