Skip to content

Commit 0a6335e

Browse files
authored
chore(cells) Switch ReactView and GenericReactView to control silo v2 (#105880)
Mulligan on #104552 which broke sentry-wizard. We render the bulk of our HTML via ReactPage. Right now that endpoint is marked as all_silo to make integration with tests simpler. Switch these endpoints to control as we're trying to have a minimal number of endpoints marked as all_silo and I want to better understand the scope of test failures from this change. This time around I've limited the excluded views to only subclasses of `ReactPage` as we *do* have HTML views that the UI code uses. Refs INFRENG-238
1 parent 9a86d09 commit 0a6335e

File tree

9 files changed

+59
-30
lines changed

9 files changed

+59
-30
lines changed

src/sentry/management/commands/generate_controlsilo_urls.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from django.urls import URLPattern, URLResolver
1414

1515
from sentry.silo.base import SiloMode
16+
from sentry.web.frontend.react_page import GenericReactPageView, ReactPageView
1617

1718
# There are a handful of catchall routes
1819
# that we don't want to handle in controlsilo
@@ -95,7 +96,7 @@ def handle(self, **options):
9596
func = info.callable
9697
if isinstance(func, functools.partial):
9798
func = func.func
98-
if hasattr(func, "view_class"):
99+
elif hasattr(func, "view_class"):
99100
func = func.view_class
100101

101102
if hasattr(func, "__name__"):
@@ -114,7 +115,21 @@ def handle(self, **options):
114115
except ImportError as err:
115116
raise CommandError(f"Could not load view in {module}: {err}")
116117

117-
if not hasattr(view_func, "silo_limit"):
118+
# If a view/endpoint doesn't have a silo_limit it is likely a basic django view.
119+
# We have tests in tests/sentry/silo/test_base.py that ensure all views/endpoints
120+
# have silo annotations on them. We skip including URLs for react views
121+
# as the UI doesn't make requests to them, but the UI code *does* make
122+
# requests to other HTML views (like sentry-wizard)
123+
if not hasattr(view_func, "silo_limit") or (
124+
isinstance(view_func, type)
125+
and issubclass(
126+
view_func,
127+
(
128+
ReactPageView,
129+
GenericReactPageView,
130+
),
131+
)
132+
):
118133
continue
119134

120135
silo_limit = view_func.silo_limit

src/sentry/testutils/cases.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -719,8 +719,9 @@ def path_2fa(self):
719719
return reverse("sentry-account-settings-security")
720720

721721
def enable_org_2fa(self, organization):
722-
organization.flags.require_2fa = True
723-
organization.save()
722+
with assume_test_silo_mode(SiloMode.REGION):
723+
organization.flags.require_2fa = True
724+
organization.save()
724725

725726
def api_enable_org_2fa(self, organization, user):
726727
self.login_as(user)

src/sentry/web/frontend/react_page.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,7 @@
2323
)
2424
from sentry.utils.http import is_using_customer_domain, query_string
2525
from sentry.web.client_config import get_client_config
26-
from sentry.web.frontend.base import (
27-
BaseView,
28-
ControlSiloOrganizationView,
29-
all_silo_view,
30-
control_silo_view,
31-
)
26+
from sentry.web.frontend.base import BaseView, ControlSiloOrganizationView, control_silo_view
3227
from sentry.web.helpers import render_to_response
3328

