ref(hc): Region name is set for organization mappings (#50257)

src/sentry/conf/server.py

@@ -3331,6 +3331,7 @@ SENTRY_ISSUE_PLATFORM_FUTURES_MAX_LIMIT = 10000
 
 SENTRY_REGION = os.environ.get("SENTRY_REGION", None)
 SENTRY_REGION_CONFIG: Union[Iterable[Region], str] = ()
+SENTRY_MONOLITH_REGION: str = "--monolith--"
 
 # Enable siloed development environment.
 USE_SILOS = os.environ.get("SENTRY_USE_SILOS", None)

src/sentry/receivers/outbox/region.py

@@ -33,6 +33,7 @@ from sentry.services.hybrid_cloud.organizationmember_mapping import (
     organizationmember_mapping_service,
 )
 from sentry.signals import member_joined
+from sentry.types.region import get_local_region
 
 
 @receiver(process_region_outbox, sender=OutboxCategory.AUDIT_LOG_EVENT)
@@ -108,7 +109,7 @@ def process_organization_updates(object_identifier: int, **kwds: Any):
         organization_mapping_service.delete(organization_id=object_identifier)
         return
 
-    update = update_organization_mapping_from_instance(org)
+    update = update_organization_mapping_from_instance(org, get_local_region())
     organization_mapping_service.upsert(organization_id=org.id, update=update)
 
 

src/sentry/services/hybrid_cloud/organization_mapping/impl.py

@@ -1,13 +1,12 @@
 from typing import Optional
 
-from django.db import transaction
-
 from sentry.models.organizationmapping import OrganizationMapping
 from sentry.services.hybrid_cloud.organization_mapping import (
     OrganizationMappingService,
     RpcOrganizationMapping,
     RpcOrganizationMappingUpdate,
 )
+from sentry.services.hybrid_cloud.organization_mapping.serial import serialize_organization_mapping
 
 
 class DatabaseBackedOrganizationMappingService(OrganizationMappingService):
@@ -49,21 +48,20 @@ class DatabaseBackedOrganizationMappingService(OrganizationMappingService):
         pass
 
     def update(self, organization_id: int, update: RpcOrganizationMappingUpdate) -> None:
-        with transaction.atomic():
-            (
-                OrganizationMapping.objects.filter(organization_id=organization_id)
-                .select_for_update()
-                .update(**update)
-            )
+        # TODO: REMOVE FROM GETSENTRY!
+        try:
+            OrganizationMapping.objects.get(organization_id=organization_id).update(**update)
+        except OrganizationMapping.DoesNotExist:
+            pass
 
     def upsert(
         self, organization_id: int, update: RpcOrganizationMappingUpdate
-    ) -> OrganizationMapping:
+    ) -> RpcOrganizationMapping:
         org_mapping, _created = OrganizationMapping.objects.update_or_create(
             organization_id=organization_id, defaults=update
         )
 
-        return org_mapping
+        return serialize_organization_mapping(org_mapping)
 
     def verify_mappings(self, organization_id: int, slug: str) -> None:
         try:

src/sentry/services/hybrid_cloud/organization_mapping/model.py

@@ -37,3 +37,4 @@ class RpcOrganizationMappingUpdate(TypedDict):
     customer_id: Optional[str]
     status: OrganizationStatus
     slug: str
+    region_name: str

src/sentry/services/hybrid_cloud/organization_mapping/serial.py

@@ -5,16 +5,20 @@ from sentry.services.hybrid_cloud.organization_mapping import (
     RpcOrganizationMapping,
     RpcOrganizationMappingUpdate,
 )
+from sentry.types.region import Region
 
 
 def update_organization_mapping_from_instance(
     organization: Organization,
+    region: Region,
 ) -> RpcOrganizationMappingUpdate:
