Browse Source

feat(vsts): Add request parser for vsts (#52387)

Alberto Leal 1 year ago
parent
commit
ffcccc2201

+ 1 - 1
CHANGES

@@ -15,7 +15,7 @@ By: @armenzg (#51040)
 
 ### Search Shortcuts (ongoing)
 
-Make common searches more accessible and discoverable for users. [notion page](https://www.notion.so/sentry/Search-Shortcuts-fka-Assignee-Filter-on-Issue-Stream-648954e782d04805bc64f1983e5cbd16?pvs=4) 
+Make common searches more accessible and discoverable for users. [notion page](https://www.notion.so/sentry/Search-Shortcuts-fka-Assignee-Filter-on-Issue-Stream-648954e782d04805bc64f1983e5cbd16?pvs=4)
 
 By: @scttcper (#51564, #51565, #51194)
 

+ 11 - 4
src/sentry/integrations/vsts/webhooks.py

@@ -23,17 +23,24 @@ logger = logging.getLogger("sentry.integrations")
 PROVIDER_KEY = "vsts"
 
 
+class VstsWebhookMixin:
+    def get_external_id(self, request: Request) -> str:
+        data = request.data
+        external_id = data["resourceContainers"]["collection"]["id"]
+        return str(external_id)
+
+
 @region_silo_endpoint
-class WorkItemWebhook(Endpoint):
+class WorkItemWebhook(Endpoint, VstsWebhookMixin):
     authentication_classes = ()
     permission_classes = ()
 
     def post(self, request: Request, *args: Any, **kwargs: Any) -> Response:
-        data = request.data
         try:
+            data = request.data
             event_type = data["eventType"]
-            external_id = data["resourceContainers"]["collection"]["id"]
-        except KeyError as e:
+            external_id = self.get_external_id(request=request)
+        except Exception as e:
             logger.info("vsts.invalid-webhook-payload", extra={"error": str(e)})
             return self.respond(status=status.HTTP_400_BAD_REQUEST)
 

+ 2 - 0
src/sentry/middleware/integrations/integration_control.py

@@ -13,6 +13,7 @@ from .parsers import (
     JiraRequestParser,
     MsTeamsRequestParser,
     SlackRequestParser,
+    VstsRequestParser,
 )
 from .parsers.base import BaseRequestParser
 
@@ -25,6 +26,7 @@ ACTIVE_PARSERS = [
     SlackRequestParser,
     MsTeamsRequestParser,
     BitbucketRequestParser,
+    VstsRequestParser,
 ]
 
 

+ 2 - 0
src/sentry/middleware/integrations/parsers/__init__.py

@@ -4,6 +4,7 @@ from .gitlab import GitlabRequestParser
 from .jira import JiraRequestParser
 from .msteams import MsTeamsRequestParser
 from .slack import SlackRequestParser
+from .vsts import VstsRequestParser
 
 __all__ = (
     "BitbucketRequestParser",
@@ -12,4 +13,5 @@ __all__ = (
     "JiraRequestParser",
     "MsTeamsRequestParser",
     "SlackRequestParser",
+    "VstsRequestParser",
 )

+ 42 - 0
src/sentry/middleware/integrations/parsers/vsts.py

@@ -0,0 +1,42 @@
+from __future__ import annotations
+
+import logging
+from typing import cast
+
+from django.http import HttpResponse
+from rest_framework.request import Request
+
+from sentry.integrations.vsts.webhooks import VstsWebhookMixin, WorkItemWebhook
+from sentry.middleware.integrations.parsers.base import BaseRequestParser
+from sentry.models.integrations.integration import Integration
+from sentry.models.outbox import WebhookProviderIdentifier
+from sentry.services.hybrid_cloud.util import control_silo_function
+
+logger = logging.getLogger(__name__)
+
+
+class VstsRequestParser(BaseRequestParser, VstsWebhookMixin):
+    provider = "vsts"
+    webhook_identifier = WebhookProviderIdentifier.VSTS
+
+    region_view_classes = [WorkItemWebhook]
+
+    @control_silo_function
+    def get_integration_from_request(self) -> Integration | None:
+        try:
+            external_id = self.get_external_id(request=cast(Request, self.request))
+        except Exception:
+            return None
+        return Integration.objects.filter(external_id=external_id, provider=self.provider).first()
+
+    def get_response(self) -> HttpResponse:
+        view_class = self.match.func.view_class  # type: ignore
+        if view_class not in self.region_view_classes:
+            return self.get_response_from_control_silo()
+
+        regions = self.get_regions_from_organizations()
+        if len(regions) == 0:
+            logger.error("no_regions", extra={"path": self.request.path})
+            return self.get_response_from_control_silo()
+
+        return self.get_response_from_outbox_creation(regions=regions)

+ 1 - 0
src/sentry/models/outbox.py

@@ -101,6 +101,7 @@ class WebhookProviderIdentifier(IntEnum):
     GITLAB = 3
     MSTEAMS = 4
     BITBUCKET = 5
+    VSTS = 6
 
 
 def _ensure_not_null(k: str, v: Any) -> Any:

+ 172 - 0
tests/sentry/middleware/integrations/parsers/test_vsts.py

@@ -0,0 +1,172 @@
+from copy import deepcopy
+from typing import Any
+from unittest import mock
+from unittest.mock import MagicMock
+
+from django.http import HttpResponse
+from django.test import override_settings
+from django.urls import reverse
+from rest_framework.test import APIRequestFactory
+
+from fixtures.vsts import WORK_ITEM_UNASSIGNED, WORK_ITEM_UPDATED, WORK_ITEM_UPDATED_STATUS
+from sentry.middleware.integrations.integration_control import IntegrationControlMiddleware
+from sentry.middleware.integrations.parsers.vsts import VstsRequestParser
+from sentry.models import Integration
+from sentry.models.outbox import (
+    ControlOutbox,
+    OutboxCategory,
+    OutboxScope,
+    WebhookProviderIdentifier,
+)
+from sentry.silo.base import SiloMode
+from sentry.testutils import TestCase
+from sentry.testutils.silo import control_silo_test
+from sentry.types.region import Region, RegionCategory
+
+
+@control_silo_test(stable=True)
+class VstsRequestParserTest(TestCase):
+    get_response = MagicMock(return_value=HttpResponse(content=b"no-error", status=200))
+    factory = APIRequestFactory()
+    path = f"{IntegrationControlMiddleware.integration_prefix}vsts/issue-updated/"
+    region = Region("na", 1, "https://na.testserver", RegionCategory.MULTI_TENANT)
+
+    def setUp(self):
+        super().setUp()
+        self.shared_secret = "1234567890"
+
+    def set_workitem_state(self, old_value, new_value):
+        work_item = deepcopy(WORK_ITEM_UPDATED_STATUS)
+        state = work_item["resource"]["fields"]["System.State"]  # type: ignore
+
+        if old_value is None:
+            del state["oldValue"]
+        else:
+            state["oldValue"] = old_value
+        state["newValue"] = new_value
+
+        return work_item
+
+    @override_settings(SILO_MODE=SiloMode.CONTROL)
+    def test_routing_properly(self):
+        request = self.factory.post(
+            self.path,
+            data=WORK_ITEM_UPDATED,
+            format="json",
+            HTTP_SHARED_SECRET=self.shared_secret,
+        )
+        parser = VstsRequestParser(request=request, response_handler=self.get_response)
+
+        # No regions identified
+        with mock.patch.object(
+            parser, "get_response_from_control_silo"
+        ) as get_response_from_control_silo, mock.patch.object(
+            parser, "get_regions_from_organizations", return_value=[]
+        ):
+            parser.get_response()
+            assert get_response_from_control_silo.called
+
+        # Regions found
+        with mock.patch.object(
+            parser, "get_response_from_outbox_creation"
+        ) as get_response_from_outbox_creation, mock.patch.object(
+            parser, "get_regions_from_organizations", return_value=[self.region]
+        ):
+            parser.get_response()
+            assert get_response_from_outbox_creation.called
+
+        # Non-webhook urls
+        with mock.patch.object(
+            parser, "get_response_from_outbox_creation"
+        ) as get_response_from_outbox_creation, mock.patch.object(
+            parser, "get_response_from_control_silo"
+        ) as get_response_from_control_silo:
+            parser.request = self.factory.get(
+                reverse("vsts-extension-configuration"), data={"targetId": "1", "targetName": "foo"}
+            )
+            parser.get_response()
+            assert get_response_from_control_silo.called
+            assert not get_response_from_outbox_creation.called
+
+            parser.request = self.factory.get(
+                reverse(
+                    "sentry-extensions-vsts-search",
+                    kwargs={"organization_slug": "albertos-apples", "integration_id": 1234},
+                ),
+            )
+            parser.get_response()
+            assert get_response_from_control_silo.called
+
+    @override_settings(SILO_MODE=SiloMode.CONTROL)
+    def test_get_integration_from_request(self):
+        account_id = "80ded3e8-3cd3-43b1-9f96-52032624aa3a"
+        expected_integration = Integration.objects.create(
+            provider="vsts",
+            external_id=account_id,
+            name="vsts_name",
+            metadata={
+                "domain_name": "https://instance.visualstudio.com/",
+                "subscription": {"id": 1234, "secret": self.shared_secret},
+            },
+        )
+
+        request = self.factory.post(
+            self.path,
+            format="json",
+            HTTP_SHARED_SECRET=self.shared_secret,
+        )
+
+        region_silo_payloads = [WORK_ITEM_UNASSIGNED, WORK_ITEM_UPDATED, WORK_ITEM_UPDATED_STATUS]
+
+        for payload in region_silo_payloads:
+            request.data = payload  # type:ignore
+            parser = VstsRequestParser(request=request, response_handler=self.get_response)
+            integration = parser.get_integration_from_request()
+            assert integration == expected_integration
+
+        # Invalid payload
+        request.data = {"nonsense": True}  # type:ignore
+        parser = VstsRequestParser(request=request, response_handler=self.get_response)
+        integration = parser.get_integration_from_request()
+        assert integration is None
+
+    @override_settings(SILO_MODE=SiloMode.CONTROL)
+    def test_webhook_outbox_creation(self):
+        request = self.factory.post(
+            self.path,
+            data=WORK_ITEM_UPDATED,
+            format="json",
+            HTTP_SHARED_SECRET=self.shared_secret,
+        )
+        parser = VstsRequestParser(request=request, response_handler=self.get_response)
+
+        assert ControlOutbox.objects.count() == 0
+        with mock.patch.object(
+            parser, "get_regions_from_organizations", return_value=[self.region]
+        ):
+            parser.get_response()
+
+            assert ControlOutbox.objects.count() == 1
+            outbox = ControlOutbox.objects.first()
+            expected_payload: Any = {
+                "method": "POST",
+                "path": self.path,
+                "uri": f"http://testserver{self.path}",
+                "headers": {
+                    "Shared-Secret": self.shared_secret,
+                    "Content-Length": "4652",
+                    "Content-Type": "application/json",
+                    "Cookie": "",
+                },
+                "body": request.body.decode(encoding="utf-8"),
+            }
+            assert outbox.payload == expected_payload
+            assert outbox == ControlOutbox(
+                id=outbox.id,
+                shard_scope=OutboxScope.WEBHOOK_SCOPE,
+                shard_identifier=WebhookProviderIdentifier.VSTS,
+                object_identifier=1,
+                category=OutboxCategory.WEBHOOK_PROXY,
+                region_name=self.region.name,
+                payload=expected_payload,
+            )