fix(hybridcloud) Fix View classes in region domains (#60407)

Previously when a View subclass was reached via a subdomain + org slug,
the application would incorrect assume that the subdomain was an org
slug.

We need to reach a few export URLs via region domains so we need to not
treat region domains as customer domains. I decided to move the 'is this
subdomain a region' check into types.region with other region methods so
that it could be imported from both the api and web packages.

src/sentry/api/base.py

@@ -4,7 +4,7 @@ import functools
 import logging
 import time
 from datetime import datetime, timedelta, timezone
-from typing import Any, Callable, Iterable, Mapping, Optional, Tuple, Type
+from typing import Any, Callable, Iterable, Mapping, Tuple, Type
 from urllib.parse import quote as urlquote
 
 import sentry_sdk
@@ -30,7 +30,6 @@ from sentry.models.environment import Environment
 from sentry.ratelimits.config import DEFAULT_RATE_LIMIT_CONFIG, RateLimitConfig
 from sentry.silo import SiloLimit, SiloMode
 from sentry.types.ratelimit import RateLimit, RateLimitCategory
-from sentry.types.region import is_region_name
 from sentry.utils import json
 from sentry.utils.audit import create_audit_entry
 from sentry.utils.cursors import Cursor
@@ -581,15 +580,6 @@ class ReleaseAnalyticsMixin:
         )
 
 
-def resolve_region(request: HttpRequest) -> Optional[str]:
-    subdomain = getattr(request, "subdomain", None)
-    if subdomain is None:
-        return None
-    if is_region_name(subdomain):
-        return subdomain
-    return None
-
-
 class EndpointSiloLimit(SiloLimit):
     def modify_endpoint_class(self, decorated_class: Type[Endpoint]) -> type:
         dispatch_override = self.create_override(decorated_class.dispatch)

src/sentry/api/bases/organization.py

@@ -9,7 +9,7 @@ from rest_framework.exceptions import ParseError, PermissionDenied
 from rest_framework.permissions import BasePermission
 from rest_framework.request import Request
 
-from sentry.api.base import Endpoint, resolve_region
+from sentry.api.base import Endpoint
 from sentry.api.exceptions import ResourceDoesNotExist
 from sentry.api.helpers.environments import get_environments
 from sentry.api.permissions import SentryPermission
@@ -31,6 +31,7 @@ from sentry.services.hybrid_cloud.organization import (
     RpcUserOrganizationContext,
     organization_service,
 )
+from sentry.types.region import subdomain_is_region
 from sentry.utils import auth
 from sentry.utils.hashlib import hash_values
 from sentry.utils.numbers import format_grouped_length
@@ -208,7 +209,7 @@ class ControlSiloOrganizationEndpoint(Endpoint):
     def convert_args(
         self, request: Request, organization_slug: str | None = None, *args: Any, **kwargs: Any
     ) -> tuple[tuple[Any, ...], dict[str, Any]]:
-        if resolve_region(request) is None:
+        if not subdomain_is_region(request):
             subdomain = getattr(request, "subdomain", None)
             if subdomain is not None and subdomain != organization_slug:
                 raise ResourceDoesNotExist
@@ -451,7 +452,7 @@ class OrganizationEndpoint(Endpoint):
     def convert_args(
         self, request: Request, organization_slug: str | None = None, *args: Any, **kwargs: Any
     ) -> tuple[tuple[Any, ...], dict[str, Any]]:
-        if resolve_region(request) is None:
+        if not subdomain_is_region(request):
             subdomain = getattr(request, "subdomain", None)
             if subdomain is not None and subdomain != organization_slug:
                 raise ResourceDoesNotExist

src/sentry/middleware/customer_domain.py

@@ -10,9 +10,9 @@ from django.http.request import HttpRequest
 from django.http.response import HttpResponseBase
 from django.urls import resolve, reverse
 
-from sentry.api.base import resolve_region
 from sentry.api.utils import generate_organization_url
 from sentry.services.hybrid_cloud.organization import organization_service
+from sentry.types.region import subdomain_is_region
 from sentry.utils import auth
 from sentry.utils.http import absolute_uri
 
@@ -84,7 +84,7 @@ class CustomerDomainMiddleware:
         ):
             return self.get_response(request)
         subdomain = request.subdomain
