feat(github-enterprise): Route installation_repositories to control silo#112245
Open
wedamija wants to merge 1 commit intodanf/repo-sync-audit-logsfrom
Open
feat(github-enterprise): Route installation_repositories to control silo#112245wedamija wants to merge 1 commit intodanf/repo-sync-audit-logsfrom
wedamija wants to merge 1 commit intodanf/repo-sync-audit-logsfrom
Conversation
Remove the should_route_to_control_silo override in GithubEnterpriseRequestParser so it inherits the parent's routing, which sends installation_repositories events to control silo. We do this to make sure the webhooks for GHE will also route to the right place
Comment on lines
18
to
23
| webhook_identifier = WebhookProviderIdentifier.GITHUB_ENTERPRISE | ||
| webhook_endpoint = GitHubEnterpriseWebhookEndpoint | ||
|
|
||
| def should_route_to_control_silo( | ||
| self, parsed_event: Mapping[str, Any], request: HttpRequest | ||
| ) -> bool: | ||
| # GHE only routes installation events to control silo. | ||
| # installation_repositories is not yet supported for GHE. | ||
| return request.META.get(GITHUB_WEBHOOK_TYPE_HEADER) == GithubWebhookType.INSTALLATION | ||
|
|
||
| def _get_external_id(self, event: Mapping[str, Any]) -> str | None: | ||
| host = get_host(request=self.request) | ||
| if not host: |
Contributor
There was a problem hiding this comment.
Bug: Removing the should_route_to_control_silo override routes installation_repositories webhooks for GHE, but the GitHubEnterpriseWebhookEndpoint lacks a handler, causing these events to be silently dropped.
Severity: HIGH
Suggested Fix
Implement a GitHubEnterpriseInstallationRepositoriesEventWebhook handler, similar to the existing InstallationRepositoriesEventWebhook for regular GitHub. Register this new handler in the _handlers dictionary within GitHubEnterpriseWebhookEndpoint for the GithubWebhookType.INSTALLATION_REPOSITORIES event type. This will ensure that repository changes for GitHub Enterprise installations are processed correctly.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/middleware/integrations/parsers/github_enterprise.py#L18-L23
Potential issue: The pull request removes an override in
`GithubEnterpriseRequestParser`, causing it to inherit the parent's behavior of routing
`installation_repositories` webhooks to the control silo. However, the
`GitHubEnterpriseWebhookEndpoint` does not have a handler registered for this event
type. As a result, when a GitHub Enterprise instance sends an
`installation_repositories` webhook, the endpoint's `_handle` method will find no
handler and silently return a `204` status. This prevents the necessary repository
synchronization tasks from running when repositories are added to or removed from a GHE
app installation.
Did we get this right? 👍 / 👎 to inform future reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove the should_route_to_control_silo override in GithubEnterpriseRequestParser so it inherits the parent's routing, which sends installation_repositories events to control silo.
We do this to make sure the webhooks for GHE will also route to the right place