Skip to content

Commit 3d1b80d

Browse files
authored
feat(preprod): Add new vcs related fields to preprod artifact assemble endpoint (#97332)
Pending FK being added on PreprodArtifact to CommitComparison, this wires up the assemble endpoint to take VCS-related params and create the CommitComparison model. sentry-cli changes to come. Notably, this removes the unused `git_sha` param in favor of the new fields.
1 parent b87fc1a commit 3d1b80d

File tree

4 files changed

+247
-36
lines changed

4 files changed

+247
-36
lines changed

src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,25 @@ def validate_preprod_artifact_schema(request_body: bytes) -> tuple[dict, str | N
3434
"items": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
3535
},
3636
# Optional metadata
37-
"git_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
3837
"build_configuration": {"type": "string"},
38+
# VCS parameters
39+
"head_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
40+
"base_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
41+
"provider": {
42+
"type": "string",
43+
"enum": [
44+
"github",
45+
"github_enterprise",
46+
"gitlab",
47+
"bitbucket",
48+
"bitbucket_server",
49+
],
50+
},
51+
"head_repo_name": {"type": "string", "maxLength": 255},
52+
"base_repo_name": {"type": "string", "maxLength": 255},
53+
"head_ref": {"type": "string", "maxLength": 255},
54+
"base_ref": {"type": "string", "maxLength": 255},
55+
"pr_number": {"type": "integer", "minimum": 1},
3956
},
4057
"required": ["checksum", "chunks"],
4158
"additionalProperties": False,
@@ -44,8 +61,15 @@ def validate_preprod_artifact_schema(request_body: bytes) -> tuple[dict, str | N
4461
error_messages = {
4562
"checksum": "The checksum field is required and must be a 40-character hexadecimal string.",
4663
"chunks": "The chunks field is required and must be provided as an array of 40-character hexadecimal strings.",
47-
"git_sha": "The git_sha field must be a 40-character hexadecimal SHA1 string (no uppercase letters).",
4864
"build_configuration": "The build_configuration field must be a string.",
65+
"head_sha": "The head_sha field must be a 40-character hexadecimal SHA1 string (no uppercase letters).",
66+
"base_sha": "The base_sha field must be a 40-character hexadecimal SHA1 string (no uppercase letters).",
67+
"provider": "The provider field must be a string with one of the following values: github, github_enterprise, gitlab, bitbucket, bitbucket_server.",
68+
"head_repo_name": "The head_repo_name field must be a string with maximum length of 255 characters.",
69+
"base_repo_name": "The base_repo_name field must be a string with maximum length of 255 characters.",
70+
"head_ref": "The head_ref field must be a string with maximum length of 255 characters.",
71+
"base_ref": "The base_ref field must be a string with maximum length of 255 characters.",
72+
"pr_number": "The pr_number field must be a positive integer.",
4973
}
5074

5175
try:
@@ -144,6 +168,16 @@ def post(self, request: Request, project) -> Response:
144168
"checksum": checksum,
145169
"chunks": chunks,
146170
"artifact_id": artifact_id,
171+
"build_configuration": data.get("build_configuration"),
172+
# VCS parameters
173+
"head_sha": data.get("head_sha"),
174+
"base_sha": data.get("base_sha"),
175+
"provider": data.get("provider"),
176+
"head_repo_name": data.get("head_repo_name"),
177+
"base_repo_name": data.get("base_repo_name"),
178+
"head_ref": data.get("head_ref"),
179+
"base_ref": data.get("base_ref"),
180+
"pr_number": data.get("pr_number"),
147181
}
148182
)
149183
if is_org_auth_token_auth(request.auth):

src/sentry/preprod/tasks.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import sentry_sdk
1010
from django.db import router, transaction
1111

12+
from sentry.models.commitcomparison import CommitComparison
1213
from sentry.models.organization import Organization
1314
from sentry.models.project import Project
1415
from sentry.preprod.api.models.project_preprod_size_analysis_models import SizeAnalysisResults
@@ -104,6 +105,14 @@ def assemble_preprod_artifact(
104105
project_id=project_id,
105106
organization_id=org_id,
106107
artifact_id=artifact_id,
108+
head_sha=kwargs.get("head_sha"),
109+
base_sha=kwargs.get("base_sha"),
110+
provider=kwargs.get("provider"),
111+
head_repo_name=kwargs.get("head_repo_name"),
112+
base_repo_name=kwargs.get("base_repo_name"),
113+
head_ref=kwargs.get("head_ref"),
114+
base_ref=kwargs.get("base_ref"),
115+
pr_number=kwargs.get("pr_number"),
107116
)
108117