-        if subdomain is None or resolve_region(request) is not None:
+        if subdomain is None or subdomain_is_region(request):
             return self.get_response(request)
 
         if (

src/sentry/types/region.py

@@ -6,6 +6,7 @@ from urllib.parse import urljoin
 
 import sentry_sdk
 from django.conf import settings
+from django.http import HttpRequest
 from pydantic.dataclasses import dataclass
 from pydantic.tools import parse_obj_as
 
@@ -218,6 +219,13 @@ def is_region_name(name: str) -> bool:
     return name in load_global_regions().by_name
 
 
+def subdomain_is_region(request: HttpRequest) -> bool:
+    subdomain = getattr(request, "subdomain", None)
+    if subdomain is None:
+        return False
+    return is_region_name(subdomain)
+
+
 @control_silo_function
 def get_region_for_organization(organization_slug: str) -> Region:
     """Resolve an organization to the region where its data is stored."""

src/sentry/web/frontend/base.py

@@ -40,6 +40,7 @@ from sentry.services.hybrid_cloud.organization import (
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.silo import SiloLimit
 from sentry.silo.base import SiloMode
+from sentry.types.region import subdomain_is_region
 from sentry.utils import auth
 from sentry.utils.audit import create_audit_entry
 from sentry.utils.auth import construct_link_with_query, is_valid_redirect
@@ -343,9 +344,8 @@ class BaseView(View, OrganizationMixin):
            check unconditionally again.
 
         """
-
         organization_slug = kwargs.get("organization_slug", None)
-        if request and is_using_customer_domain(request):
+        if request and is_using_customer_domain(request) and not subdomain_is_region(request):
             organization_slug = request.subdomain
         self.determine_active_organization(request, organization_slug)
 

tests/sentry/api/test_base.py

@@ -8,7 +8,7 @@ from rest_framework.response import Response
 from sentry_sdk import Scope
 from sentry_sdk.utils import exc_info_from_error
 
-from sentry.api.base import Endpoint, EndpointSiloLimit, resolve_region
+from sentry.api.base import Endpoint, EndpointSiloLimit
 from sentry.api.paginator import GenericOffsetPaginator
 from sentry.models.apikey import ApiKey
 from sentry.services.hybrid_cloud.util import FunctionSiloLimit
@@ -17,7 +17,7 @@ from sentry.testutils.cases import APITestCase
 from sentry.testutils.helpers.options import override_options
 from sentry.testutils.region import override_region_config
 from sentry.testutils.silo import all_silo_test, assume_test_silo_mode
-from sentry.types.region import RegionCategory, clear_global_regions
+from sentry.types.region import RegionCategory, clear_global_regions, subdomain_is_region
 from sentry.utils.cursors import Cursor
 
 
@@ -507,7 +507,7 @@ class CustomerDomainTest(APITestCase):
         def request_with_subdomain(subdomain):
             request = self.make_request(method="GET")
             request.subdomain = subdomain
-            return resolve_region(request)
+            return subdomain_is_region(request)
 
         region_config = [
             {
@@ -524,9 +524,9 @@ class CustomerDomainTest(APITestCase):
             },
         ]
         with override_region_config(region_config):
-            assert request_with_subdomain("us") == "us"
-            assert request_with_subdomain("eu") == "eu"
-            assert request_with_subdomain("sentry") is None
+            assert request_with_subdomain("us")
+            assert request_with_subdomain("eu")
+            assert not request_with_subdomain("sentry")
 
 
 class EndpointSiloLimitTest(APITestCase):

tests/sentry/types/test_region.py

@@ -3,7 +3,7 @@ from unittest.mock import patch
 import pytest
 from django.conf import settings
 from django.db import router
-from django.test import override_settings
+from django.test import RequestFactory, override_settings
 
 from sentry.models.organizationmapping import OrganizationMapping
 from sentry.services.hybrid_cloud.organization import organization_service
@@ -21,6 +21,7 @@ from sentry.types.region import (
     get_local_region,
     get_region_by_name,
     get_region_for_organization,
+    subdomain_is_region,
 )
 from sentry.utils import json
 
@@ -218,3 +219,17 @@ class RegionMappingTest(TestCase):
         with override_settings(SILO_MODE=SiloMode.CONTROL), override_region_config(region_config):
             result = find_all_multitenant_region_names()
             assert set(result) == {"us"}
+
+    def test_subdomain_is_region(self):
+        regions = [
+            Region("us", 1, "http://us.testserver", RegionCategory.MULTI_TENANT),
+        ]
+        rf = RequestFactory()
+        with override_regions(regions):
+            req = rf.get("/")
+            setattr(req, "subdomain", "us")
+            assert subdomain_is_region(req)
+
+            req = rf.get("/")
+            setattr(req, "subdomain", "acme")
+            assert not subdomain_is_region(req)

tests/sentry/web/frontend/test_group_tag_export.py

@@ -4,7 +4,12 @@ from django.urls import reverse
 
 from sentry.testutils.cases import SnubaTestCase, TestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
+from sentry.testutils.region import override_regions
 from sentry.testutils.silo import region_silo_test
+from sentry.types.region import Region, RegionCategory
+
+region = Region("us", 1, "https://us.testserver", RegionCategory.MULTI_TENANT)
+region_config = (region,)
 
 
 @region_silo_test
@@ -83,3 +88,22 @@ class GroupTagExportTest(TestCase, SnubaTestCase):
             self.url, SERVER_NAME=f"{self.project.organization.slug}.testserver"
         )
         self.verify_test(response)
+
+    def test_region_subdomain_no_conflict_with_slug(self):
+        # When a request to a web view contains both
+        # a region subdomain and org slug, we shouldn't conflate
+        # the subdomain as being an org slug.
+        # We're using this endpoint because it is the only view that
+        # accepts organization_slug at time of writing.
+        url = reverse(
+            "sentry-customer-domain-sentry-group-tag-export",
+            kwargs={
+                "project_slug": self.project.slug,
+                "group_id": self.group.id,
+                "key": self.key,
+            },
+        )
+        with override_regions(region_config):
+            resp = self.client.get(url, HTTP_HOST="us.testserver")
+            assert resp.status_code == 200
+            assert "Location" not in resp

tests/sentry_plugins/slack/test_plugin.py

@@ -40,8 +40,7 @@ class SlackPluginTest(PluginTestCase):
 
         notification = Notification(event=event, rule=rule)
 
-        with self.options({"system.url-prefix": "http://example.com"}):
-            self.plugin.notify(notification)
+        self.plugin.notify(notification)
 
         request = responses.calls[0].request
         payload = json.loads(parse_qs(request.body)["payload"][0])
@@ -56,8 +55,7 @@ class SlackPluginTest(PluginTestCase):
                     ],
                     "fallback": "[bar] Hello world",
                     "title": "Hello world",
-                    "title_link": "http://example.com/organizations/baz/issues/%s/?referrer=slack"
-                    % group.id,
+                    "title_link": group.get_absolute_url(params={"referrer": "slack"}),
                 }
             ],
         }
@@ -78,8 +76,7 @@ class SlackPluginTest(PluginTestCase):
 
         notification = Notification(event=event, rule=rule)
 
-        with self.options({"system.url-prefix": "http://example.com"}):
-            self.plugin.notify(notification)
+        self.plugin.notify(notification)
 
         request = responses.calls[0].request
         payload = json.loads(parse_qs(request.body)["payload"][0])
@@ -91,8 +88,7 @@ class SlackPluginTest(PluginTestCase):
                     "fields": [{"short": True, "value": "bar", "title": "Project"}],
                     "fallback": "[bar] Hello world",
                     "title": "Hello world",
-                    "title_link": "http://example.com/organizations/baz/issues/%s/?referrer=slack"
-                    % group.id,
+                    "title_link": group.get_absolute_url(params={"referrer": "slack"}),
                 }
             ],
         }
@@ -114,8 +110,7 @@ class SlackPluginTest(PluginTestCase):
 
         notification = Notification(event=event, rule=rule)
 
-        with self.options({"system.url-prefix": "http://example.com"}):
-            self.plugin.notify(notification)
+        self.plugin.notify(notification)
 
         request = responses.calls[0].request
         payload = json.loads(parse_qs(request.body)["payload"][0])
@@ -127,8 +122,7 @@ class SlackPluginTest(PluginTestCase):
                     "fields": [{"short": False, "value": "foo.bar", "title": "Culprit"}],
                     "fallback": "[bar] Hello world",
                     "title": "Hello world",
-                    "title_link": "http://example.com/organizations/baz/issues/%s/?referrer=slack"
-                    % group.id,
+                    "title_link": group.get_absolute_url(params={"referrer": "slack"}),
                 }
             ],
         }
@@ -148,15 +142,13 @@ class SlackPluginTest(PluginTestCase):
         notification = Notification(event=event, rule=rule)
 
         # No exception since 404s are supposed to be ignored
-        with self.options({"system.url-prefix": "http://example.com"}):
-            self.plugin.notify(notification)
+        self.plugin.notify(notification)
 
         responses.replace("POST", "http://example.com/slack", status=400)
 
         # Other exceptions should not be ignored
-        with self.options({"system.url-prefix": "http://example.com"}):
-            with pytest.raises(ApiError):
-                self.plugin.notify(notification)
+        with pytest.raises(ApiError):
+            self.plugin.notify(notification)
 
     @responses.activate
     def test_no_error_on_ignorable_slack_errors(self):
@@ -179,6 +171,5 @@ class SlackPluginTest(PluginTestCase):
         responses.replace("POST", "http://example.com/slack", status=403, body="some_other_error")
 
         # Other exceptions should not be ignored
-        with self.options({"system.url-prefix": "http://example.com"}):
-            with pytest.raises(ApiError):
-                self.plugin.notify(notification)
+        with pytest.raises(ApiError):
+            self.plugin.notify(notification)

tests/sentry_plugins/victorops/test_plugin.py

@@ -75,8 +75,7 @@ class VictorOpsPluginTest(PluginTestCase):
 
         notification = Notification(event=event, rule=rule)
 
-        with self.options({"system.url-prefix": "http://example.com"}):
-            self.plugin.notify(notification)
+        self.plugin.notify(notification)
 
         request = responses.calls[0].request
         payload = json.loads(request.body)
@@ -87,7 +86,7 @@ class VictorOpsPluginTest(PluginTestCase):
             "monitoring_tool": "sentry",
             "state_message": 'Stacktrace\n-----------\n\nStacktrace (most recent call last):\n\n  File "sentry/models/foo.py", line 29, in build_msg\n    string_max_length=self.string_max_length)\n\nMessage\n-----------\n\nHello world',
             "timestamp": int(event.datetime.strftime("%s")),
-            "issue_url": "http://example.com/organizations/baz/issues/%s/" % group.id,
+            "issue_url": group.get_absolute_url(),
             "issue_id": group.id,
             "project_id": group.project.id,
         } == payload