Skip to content

Commit f1b52f7

Browse files
Moves constants for webhook headers into the common types file, updates tests
1 parent 5e4b55a commit f1b52f7

File tree

4 files changed

+40
-30
lines changed

4 files changed

+40
-30
lines changed

src/sentry/integrations/github/webhook_types.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
GITHUB_WEBHOOK_TYPE_HEADER = "HTTP_X_GITHUB_EVENT"
44
GITHUB_WEBHOOK_TYPE_HEADER_KEY = "X-GITHUB-EVENT"
5+
GITHUB_INSTALLATION_TARGET_ID_HEADER = "X-GITHUB-HOOK-INSTALLATION-TARGET-ID"
56

67

78
class GithubWebhookType(StrEnum):

src/sentry/overwatch_webhooks/webhook_forwarder.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from sentry import options
88
from sentry.constants import ObjectStatus
99
from sentry.integrations.github.webhook_types import (
10+
GITHUB_INSTALLATION_TARGET_ID_HEADER,
1011
GITHUB_WEBHOOK_TYPE_HEADER_KEY,
1112
GithubWebhookType,
1213
)
@@ -29,10 +30,6 @@
2930
}
3031

3132

32-
GITHUB_INSTALLATION_TARGET_ID_HEADER = "X-GitHub-Hook-Installation-Target-ID"
33-
DJANGO_HTTP_GITHUB_INSTALLATION_TARGET_ID_HEADER = "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID"
34-
35-
3633
@dataclass(frozen=True)
3734
class OverwatchOrganizationContext:
3835
organization_integration: OrganizationIntegration
@@ -166,7 +163,7 @@ def forward_if_applicable(self, event: Mapping[str, Any], headers: Mapping[str,
166163

167164
raw_app_id = headers.get(
168165
GITHUB_INSTALLATION_TARGET_ID_HEADER,
169-
) or headers.get(DJANGO_HTTP_GITHUB_INSTALLATION_TARGET_ID_HEADER)
166+
)
170167
verbose_log(
171168
"overwatch.debug.raw_app_id",
172169
extra={"region_name": region_name, "raw_app_id": raw_app_id},

tests/sentry/middleware/integrations/parsers/test_github.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,10 @@ def test_overwatch_forwarder(self) -> None:
334334
self.path,
335335
data={"installation": {"id": "1"}, "action": "created"},
336336
content_type="application/json",
337-
headers={"x-github-event": GithubWebhookType.PULL_REQUEST.value},
337+
headers={
338+
"x-github-event": GithubWebhookType.PULL_REQUEST.value,
339+
"x-github-hook-installation-target-id": "123",
340+
},
338341
)
339342
parser = GithubRequestParser(
340343
request=request,
@@ -362,7 +365,7 @@ def test_overwatch_forwarder(self) -> None:
362365
}
363366
]
364367
assert json_body["webhook_body"] == {"installation": {"id": "1"}, "action": "created"}
365-
368+
assert json_body["app_id"] == 123
366369
assert json_body["webhook_headers"]["X-Github-Event"] == "pull_request"
367370
assert json_body["integration_provider"] == "github"
368371
assert json_body["region"] == "us"
@@ -381,7 +384,10 @@ def test_overwatch_forwarder_missing_region_config(self) -> None:
381384
self.path,
382385
data={"installation": {"id": "1"}, "action": "created"},
383386
content_type="application/json",
384-
headers={"x-github-event": GithubWebhookType.PULL_REQUEST.value},
387+
headers={
388+
"x-github-event": GithubWebhookType.PULL_REQUEST.value,
389+
"x-github-hook-installation-target-id": "1",
390+
},
385391
)
386392
parser = GithubRequestParser(
387393
request=request,

tests/sentry/overwatch_webhooks/test_webhook_forwarder.py

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
from django.test.utils import override_settings
77

88
from sentry.hybridcloud.models.outbox import outbox_context
9+
from sentry.integrations.github.webhook_types import (
10+
GITHUB_INSTALLATION_TARGET_ID_HEADER,
11+
GITHUB_WEBHOOK_TYPE_HEADER_KEY,
12+
)
913
from sentry.integrations.models.organization_integration import OrganizationIntegration
1014
from sentry.overwatch_webhooks.types import (
1115
DEFAULT_REQUEST_TYPE,
@@ -46,15 +50,15 @@ def test_init_creates_publisher_with_correct_provider(self):
4650

4751
def test_should_forward_to_overwatch_with_valid_events(self):
4852
for event_action in GITHUB_EVENTS_TO_FORWARD_OVERWATCH:
49-
headers = {"HTTP_X_GITHUB_EVENT": event_action}
53+
headers = {GITHUB_WEBHOOK_TYPE_HEADER_KEY: event_action}
5054
assert self.forwarder.should_forward_to_overwatch(headers) is True
5155

5256
def test_should_forward_to_overwatch_with_invalid_events(self):
5357
invalid_events = [
54-
{"HTTP_X_GITHUB_EVENT": "invalid_action"},
55-
{"HTTP_X_GITHUB_EVENT": "create"},
56-
{"HTTP_X_GITHUB_EVENT": "delete"},
57-
{"HTTP_X_GITHUB_EVENT": "some_other_action"},
58+
{GITHUB_WEBHOOK_TYPE_HEADER_KEY: "invalid_action"},
59+
{GITHUB_WEBHOOK_TYPE_HEADER_KEY: "create"},
60+
{GITHUB_WEBHOOK_TYPE_HEADER_KEY: "delete"},
61+
{GITHUB_WEBHOOK_TYPE_HEADER_KEY: "some_other_action"},
5862
{},
5963
]
6064

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

130134
with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
131-
self.forwarder.forward_if_applicable(event, headers={"HTTP_X_GITHUB_EVENT": "push"})
135+
self.forwarder.forward_if_applicable(
136+
event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "push"}
137+
)
132138
mock_enqueue.assert_not_called()
133139

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

144150
with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
145151
self.forwarder.forward_if_applicable(
146-
event, headers={"HTTP_X_GITHUB_EVENT": "invalid_action"}
152+
event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "invalid_action"}
147153
)
148154
mock_enqueue.assert_not_called()
149155

