Skip to content

Commit 4335c7e

Browse files
authored
fix(hc): Introduce OrganizationService.get_default_organization (#52372)
Fix a silo access error when running a typical dev environment in the single-org and split silo modes. Introduce a special region resolution strategy that requires there to be only one region, which is a safe assumption when there is only one organization.
1 parent c19f762 commit 4335c7e

File tree

5 files changed

+70
-6
lines changed

5 files changed

+70
-6
lines changed

Diff for: src/sentry/services/hybrid_cloud/organization/impl.py

+3
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ def get_org_by_slug(
110110
except Organization.DoesNotExist:
111111
return None
112112

113+
def get_default_organization(self) -> RpcOrganization:
114+
return serialize_rpc_organization(Organization.get_default())
115+
113116
def check_membership_by_email(
114117
self, organization_id: int, email: str
115118
) -> Optional[RpcOrganizationMember]:

Diff for: src/sentry/services/hybrid_cloud/organization/service.py

+6
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
ByOrganizationIdAttribute,
2626
ByOrganizationSlug,
2727
ByRegionName,
28+
RequireSingleOrganization,
2829
UnimplementedRegionResolution,
2930
)
3031
from sentry.services.hybrid_cloud.rpc import RpcService, regional_rpc_method
@@ -189,6 +190,11 @@ def get_organization_by_slug(
189190
return None
190191
return org_context
191192

193+
@regional_rpc_method(resolve=RequireSingleOrganization())
194+
@abstractmethod
195+
def get_default_organization(self) -> RpcOrganization:
196+
pass
197+
192198
@regional_rpc_method(resolve=ByOrganizationId())
193199
@abstractmethod
194200
def add_organization_member(

Diff for: src/sentry/services/hybrid_cloud/region.py

+29-1
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44
from dataclasses import dataclass
55
from typing import TYPE_CHECKING
66

7+
from django.conf import settings
8+
79
from sentry.services.hybrid_cloud import ArgumentDict
810
from sentry.services.hybrid_cloud.rpc import RpcServiceUnimplementedException
9-
from sentry.types.region import Region, get_region_by_name
11+
from sentry.types.region import Region, RegionResolutionError, get_region_by_name
1012

1113
if TYPE_CHECKING:
1214
from sentry.db.models import BaseManager
@@ -94,6 +96,32 @@ def resolve(self, arguments: ArgumentDict) -> Region:
9496
return self._resolve_from_mapping(mapping)
9597

9698

99+
class RequireSingleOrganization(RegionResolution):
100+
"""Resolve to the only region in a single-organization environment.
101+
102+
Calling a service method with this resolution strategy will cause an error if the
103+
environment is not configured with the "single organization" or has more than one
104+
region.
105+
"""
106+
107+
def resolve(self, arguments: ArgumentDict) -> Region:
108+
if not settings.SENTRY_SINGLE_ORGANIZATION:
109+
raise RegionResolutionError("Method is available only in single-org environment")
110+
111+
all_region_names = list(
112+
self.organization_mapping_manager.all()
113+
.values_list("region_name", flat=True)
114+
.distinct()[:2]
115+
)
116+
if len(all_region_names) == 0:
117+
return get_region_by_name(settings.SENTRY_MONOLITH_REGION)
118+
if len(all_region_names) != 1:
119+
raise RegionResolutionError("Expected single-org environment to have only one region")
120+
121+
(single_region_name,) = all_region_names
122+
return get_region_by_name(single_region_name)
123+
124+
97125
class UnimplementedRegionResolution(RegionResolution):
98126
"""Indicate that a method's region resolution logic has not been implemented yet.
99127

Diff for: src/sentry/web/frontend/auth_login.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ def get(self, request: Request, **kwargs) -> Response:
319319

320320
# Single org mode -- send them to the org-specific handler
321321
if settings.SENTRY_SINGLE_ORGANIZATION:
322-
org = Organization.get_default()
322+
org = organization_service.get_default_organization()
323323
next_uri = reverse("sentry-auth-organization", args=[org.slug])
324324
return HttpResponseRedirect(next_uri)
325325

Diff for: tests/sentry/hybrid_cloud/test_region.py

+31-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import pytest
2+
from django.test import override_settings
23

34
from sentry.db.postgres.roles import in_test_psql_role_override
45
from sentry.models.organizationmapping import OrganizationMapping
@@ -8,14 +9,15 @@
89
ByOrganizationIdAttribute,
910
ByOrganizationObject,
1011
ByOrganizationSlug,
12+
RequireSingleOrganization,
1113
UnimplementedRegionResolution,
1214
)
1315
from sentry.services.hybrid_cloud.rpc import RpcServiceUnimplementedException
1416
from sentry.silo import SiloMode
1517
from sentry.testutils import TestCase
1618
from sentry.testutils.region import override_regions
1719
from sentry.testutils.silo import assume_test_silo_mode, control_silo_test
18-
from sentry.types.region import Region, RegionCategory
20+
from sentry.types.region import Region, RegionCategory, RegionResolutionError
1921

2022

2123
@control_silo_test(stable=True)
@@ -26,11 +28,16 @@ def setUp(self):
2628
Region("europe", 2, "eu.sentry.io", RegionCategory.MULTI_TENANT),
2729
]
2830
self.target_region = self.regions[0]
29-
self.organization = self.create_organization()
30-
org_mapping = OrganizationMapping.objects.get(organization_id=self.organization.id)
31+
self.organization = self._create_org_in_region(self.target_region)
32+
33+
def _create_org_in_region(self, target_region):
34+
with override_settings(SENTRY_REGION=target_region.name):
35+
organization = self.create_organization()
36+
org_mapping = OrganizationMapping.objects.get(organization_id=organization.id)
3137
with in_test_psql_role_override("postgres"):
32-
org_mapping.region_name = self.target_region.name
38+
org_mapping.region_name = target_region.name
3339
org_mapping.save()
40+
return organization
3441

3542
def test_by_organization_object(self):
3643
with override_regions(self.regions):
@@ -65,6 +72,26 @@ def test_by_organization_id_attribute(self):
6572
actual_region = region_resolution.resolve(arguments)
6673
assert actual_region == self.target_region
6774

75+
def test_require_single_organization(self):
76+
region_resolution = RequireSingleOrganization()
77+
78+
with override_regions([self.target_region]), override_settings(
79+
SENTRY_SINGLE_ORGANIZATION=True
80+
):
81+
actual_region = region_resolution.resolve({})
82+
assert actual_region == self.target_region
83+
84+
with override_regions([self.target_region]), override_settings(
85+
SENTRY_SINGLE_ORGANIZATION=False
86+
):
87+
with pytest.raises(RegionResolutionError):
88+
region_resolution.resolve({})
89+
90+
with override_regions(self.regions), override_settings(SENTRY_SINGLE_ORGANIZATION=True):
91+
self._create_org_in_region(self.regions[1])
92+
with pytest.raises(RegionResolutionError):
93+
region_resolution.resolve({})
94+
6895
def test_unimplemented_region_resolution(self):
6996
with override_regions(self.regions):
7097
region_resolution = UnimplementedRegionResolution()

0 commit comments

Comments
 (0)