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
2 changes: 2 additions & 0 deletions src/sentry/integrations/github/webhook_types.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from enum import StrEnum

GITHUB_WEBHOOK_TYPE_HEADER = "HTTP_X_GITHUB_EVENT"
GITHUB_WEBHOOK_TYPE_HEADER_KEY = "X-GITHUB-EVENT"
GITHUB_INSTALLATION_TARGET_ID_HEADER = "X-GITHUB-HOOK-INSTALLATION-TARGET-ID"


class GithubWebhookType(StrEnum):
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/middleware/integrations/parsers/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def get_response(self) -> HttpResponseBase:

# The overwatch forwarder implements its own region-based checks
OverwatchGithubWebhookForwarder(integration=integration).forward_if_applicable(
event=event, headers=self.request.META
event=event, headers=self.request.headers
)

return response
18 changes: 10 additions & 8 deletions src/sentry/overwatch_webhooks/webhook_forwarder.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@

from sentry import options
from sentry.constants import ObjectStatus
from sentry.integrations.github.webhook_types import GITHUB_WEBHOOK_TYPE_HEADER, GithubWebhookType
from sentry.integrations.github.webhook_types import (
GITHUB_INSTALLATION_TARGET_ID_HEADER,
GITHUB_WEBHOOK_TYPE_HEADER_KEY,
GithubWebhookType,
)
from sentry.integrations.models.integration import Integration
from sentry.integrations.models.organization_integration import OrganizationIntegration
from sentry.models.organizationmapping import OrganizationMapping
Expand All @@ -26,10 +30,6 @@
}


GITHUB_INSTALLATION_TARGET_ID_HEADER = "X-GitHub-Hook-Installation-Target-ID"
DJANGO_HTTP_GITHUB_INSTALLATION_TARGET_ID_HEADER = "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID"


@dataclass(frozen=True)
class OverwatchOrganizationContext:
organization_integration: OrganizationIntegration
Expand All @@ -51,7 +51,7 @@ def __init__(self, integration: Integration):
self.integration = integration