@@ -161,8 +167,8 @@ def test_forward_if_applicable_successful_forwarding(self):
161167
self.forwarder.forward_if_applicable(
162168
event,
163169
headers={
164-
"HTTP_X_GITHUB_EVENT": "pull_request",
165-
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
170+
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
171+
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
166172
},
167173
)
168174

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

189195
with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
190196
self.forwarder.forward_if_applicable(
191-
event, headers={"HTTP_X_GITHUB_EVENT": "pull_request"}
197+
event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request"}
192198
)
193199

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

214220
with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
215221
self.forwarder.forward_if_applicable(
216-
event, headers={"HTTP_X_GITHUB_EVENT": event_type}
222+
event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: event_type}
217223
)
218224
mock_enqueue.assert_called_once()
219225

@@ -246,7 +252,7 @@ def test_forward_if_applicable_preserves_webhook_body_data(self):
246252

247253
with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
248254
self.forwarder.forward_if_applicable(
249-
complex_event, headers={"HTTP_X_GITHUB_EVENT": "pull_request"}
255+
complex_event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request"}
250256
)
251257

252258
mock_enqueue.assert_called_once()
@@ -294,8 +300,8 @@ def test_forwards_to_correct_regions(self):
294300
self.forwarder.forward_if_applicable(
295301
event,
296302
headers={
297-
"HTTP_X_GITHUB_EVENT": "pull_request",
298-
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
303+
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
304+
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
299305
},
300306
)
301307

@@ -319,8 +325,8 @@ def test_forwards_to_correct_regions(self):
319325
],
320326
"webhook_body": event,
321327
"webhook_headers": {
322-
"HTTP_X_GITHUB_EVENT": "pull_request",
323-
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
328+
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
329+
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
324330
},
325331
"integration_provider": "github",
326332
"region": "us",
@@ -342,8 +348,8 @@ def test_forwards_to_correct_regions(self):
342348
],
343349
"webhook_body": event,
344350
"webhook_headers": {
345-
"HTTP_X_GITHUB_EVENT": "pull_request",
346-
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
351+
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
352+
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
347353
},
348354
"integration_provider": "github",
349355
"region": "de",
@@ -385,8 +391,8 @@ def test_forwards_conditionally_to_some_regions(self):
385391
self.forwarder.forward_if_applicable(
386392
event,
387393
headers={
388-
"HTTP_X_GITHUB_EVENT": "pull_request",
389-
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
394+
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
395+
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
390396
},
391397
)
392398

@@ -407,8 +413,8 @@ def test_forwards_conditionally_to_some_regions(self):
407413
],
408414
"webhook_body": event,
409415
"webhook_headers": {
410-
"HTTP_X_GITHUB_EVENT": "pull_request",
411-
"HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
416+
GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
417+
GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
412418
},
413419
"integration_provider": "github",
414420
"region": "us",

0 commit comments

Comments
 (0)