Skip to content

Commit 42d3d3a

Browse files
GabeVillalobosJesse-Box
authored andcommitted
fix(eco): Refactors prevent webhook forwarder to check the full set of headers, use enums (#103067)
1 parent 96b0f60 commit 42d3d3a

File tree

3 files changed

+17
-28
lines changed

3 files changed

+17
-28
lines changed

src/sentry/integrations/github/webhook_types.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55

66
class GithubWebhookType(StrEnum):
77
INSTALLATION = "installation"
8+
INSTALLATION_REPOSITORIES = "installation_repositories"
89
ISSUE = "issues"
10+
ISSUE_COMMENT = "issue_comment"
911
PULL_REQUEST = "pull_request"
12+
PULL_REQUEST_REVIEW_COMMENT = "pull_request_review_comment"
13+
PULL_REQUEST_REVIEW = "pull_request_review"
1014
PUSH = "push"

src/sentry/middleware/integrations/parsers/github.py

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -100,18 +100,13 @@ def get_response(self) -> HttpResponseBase:
100100
if codecov_regions:
101101
self.try_forward_to_codecov(event=event)
102102

103-
logger.info(
104-
"overwatch.debug.forward_if_applicable.begin",
105-
extra={
106-
"headers_keys": list(self.request.headers.keys()),
107-
"integration_id": integration.id,
108-
},
103+
response = self.get_response_from_webhookpayload(
104+
regions=regions, identifier=integration.id, integration_id=integration.id
109105
)
106+
110107
# The overwatch forwarder implements its own region-based checks
111108
OverwatchGithubWebhookForwarder(integration=integration).forward_if_applicable(
112-
event=event, headers=self.request.headers
109+
event=event, headers=self.request.META
113110
)
114111

115-
return self.get_response_from_webhookpayload(
116-
regions=regions, identifier=integration.id, integration_id=integration.id
117-
)
112+
return response

src/sentry/overwatch_webhooks/webhook_forwarder.py

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from sentry import options
88
from sentry.constants import ObjectStatus
9+
from sentry.integrations.github.webhook_types import GITHUB_WEBHOOK_TYPE_HEADER, GithubWebhookType
910
from sentry.integrations.models.integration import Integration
1011
from sentry.integrations.models.organization_integration import OrganizationIntegration
1112
from sentry.models.organizationmapping import OrganizationMapping
@@ -16,12 +17,12 @@
1617

1718
# TODO: Double check that this includes all of the events you care about.
1819
GITHUB_EVENTS_TO_FORWARD_OVERWATCH = {
19-
"installation",
20-
"installation_repositories",
21-
"issue_comment",
22-
"pull_request",
23-
"pull_request_review_comment",
24-
"pull_request_review",
20+
GithubWebhookType.INSTALLATION,
21+
GithubWebhookType.INSTALLATION_REPOSITORIES,
22+
GithubWebhookType.ISSUE_COMMENT,
23+
GithubWebhookType.PULL_REQUEST,
24+
GithubWebhookType.PULL_REQUEST_REVIEW_COMMENT,
25+
GithubWebhookType.PULL_REQUEST_REVIEW,
2526
}
2627

2728

@@ -45,7 +46,7 @@ def __init__(self, integration: Integration):
4546
self.integration = integration
4647

4748
def should_forward_to_overwatch(self, headers: Mapping[str, str]) -> bool:
48-
return headers.get("HTTP_X_GITHUB_EVENT") in GITHUB_EVENTS_TO_FORWARD_OVERWATCH
49+
return headers.get(GITHUB_WEBHOOK_TYPE_HEADER) in GITHUB_EVENTS_TO_FORWARD_OVERWATCH
4950

5051
def _get_org_summaries_by_region_for_integration(
5152
self, integration: Integration
@@ -108,12 +109,8 @@ def forward_if_applicable(self, event: Mapping[str, Any], headers: Mapping[str,
108109
region_name = None
109110
try:
110111
enabled_regions = options.get("overwatch.enabled-regions")
111-
logger.info(
112-
"overwatch.debug.enabled_regions", extra={"enabled_regions": enabled_regions}
113-
)
114112
if not enabled_regions:
115113
# feature isn't enabled, no work to do
116-
logger.info("overwatch.debug.excluded.feature_not_enabled", extra={})
117114
return
118115

119116
orgs_by_region = self._get_org_summaries_by_region_for_integration(
@@ -177,10 +174,6 @@ def forward_if_applicable(self, event: Mapping[str, Any], headers: Mapping[str,
177174
region=region_name,
178175
app_id=app_id,
179176
)
180-
logger.info(
181-
"overwatch.debug.webhook_detail.created",
182-
extra={"region_name": region_name, "app_id": app_id},
183-
)
184177

185178
publisher = OverwatchWebhookPublisher(
186179
integration_provider=self.integration.provider,
@@ -193,9 +186,6 @@ def forward_if_applicable(self, event: Mapping[str, Any], headers: Mapping[str,
193186
sample_rate=1.0,
194187
tags={"forward_region": region_name},
195188
)
196-
logger.info(
197-
"overwatch.debug.metrics_incr.success", extra={"region_name": region_name}
198-
)
199189
except Exception:
200190
metrics.incr(
201191
"overwatch.forward-webhooks.forward-error",

0 commit comments

Comments
 (0)