ref(opsgenie): verify integration key upon key save rather than upon alert rule save (#67081)

Author: cathteng

src/sentry/api/endpoints/integrations/organization_integrations/details.py

@@ -21,7 +21,7 @@
 from sentry.models.integrations.organization_integration import OrganizationIntegration
 from sentry.models.scheduledeletion import ScheduledDeletion
 from sentry.services.hybrid_cloud.organization import RpcUserOrganizationContext
-from sentry.shared_integrations.exceptions import IntegrationError
+from sentry.shared_integrations.exceptions import ApiError, IntegrationError
 from sentry.utils.audit import create_audit_entry
 from sentry.web.decorators import set_referrer_policy
 
@@ -118,7 +118,7 @@ def post(
         )
         try:
             installation.update_organization_config(request.data)
-        except IntegrationError as e:
-            return self.respond({"detail": str(e)}, status=400)
+        except (IntegrationError, ApiError) as e:
+            return self.respond({"detail": [str(e)]}, status=400)
 
         return self.respond(status=200)

src/sentry/incidents/logic.py

@@ -5,7 +5,7 @@
 from copy import deepcopy
 from dataclasses import replace
 from datetime import datetime, timedelta, timezone
-from typing import Any, cast
+from typing import Any
 from uuid import uuid4
 
 from django.db import router, transaction
@@ -58,7 +58,6 @@
 from sentry.services.hybrid_cloud.integration import RpcIntegration, integration_service
 from sentry.services.hybrid_cloud.integration.model import RpcOrganizationIntegration
 from sentry.shared_integrations.exceptions import (
-    ApiError,
     ApiTimeoutError,
     DuplicateDisplayNameError,
     IntegrationError,
@@ -1476,7 +1475,6 @@ def get_alert_rule_trigger_action_opsgenie_team(
     input_channel_id=None,
     integrations=None,
 ) -> tuple[str, str]:
-    from sentry.integrations.opsgenie.integration import OpsgenieIntegration
     from sentry.integrations.opsgenie.utils import get_team
 
     integration, oi = integration_service.get_organization_context(
@@ -1489,17 +1487,6 @@ def get_alert_rule_trigger_action_opsgenie_team(
     if not team:
         raise InvalidTriggerActionError("No Opsgenie team found.")
 
-    install = cast(
-        "OpsgenieIntegration",
-        integration.get_installation(organization_id=organization.id),
-    )
-    client = install.get_keyring_client(keyid=team["id"])
-
-    try:
-        client.authorize_integration(type="sentry")
-    except ApiError as e:
-        logger.info("opsgenie.authorization_error", extra={"error": str(e)})
-        raise InvalidTriggerActionError("Invalid integration key.")
     return team["id"], team["team"]
 
 

src/sentry/integrations/opsgenie/client.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 from typing import Literal
-from urllib.parse import quote
 
 from sentry.eventstore.models import Event, GroupEvent
 from sentry.integrations.client import ApiClient
@@ -33,13 +32,9 @@ def metadata(self):
     def _get_auth_headers(self):
         return {"Authorization": f"GenieKey {self.integration_key}"}
 
-    # This doesn't work if the team name is "." or "..", which Opsgenie allows for some reason
-    # despite their API not working with these names.
-    def get_team_id(self, team_name: str) -> BaseApiResponseX:
-        params = {"identifierType": "name"}
-        quoted_name = quote(team_name)
-        path = f"/teams/{quoted_name}"
-        return self.get(path=path, headers=self._get_auth_headers(), params=params)
+    def get_alerts(self, limit: int | None = 1) -> BaseApiResponseX:
+        path = f"/alerts?limit={limit}"
+        return self.get(path=path, headers=self._get_auth_headers())
 
     def authorize_integration(self, type: str) -> BaseApiResponseX:
         body = {"type": type}

src/sentry/integrations/opsgenie/integration.py

@@ -21,6 +21,12 @@
 from sentry.models.integrations.organization_integration import OrganizationIntegration
 from sentry.pipeline import PipelineView
 from sentry.services.hybrid_cloud.organization import RpcOrganizationSummary
+from sentry.shared_integrations.exceptions import (
+    ApiError,
+    ApiRateLimitedError,
+    ApiUnauthorized,
+    IntegrationError,
+)
 from sentry.tasks.integrations import migrate_opsgenie_plugin
 from sentry.web.helpers import render_to_response
 
@@ -147,6 +153,8 @@ def get_organization_config(self) -> Sequence[Any]:
         return fields
 
     def update_organization_config(self, data: MutableMapping[str, Any]) -> None:
+        from sentry.services.hybrid_cloud.integration import integration_service
+
         # add the integration ID to a newly added row
         if not self.org_integration:
             return
@@ -156,10 +164,57 @@ def update_organization_config(self, data: MutableMapping[str, Any]) -> None:
         # this is not instantaneous, so you could add the same team a bunch of times in a row
         # but I don't anticipate this being too much of an issue
         added_names = {team["team"] for team in teams if team not in unsaved_teams}
+        existing_team_key_pairs = {
+            (team["team"], team["integration_key"]) for team in teams if team not in unsaved_teams
+        }
+
+        integration = integration_service.get_integration(
+            organization_integration_id=self.org_integration.id
+        )
+        if not integration:
+            raise IntegrationError("Integration does not exist")
+
         for team in unsaved_teams:
             if team["team"] in added_names:
                 raise ValidationError({"duplicate_name": ["Duplicate team name."]})
             team["id"] = str(self.org_integration.id) + "-" + team["team"]
+
+        invalid_keys = []
+        for team in teams:
+            # skip if team, key pair already exist in config
+            if (team["team"], team["integration_key"]) in existing_team_key_pairs:
+                continue
+
+            integration_key = team["integration_key"]
+
+            # validate integration keys
+            client = OpsgenieClient(
+                integration=integration,
+                integration_key=integration_key,
+            )
+            # call an API to test the integration key
+            try:
+                client.get_alerts()
+            except ApiError as e:
+                logger.info(
+                    "opsgenie.authorization_error",
+                    extra={"error": str(e), "status_code": e.code},
+                )
+                if e.code == 429:
+                    raise ApiRateLimitedError(
+                        "Too many requests. Please try updating one team/key at a time."
+                    )
+                elif e.code == 401:
+                    invalid_keys.append(integration_key)
+                    pass
+                elif e.json and e.json.get("message"):
+                    raise ApiError(e.json["message"])
+                else:
+                    raise
+
+        if invalid_keys:
+            raise ApiUnauthorized(f"Invalid integration key: {str(invalid_keys)}")
+
         return super().update_organization_config(data)
 
     def schedule_migrate_opsgenie_plugin(self):

tests/sentry/api/endpoints/test_organization_integration_details.py

@@ -1,8 +1,12 @@
+from unittest.mock import patch
+
+from sentry.integrations.base import IntegrationInstallation
 from sentry.models.identity import Identity
 from sentry.models.integrations.integration import Integration
 from sentry.models.integrations.organization_integration import OrganizationIntegration
 from sentry.models.repository import Repository
 from sentry.models.scheduledeletion import ScheduledDeletion
+from sentry.shared_integrations.exceptions import ApiError, IntegrationError
 from sentry.silo import SiloMode
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import assume_test_silo_mode, control_silo_test
@@ -58,6 +62,22 @@ def test_update_config(self):
 
         assert org_integration.config == config
 
+    @patch.object(IntegrationInstallation, "update_organization_config")
+    def test_update_config_error(self, mock_update_config):
+        config = {"setting": "new_value", "setting2": "baz"}
+
+        mock_update_config.side_effect = IntegrationError("hello")
+        response = self.get_error_response(
+            self.organization.slug, self.integration.id, **config, status_code=400
+        )
+        assert response.data["detail"] == ["hello"]
+
+        mock_update_config.side_effect = ApiError("hi")
+        response = self.get_error_response(
+            self.organization.slug, self.integration.id, **config, status_code=400
+        )
+        assert response.data["detail"] == ["hi"]
+
 
 @control_silo_test
 class OrganizationIntegrationDetailsDeleteTest(OrganizationIntegrationDetailsTest):

tests/sentry/integrations/opsgenie/test_integration.py

@@ -9,6 +9,7 @@
 from sentry.models.integrations.integration import Integration
 from sentry.models.integrations.organization_integration import OrganizationIntegration
 from sentry.models.rule import Rule
+from sentry.shared_integrations.exceptions import ApiRateLimitedError, ApiUnauthorized
 from sentry.tasks.integrations.migrate_opsgenie_plugins import (
     ALERT_LEGACY_INTEGRATIONS,
     ALERT_LEGACY_INTEGRATIONS_WITH_NAME,
@@ -134,6 +135,10 @@ def test_update_config_valid(self):
         integration = Integration.objects.get(provider=self.provider.key)
         org_integration = OrganizationIntegration.objects.get(integration_id=integration.id)
 
+        responses.add(
+            responses.GET, url="https://api.opsgenie.com/v2/alerts?limit=1", status=200, json={}
+        )
+
         data = {"team_table": [{"id": "", "team": "cool-team", "integration_key": "1234-5678"}]}
         installation.update_organization_config(data)
         team_id = str(org_integration.id) + "-" + "cool-team"
@@ -153,6 +158,10 @@ def test_update_config_invalid(self):
         org_integration = OrganizationIntegration.objects.get(integration_id=integration.id)
         team_id = str(org_integration.id) + "-" + "cool-team"
 
+        responses.add(
+            responses.GET, url="https://api.opsgenie.com/v2/alerts?limit=1", status=200, json={}
+        )
+
         # valid
         data = {"team_table": [{"id": "", "team": "cool-team", "integration_key": "1234"}]}
         installation.update_organization_config(data)
@@ -173,6 +182,44 @@ def test_update_config_invalid(self):
             "team_table": [{"id": team_id, "team": "cool-team", "integration_key": "1234"}]
         }
 
+    @responses.activate
+    def test_update_config_invalid_rate_limited(self):
+        integration = self.create_provider_integration(
+            provider="opsgenie", name="test-app", external_id=EXTERNAL_ID, metadata=METADATA
+        )
+        integration.add_organization(self.organization, self.user)
+        installation = integration.get_installation(self.organization.id)
+
+        data = {
+            "team_table": [
+                {"id": "", "team": "rad-team", "integration_key": "4321"},
+                {"id": "cool-team", "team": "cool-team", "integration_key": "1234"},
+            ]
+        }
+        responses.add(responses.GET, url="https://api.opsgenie.com/v2/alerts?limit=1", status=429)
+
+        with pytest.raises(ApiRateLimitedError):
+            installation.update_organization_config(data)
+
+    @responses.activate
+    def test_update_config_invalid_integration_key(self):
+        integration = self.create_provider_integration(
+            provider="opsgenie", name="test-app", external_id=EXTERNAL_ID, metadata=METADATA
+        )
+        integration.add_organization(self.organization, self.user)
+        installation = integration.get_installation(self.organization.id)
+
+        data = {
+            "team_table": [
+                {"id": "cool-team", "team": "cool-team", "integration_key": "1234"},
+                {"id": "", "team": "rad-team", "integration_key": "4321"},
+            ]
+        }
+        responses.add(responses.GET, url="https://api.opsgenie.com/v2/alerts?limit=1", status=401)
+
+        with pytest.raises(ApiUnauthorized):
+            installation.update_organization_config(data)
+
 
 @region_silo_test
 class OpsgenieMigrationIntegrationTest(APITestCase):