Browse Source

fix(hybrid-cloud): Fixes gitlab webhook processing failures (#61501)

Gabe Villalobos 1 year ago
parent
commit
290d71ff5d

+ 0 - 1
pyproject.toml

@@ -500,7 +500,6 @@ module = [
     "sentry.profiles.task",
     "sentry.profiles.utils",
     "sentry.quotas.redis",
-    "sentry.receivers.outbox.control",
     "sentry.receivers.releases",
     "sentry.receivers.sentry_apps",
     "sentry.release_health.metrics_sessions_v2",

+ 2 - 2
src/sentry/integrations/gitlab/webhooks.py

@@ -276,7 +276,7 @@ class GitlabWebhookEndpoint(Endpoint, GitlabWebhookMixin):
             logger.info("gitlab.webhook.invalid-organization", extra=extra)
             extra["reason"] = "There is no integration that matches your organization."
             logger.error(extra["reason"])
-            return HttpResponse(status=400, reason=extra["reason"])
+            return HttpResponse(status=409, reason=extra["reason"])
 
         extra = {
             **extra,
@@ -305,7 +305,7 @@ class GitlabWebhookEndpoint(Endpoint, GitlabWebhookMixin):
                 "reason"
             ] = "Gitlab's webhook secret does not match. Refresh token (or re-install the integration) by following this https://docs.sentry.io/product/integrations/integration-platform/public-integration/#refreshing-tokens."
             logger.exception(extra["reason"])
-            return HttpResponse(status=400, reason=extra["reason"])
+            return HttpResponse(status=409, reason=extra["reason"])
 
         try:
             event = json.loads(request.body.decode("utf-8"))

+ 22 - 16
src/sentry/receivers/outbox/control.py

@@ -19,6 +19,7 @@ from sentry.models.outbox import OutboxCategory, process_control_outbox
 from sentry.receivers.outbox import maybe_process_tombstone
 from sentry.services.hybrid_cloud.issue import issue_service
 from sentry.services.hybrid_cloud.organization import RpcOrganizationSignal, organization_service
+from sentry.shared_integrations.exceptions import ApiConflictError
 from sentry.silo.base import SiloMode
 
 logger = logging.getLogger(__name__)
@@ -68,22 +69,27 @@ def process_async_webhooks(payload: Mapping[str, Any], region_name: str, **kwds:
 
     if SiloMode.get_current_mode() == SiloMode.CONTROL:
         # By default, these clients will raise errors on non-20x response codes
-        response = RegionSiloClient(region=region).request(
-            method=webhook_payload.method,
-            path=webhook_payload.path,
-            headers=webhook_payload.headers,
-            # We need to send the body as raw bytes to avoid interfering with webhook signatures
-            data=webhook_payload.body,
-            json=False,
-        )
-        logger.info(
-            "webhook_proxy.complete",
-            extra={
-                "status": response.status_code,
-                "request_path": webhook_payload.path,
-                "request_method": webhook_payload.method,
-            },
-        )
+        try:
+            response = RegionSiloClient(region=region).request(
+                method=webhook_payload.method,
+                path=webhook_payload.path,
+                headers=webhook_payload.headers,
+                # We need to send the body as raw bytes to avoid interfering with webhook signatures
+                data=webhook_payload.body,
+                json=False,
+            )
+            logger.info(
+                "webhook_proxy.complete",
+                extra={
+                    "status": getattr(
+                        response, "status_code", 204
+                    ),  # Request returns empty dict instead of a response object when the code is a 204
+                    "request_path": webhook_payload.path,
+                    "request_method": webhook_payload.method,
+                },
+            )
+        except ApiConflictError as e:
+            logger.warning("webhook_proxy.conflict_occurred", extra={"conflict_text": e.text})
 
 
 @receiver(process_control_outbox, sender=OutboxCategory.SEND_SIGNAL)

+ 4 - 0
src/sentry/shared_integrations/exceptions/__init__.py

@@ -59,6 +59,10 @@ class ApiRateLimitedError(ApiError):
     code = 429
 
 
+class ApiConflictError(ApiError):
+    code = 409
+
+
 class ApiConnectionResetError(ApiError):
     code = errno.ECONNRESET
 

+ 8 - 1
src/sentry/shared_integrations/exceptions/base.py

@@ -63,10 +63,17 @@ class ApiError(Exception):
 
     @classmethod
     def from_response(cls, response: Response, url: str | None = None) -> ApiError:
-        from sentry.shared_integrations.exceptions import ApiRateLimitedError, ApiUnauthorized
+        from sentry.shared_integrations.exceptions import (
+            ApiConflictError,
+            ApiRateLimitedError,
+            ApiUnauthorized,
+        )
 
         if response.status_code == 401:
             return ApiUnauthorized(response.text, url=url)
         elif response.status_code == 429:
             return ApiRateLimitedError(response.text, url=url)
+        elif response.status_code == 409:
+            return ApiConflictError(response.text, url=url)
+
         return cls(response.text, response.status_code, url=url)

+ 20 - 2
tests/sentry/integrations/gitlab/test_webhook.py

@@ -9,9 +9,10 @@ from fixtures.gitlab import (
 from sentry.models.commit import Commit
 from sentry.models.commitauthor import CommitAuthor
 from sentry.models.grouplink import GroupLink
+from sentry.models.integrations import Integration
 from sentry.models.pullrequest import PullRequest
 from sentry.silo import SiloMode
-from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
+from sentry.testutils.silo import assume_test_silo_mode, assume_test_silo_mode_of, region_silo_test
 from sentry.utils import json
 
 
@@ -88,7 +89,7 @@ class WebhookTest(GitLabTestCase):
             HTTP_X_GITLAB_TOKEN=f"{EXTERNAL_ID}:wrong",
             HTTP_X_GITLAB_EVENT="Push Hook",
         )
-        assert response.status_code == 400
+        assert response.status_code == 409
         assert (
             response.reason_phrase
             == "Gitlab's webhook secret does not match. Refresh token (or re-install the integration) by following this https://docs.sentry.io/product/integrations/integration-platform/public-integration/#refreshing-tokens."
@@ -385,3 +386,20 @@ class WebhookTest(GitLabTestCase):
         # url has been updated
         repo_out_of_date_url.refresh_from_db()
         assert repo_out_of_date_url.url == "http://example.com/cool-group/sentry"
+
+    def test_no_valid_integration_for_organization(self):
+        self.create_repo("getsentry/sentry")
+        self.create_group(project=self.project, short_id=9)
+
+        with assume_test_silo_mode_of(Integration):
+            self.integration.delete()
+
+        response = self.client.post(
+            self.url,
+            data=MERGE_REQUEST_OPENED_EVENT,
+            content_type="application/json",
+            HTTP_X_GITLAB_TOKEN=WEBHOOK_TOKEN,
+            HTTP_X_GITLAB_EVENT="Merge Request Hook",
+        )
+        assert response.status_code == 409
+        assert response.reason_phrase == "There is no integration that matches your organization."