3429
logger = logging.getLogger(__name__)
@@ -194,7 +189,7 @@ def handle_react(
194189

195190
# While most of our HTML views are rendered in control silo,
196191
# our tests frequently redirect to HTML views
197-
@all_silo_view
192+
@control_silo_view
198193
class ReactPageView(ControlSiloOrganizationView, ReactMixin):
199194
def handle_auth_required(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
200195
# If user is a superuser (but not active, because otherwise this method would never be called)
@@ -221,7 +216,7 @@ def handle(self, request: HttpRequest, organization, **kwargs) -> HttpResponse:
221216
return self.handle_react(request, organization=organization)
222217

223218

224-
@all_silo_view
219+
@control_silo_view
225220
class GenericReactPageView(BaseView, ReactMixin):
226221
def handle(self, request: HttpRequest, **kwargs) -> HttpResponse:
227222
return self.handle_react(request, **kwargs)

src/sentry/web/urls.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1089,7 +1089,7 @@
10891089
re_path(
10901090
r"^(?P<organization_slug>[^/]+)/projects/(?P<project_id_or_slug>[^/]+)/events/(?P<client_event_id>[^/]+)/$",
10911091
ProjectEventRedirect.as_view(),
1092-
name="sentry-project-event-redirect",
1092+
name="sentry-organization-project-event-redirect",
10931093
),
10941094
re_path(
10951095
r"^(?P<organization_slug>[^/]+)/api-keys/$",

static/app/data/controlsiloUrlPatterns.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ const patterns: RegExp[] = [
160160
new RegExp('^api/0/tempest-ips/$'),
161161
new RegExp('^api/0/secret-scanning/github/$'),
162162
new RegExp('^api/hooks/mailgun/inbound/'),
163-
new RegExp('^auth-v2/'),
164163
new RegExp('^oauth/authorize/$'),
165164
new RegExp('^oauth/token/$'),
166165
new RegExp('^oauth/userinfo/$'),
@@ -194,10 +193,8 @@ const patterns: RegExp[] = [
194193
new RegExp('^auth/sso/account/settings/social/associate/complete/[^/]+/$'),
195194
new RegExp('^out/$'),
196195
new RegExp('^settings/organization/auth/configure/$'),
197-
new RegExp('^disabled-member/'),
198196
new RegExp('^organizations/[^/]+/auth/configure/$'),
199197
new RegExp('^organizations/[^/]+/integrations/[^/]+/setup/$'),
200-
new RegExp('^organizations/[^/]+/disabled-member/$'),
201198
new RegExp('^avatar/[^/]+/$'),
202199
new RegExp('^sentry-app-avatar/[^/]+/$'),
203200
new RegExp('^doc-integration-avatar/[^/]+/$'),
@@ -229,7 +226,6 @@ const patterns: RegExp[] = [
229226
new RegExp('^extensions/discord/configure/$'),
230227
new RegExp('^extensions/discord/link-identity/[^/]+/$'),
231228
new RegExp('^extensions/discord/unlink-identity/[^/]+/$'),
232-
new RegExp('^share/(?:group|issue)/[^/]+/$'),
233229
];
234230

235231
export default patterns;

tests/sentry/core/endpoints/test_organization_index.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@
1818
from sentry.silo.base import SiloMode
1919
from sentry.testutils.cases import APITestCase, TwoFactorAPITestCase
2020
from sentry.testutils.hybrid_cloud import HybridCloudTestMixin
21-
from sentry.testutils.silo import assume_test_silo_mode, create_test_regions, region_silo_test
21+
from sentry.testutils.silo import (
22+
assume_test_silo_mode,
23+
control_silo_test,
24+
create_test_regions,
25+
region_silo_test,
26+
)
2227
from sentry.users.models.authenticator import Authenticator
2328
from sentry.utils.slug import ORG_SLUG_PATTERN
2429

@@ -328,7 +333,7 @@ def test_streamline_only_is_true(self) -> None:
328333

329334

330335
@region_silo_test(regions=create_test_regions("de", "us"))
331-
class OrganizationsCreateInRegionTest(OrganizationIndexTest, HybridCloudTestMixin):
336+
class OrganizationsCreateInRegionTest(OrganizationIndexTest):
332337
method = "post"
333338

334339
@override_settings(SENTRY_MONOLITH_REGION="us", SENTRY_REGION="de")
@@ -348,7 +353,9 @@ def test_success(self) -> None:
348353
assert mapping.region_name == "de"
349354

350355

356+
@control_silo_test
351357
class OrganizationIndex2faTest(TwoFactorAPITestCase):
358+
# This is the HTML view, not an API endpoint
352359
endpoint = "sentry-organization-home"
353360

354361
def setUp(self) -> None:
@@ -365,8 +372,7 @@ def test_preexisting_members_must_enable_2fa(self) -> None:
365372
self.login_as(self.no_2fa_user)
366373
self.assert_redirected_to_2fa()
367374

368-
with assume_test_silo_mode(SiloMode.CONTROL):
369-
TotpInterface().enroll(self.no_2fa_user)
375+
TotpInterface().enroll(self.no_2fa_user)
370376
self.get_success_response(self.org_2fa.slug)
371377

372378
def test_new_member_must_enable_2fa(self) -> None:
@@ -396,7 +402,9 @@ def test_superuser_can_access_org_home(self) -> None:
396402
self.get_success_response(self.org_2fa.slug)
397403

398404

405+
@control_silo_test
399406
class OrganizationIndexMemberLimitTest(APITestCase):
407+
# This is a react view, not an API endpoint
400408
endpoint = "sentry-organization-index"
401409

402410
def setup_user(self, is_superuser=False):
@@ -417,4 +425,5 @@ def test_member_limit_redirect(self) -> None:
417425

418426
def test_member_limit_superuser_no_redirect(self) -> None:
419427
self.setup_user(is_superuser=True)
420-
self.get_success_response(self.organization.slug, status_code=200)
428+
response = self.get_success_response(self.organization.slug, status_code=200)
429+
assert response.headers["Content-Type"] == "text/html"

tests/sentry/management/commands/test_generate_controlsilo_urls.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,14 @@ def test_write_file(self) -> None:
3737
tf.seek(0)
3838
result = tf.read().decode("utf8")
3939
assert "This is generated code" in result
40-
assert "new RegExp('^api/0/users/$')," in result
4140
assert "const patterns" in result
4241
assert "export default patterns;" in result
42+
assert "new RegExp('^api/0/users/$')," in result
43+
# Wizard is an HTML view that gets POST from UI code.
44+
assert "new RegExp('^account/settings/wizard/[^/]+/$')," in result
45+
# Views that render react application (like /dashboards)
46+
# are not be used by UI code, and shouldn't be in the URL list
47+
assert "new RegExp('^dashboards" not in result
4348

4449
def test_no_missing_urls(self) -> None:
4550
pattern_file = "static/app/data/controlsiloUrlPatterns.ts"

tests/sentry/middleware/test_customer_domain.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,15 @@
1212

1313
from sentry.api.base import Endpoint
1414
from sentry.middleware.customer_domain import CustomerDomainMiddleware
15+
from sentry.silo.base import SiloMode
1516
from sentry.testutils.cases import APITestCase, TestCase
1617
from sentry.testutils.helpers import with_feature
17-
from sentry.testutils.silo import all_silo_test, create_test_regions, no_silo_test
18+
from sentry.testutils.silo import (
19+
all_silo_test,
20+
assume_test_silo_mode,
21+
create_test_regions,
22+
no_silo_test,
23+
)
1824
from sentry.web.frontend.auth_logout import AuthLogoutView
1925

2026

@@ -162,11 +168,12 @@ def test_billing_route(self) -> None:
162168
self.login_as(user=non_staff_user)
163169
self.create_organization(name="albertos-apples", owner=non_staff_user)
164170

165-
response = self.client.get(
166-
"/settings/billing/overview/",
167-
data={"querystring": "value"},
168-
follow=True,
169-
)
171+
with assume_test_silo_mode(SiloMode.CONTROL):
172+
response = self.client.get(
173+
"/settings/billing/overview/",
174+
data={"querystring": "value"},
175+
follow=True,
176+
)
170177
assert response.status_code == 200
171178
assert response.redirect_chain == [
172179
("http://albertos-apples.testserver/settings/billing/overview/?querystring=value", 302)

tests/sentry/web/frontend/test_project_event.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ def test_redirect_to_event(self) -> None:
2929
self.assertRedirects(
3030
resp,
3131
f"http://testserver/organizations/{self.org.slug}/issues/{self.event.group_id}/events/{self.event.event_id}/",
32+
fetch_redirect_response=False,
3233
)
3334

3435
def test_event_not_found(self) -> None:
@@ -79,7 +80,7 @@ def test_redirect_to_event_customer_domain(self) -> None:
7980
self.org.refresh_from_db()
8081
resp = self.client.get(
8182
reverse(
82-
"sentry-project-event-redirect",
83+
"sentry-organization-project-event-redirect",
8384
args=[self.org.slug, self.project.slug, self.event.event_id],
8485
)
8586
)

0 commit comments

Comments
 (0)