-    attributes = {
-        attr_name: getattr(organization, attr_name)
-        for attr_name in RpcOrganizationMappingUpdate.__annotations__.keys()
-    }
-    return RpcOrganizationMappingUpdate(**attributes)  # type: ignore
+    return RpcOrganizationMappingUpdate(
+        name=organization.name,
+        customer_id=organization.customer_id,
+        status=organization.status,
+        slug=organization.slug,
+        region_name=region.name,
+    )
 
 
 def serialize_organization_mapping(org_mapping: OrganizationMapping) -> RpcOrganizationMapping:

src/sentry/services/hybrid_cloud/organization_mapping/service.py

@@ -6,7 +6,6 @@
 from abc import abstractmethod
 from typing import Optional, cast
 
-from sentry.models import OrganizationMapping
 from sentry.services.hybrid_cloud.organization_mapping import (
     RpcOrganizationMapping,
     RpcOrganizationMappingUpdate,
@@ -68,7 +67,7 @@ class OrganizationMappingService(RpcService):
     @abstractmethod
     def upsert(
         self, *, organization_id: int, update: RpcOrganizationMappingUpdate
-    ) -> OrganizationMapping:
+    ) -> RpcOrganizationMapping:
         pass
 
     @rpc_method

src/sentry/testutils/region.py

@@ -1,12 +1,11 @@
 from contextlib import contextmanager
 from typing import Iterable
-from unittest import mock
 
-from sentry.types.region import Region, _RegionMapping
+from sentry.types import region
 
 
 @contextmanager
-def override_regions(regions: Iterable[Region]):
+def override_regions(regions: Iterable[region.Region]):
     """Override the global set of existing regions.
 
     The overriding value takes the place of the `SENTRY_REGION_CONFIG` setting and
@@ -15,10 +14,15 @@ def override_regions(regions: Iterable[Region]):
     because the region mapping may already be cached.
     """
 
-    mapping = _RegionMapping(regions)
+    mapping = region.GlobalRegionDirectory(regions)
 
-    def override() -> _RegionMapping:
+    def override() -> region.GlobalRegionDirectory:
         return mapping
 
-    with mock.patch("sentry.types.region._load_global_regions", new=override):
+    existing = region.load_global_regions
+    region.load_global_regions = override
+
+    try:
         yield
+    finally:
+        region.load_global_regions = existing

src/sentry/types/region.py

@@ -1,11 +1,12 @@
 from __future__ import annotations
 
-import functools
 from dataclasses import dataclass
 from enum import Enum
-from typing import TYPE_CHECKING, Iterable, Set
+from typing import TYPE_CHECKING, Collection, Iterable, Set
 from urllib.parse import urljoin
 
+from django.conf import settings
+
 from sentry.services.hybrid_cloud.util import control_silo_function
 from sentry.silo import SiloMode
 from sentry.utils import json
@@ -13,8 +14,6 @@ from sentry.utils import json
 if TYPE_CHECKING:
     from sentry.models import Organization
 
-MONOLITH_REGION_NAME = "--monolith--"
-
 
 class RegionCategory(Enum):
     MULTI_TENANT = "MULTI_TENANT"
@@ -97,10 +96,19 @@ class RegionContextError(Exception):
     """Indicate that the server is not in a state to resolve a region."""
 
 
-class _RegionMapping:
+class GlobalRegionDirectory:
     """The set of all regions in this Sentry platform instance."""
 
-    def __init__(self, regions: Iterable[Region]) -> None:
+    def __init__(self, regions: Collection[Region]) -> None:
+        if not any(r.name == settings.SENTRY_MONOLITH_REGION for r in regions):
+            default_monolith_region = Region(
+                name=settings.SENTRY_MONOLITH_REGION,
+                snowflake_id=0,
+                address="/",
+                category=RegionCategory.MULTI_TENANT,
+            )
+            regions = [default_monolith_region, *regions]
+
         self.regions = frozenset(regions)
         self.by_name = {r.name: r for r in self.regions}
 
@@ -112,8 +120,14 @@ def _parse_config(region_config: str) -> Iterable[Region]:
         yield Region(**config_value)
 
 