def should_forward_to_overwatch(self, headers: Mapping[str, str]) -> bool:
event_type = headers.get(GITHUB_WEBHOOK_TYPE_HEADER)
event_type = headers.get(GITHUB_WEBHOOK_TYPE_HEADER_KEY)
verbose_log(
"overwatch.debug.should_forward_to_overwatch.checked",
extra={
Expand Down Expand Up @@ -163,7 +163,7 @@ def forward_if_applicable(self, event: Mapping[str, Any], headers: Mapping[str,

raw_app_id = headers.get(
GITHUB_INSTALLATION_TARGET_ID_HEADER,
) or headers.get(DJANGO_HTTP_GITHUB_INSTALLATION_TARGET_ID_HEADER)
)
verbose_log(
"overwatch.debug.raw_app_id",
extra={"region_name": region_name, "raw_app_id": raw_app_id},
Expand All @@ -179,10 +179,12 @@ def forward_if_applicable(self, event: Mapping[str, Any], headers: Mapping[str,
)
app_id = None

formatted_headers = {k: v for k, v in headers.items()}

webhook_detail = WebhookDetails(
organizations=org_summaries,
webhook_body=dict(event),
webhook_headers=headers,
webhook_headers=formatted_headers,
integration_provider=self.integration.provider,
region=region_name,
app_id=app_id,
Expand Down
115 changes: 114 additions & 1 deletion tests/sentry/middleware/integrations/parsers/test_github.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import orjson
import pytest
import responses
from django.db import router, transaction
Expand All @@ -17,7 +18,7 @@
from sentry.testutils.helpers.options import override_options
from sentry.testutils.outbox import assert_no_webhook_payloads, assert_webhook_payloads_for_mailbox
from sentry.testutils.region import override_regions
from sentry.testutils.silo import control_silo_test
from sentry.testutils.silo import control_silo_test, create_test_regions
from sentry.types.region import Region, RegionCategory

region = Region("us", 1, "https://us.testserver", RegionCategory.MULTI_TENANT)
Expand Down Expand Up @@ -304,3 +305,115 @@ class GithubRequestParserTypeRoutingTest(GithubRequestParserTest):
def setup(self):
with override_options({"github.webhook-type-routing.enabled": True}):
yield


@control_silo_test(regions=create_test_regions("us"))
class GithubRequestParserOverwatchForwarderTest(TestCase):
factory = RequestFactory()
path = reverse("sentry-integration-github-webhook")

@pytest.fixture(autouse=True)
def setup(self):
with override_options({"github.webhook-type-routing.enabled": True}):
yield

def setUp(self) -> None:
super().setUp()
self.organization = self.create_organization(
name="Test Org",
slug="test-org",
region="us",
owner=self.create_user(email="test@example.com"),
)
self.integration = self.create_integration(
provider="github",
external_id="1",
name="Test Integration",
organization=self.organization,
)

@responses.activate
def test_overwatch_forwarder(self) -> None:
with (
override_options({"overwatch.enabled-regions": ["us"]}),
override_settings(
OVERWATCH_REGION_URLS={"us": "https://us.example.com/api"},
OVERWATCH_WEBHOOK_SECRET="test-secret",
),
):
responses.add(
responses.POST,
"https://us.example.com/api/webhooks/sentry",
status=200,
)

request = self.factory.post(
self.path,
data={"installation": {"id": "1"}, "action": "created"},
content_type="application/json",
headers={
"x-github-event": GithubWebhookType.PULL_REQUEST.value,
"x-github-hook-installation-target-id": "123",
},
)
parser = GithubRequestParser(
request=request,
response_handler=lambda _: HttpResponse(status=200, content="passthrough"),
)

response = parser.get_response()
assert isinstance(response, HttpResponse)
assert response.status_code == status.HTTP_202_ACCEPTED
assert response.content == b""

assert len(responses.calls) == 1
assert responses.calls[0].request.url == "https://us.example.com/api/webhooks/sentry"
assert responses.calls[0].request.method == "POST"
json_body = orjson.loads(responses.calls[0].request.body)

assert json_body["organizations"] == [
{
"name": "Test Org",
"slug": "test-org",
"id": self.organization.id,
"region": "us",
"github_integration_id": self.integration.id,
"organization_integration_id": self.organization_integration.id,
}
]
assert json_body["webhook_body"] == {"installation": {"id": "1"}, "action": "created"}
assert json_body["app_id"] == 123
assert json_body["webhook_headers"]["X-Github-Event"] == "pull_request"
assert json_body["integration_provider"] == "github"
assert json_body["region"] == "us"
assert json_body["event_type"] == "github"

@responses.activate
def test_overwatch_forwarder_missing_region_config(self) -> None:
with (
override_options({"overwatch.enabled-regions": ["us"]}),
override_settings(
OVERWATCH_REGION_URLS={"de": "https://de.example.com/api"},
OVERWATCH_WEBHOOK_SECRET="test-secret",
),
):
request = self.factory.post(
self.path,
data={"installation": {"id": "1"}, "action": "created"},
content_type="application/json",
headers={
"x-github-event": GithubWebhookType.PULL_REQUEST.value,
"x-github-hook-installation-target-id": "1",
},
)
parser = GithubRequestParser(
request=request,
response_handler=lambda _: HttpResponse(status=200, content="passthrough"),
)

response = parser.get_response()
assert isinstance(response, HttpResponse)
assert response.status_code == status.HTTP_202_ACCEPTED
assert response.content == b""

assert len(responses.calls) == 0
50 changes: 28 additions & 22 deletions tests/sentry/overwatch_webhooks/test_webhook_forwarder.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
from django.test.utils import override_settings

from sentry.hybridcloud.models.outbox import outbox_context
from sentry.integrations.github.webhook_types import (
GITHUB_INSTALLATION_TARGET_ID_HEADER,
GITHUB_WEBHOOK_TYPE_HEADER_KEY,
)
from sentry.integrations.models.organization_integration import OrganizationIntegration
from sentry.overwatch_webhooks.types import (
DEFAULT_REQUEST_TYPE,
Expand Down Expand Up @@ -46,15 +50,15 @@ def test_init_creates_publisher_with_correct_provider(self):

def test_should_forward_to_overwatch_with_valid_events(self):
for event_action in GITHUB_EVENTS_TO_FORWARD_OVERWATCH:
headers = {"HTTP_X_GITHUB_EVENT": event_action}
headers = {GITHUB_WEBHOOK_TYPE_HEADER_KEY: event_action}
assert self.forwarder.should_forward_to_overwatch(headers) is True

def test_should_forward_to_overwatch_with_invalid_events(self):
invalid_events = [
{"HTTP_X_GITHUB_EVENT": "invalid_action"},
{"HTTP_X_GITHUB_EVENT": "create"},
{"HTTP_X_GITHUB_EVENT": "delete"},
{"HTTP_X_GITHUB_EVENT": "some_other_action"},
{GITHUB_WEBHOOK_TYPE_HEADER_KEY: "invalid_action"},
{GITHUB_WEBHOOK_TYPE_HEADER_KEY: "create"},
{GITHUB_WEBHOOK_TYPE_HEADER_KEY: "delete"},
{GITHUB_WEBHOOK_TYPE_HEADER_KEY: "some_other_action"},
{},
]

Expand Down Expand Up @@ -128,7 +132,9 @@ def test_forward_if_applicable_no_organizations(self):
event = {"action": "push", "data": "test"}

with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
self.forwarder.forward_if_applicable(event, headers={"HTTP_X_GITHUB_EVENT": "push"})
self.forwarder.forward_if_applicable(
event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "push"}
)
mock_enqueue.assert_not_called()

@override_options({"overwatch.enabled-regions": ["us"]})
Expand All @@ -143,7 +149,7 @@ def test_forward_if_applicable_event_not_eligible_for_forwarding(self):

with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
self.forwarder.forward_if_applicable(
event, headers={"HTTP_X_GITHUB_EVENT": "invalid_action"}
event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "invalid_action"}
)
mock_enqueue.assert_not_called()

Expand All @@ -161,8 +167,8 @@ def test_forward_if_applicable_successful_forwarding(self):
self.forwarder.forward_if_applicable(
event,
headers={
"HTTP_X_GITHUB_EVENT": "pull_request",
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
},
)

Expand All @@ -188,7 +194,7 @@ def test_forward_if_applicable_multiple_organizations(self):

with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
self.forwarder.forward_if_applicable(
event, headers={"HTTP_X_GITHUB_EVENT": "pull_request"}
event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request"}
)

mock_enqueue.assert_called_once()
Expand All @@ -213,7 +219,7 @@ def test_forward_if_applicable_all_valid_event_actions(self):

with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
self.forwarder.forward_if_applicable(
event, headers={"HTTP_X_GITHUB_EVENT": event_type}
event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: event_type}
)
mock_enqueue.assert_called_once()

Expand Down Expand Up @@ -246,7 +252,7 @@ def test_forward_if_applicable_preserves_webhook_body_data(self):

with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
self.forwarder.forward_if_applicable(
complex_event, headers={"HTTP_X_GITHUB_EVENT": "pull_request"}
complex_event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request"}
)

mock_enqueue.assert_called_once()
Expand Down Expand Up @@ -294,8 +300,8 @@ def test_forwards_to_correct_regions(self):
self.forwarder.forward_if_applicable(
event,
headers={
"HTTP_X_GITHUB_EVENT": "pull_request",
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
},
)

Expand All @@ -319,8 +325,8 @@ def test_forwards_to_correct_regions(self):
],
"webhook_body": event,
"webhook_headers": {
"HTTP_X_GITHUB_EVENT": "pull_request",
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
},
"integration_provider": "github",
"region": "us",
Expand All @@ -342,8 +348,8 @@ def test_forwards_to_correct_regions(self):
],
"webhook_body": event,
"webhook_headers": {
"HTTP_X_GITHUB_EVENT": "pull_request",
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
},
"integration_provider": "github",
"region": "de",
Expand Down Expand Up @@ -385,8 +391,8 @@ def test_forwards_conditionally_to_some_regions(self):
self.forwarder.forward_if_applicable(
event,
headers={
"HTTP_X_GITHUB_EVENT": "pull_request",
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
},
)

Expand All @@ -407,8 +413,8 @@ def test_forwards_conditionally_to_some_regions(self):
],
"webhook_body": event,
"webhook_headers": {
"HTTP_X_GITHUB_EVENT": "pull_request",
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
},
"integration_provider": "github",
"region": "us",
Expand Down
Loading