109118
except Exception as e:
@@ -138,13 +147,52 @@ def create_preprod_artifact(
138147
project_id,
139148
checksum,
140149
build_configuration=None,
150+
head_sha=None,
151+
base_sha=None,
152+
provider=None,
153+
head_repo_name=None,
154+
base_repo_name=None,
155+
head_ref=None,
156+
base_ref=None,
157+
pr_number=None,
141158
) -> str | None:
142159
try:
143160
organization = Organization.objects.get_from_cache(pk=org_id)
144161
project = Project.objects.get(id=project_id, organization=organization)
145162
bind_organization_context(organization)
146163

147164
with transaction.atomic(router.db_for_write(PreprodArtifact)):
165+
# Create CommitComparison if git information is provided
166+
commit_comparison = None
167+
if head_sha and head_repo_name and provider and head_ref:
168+
commit_comparison, _ = CommitComparison.objects.get_or_create(
169+
organization_id=org_id,
170+
head_sha=head_sha,
171+
base_sha=base_sha,
172+
provider=provider,
173+
head_repo_name=head_repo_name,
174+
base_repo_name=base_repo_name,
175+
head_ref=head_ref,
176+
base_ref=base_ref,
177+
pr_number=pr_number,
178+
)
179+
else:
180+
logger.info(
181+
"Skipping commit comparison creation because required vcs information is not provided",
182+
extra={
183+
"project_id": project_id,
184+
"organization_id": org_id,
185+
"head_sha": head_sha,
186+
"head_repo_name": head_repo_name,
187+
"provider": provider,
188+
"head_ref": head_ref,
189+
"base_sha": base_sha,
190+
"base_repo_name": base_repo_name,
191+
"base_ref": base_ref,
192+
"pr_number": pr_number,
193+
},
194+
)
195+
148196
build_config = None
149197
if build_configuration:
150198
build_config, _ = PreprodBuildConfiguration.objects.get_or_create(
@@ -156,6 +204,7 @@ def create_preprod_artifact(
156204
project=project,
157205
build_configuration=build_config,
158206
state=PreprodArtifact.ArtifactState.UPLOADING,
207+
commit_comparison=commit_comparison,
159208
)
160209

161210
logger.info(

tests/sentry/preprod/api/endpoints/test_organization_preprod_artifact_assemble.py

Lines changed: 94 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,35 @@ def test_valid_full_schema(self) -> None:
3838
data = {
3939
"checksum": "a" * 40,
4040
"chunks": ["b" * 40, "c" * 40],
41-
"git_sha": "d" * 40,
4241
"build_configuration": "release",
42+
"head_sha": "e" * 40,
43+
"base_sha": "f" * 40,
44+
"provider": "github",
45+
"head_repo_name": "owner/repo",
46+
"base_repo_name": "owner/repo",
47+
"head_ref": "feature/xyz",
48+
"base_ref": "main",
49+
"pr_number": 123,
50+
}
51+
body = orjson.dumps(data)
52+
result, error = validate_preprod_artifact_schema(body)
53+
assert error is None
54+
assert result == data
55+
56+
def test_valid_schema_with_commit_comparison(self) -> None:
57+
"""Test valid schema with CommitComparison fields passes validation."""
58+
data = {
59+
"checksum": "a" * 40,
60+
"chunks": ["b" * 40, "c" * 40],
61+
"build_configuration": "release",
62+
"head_sha": "e" * 40,
63+
"base_sha": "f" * 40,
64+
"provider": "github",
65+
"head_repo_name": "owner/repo",
66+
"base_repo_name": "owner/repo",
67+
"head_ref": "feature/xyz",
68+
"base_ref": "main",
69+
"pr_number": 123,
4370
}
4471
body = orjson.dumps(data)
4572
result, error = validate_preprod_artifact_schema(body)
@@ -105,24 +132,46 @@ def test_chunks_invalid_item_type(self) -> None:
105132
assert error is not None
106133
assert result == {}
107134

108-
def test_git_sha_wrong_type(self) -> None:
109-
"""Test non-string git_sha returns error."""
110-
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "git_sha": 123})
135+
def test_build_configuration_wrong_type(self) -> None:
136+
"""Test non-string build_configuration returns error."""
137+
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "build_configuration": 123})
138+
result, error = validate_preprod_artifact_schema(body)
139+
assert error is not None
140+
assert result == {}
141+
142+
def test_head_sha_invalid_format(self) -> None:
143+
"""Test invalid head_sha format returns error."""
144+
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "head_sha": "invalid"})
111145
result, error = validate_preprod_artifact_schema(body)
112146
assert error is not None
147+
assert "head_sha" in error
113148
assert result == {}
114149

115-
def test_git_sha_invalid_format(self) -> None:
116-
"""Test invalid git_sha format returns error."""
117-
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "git_sha": "invalid"})
150+
def test_base_sha_invalid_format(self) -> None:
151+
"""Test invalid base_sha format returns error."""
152+
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "base_sha": "invalid"})
118153
result, error = validate_preprod_artifact_schema(body)
119154
assert error is not None
120-
assert "git_sha" in error
155+
assert "base_sha" in error
121156
assert result == {}
122157

123-
def test_build_configuration_wrong_type(self) -> None:
124-
"""Test non-string build_configuration returns error."""
125-
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "build_configuration": 123})
158+
def test_provider_too_long(self) -> None:
159+
"""Test provider field too long returns error."""
160+
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "provider": "a" * 65})
161+
result, error = validate_preprod_artifact_schema(body)
162+
assert error is not None
163+
assert result == {}
164+
165+
def test_head_repo_name_too_long(self) -> None:
166+
"""Test head_repo_name field too long returns error."""
167+
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "head_repo_name": "a" * 256})
168+
result, error = validate_preprod_artifact_schema(body)
169+
assert error is not None
170+
assert result == {}
171+
172+
def test_pr_number_invalid(self) -> None:
173+
"""Test invalid pr_number returns error."""
174+
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "pr_number": 0})
126175
result, error = validate_preprod_artifact_schema(body)
127176
assert error is not None
128177
assert result == {}
@@ -255,26 +304,6 @@ def test_assemble_json_schema_chunks_invalid_item_type(self) -> None:
255304
)
256305
assert response.status_code == 400, response.content
257306

258-
def test_assemble_json_schema_git_sha_wrong_type(self) -> None:
259-
"""Test that non-string git_sha is rejected."""
260-
checksum = sha1(b"1").hexdigest()
261-
response = self.client.post(
262-
self.url,
263-
data={"checksum": checksum, "chunks": [], "git_sha": 123},
264-
HTTP_AUTHORIZATION=f"Bearer {self.token.token}",
265-
)
266-
assert response.status_code == 400, response.content
267-
268-
def test_assemble_json_schema_git_sha_invalid_format(self) -> None:
269-
"""Test that invalid git_sha format is rejected."""
270-
checksum = sha1(b"1").hexdigest()
271-
response = self.client.post(
272-
self.url,
273-
data={"checksum": checksum, "chunks": [], "git_sha": "invalid_sha"},
274-
HTTP_AUTHORIZATION=f"Bearer {self.token.token}",
275-
)
276-
assert response.status_code == 400, response.content
277-
278307
def test_assemble_json_schema_build_configuration_wrong_type(self) -> None:
279308
"""Test that non-string build_configuration is rejected."""
280309
checksum = sha1(b"1").hexdigest()
@@ -304,8 +333,15 @@ def test_assemble_json_schema_optional_fields(self) -> None:
304333
data={
305334
"checksum": checksum,
306335
"chunks": [],
307-
"git_sha": "c076e3b84d9d7c43f456908535ea78b9de6ec59b",
308336
"build_configuration": "release",
337+
"head_sha": "e" * 40,
338+
"base_sha": "f" * 40,
339+
"provider": "github",
340+
"head_repo_name": "owner/repo",
341+
"base_repo_name": "owner/repo",
342+
"head_ref": "feature/xyz",
343+
"base_ref": "main",
344+
"pr_number": 123,
309345
},
310346
HTTP_AUTHORIZATION=f"Bearer {self.token.token}",
311347
)
@@ -356,6 +392,15 @@ def test_assemble_basic(
356392
"checksum": total_checksum,
357393
"chunks": [blob.checksum],
358394
"artifact_id": artifact_id,
395+
"build_configuration": None,
396+
"head_sha": None,
397+
"base_sha": None,
398+
"provider": None,
399+
"head_repo_name": None,
400+
"base_repo_name": None,
401+
"head_ref": None,
402+
"base_ref": None,
403+
"pr_number": None,
359404
}
360405
)
361406

@@ -382,8 +427,15 @@ def test_assemble_with_metadata(
382427
data={
383428
"checksum": total_checksum,
384429
"chunks": [blob.checksum],
385-
"git_sha": "c076e3b84d9d7c43f456908535ea78b9de6ec59b",
386430
"build_configuration": "release",
431+
"head_sha": "e" * 40,
432+
"base_sha": "f" * 40,
433+
"provider": "github",
434+
"head_repo_name": "owner/repo",
435+
"base_repo_name": "owner/repo",
436+
"head_ref": "feature/xyz",
437+
"base_ref": "main",
438+
"pr_number": 123,
387439
},
388440
HTTP_AUTHORIZATION=f"Bearer {self.token.token}",
389441
)
@@ -406,6 +458,15 @@ def test_assemble_with_metadata(
406458
"checksum": total_checksum,
407459
"chunks": [blob.checksum],
408460
"artifact_id": artifact_id,
461+
"build_configuration": "release",
462+
"head_sha": "e" * 40,
463+
"base_sha": "f" * 40,
464+
"provider": "github",
465+
"head_repo_name": "owner/repo",
466+
"base_repo_name": "owner/repo",
467+
"head_ref": "feature/xyz",
468+
"base_ref": "main",
469+
"pr_number": 123,
409470
}
410471
)
411472

0 commit comments

Comments
 (0)