-@functools.lru_cache(maxsize=1)
-def _load_global_regions() -> _RegionMapping:
+_global_regions: GlobalRegionDirectory | None = None
+
+
+def load_global_regions() -> GlobalRegionDirectory:
+    global _global_regions
+    if _global_regions is not None:
+        return _global_regions
+
     from django.conf import settings
 
     # For now, assume that all region configs can be taken in through Django
@@ -121,14 +135,20 @@ def _load_global_regions() -> _RegionMapping:
     # production.
     config = settings.SENTRY_REGION_CONFIG
     if isinstance(config, str):
-        config = _parse_config(config)
-    return _RegionMapping(config)
+        config = list(_parse_config(config))
+    _global_regions = GlobalRegionDirectory(config)
+    return _global_regions
+
+
+def clear_global_regions() -> None:
+    global _global_regions
+    _global_regions = None
 
 
 def get_region_by_name(name: str) -> Region:
     """Look up a region by name."""
     try:
-        return _load_global_regions().by_name[name]
+        return load_global_regions().by_name[name]
     except KeyError:
         raise RegionResolutionError(f"No region with name: {name!r}")
 
@@ -139,7 +159,7 @@ def get_region_for_organization(organization: Organization) -> Region:
     Raises RegionContextError if this Sentry platform instance is configured to
     run only in monolith mode.
     """
-    mapping = _load_global_regions()
+    mapping = load_global_regions()
 
     if not mapping.regions:
         raise RegionContextError("No regions are configured")
@@ -158,13 +178,7 @@ def get_local_region() -> Region:
     from django.conf import settings
 
     if SiloMode.get_current_mode() == SiloMode.MONOLITH:
-        # This is a dummy value used to make region.to_url work
-        return Region(
-            name=MONOLITH_REGION_NAME,
-            snowflake_id=0,
-            address="/",
-            category=RegionCategory.MULTI_TENANT,
-        )
+        return get_region_by_name(settings.SENTRY_MONOLITH_REGION)
 
     if SiloMode.get_current_mode() != SiloMode.REGION:
         raise RegionContextError("Not a region silo")
@@ -188,25 +202,23 @@ def find_regions_for_orgs(org_ids: Iterable[int]) -> Set[str]:
     from sentry.models import OrganizationMapping
 
     if SiloMode.get_current_mode() == SiloMode.MONOLITH:
-        return {
-            MONOLITH_REGION_NAME,
-        }
+        return {settings.SENTRY_MONOLITH_REGION}
     else:
-        return {
-            t["region_name"]
-            for t in OrganizationMapping.objects.filter(organization_id__in=org_ids).values(
-                "region_name"
+        return set(
+            OrganizationMapping.objects.filter(organization_id__in=org_ids).values_list(
+                "region_name", flat=True
             )
-        }
+        )
 
 
 @control_silo_function
 def find_regions_for_user(user_id: int) -> Set[str]:
+    if SiloMode.get_current_mode() == SiloMode.MONOLITH:
+        return {settings.SENTRY_MONOLITH_REGION}
+
     org_ids = _find_orgs_for_user(user_id)
     return find_regions_for_orgs(org_ids)
 
 
 def find_all_region_names() -> Iterable[str]:
-    if SiloMode.get_current_mode() == SiloMode.MONOLITH:
-        return {MONOLITH_REGION_NAME}
-    return _load_global_regions().by_name.keys()
+    return load_global_regions().by_name.keys()

tests/sentry/hybrid_cloud/test_organizationmapping.py

@@ -38,35 +38,6 @@ class OrganizationMappingTest(TransactionTestCase):
         assert self.organization.slug == org_mapping.slug
         assert self.organization.name == org_mapping.name
 
-    def test_update(self):
-        self.organization = Factories.create_organization(name="test name")
-
-        organization_mapping = OrganizationMapping.objects.get(organization_id=self.organization.id)
-        assert not organization_mapping.customer_id
-
-        organization_mapping_service.update(
-            organization_id=self.organization.id,
-            update=RpcOrganizationMappingUpdate(customer_id="test"),
-        )
-        org_mapping = OrganizationMapping.objects.get(organization_id=self.organization.id)
-        assert org_mapping.customer_id == "test"
-
-        organization_mapping_service.update(
-            organization_id=self.organization.id,
-            update=RpcOrganizationMappingUpdate(name="new name!"),
-        )
-        org_mapping = OrganizationMapping.objects.get(organization_id=self.organization.id)
-        assert org_mapping.customer_id == "test"
-        assert org_mapping.name == "new name!"
-        assert org_mapping.slug == self.organization.slug
-        assert org_mapping.status == self.organization.status
-
-        organization_mapping_service.update(
-            organization_id=self.organization.id, update=RpcOrganizationMappingUpdate()
-        )
-        # Does not overwrite with empty value.
-        assert org_mapping.name == "new name!"
-
     def test_upsert__create_if_not_found(self):
         self.organization = self.create_organization(
             name="test name",

tests/sentry/models/test_outbox.py

@@ -5,7 +5,8 @@ from unittest.mock import call, patch
 
 import pytest
 import responses
-from django.test import RequestFactory, override_settings
+from django.conf import settings
+from django.test import RequestFactory
 from freezegun import freeze_time
 from pytest import raises
 from rest_framework import status
@@ -28,8 +29,9 @@ from sentry.tasks.deliver_from_outbox import enqueue_outbox_jobs
 from sentry.testutils import TestCase
 from sentry.testutils.factories import Factories
 from sentry.testutils.outbox import outbox_runner
+from sentry.testutils.region import override_regions
 from sentry.testutils.silo import control_silo_test, exempt_from_silo_limits, region_silo_test
-from sentry.types.region import MONOLITH_REGION_NAME, Region, RegionCategory
+from sentry.types.region import Region, RegionCategory
 
 
 @pytest.fixture(autouse=True, scope="function")
@@ -89,7 +91,7 @@ class ControlOutboxTest(TestCase):
             org = Factories.create_organization()
 
             org_mapping = OrganizationMapping.objects.get(organization_id=org.id)
-            org_mapping.region_name = MONOLITH_REGION_NAME
+            org_mapping.region_name = settings.SENTRY_MONOLITH_REGION
             org_mapping.save()
 
             user1 = Factories.create_user()
@@ -112,14 +114,14 @@ class ControlOutboxTest(TestCase):
 
         for inst in ControlOutbox.for_webhook_update(
             webhook_identifier=WebhookProviderIdentifier.SLACK,
-            region_names=[MONOLITH_REGION_NAME, "special-slack-region"],
+            region_names=[settings.SENTRY_MONOLITH_REGION, "special-slack-region"],
             request=request,
         ):
             inst.save()
 
         for inst in ControlOutbox.for_webhook_update(
             webhook_identifier=WebhookProviderIdentifier.GITHUB,
-            region_names=[MONOLITH_REGION_NAME, "special-github-region"],
+            region_names=[settings.SENTRY_MONOLITH_REGION, "special-github-region"],
             request=request,
         ):
             inst.save()
@@ -129,17 +131,17 @@ class ControlOutboxTest(TestCase):
             for row in ControlOutbox.find_scheduled_shards()
         }
         assert shards == {
-            (OutboxScope.USER_SCOPE.value, user1.id, MONOLITH_REGION_NAME),
-            (OutboxScope.USER_SCOPE.value, user2.id, MONOLITH_REGION_NAME),
+            (OutboxScope.USER_SCOPE.value, user1.id, settings.SENTRY_MONOLITH_REGION),
+            (OutboxScope.USER_SCOPE.value, user2.id, settings.SENTRY_MONOLITH_REGION),
             (
                 OutboxScope.WEBHOOK_SCOPE.value,
                 WebhookProviderIdentifier.SLACK,
-                MONOLITH_REGION_NAME,
+                settings.SENTRY_MONOLITH_REGION,
             ),
             (
                 OutboxScope.WEBHOOK_SCOPE.value,
                 WebhookProviderIdentifier.GITHUB,
-                MONOLITH_REGION_NAME,
+                settings.SENTRY_MONOLITH_REGION,
             ),
             (
                 OutboxScope.WEBHOOK_SCOPE.value,
@@ -187,51 +189,51 @@ class ControlOutboxTest(TestCase):
         assert outbox.payload["body"] == '{"installation": {"id": "github:1"}}'
 
     @responses.activate
-    @override_settings(SENTRY_REGION_CONFIG=region_config)
     def test_drains_successful_success(self):
-        mock_response = responses.add(
-            self.webhook_request.method,
-            f"{self.region.address}{self.webhook_request.path}",
-            status=status.HTTP_200_OK,
-        )
-        expected_request_count = 1 if SiloMode.get_current_mode() == SiloMode.CONTROL else 0
-        [outbox] = ControlOutbox.for_webhook_update(
-            webhook_identifier=WebhookProviderIdentifier.GITHUB,
-            region_names=[self.region.name],
-            request=self.webhook_request,
-        )
-        outbox.save()
+        with override_regions(self.region_config):
+            mock_response = responses.add(
+                self.webhook_request.method,
+                f"{self.region.address}{self.webhook_request.path}",
+                status=status.HTTP_200_OK,
+            )
+            expected_request_count = 1 if SiloMode.get_current_mode() == SiloMode.CONTROL else 0
+            [outbox] = ControlOutbox.for_webhook_update(
+                webhook_identifier=WebhookProviderIdentifier.GITHUB,
+                region_names=[self.region.name],
+                request=self.webhook_request,
+            )
+            outbox.save()
 
-        assert ControlOutbox.objects.filter(id=outbox.id).exists()
-        outbox.drain_shard()
-        assert mock_response.call_count == expected_request_count
-        assert not ControlOutbox.objects.filter(id=outbox.id).exists()
+            assert ControlOutbox.objects.filter(id=outbox.id).exists()
+            outbox.drain_shard()
+            assert mock_response.call_count == expected_request_count
+            assert not ControlOutbox.objects.filter(id=outbox.id).exists()
 
     @responses.activate
-    @override_settings(SENTRY_REGION_CONFIG=region_config)
     def test_drains_webhook_failure(self):
-        mock_response = responses.add(
-            self.webhook_request.method,
-            f"{self.region.address}{self.webhook_request.path}",
-            status=status.HTTP_502_BAD_GATEWAY,
-        )
-        [outbox] = ControlOutbox.for_webhook_update(
-            webhook_identifier=WebhookProviderIdentifier.GITHUB,
-            region_names=[self.region.name],
-            request=self.webhook_request,
-        )
-        outbox.save()
+        with override_regions(self.region_config):
+            mock_response = responses.add(
+                self.webhook_request.method,
+                f"{self.region.address}{self.webhook_request.path}",
+                status=status.HTTP_502_BAD_GATEWAY,
+            )
+            [outbox] = ControlOutbox.for_webhook_update(
+                webhook_identifier=WebhookProviderIdentifier.GITHUB,
+                region_names=[self.region.name],
+                request=self.webhook_request,
+            )
+            outbox.save()
 
-        assert ControlOutbox.objects.filter(id=outbox.id).exists()
-        if SiloMode.get_current_mode() == SiloMode.CONTROL:
-            with raises(ApiError):
-                outbox.drain_shard()
-            assert mock_response.call_count == 1
             assert ControlOutbox.objects.filter(id=outbox.id).exists()
-        else:
-            outbox.drain_shard()
-            assert mock_response.call_count == 0
-            assert not ControlOutbox.objects.filter(id=outbox.id).exists()
+            if SiloMode.get_current_mode() == SiloMode.CONTROL:
+                with raises(ApiError):
+                    outbox.drain_shard()
+                assert mock_response.call_count == 1
+                assert ControlOutbox.objects.filter(id=outbox.id).exists()
+            else:
+                outbox.drain_shard()
+                assert mock_response.call_count == 0
+                assert not ControlOutbox.objects.filter(id=outbox.id).exists()
 
 
 @region_silo_test(stable=True)