Skip to content

Commit 951b588

Browse files
authored
feat(hybridcloud) Fix silo issues in shared issue HTML view (#57978)
Add a small RPC service for handling the group share view HTML response. We need to embed a small amount of group data into the HTML for embed data. While in saas/multiregion setups we can use the subdomain to locate the region and get the group, but in self-hosted we cannot so we need to fallback to the monolith_region. Refs HC-878
1 parent 09f5468 commit 951b588

File tree

9 files changed

+183
-8
lines changed

9 files changed

+183
-8
lines changed

src/sentry/models/group.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ def get_recommended_event_for_environments(
294294
return None
295295

296296

297-
class GroupManager(BaseManager):
297+
class GroupManager(BaseManager["Group"]):
298298
use_for_related_fields = True
299299

300300
def by_qualified_short_id(self, organization_id: int, short_id: str):
@@ -541,7 +541,7 @@ class Group(Model):
541541
short_id = BoundedBigIntegerField(null=True)
542542
type = BoundedPositiveIntegerField(default=ErrorGroupType.type_id, db_index=True)
543543

544-
objects = GroupManager(cache_fields=("id",))
544+
objects: GroupManager = GroupManager(cache_fields=("id",))
545545

546546
class Meta:
547547
app_label = "sentry"

src/sentry/services/hybrid_cloud/issue/__init__.py

Whitespace-only changes.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Please do not use
2+
# from __future__ import annotations
3+
# in modules such as this one where hybrid cloud data models or service classes are
4+
# defined, because we want to reflect on type annotations and avoid forward references.
5+
6+
7+
from typing import Optional
8+
9+
from sentry.models.group import Group
10+
from sentry.models.organization import Organization
11+
from sentry.services.hybrid_cloud.issue.model import RpcGroupShareMetadata
12+
from sentry.services.hybrid_cloud.issue.service import IssueService
13+
14+
15+
class DatabaseBackedIssueService(IssueService):
16+
def get_shared_for_org(self, *, slug: str, share_id: str) -> Optional[RpcGroupShareMetadata]:
17+
try:
18+
organization = Organization.objects.get(slug=slug)
19+
except Organization.DoesNotExist:
20+
return None
21+
if organization.flags.disable_shared_issues:
22+
return None
23+
try:
24+
group = Group.objects.from_share_id(share_id)
25+
except Group.DoesNotExist:
26+
return None
27+
28+
return RpcGroupShareMetadata(title=group.title, message=group.message)
29+
30+
def get_shared_for_region(
31+
self, *, region_name: str, share_id: str
32+
) -> Optional[RpcGroupShareMetadata]:
33+
try:
34+
group = Group.objects.from_share_id(share_id)
35+
except Group.DoesNotExist:
36+
return None
37+
if group.organization.flags.disable_shared_issues:
38+
return None
39+
40+
return RpcGroupShareMetadata(title=group.title, message=group.message)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
from sentry.services.hybrid_cloud import RpcModel
2+
3+
4+
class RpcGroupShareMetadata(RpcModel):
5+
title: str
6+
message: str
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# Please do not use
2+
# from __future__ import annotations
3+
# in modules such as this one where hybrid cloud data models or service classes are
4+
# defined, because we want to reflect on type annotations and avoid forward references.
5+
6+
7+
from abc import abstractmethod
8+
from typing import Optional, cast
9+
10+
from sentry.services.hybrid_cloud.issue.model import RpcGroupShareMetadata
11+
from sentry.services.hybrid_cloud.region import ByOrganizationSlug, ByRegionName
12+
from sentry.services.hybrid_cloud.rpc import RpcService, regional_rpc_method
13+
from sentry.silo.base import SiloMode
14+
15+
16+
class IssueService(RpcService):
17+
"""
18+
Avoid the temptation to expand this service.
19+
20+
We want as little access to issues and events in control as possible.
21+
22+
Unfortunately the issue public share link view requires it as
23+
we need issue data to render the initial HTML so that open-graph
24+
data can be included.
25+
"""
26+
27+
key = "issue"
28+
local_mode = SiloMode.REGION
29+
30+
@classmethod
31+
def get_local_implementation(cls) -> RpcService:
32+
from sentry.services.hybrid_cloud.issue.impl import DatabaseBackedIssueService
33+
34+
return DatabaseBackedIssueService()
35+
36+
@regional_rpc_method(resolve=ByOrganizationSlug(), return_none_if_mapping_not_found=True)
37+
@abstractmethod
38+
def get_shared_for_org(self, *, slug: str, share_id: str) -> Optional[RpcGroupShareMetadata]:
39+
pass
40+
41+
@regional_rpc_method(resolve=ByRegionName(), return_none_if_mapping_not_found=True)
42+
@abstractmethod
43+
def get_shared_for_region(
44+
self, *, region_name: str, share_id: str
45+
) -> Optional[RpcGroupShareMetadata]:
46+
pass
47+
48+
49+
issue_service = cast(IssueService, IssueService.create_delegation())

src/sentry/web/frontend/shared_group_details.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,24 @@
1+
from django.conf import settings
12
from rest_framework.request import Request
23

3-
from sentry.models.group import Group
4+
from sentry.services.hybrid_cloud.issue.service import issue_service
5+
from sentry.web.frontend.base import control_silo_view
46
from sentry.web.frontend.react_page import GenericReactPageView
57

68

9+
@control_silo_view
710
class SharedGroupDetailsView(GenericReactPageView):
811
def meta_tags(self, request: Request, share_id):
9-
try:
10-
group = Group.objects.from_share_id(share_id)
11-
except Group.DoesNotExist:
12-
return {}
13-
if group.organization.flags.disable_shared_issues:
12+
org_slug = getattr(request, "subdomain", None)
13+
if org_slug:
14+
group = issue_service.get_shared_for_org(slug=org_slug, share_id=share_id)
15+
else:
16+
# Backwards compatibility for Self-hosted and single tenants
17+
group = issue_service.get_shared_for_region(
18+
region_name=settings.SENTRY_MONOLITH_REGION, share_id=share_id
19+
)
20+
21+
if not group:
1422
return {}
1523
return {
1624
"og:type": "website",

static/app/data/controlsiloUrlPatterns.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ const patterns: RegExp[] = [
2828
new RegExp('^api/0/sponsored_education_account/$'),
2929
new RegExp('^api/0/organizations/[^/]+/broadcasts/$'),
3030
new RegExp('^api/0/auth-details/$'),
31+
new RegExp('^api/0/_admin/instance-level-oauth/$'),
32+
new RegExp('^api/0/_admin/instance-level-oauth/[^/]+/$'),
3133
new RegExp('^_admin/'),
3234
new RegExp('^api/0/organizations/[^/]+/api-keys/$'),
3335
new RegExp('^api/0/organizations/[^/]+/api-keys/[^/]+/$'),
@@ -149,6 +151,7 @@ const patterns: RegExp[] = [
149151
new RegExp('^extensions/msteams/unlink-identity/[^/]+/$'),
150152
new RegExp('^extensions/discord/link-identity/[^/]+/$'),
151153
new RegExp('^extensions/discord/unlink-identity/[^/]+/$'),
154+
new RegExp('^share/(?:group|issue)/[^/]+/$'),
152155
];
153156

154157
export default patterns;

tests/sentry/event_manager/test_event_manager.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,7 @@ def test_unresolves_group_with_auto_resolve(self, mock_is_resolved):
914914
assert event.group_id == event2.group_id
915915

916916
group = Group.objects.get(id=event.group.id)
917+
assert group.active_at
917918
assert group.active_at.replace(second=0) == event2.datetime.replace(second=0)
918919
assert group.active_at.replace(second=0) != event.datetime.replace(second=0)
919920

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
from typing import Any
2+
3+
from sentry.models.groupshare import GroupShare
4+
from sentry.silo.base import SiloMode
5+
from sentry.testutils.cases import TestCase
6+
from sentry.testutils.silo import assume_test_silo_mode, control_silo_test
7+
8+
9+
@control_silo_test(stable=True)
10+
class SharedGroupDetailsTest(TestCase):
11+
def setUp(self):
12+
self.group = self.create_group(project=self.project)
13+
self.org_domain = f"{self.organization.slug}.testserver"
14+
15+
def share_group(self):
16+
with assume_test_silo_mode(SiloMode.REGION):
17+
return GroupShare.objects.create(
18+
project=self.project, group=self.group, user_id=self.user.id
19+
)
20+
21+
def assert_group_metadata_present(self, response: Any):
22+
response_body = response.content.decode("utf8")
23+
assert f'<meta property="og:title" content="{self.group.title}"' in response_body
24+
assert f'<meta property="og:description" content="{self.group.message}"' in response_body
25+
assert '<meta property="twitter:card" content="summary"' in response_body
26+
assert f'<meta property="twitter:title" content="{self.group.title}"' in response_body
27+
assert (
28+
f'<meta property="twitter:description" content="{self.group.message}"' in response_body
29+
)
30+
31+
def assert_group_metadata_absent(self, response: Any):
32+
response_body = response.content.decode("utf8")
33+
assert f'<meta property="og:title" content="{self.group.title}"' not in response_body
34+
assert (
35+
f'<meta property="og:description" content="{self.group.message}"' not in response_body
36+
)
37+
assert '<meta property="twitter:card" content="summary"' not in response_body
38+
assert f'<meta property="twitter:title" content="{self.group.title}"' not in response_body
39+
assert (
40+
f'<meta property="twitter:description" content="{self.group.message}"'
41+
not in response_body
42+
)
43+
44+
def test_get_not_found(self):
45+
response = self.client.get("/share/issue/lolnope/", HTTP_HOST=self.org_domain)
46+
assert response.status_code == 200
47+
self.assert_group_metadata_absent(response)
48+
49+
def test_get_org_disable_sharing(self):
50+
share = self.share_group()
51+
with assume_test_silo_mode(SiloMode.REGION):
52+
self.organization.flags.disable_shared_issues = True
53+
self.organization.save()
54+
response = self.client.get(f"/share/issue/{share.uuid}/", HTTP_HOST=self.org_domain)
55+
assert response.status_code == 200
56+
self.assert_group_metadata_absent(response)
57+
58+
def test_get_no_subdomain(self):
59+
share = self.share_group()
60+
response = self.client.get(f"/share/issue/{share.uuid}/")
61+
assert response.status_code == 200
62+
self.assert_group_metadata_present(response)
63+
64+
def test_get_success(self):
65+
share = self.share_group()
66+
response = self.client.get(f"/share/issue/{share.uuid}/", HTTP_HOST=self.org_domain)
67+
assert response.status_code == 200
68+
self.assert_group_metadata_present(response)

0 commit comments

Comments
 (0)