From e0419dba3d15f282498ca4529ef26bb89b89c7cb Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 7 May 2026 17:04:14 -0400 Subject: [PATCH 1/2] Sketch out changes required for control org create --- .../core/endpoints/organization_index.py | 9 +++++---- .../core/endpoints/test_organization_index.py | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/sentry/core/endpoints/organization_index.py b/src/sentry/core/endpoints/organization_index.py index 94f557acd8b1de..48b68243edfefe 100644 --- a/src/sentry/core/endpoints/organization_index.py +++ b/src/sentry/core/endpoints/organization_index.py @@ -53,6 +53,8 @@ class OrganizationPostSerializer(BaseOrganizationSerializer): agreeTerms = serializers.BooleanField(required=True) aggregatedDataConsent = serializers.BooleanField(required=False) idempotencyKey = serializers.CharField(max_length=IDEMPOTENCY_KEY_LENGTH, required=False) + # TODO add dataStorageLocation and validate it. If the parameter is not provided + # Use `us` as the locality name. def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -329,10 +331,7 @@ def post(self, request: Request) -> Response: terms of service and privacy policy. :auth: required, user-context-needed """ - # TODO(cells): Move org creation to control as part of the broader - # org-listing/org-provisioning cutover. Since POST is private, the - # legacy cell-side path can be removed once the control implementation - # is ready. + # TODO remove this check as the endpoint should be safe in both silo modes now. if SiloMode.get_current_mode() == SiloMode.CONTROL: return Response(status=404) @@ -403,6 +402,8 @@ def post(self, request: Request) -> Response: ) rpc_org = organization_provisioning_service.provision_organization_in_cell( + # TODO resolve the cell name from request data. The validator logic should handle + # the fallback to local cell (in region mode) or monolith region cell_name=settings.SENTRY_LOCAL_CELL or settings.SENTRY_MONOLITH_REGION, provisioning_options=provision_args, ) diff --git a/tests/sentry/core/endpoints/test_organization_index.py b/tests/sentry/core/endpoints/test_organization_index.py index f4f55d44e7f238..82163704524f6b 100644 --- a/tests/sentry/core/endpoints/test_organization_index.py +++ b/tests/sentry/core/endpoints/test_organization_index.py @@ -234,6 +234,24 @@ def test_response_compatible_with_cell(self) -> None: } +@control_silo_test +class OrganizationsCreateControlTest(OrganizationIndexTest, HybridCloudTestMixin): + def test_implicit_data_storage_location(self) -> None: + # TODO write this test. When no dataStorageLocation is provided fallback to us. + pass + + def test_invalid_data_storage_location(self) -> None: + # TODO write this test. An invalid region name should fail + pass + + def test_locality_to_cell_resolution(self) -> None: + # TODO write this test. The request data is a locality. + # The endpoint should resolve the locality into the new_org_cell Cell. + pass + + +# TODO make this both control and cell silo +# Will need to setup a us/de cell and class OrganizationsCreateTest(OrganizationIndexTest, HybridCloudTestMixin): method = "post" From b22ff7dd43b51ffebda8d23aef5d927b474857e8 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 8 May 2026 18:11:21 -0400 Subject: [PATCH 2/2] feat(cells) Make organization-create work in control silo With all the RPC refactoring done, we can make `POST /organizations` work in control silo. Having this endpoint work in both cell & control silo modes is necessary so that we can update internal clients incrementally. These changes also add support for `dataStorageLocation` with locality -> cell resolution which is required for control based provisioning. Refs INFRENG-186 --- .../api/serializers/models/organization.py | 21 +- .../core/endpoints/organization_index.py | 58 +++-- .../core/endpoints/test_organization_index.py | 212 +++++++++++++++++- 3 files changed, 258 insertions(+), 33 deletions(-) diff --git a/src/sentry/api/serializers/models/organization.py b/src/sentry/api/serializers/models/organization.py index d9ccdbaaa738ea..7a90bc0f60f3b8 100644 --- a/src/sentry/api/serializers/models/organization.py +++ b/src/sentry/api/serializers/models/organization.py @@ -194,7 +194,11 @@ def validate_name(self, value: str) -> str: return value - def validate_slug(self, value: str) -> str: + def _validate_slug_shape(self, value: str) -> str: + """ + Validate slug values without any DB queries. + This method is re-used across cell + control silos. + """ # Historically, the only check just made sure there was more than 1 # character for the slug, but since then, there are many slugs that # fit within this new imposed limit. We're not fixing existing, but @@ -209,12 +213,6 @@ def validate_slug(self, value: str) -> str: ) if value in RESERVED_ORGANIZATION_SLUGS: raise serializers.ValidationError(f'This slug "{value}" is reserved and not allowed.') - qs = Organization.objects.filter(slug=value) - if "organization" in self.context: - qs = qs.exclude(id=self.context["organization"].id) - if qs.exists(): - raise serializers.ValidationError(f'The slug "{value}" is already in use.') - contains_whitespace = any(c.isspace() for c in self.initial_data["slug"]) if contains_whitespace: raise serializers.ValidationError( @@ -222,6 +220,15 @@ def validate_slug(self, value: str) -> str: ) return value + def validate_slug(self, value: str) -> str: + value = self._validate_slug_shape(value) + qs = Organization.objects.filter(slug=value) + if "organization" in self.context: + qs = qs.exclude(id=self.context["organization"].id) + if qs.exists(): + raise serializers.ValidationError(f'The slug "{value}" is already in use.') + return value + class TrustedRelaySerializerResponse(TypedDict, total=False): name: str diff --git a/src/sentry/core/endpoints/organization_index.py b/src/sentry/core/endpoints/organization_index.py index 48b68243edfefe..cc6eadf951007f 100644 --- a/src/sentry/core/endpoints/organization_index.py +++ b/src/sentry/core/endpoints/organization_index.py @@ -33,6 +33,7 @@ from sentry.models.organizationmapping import OrganizationMapping from sentry.models.organizationmember import OrganizationMember from sentry.models.organizationmembermapping import OrganizationMemberMapping +from sentry.organizations.services.organization import organization_service from sentry.search.utils import tokenize_query from sentry.services.organization import ( OrganizationOptions, @@ -41,6 +42,12 @@ ) from sentry.services.organization.provisioning import organization_provisioning_service from sentry.silo.base import SiloMode +from sentry.types.cell import ( + CellResolutionError, + RegionCategory, + get_locality_by_name, + get_new_org_cell_for_locality, +) from sentry.users.services.user.serial import serialize_generic_user from sentry.users.services.user.service import user_service from sentry.utils.pagination_factory import PaginatorLike @@ -53,8 +60,7 @@ class OrganizationPostSerializer(BaseOrganizationSerializer): agreeTerms = serializers.BooleanField(required=True) aggregatedDataConsent = serializers.BooleanField(required=False) idempotencyKey = serializers.CharField(max_length=IDEMPOTENCY_KEY_LENGTH, required=False) - # TODO add dataStorageLocation and validate it. If the parameter is not provided - # Use `us` as the locality name. + dataStorageLocation = serializers.CharField(required=False) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -68,6 +74,37 @@ def validate_agreeTerms(self, value): raise serializers.ValidationError("This attribute is required.") return value + def validate_slug(self, value: str) -> str: + if SiloMode.get_current_mode() == SiloMode.CONTROL: + value = self._validate_slug_shape(value) + if OrganizationMapping.objects.filter(slug=value).exists(): + raise serializers.ValidationError(f'The slug "{value}" is already in use.') + + return value + # TODO(cells) remove this path when cell scoped provisioning is removed. + return super().validate_slug(value) + + def validate_dataStorageLocation(self, value: str) -> str: + try: + locality = get_locality_by_name(value) + except CellResolutionError: + raise serializers.ValidationError(f"Unknown data storage location {value!r}.") + if locality.category != RegionCategory.MULTI_TENANT or not locality.visible: + raise serializers.ValidationError(f"Unknown data storage location {value!r}.") + return value + + def validate(self, attrs): + attrs = super().validate(attrs) + + locality_name = attrs.get("dataStorageLocation") + if locality_name is None and SiloMode.get_current_mode() == SiloMode.CONTROL: + locality_name = settings.SENTRY_MONOLITH_REGION + + if locality_name is not None: + attrs["cell_name"] = get_new_org_cell_for_locality(locality_name).name + + return attrs + @extend_schema(tags=["Users"]) @all_silo_endpoint @@ -314,7 +351,6 @@ def _get_from_control(self, request: Request) -> Response: paginator_cls=paginator_cls, ) - # XXX: endpoint useless for end-users as it needs user context. def post(self, request: Request) -> Response: """ Create a New Organization @@ -331,10 +367,6 @@ def post(self, request: Request) -> Response: terms of service and privacy policy. :auth: required, user-context-needed """ - # TODO remove this check as the endpoint should be safe in both silo modes now. - if SiloMode.get_current_mode() == SiloMode.CONTROL: - return Response(status=404) - if not request.user.is_authenticated: return Response({"detail": "This endpoint requires user info"}, status=401) @@ -402,18 +434,14 @@ def post(self, request: Request) -> Response: ) rpc_org = organization_provisioning_service.provision_organization_in_cell( - # TODO resolve the cell name from request data. The validator logic should handle - # the fallback to local cell (in region mode) or monolith region - cell_name=settings.SENTRY_LOCAL_CELL or settings.SENTRY_MONOLITH_REGION, + cell_name=result.get("cell_name"), provisioning_options=provision_args, ) - org = Organization.objects.get(id=rpc_org.id) - - # TODO(hybrid-cloud): We'll need to catch a more generic error - # when the internal RPC is implemented. except IntegrityError: return Response( {"detail": "An organization with this slug already exists."}, status=409 ) - return Response(serialize(org, request.user), status=201) + org_data = organization_service.serialize_organization(id=rpc_org.id, as_user=rpc_user) + + return Response(org_data, status=201) diff --git a/tests/sentry/core/endpoints/test_organization_index.py b/tests/sentry/core/endpoints/test_organization_index.py index 82163704524f6b..ce8ae123d301aa 100644 --- a/tests/sentry/core/endpoints/test_organization_index.py +++ b/tests/sentry/core/endpoints/test_organization_index.py @@ -25,16 +25,19 @@ from sentry.silo.base import SiloMode from sentry.testutils.asserts import assert_org_audit_log_exists from sentry.testutils.cases import APITestCase, TwoFactorAPITestCase +from sentry.testutils.cell import get_test_env_directory from sentry.testutils.helpers.analytics import assert_any_analytics_event from sentry.testutils.helpers.options import override_options from sentry.testutils.hybrid_cloud import HybridCloudTestMixin from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import ( assume_test_silo_mode, + assume_test_silo_mode_of, cell_silo_test, control_silo_test, create_test_cells, ) +from sentry.types.cell import Cell, Locality, RegionCategory from sentry.users.models.authenticator import Authenticator from sentry.utils.slug import ORG_SLUG_PATTERN @@ -234,25 +237,212 @@ def test_response_compatible_with_cell(self) -> None: } -@control_silo_test +@control_silo_test(cells=create_test_cells("us", "de")) class OrganizationsCreateControlTest(OrganizationIndexTest, HybridCloudTestMixin): + method = "post" + def test_implicit_data_storage_location(self) -> None: - # TODO write this test. When no dataStorageLocation is provided fallback to us. - pass + response = self.get_success_response(name="implicit org", slug="implicit-org") + + organization_id = response.data["id"] + with assume_test_silo_mode(SiloMode.CONTROL): + mapping = OrganizationMapping.objects.get(organization_id=organization_id) + assert mapping.cell_name == "us" def test_invalid_data_storage_location(self) -> None: - # TODO write this test. An invalid region name should fail - pass + response = self.get_error_response( + name="bad locality", + slug="bad-locality", + dataStorageLocation="atlantis", + status_code=400, + ) + assert "dataStorageLocation" in response.data def test_locality_to_cell_resolution(self) -> None: - # TODO write this test. The request data is a locality. - # The endpoint should resolve the locality into the new_org_cell Cell. - pass + cells = [ + Cell( + name="us", + snowflake_id=1, + category=RegionCategory.MULTI_TENANT, + address="10.0.0.1", + visible=True, + ), + Cell( + name="us2", + snowflake_id=3, + category=RegionCategory.MULTI_TENANT, + address="10.0.0.2", + visible=True, + ), + Cell( + name="de", + snowflake_id=4, + category=RegionCategory.MULTI_TENANT, + address="10.0.0.4", + visible=True, + ), + ] + + localities = [ + Locality( + name="us", + cells=frozenset(["us", "us2"]), + category=RegionCategory.MULTI_TENANT, + new_org_cell="us2", + visible=True, + ), + Locality( + name="de", + cells=frozenset(["de"]), + category=RegionCategory.MULTI_TENANT, + new_org_cell="de", + visible=True, + ), + ] + with get_test_env_directory().swap_state(cells, localities): + data = {"name": "Acme co", "slug": "acme-co", "dataStorageLocation": "us"} + resp = self.get_success_response(**data) + assert "acme-co" == resp.data["slug"] + org_id = resp.data["id"] + + with assume_test_silo_mode_of(OrganizationMapping): + org_mapping = OrganizationMapping.objects.get(slug="acme-co", organization_id=org_id) + assert org_mapping.cell_name == "us2", "Should be created in us2 cell, not us" + + with assume_test_silo_mode_of(Organization): + org = Organization.objects.get(id=org_id) + assert org.slug == org_mapping.slug + + # Validate ownership of the new org + owners = [owner.id for owner in org.get_owners()] + assert [self.user.id] == owners + + def test_with_default_team_true(self) -> None: + data = {"name": "hello world", "slug": "foobar", "defaultTeam": True} + response = self.get_success_response(**data) + + organization_id = response.data["id"] + + with assume_test_silo_mode_of(Organization): + Organization.objects.get(id=organization_id) + team = Team.objects.get(organization_id=organization_id) + assert team.name == "hello world" + + org_member = OrganizationMember.objects.get( + organization_id=organization_id, user_id=self.user.id + ) + OrganizationMemberTeam.objects.get(organizationmember_id=org_member.id, team_id=team.id) + + def test_invalid_slug_values(self) -> None: + with self.options({"api.rate-limit.org-create": 9001}): + self.get_error_response(name="name", slug=" i have whitespace ", status_code=400) + self.get_error_response(name="name", slug="bird-company!", status_code=400) + + def test_conflicting_slug(self) -> None: + self.create_organization(slug="acme-co") + + resp = self.get_error_response(name="name", slug="acme-co", status_code=400) + assert 'The slug "acme-co" is already in use' in str(resp.data) + + def test_name_with_url_scheme_rejected(self) -> None: + with self.options({"api.rate-limit.org-create": 9001}): + self.get_error_response( + name="https://evil.com Click Here", slug="legit-slug", status_code=400 + ) + self.get_error_response(name="http://evil.com", slug="legit-slug-2", status_code=400) + + def test_name_with_spam_signals_rejected(self) -> None: + response = self.get_error_response( + name="Win $50 ETH bit.ly/offer Claim Now", + slug="spam-org", + status_code=400, + ) + assert "disallowed content" in str(response.data) + + def test_name_with_single_signal_allowed(self) -> None: + response = self.get_success_response(name="BTC Analytics", slug="btc-analytics") + with assume_test_silo_mode_of(Organization): + org = Organization.objects.get(id=response.data["id"]) + assert org.name == "BTC Analytics" + + def test_required_terms_with_terms_url(self) -> None: + data: dict[str, Any] = {"name": "hello world"} + with self.settings(PRIVACY_URL=None, TERMS_URL="https://example.com/terms"): + self.get_success_response(**data) + + with self.settings(TERMS_URL=None, PRIVACY_URL="https://example.com/privacy"): + self.get_success_response(**data) + + with self.settings( + TERMS_URL="https://example.com/terms", PRIVACY_URL="https://example.com/privacy" + ): + data = {"name": "hello world", "agreeTerms": False} + self.get_error_response(status_code=400, **data) + + data = {"name": "hello world", "agreeTerms": True} + self.get_success_response(**data) + + @mock.patch("sentry.analytics.record") + def test_success_analytics_in_rpc_call(self, mock_record: mock.MagicMock) -> None: + self.login_as(user=self.user) + + with outbox_runner(): + data = { + "name": "org name", + "aggregatedDataConsent": True, + "agreeTerms": True, + "defaultTeam": True, + } + response = self.get_success_response(**data) + assert response.status_code == 201 + + with assume_test_silo_mode_of(Organization): + org = Organization.objects.get(slug="org-name") + + assert_any_analytics_event( + mock_record, + OrganizationCreatedEvent( + id=org.id, + actor_id=self.user.id, + name=org.name, + slug=org.slug, + ), + ) + assert_any_analytics_event( + mock_record, AggregatedDataConsentOrganizationCreatedEvent(organization_id=org.id) + ) + assert_org_audit_log_exists( + organization=org, + event=audit_log.get_event_id("ORG_ADD"), + ) + with assume_test_silo_mode_of(Organization): + assert org.get_option("sentry:aggregated_data_consent") is True + assert org.get_option("sentry:streamline_ui_only") is True + assert OrganizationMember.objects.filter( + organization_id=org.id, user_id=self.user.id + ).exists() + assert Team.objects.filter(organization_id=org.id).exists() + + def test_demo_user_cannot_create_organization(self) -> None: + demo_user = self.create_user("demo@example.com") + self.login_as(demo_user) + with override_options({"demo-mode.enabled": True, "demo-mode.users": [demo_user.id]}): + self.get_error_response(name="demo org", slug="demo-org", status_code=403) + + with assume_test_silo_mode_of(Organization): + assert not Organization.objects.filter(slug="demo-org").exists() + + def test_demo_user_cannot_create_organization_when_demo_mode_disabled(self) -> None: + demo_user = self.create_user("demo@example.com") + self.login_as(demo_user) + with override_options({"demo-mode.enabled": False, "demo-mode.users": [demo_user.id]}): + self.get_error_response(name="demo org", slug="demo-org", status_code=403) + + with assume_test_silo_mode_of(Organization): + assert not Organization.objects.filter(slug="demo-org").exists() -# TODO make this both control and cell silo -# Will need to setup a us/de cell and -class OrganizationsCreateTest(OrganizationIndexTest, HybridCloudTestMixin): +class OrganizationsCreateInCellTest(OrganizationIndexTest, HybridCloudTestMixin): method = "post" def test_missing_params(self) -> None: