Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def validate_preprod_artifact_schema(request_body: bytes) -> tuple[dict[str, Any
# Optional metadata
"build_configuration": {"type": "string"},
"release_notes": {"type": "string"},
"date_built": {"type": "string"},
# VCS parameters - allow empty strings to support clearing auto-filled values
"head_sha": {"type": "string", "pattern": "^(|[0-9a-f]{40})$"},
"base_sha": {"type": "string", "pattern": "^(|[0-9a-f]{40})$"},
Expand All @@ -97,6 +98,7 @@ def validate_preprod_artifact_schema(request_body: bytes) -> tuple[dict[str, Any
"chunks": "The chunks field is required and must be provided as an array of 40-character hexadecimal strings.",
"build_configuration": "The build_configuration field must be a string.",
"release_notes": "The release_notes field msut be a string.",
"date_built": "The date_built field must be an ISO 8601 formatted date-time string.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured with ISO-8601 we can set and send the local time zone information from the gradle plugin.

"head_sha": "The head_sha field must be a 40-character hexadecimal SHA1 string (no uppercase letters).",
"base_sha": "The base_sha field must be a 40-character hexadecimal SHA1 string (no uppercase letters).",
"provider": "The provider field must be a string with maximum length of 255 characters containing the domain of the VCS provider (ex. github.com)",
Expand Down Expand Up @@ -204,6 +206,7 @@ def post(self, request: Request, project: Project) -> Response:
checksum=checksum,
build_configuration_name=data.get("build_configuration"),
release_notes=data.get("release_notes"),
date_built=data.get("date_built"),
head_sha=data.get("head_sha"),
base_sha=data.get("base_sha"),
provider=data.get("provider"),
Expand Down
19 changes: 18 additions & 1 deletion src/sentry/preprod/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ def create_preprod_artifact(
checksum: str,
build_configuration_name: str | None = None,
release_notes: str | None = None,
date_built: str | None = None,
head_sha: str | None = None,
base_sha: str | None = None,
provider: str | None = None,
Expand Down Expand Up @@ -236,14 +237,30 @@ def create_preprod_artifact(
if release_notes:
extras = {"release_notes": release_notes}

preprod_artifact, _ = PreprodArtifact.objects.get_or_create(
# Parse date_built if provided
parsed_date_built = None
if date_built:
try:
parsed_date_built = datetime.datetime.fromisoformat(date_built)
except (ValueError, TypeError) as e:
logger.warning(
"Failed to parse date_built",
extra={"date_built": date_built, "error": str(e)},
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Invalid date_built strings silently ignored after validation

The schema accepts any string for date_built without validating ISO 8601 format. Invalid date strings like "invalid-date" pass schema validation but fail during datetime.fromisoformat() parsing, causing the error to be logged as a warning while the artifact is created successfully with date_built set to null. Users receive no error response indicating their timestamp was invalid and discarded, leading to silent data loss.

Additional Locations (1)

Fix in Cursor Fix in Web


preprod_artifact, created = PreprodArtifact.objects.get_or_create(
project=project,
build_configuration=build_config,
state=PreprodArtifact.ArtifactState.UPLOADING,
commit_comparison=commit_comparison,
extras=extras,
)

# Set date_built if provided and artifact was just created
if created and parsed_date_built:
preprod_artifact.date_built = parsed_date_built
preprod_artifact.save(update_fields=["date_built"])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: date_built not updated for existing artifacts

The date_built field is only set when a new artifact is created (created == True), but not when an existing artifact is retrieved by get_or_create. If the same artifact is uploaded multiple times (e.g., retries, CI reruns), subsequent uploads with a different or corrected date_built timestamp will silently ignore the new value, leaving the original timestamp unchanged. This breaks the intended functionality of capturing accurate build timestamps.

Fix in Cursor Fix in Web


# TODO(preprod): add gating to only create if has quota
PreprodArtifactSizeMetrics.objects.get_or_create(
preprod_artifact=preprod_artifact,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def test_valid_full_schema(self) -> None:
"checksum": "a" * 40,
"chunks": ["b" * 40, "c" * 40],
"build_configuration": "release",
"date_built": "2025-11-26T10:30:00",
"head_sha": "e" * 40,
"base_sha": "f" * 40,
"provider": "github",
Expand Down Expand Up @@ -164,6 +165,21 @@ def test_pr_number_invalid(self) -> None:
assert error is not None
assert result == {}

def test_date_built_valid_string(self) -> None:
"""Test valid date_built string is accepted."""
data = {"checksum": "a" * 40, "chunks": [], "date_built": "2025-11-26T10:30:00"}
body = orjson.dumps(data)
result, error = validate_preprod_artifact_schema(body)
assert error is None
assert result == data

def test_date_built_wrong_type(self) -> None:
"""Test non-string date_built returns error."""
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "date_built": 123})
result, error = validate_preprod_artifact_schema(body)
assert error is not None
assert result == {}

def test_additional_properties_rejected(self) -> None:
"""Test additional properties are rejected."""
body = orjson.dumps({"checksum": "a" * 40, "chunks": [], "extra_field": "value"})
Expand Down
23 changes: 23 additions & 0 deletions tests/sentry/preprod/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,29 @@ def test_create_preprod_artifact_with_release_notes(self) -> None:
# Clean up
delete_assemble_status(AssembleTask.PREPROD_ARTIFACT, self.project.id, total_checksum)

def test_create_preprod_artifact_with_date_built(self) -> None:
"""Test that create_preprod_artifact stores date_built field"""
content = b"test preprod artifact with date_built"
total_checksum = sha1(content).hexdigest()
date_built_str = "2025-11-26T10:30:00"

# Create preprod artifact with date_built
artifact = create_preprod_artifact(
org_id=self.organization.id,
project_id=self.project.id,
checksum=total_checksum,
build_configuration_name="release",
date_built=date_built_str,
)
assert artifact is not None

# Verify the artifact was created with date_built
assert artifact.date_built is not None
assert artifact.date_built.isoformat() == date_built_str

# Clean up
delete_assemble_status(AssembleTask.PREPROD_ARTIFACT, self.project.id, total_checksum)

def test_assemble_preprod_artifact_with_commit_comparison(self) -> None:
content = b"test preprod artifact with commit comparison"
fileobj = ContentFile(content)
Expand Down
Loading