Skip to content

Commit 79b78e8

Browse files
NicoHinderlingNicoHinderling
authored
feat(preprod): Make snapshots endpoint org scoped (#109575)
<!-- Describe your PR here. --> <!-- Sentry employees and contractors can delete or ignore the following. --> ### Legal Boilerplate Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
1 parent 460d142 commit 79b78e8

File tree

8 files changed

+59
-51
lines changed

8 files changed

+59
-51
lines changed

‎src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py‎

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
from sentry.api.api_owners import ApiOwner
1515
from sentry.api.api_publish_status import ApiPublishStatus
1616
from sentry.api.base import region_silo_endpoint
17+
from sentry.api.bases.organization import OrganizationEndpoint, OrganizationReleasePermission
1718
from sentry.api.bases.project import ProjectEndpoint, ProjectReleasePermission
1819
from sentry.api.paginator import OffsetPaginator
1920
from sentry.models.commitcomparison import CommitComparison
21+
from sentry.models.organization import Organization
2022
from sentry.models.project import Project
2123
from sentry.objectstore import get_preprod_session
2224
from sentry.preprod.analytics import PreprodArtifactApiGetSnapshotDetailsEvent
@@ -79,35 +81,22 @@ def validate_preprod_snapshot_post_schema(request_body: bytes) -> tuple[dict[str
7981

8082

8183
@region_silo_endpoint
82-
class ProjectPreprodSnapshotEndpoint(ProjectEndpoint):
84+
class OrganizationPreprodSnapshotEndpoint(OrganizationEndpoint):
8385
owner = ApiOwner.EMERGE_TOOLS
8486
publish_status = {
8587
"GET": ApiPublishStatus.EXPERIMENTAL,
86-
"POST": ApiPublishStatus.EXPERIMENTAL,
8788
}
88-
permission_classes = (ProjectReleasePermission,)
89-
90-
rate_limits = RateLimitConfig(
91-
limit_overrides={
92-
"POST": {
93-
RateLimitCategory.ORGANIZATION: RateLimit(limit=100, window=60),
94-
}
95-
}
96-
)
97-
98-
def get(self, request: Request, project: Project, snapshot_id: str) -> Response:
99-
"""
100-
Retrieves snapshot data including manifest images and VCS info.
101-
"""
89+
permission_classes = (OrganizationReleasePermission,)
10290

91+
def get(self, request: Request, organization: Organization, snapshot_id: str) -> Response:
10392
if not settings.IS_DEV and not features.has(
104-
"organizations:preprod-snapshots", project.organization, actor=request.user
93+
"organizations:preprod-snapshots", organization, actor=request.user
10594
):
10695
return Response({"detail": "Feature not enabled"}, status=403)
10796

10897
try:
10998
artifact = PreprodArtifact.objects.select_related("commit_comparison").get(
110-
id=snapshot_id, project_id=project.id
99+
id=snapshot_id, project__organization_id=organization.id
111100
)
112101
except PreprodArtifact.DoesNotExist:
113102
return Response({"detail": "Snapshot not found"}, status=404)
@@ -122,7 +111,7 @@ def get(self, request: Request, project: Project, snapshot_id: str) -> Response:
122111
return Response({"detail": "Manifest key not found"}, status=404)
123112

124113
try:
125-
session = get_preprod_session(project.organization_id, project.id)
114+
session = get_preprod_session(organization.id, artifact.project_id)
126115
get_response = session.get(manifest_key)
127116
manifest_data = orjson.loads(get_response.payload.read())
128117
manifest = SnapshotManifest(**manifest_data)
@@ -196,8 +185,8 @@ def get(self, request: Request, project: Project, snapshot_id: str) -> Response:
196185

197186
analytics.record(
198187
PreprodArtifactApiGetSnapshotDetailsEvent(
199-
organization_id=project.organization_id,
200-
project_id=project.id,
188+
organization_id=organization.id,
189+
project_id=artifact.project_id,
201190
user_id=request.user.id if request.user and request.user.is_authenticated else None,
202191
artifact_id=str(artifact.id),
203192
)
@@ -269,8 +258,8 @@ def on_results(images: list[SnapshotImageResponse]) -> dict[str, Any]:
269258
)
270259

271260
result = response.dict()
272-
result["org_id"] = str(project.organization_id)
273-
result["project_id"] = str(project.id)
261+
result["org_id"] = str(organization.id)
262+
result["project_id"] = str(artifact.project_id)
274263
result["comparison_type"] = comparison_type
275264
if comparison_state is not None:
276265
result["comparison_state"] = comparison_state
@@ -286,6 +275,23 @@ def on_results(images: list[SnapshotImageResponse]) -> dict[str, Any]:
286275
max_per_page=100,
287276
)
288277

278+
279+
@region_silo_endpoint
280+
class ProjectPreprodSnapshotEndpoint(ProjectEndpoint):
281+
owner = ApiOwner.EMERGE_TOOLS
282+
publish_status = {
283+
"POST": ApiPublishStatus.EXPERIMENTAL,
284+
}
285+
permission_classes = (ProjectReleasePermission,)
286+
287+
rate_limits = RateLimitConfig(
288+
limit_overrides={
289+
"POST": {
290+
RateLimitCategory.ORGANIZATION: RateLimit(limit=100, window=60),
291+
}
292+
}
293+
)
294+
289295
def post(self, request: Request, project: Project) -> Response:
290296
if not settings.IS_DEV and not features.has(
291297
"organizations:preprod-snapshots", project.organization, actor=request.user

‎src/sentry/preprod/api/endpoints/urls.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@
2727
PreprodArtifactRerunAnalysisEndpoint,
2828
)
2929
from .preprod_artifact_rerun_status_checks import PreprodArtifactRerunStatusChecksEndpoint
30-
from .preprod_artifact_snapshot import ProjectPreprodSnapshotEndpoint
30+
from .preprod_artifact_snapshot import (
31+
OrganizationPreprodSnapshotEndpoint,
32+
ProjectPreprodSnapshotEndpoint,
33+
)
3134
from .preprod_snapshot_recompare import PreprodSnapshotRecompareEndpoint
3235
from .project_installable_preprod_artifact_download import (
3336
ProjectInstallablePreprodArtifactDownloadEndpoint,
@@ -76,11 +79,6 @@
7679
ProjectPreprodUploadOptionsEndpoint.as_view(),
7780
name="sentry-api-0-project-preprod-snapshots-upload-options",
7881
),
79-
re_path(
80-
r"^(?P<organization_id_or_slug>[^/]+)/(?P<project_id_or_slug>[^/]+)/preprodartifacts/snapshots/(?P<snapshot_id>[^/]+)/$",
81-
ProjectPreprodSnapshotEndpoint.as_view(),
82-
name="sentry-api-0-project-preprod-snapshots-detail",
83-
),
8482
re_path(
8583
r"^(?P<organization_id_or_slug>[^/]+)/(?P<project_id_or_slug>[^/]+)/preprodartifacts/check-for-updates/$",
8684
ProjectPreprodArtifactCheckForUpdatesEndpoint.as_view(),
@@ -188,6 +186,11 @@
188186
name="sentry-api-0-organization-preprod-artifact-public-size-analysis",
189187
),
190188
# Snapshots
189+
re_path(
190+
r"^(?P<organization_id_or_slug>[^/]+)/preprodartifacts/snapshots/(?P<snapshot_id>[^/]+)/$",
191+
OrganizationPreprodSnapshotEndpoint.as_view(),
192+
name="sentry-api-0-project-preprod-snapshots-detail",
193+
),
191194
re_path(
192195
r"^(?P<organization_id_or_slug>[^/]+)/preprodartifacts/snapshots/(?P<snapshot_id>[^/]+)/recompare/$",
193196
PreprodSnapshotRecompareEndpoint.as_view(),

‎static/app/router/routes.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2429,7 +2429,7 @@ function buildRoutes(): RouteObject[] {
24292429
],
24302430
},
24312431
{
2432-
path: ':projectSlug/snapshots/:snapshotId/',
2432+
path: 'snapshots/:snapshotId/',
24332433
component: make(() => import('sentry/views/preprod/snapshots/snapshots')),
24342434
},
24352435
// TODO(EME-735): Remove old routes after backend deployment

‎static/app/utils/api/knownSentryApiUrls.generated.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,7 @@ export type KnownSentryApiUrls =
474474
| '/organizations/$organizationIdOrSlug/preprodartifacts/$artifactId/install-details/'
475475
| '/organizations/$organizationIdOrSlug/preprodartifacts/$artifactId/size-analysis/'
476476
| '/organizations/$organizationIdOrSlug/preprodartifacts/list-builds/'
477+
| '/organizations/$organizationIdOrSlug/preprodartifacts/snapshots/$snapshotId/'
477478
| '/organizations/$organizationIdOrSlug/preprodartifacts/snapshots/$snapshotId/recompare/'
478479
| '/organizations/$organizationIdOrSlug/prevent/owner/$owner/repositories/'
479480
| '/organizations/$organizationIdOrSlug/prevent/owner/$owner/repositories/sync/'
@@ -673,7 +674,6 @@ export type KnownSentryApiUrls =
673674
| '/projects/$organizationIdOrSlug/$projectIdOrSlug/preprodartifacts/size-analysis/compare/$headArtifactId/$baseArtifactId/'
674675
| '/projects/$organizationIdOrSlug/$projectIdOrSlug/preprodartifacts/size-analysis/compare/$headSizeMetricId/$baseSizeMetricId/download/'
675676
| '/projects/$organizationIdOrSlug/$projectIdOrSlug/preprodartifacts/snapshots/'
676-
| '/projects/$organizationIdOrSlug/$projectIdOrSlug/preprodartifacts/snapshots/$snapshotId/'
677677
| '/projects/$organizationIdOrSlug/$projectIdOrSlug/preprodartifacts/snapshots/upload-options/'
678678
| '/projects/$organizationIdOrSlug/$projectIdOrSlug/processing-errors/'
679679
| '/projects/$organizationIdOrSlug/$projectIdOrSlug/processing-errors/$uuid/'

‎static/app/views/preprod/snapshots/main/snapshotMainContent.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ interface SnapshotMainContentProps {
1414
currentGroupName: string | null;
1515
onVariantChange: (index: number) => void;
1616
organizationSlug: string;
17-
projectSlug: string;
17+
projectId: string;
1818
variantIndex: number;
1919
}
2020

@@ -24,7 +24,7 @@ export function SnapshotMainContent({
2424
variantIndex,
2525
onVariantChange,
2626
organizationSlug,
27-
projectSlug,
27+
projectId,
2828
}: SnapshotMainContentProps) {
2929
const selectedImage = currentGroupImages[variantIndex];
3030
if (!currentGroupName || !selectedImage) {
@@ -35,7 +35,7 @@ export function SnapshotMainContent({
3535
);
3636
}
3737

38-
const imageUrl = `/api/0/projects/${organizationSlug}/${projectSlug}/files/images/${selectedImage.key}/`;
38+
const imageUrl = `/api/0/projects/${organizationSlug}/${projectId}/files/images/${selectedImage.key}/`;
3939
const totalVariants = currentGroupImages.length;
4040
const displayName = selectedImage.display_name ?? selectedImage.image_file_name;
4141

‎static/app/views/preprod/snapshots/snapshots.tsx‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@ import {SnapshotSidebarContent} from './sidebar/snapshotSidebarContent';
2323

2424
export default function SnapshotsPage() {
2525
const organization = useOrganization();
26-
const {projectId, projectSlug, snapshotId} = useParams<{
27-
projectId: string;
28-
projectSlug: string;
26+
const {snapshotId} = useParams<{
2927
snapshotId: string;
3028
}>();
3129

@@ -34,19 +32,18 @@ export default function SnapshotsPage() {
3432
queryKey: [
3533
'infinite',
3634
getApiUrl(
37-
'/projects/$organizationIdOrSlug/$projectIdOrSlug/preprodartifacts/snapshots/$snapshotId/',
35+
'/organizations/$organizationIdOrSlug/preprodartifacts/snapshots/$snapshotId/',
3836
{
3937
path: {
4038
organizationIdOrSlug: organization.slug,
41-
projectIdOrSlug: projectSlug,
4239
snapshotId,
4340
},
4441
}
4542
),
4643
{query: {per_page: 20}},
4744
],
4845
staleTime: 0,
49-
enabled: !!projectSlug && !!snapshotId,
46+
enabled: !!snapshotId,
5047
});
5148

5249
const [searchQuery, setSearchQuery] = useState('');
@@ -129,7 +126,10 @@ export default function SnapshotsPage() {
129126
<SentryDocumentTitle title={t('Snapshot')}>
130127
<Layout.Page>
131128
<Layout.Header>
132-
<SnapshotHeaderContent projectId={projectId} data={firstPageData} />
129+
<SnapshotHeaderContent
130+
projectId={firstPageData.project_id}
131+
data={firstPageData}
132+
/>
133133
</Layout.Header>
134134
<Flex direction="row" gap="0" height="100%" width="100%">
135135
<SnapshotSidebarContent
@@ -149,7 +149,7 @@ export default function SnapshotsPage() {
149149
variantIndex={variantIndex}
150150
onVariantChange={setVariantIndex}
151151
organizationSlug={organization.slug}
152-
projectSlug={projectSlug}
152+
projectId={firstPageData.project_id}
153153
/>
154154
</Flex>
155155
</Layout.Page>

‎static/app/views/preprod/types/snapshotTypes.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export interface SnapshotDetailsApiResponse {
2121
head_artifact_id: string;
2222
image_count: number;
2323
images: SnapshotImage[];
24+
project_id: string;
2425
state: string;
2526
vcs_info: BuildDetailsVcsInfo;
2627

‎tests/sentry/preprod/api/endpoints/test_preprod_artifact_snapshot.py‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,9 @@ def _get_create_url(self):
2323
)
2424

2525
def _get_detail_url(self, snapshot_id):
26-
"""URL for GET (retrieving snapshots)"""
2726
return reverse(
2827
"sentry-api-0-project-preprod-snapshots-detail",
29-
args=[self.org.slug, self.project.slug, snapshot_id],
28+
args=[self.org.slug, snapshot_id],
3029
)
3130

3231
def test_successful_snapshot_upload(self):
@@ -141,9 +140,7 @@ def test_snapshot_with_empty_images(self):
141140

142141
def test_snapshot_missing_required_field(self):
143142
url = self._get_create_url()
144-
data: dict[str, str] = {
145-
# Missing images field
146-
}
143+
data: dict[str, str] = {}
147144

148145
with self.feature("organizations:preprod-snapshots"):
149146
response = self.client.post(url, data, format="json")
@@ -264,7 +261,7 @@ def setUp(self):
264261
def _get_detail_url(self, snapshot_id):
265262
return reverse(
266263
"sentry-api-0-project-preprod-snapshots-detail",
267-
args=[self.org.slug, self.project.slug, snapshot_id],
264+
args=[self.org.slug, snapshot_id],
268265
)
269266

270267
def _create_artifact_with_manifest(self, images=None, commit_comparison=None):
@@ -400,9 +397,10 @@ def test_get_snapshot_not_found(self):
400397
assert response.status_code == 404
401398
assert response.data["detail"] == "Snapshot not found"
402399

403-
def test_get_snapshot_wrong_project(self):
404-
"""Artifact belonging to a different project should return 404 (IDOR protection)."""
405-
other_project = self.create_project(organization=self.org)
400+
def test_get_snapshot_wrong_organization(self):
401+
"""Artifact belonging to a different organization should return 404 (IDOR protection)."""
402+
other_org = self.create_organization()
403+
other_project = self.create_project(organization=other_org)
406404
artifact = PreprodArtifact.objects.create(
407405
project=other_project,
408406
state=PreprodArtifact.ArtifactState.UPLOADED,

0 commit comments

Comments
 (0)