Skip to content
Merged
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
10 changes: 0 additions & 10 deletions src/sentry/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -3921,16 +3921,6 @@
flags=FLAG_MODIFIABLE_RATE | FLAG_AUTOMATOR_MODIFIABLE,
)

# Controls whether to validate webhook payloads with Pydantic before sending to Seer
# This is disabled by default to avoid potential issues with enum key serialization
# until the validation is fully tested and deployed
register(
"seer.code_review.validate_webhook_payload",
type=Bool,
default=False,
flags=FLAG_AUTOMATOR_MODIFIABLE,
)

# Enabled Prebuilt Dashboard IDs
register(
"dashboards.prebuilt-dashboard-ids",
Expand Down
6 changes: 2 additions & 4 deletions src/sentry/seer/code_review/webhooks/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

from urllib3.exceptions import HTTPError

from sentry import options
from sentry.integrations.github.webhook_types import GithubWebhookType
from sentry.models.organization import Organization
from sentry.models.repository import Repository
Expand Down Expand Up @@ -98,9 +97,8 @@ def process_github_webhook_event(
try:
path = get_seer_endpoint_for_event(github_event).value

# Validate payload with Pydantic if enabled (except for CHECK_RUN events which use minimal payload)
should_validate = options.get("seer.code_review.validate_webhook_payload", False)
if should_validate and github_event != GithubWebhookType.CHECK_RUN:
# Validate payload with Pydantic (except for CHECK_RUN events which use minimal payload)
if github_event != GithubWebhookType.CHECK_RUN:
# Parse with appropriate model based on request type to enforce
# organization_id and integration_id requirements for PR closed
request_type = event_payload.get("request_type")
Expand Down
47 changes: 3 additions & 44 deletions tests/sentry/seer/code_review/test_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
process_github_webhook_event,
)
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers.options import override_options


class ProcessGitHubWebhookEventTest(TestCase):
Expand Down Expand Up @@ -447,6 +446,7 @@ def test_check_run_and_pr_events_processed_separately(self, mock_request: MagicM
"owner": "test-owner",
"name": "test-repo",
"external_id": "123456",
"base_commit_sha": "abc123",
},
"pr_id": 123,
"bug_prediction_specific_information": {
Expand Down Expand Up @@ -475,12 +475,9 @@ def test_check_run_and_pr_events_processed_separately(self, mock_request: MagicM
pr_call = mock_request.call_args
assert pr_call[1]["path"] == "/v1/automation/overwatch-request"

@override_options({"seer.code_review.validate_webhook_payload": True})
@patch("sentry.seer.code_review.utils.make_signed_seer_api_request")
def test_validation_enabled_converts_enum_keys_to_strings(
self, mock_request: MagicMock
) -> None:
"""Test that when validation is enabled, enum keys are properly converted to strings.
def test_validation_converts_enum_keys_to_strings(self, mock_request: MagicMock) -> None:
"""Test that validation converts enum keys to strings for JSON serialization.

This test verifies the fix for the Pydantic v1 enum key serialization bug:
- Pydantic v1 converts string keys to enum members during parsing
Expand Down Expand Up @@ -541,41 +538,6 @@ def test_validation_enabled_converts_enum_keys_to_strings(
for key in features.keys():
assert isinstance(key, str), f"Expected string key, got {type(key)}"

@override_options({"seer.code_review.validate_webhook_payload": False})
@patch("sentry.seer.code_review.utils.make_signed_seer_api_request")
def test_validation_disabled_skips_pydantic_parsing(self, mock_request: MagicMock) -> None:
"""Test that when validation is disabled, payload is passed through without Pydantic parsing."""
mock_request.return_value = self._mock_response(200, b"{}")

event_payload = {
"request_type": "pr-review",
"external_owner_id": "456",
"data": {
"repo": {
"provider": "github",
"owner": "test-owner",
"name": "test-repo",
"external_id": "123456",
"base_commit_sha": "abc123",
},
"pr_id": 123,
"bug_prediction_specific_information": {
"organization_id": 789,
"organization_slug": "test-org",
},
},
}

process_github_webhook_event._func(
github_event=GithubWebhookType.PULL_REQUEST,
event_payload=event_payload,
enqueued_at_str=self.enqueued_at_str,
)

# Verify the request was made with the original payload (no validation)
assert mock_request.call_count == 1

@override_options({"seer.code_review.validate_webhook_payload": True})
@patch("sentry.seer.code_review.utils.make_signed_seer_api_request")
def test_pr_review_validation_passes_without_organization_id(
self, mock_request: MagicMock
Expand Down Expand Up @@ -616,7 +578,6 @@ def test_pr_review_validation_passes_without_organization_id(

assert mock_request.call_count == 1

@override_options({"seer.code_review.validate_webhook_payload": True})
@patch("sentry.seer.code_review.utils.make_signed_seer_api_request")
def test_pr_closed_validation_fails_without_organization_id(
self, mock_request: MagicMock
Expand Down Expand Up @@ -663,7 +624,6 @@ def test_pr_closed_validation_fails_without_organization_id(
errors = exc_info.value.errors()
assert any("organization_id" in str(error) for error in errors)

@override_options({"seer.code_review.validate_webhook_payload": True})
@patch("sentry.seer.code_review.utils.make_signed_seer_api_request")
def test_pr_closed_validation_fails_without_integration_id(
self, mock_request: MagicMock
Expand Down Expand Up @@ -710,7 +670,6 @@ def test_pr_closed_validation_fails_without_integration_id(
errors = exc_info.value.errors()
assert any("integration_id" in str(error) for error in errors)

@override_options({"seer.code_review.validate_webhook_payload": True})
@patch("sentry.seer.code_review.utils.make_signed_seer_api_request")
def test_pr_closed_validation_passes_with_required_fields(
self, mock_request: MagicMock
Expand Down
Loading