Browse Source

fix(opsgenie): fix alert rule migration behavior (#55598)

Alert rules were not being correctly migrated because 1) the action for
legacy alerts changed and 2) the account ID was incorrect. Fix these
issues.
Michelle Fu 1 year ago
parent
commit
8595bb0a8b

+ 5 - 5
src/sentry/tasks/integrations/migrate_opsgenie_plugin.py

@@ -7,7 +7,8 @@ from sentry.models.integrations.organization_integration import OrganizationInte
 from sentry.services.hybrid_cloud.integration.service import integration_service
 from sentry.tasks.base import instrumented_task, retry
 
-ALERT_LEGACY_INTEGRATIONS = {
+ALERT_LEGACY_INTEGRATIONS = {"id": "sentry.rules.actions.notify_event.NotifyEventAction"}
+ALERT_LEGACY_INTEGRATIONS_WITH_NAME = {
     "id": "sentry.rules.actions.notify_event.NotifyEventAction",
     "name": "Send a notification (for all legacy integrations)",
 }
@@ -52,7 +53,7 @@ def migrate_opsgenie_plugin(integration_id: int, organization_id: int) -> None:
             seen_keys[api_key] = len(team_table)
             team = {
                 "team": f"{project.name} [MIGRATED]",
-                "id": f"{str(organization_id)}-{project.name}",
+                "id": f"{str(organization_integration.id)}-{project.name}",
                 "integration_key": api_key,
             }
             team_table.append(team)
@@ -68,7 +69,6 @@ def migrate_opsgenie_plugin(integration_id: int, organization_id: int) -> None:
         extra={
             "integration_id": integration_id,
             "organization_id": organization_id,
-            "project_id": project.id,
             "plugin": plugin.slug,
         },
     )
@@ -81,14 +81,14 @@ def migrate_opsgenie_plugin(integration_id: int, organization_id: int) -> None:
             rule
             for rule in Rule.objects.filter(project_id=project.id)
             if ALERT_LEGACY_INTEGRATIONS in rule.data["actions"]
+            or ALERT_LEGACY_INTEGRATIONS_WITH_NAME in rule.data["actions"]
         ]
-
         with transaction.atomic(router.db_for_write(Rule)):
             for rule in rules_to_migrate:
                 actions = rule.data["actions"]
                 new_action = {
                     "id": "sentry.integrations.opsgenie.notify_action.OpsgenieNotifyTeamAction",
-                    "account": organization_integration.id,
+                    "account": integration.id,
                     "team": team["id"],
                 }
                 if new_action not in actions:

+ 63 - 18
tests/sentry/integrations/opsgenie/test_integration.py

@@ -8,7 +8,10 @@ from sentry.integrations.opsgenie.integration import OpsgenieIntegrationProvider
 from sentry.models import Rule
 from sentry.models.integrations.integration import Integration
 from sentry.models.integrations.organization_integration import OrganizationIntegration
-from sentry.tasks.integrations.migrate_opsgenie_plugin import ALERT_LEGACY_INTEGRATIONS
+from sentry.tasks.integrations.migrate_opsgenie_plugin import (
+    ALERT_LEGACY_INTEGRATIONS,
+    ALERT_LEGACY_INTEGRATIONS_WITH_NAME,
+)
 from sentry.testutils.cases import APITestCase, IntegrationTestCase
 from sentry.testutils.silo import control_silo_test
 from sentry_plugins.opsgenie.plugin import OpsGeniePlugin
@@ -181,8 +184,8 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
             self.installation.schedule_migrate_opsgenie_plugin()
 
         org_integration = OrganizationIntegration.objects.get(integration_id=self.integration.id)
-        id1 = str(self.organization.id) + "-thonk"
-        id2 = str(self.organization.id) + "-thinkies"
+        id1 = str(self.organization_integration.id) + "-thonk"
+        id2 = str(self.organization_integration.id) + "-thinkies"
         assert org_integration.config == {
             "team_table": [
                 {"id": id1, "team": "thonk [MIGRATED]", "integration_key": "123-key"},
@@ -199,7 +202,7 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
             ALERT_LEGACY_INTEGRATIONS,
             {
                 "id": "sentry.integrations.opsgenie.notify_action.OpsgenieNotifyTeamAction",
-                "account": org_integration.id,
+                "account": self.integration.id,
                 "team": id1,
             },
         ]
@@ -212,7 +215,7 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
             ALERT_LEGACY_INTEGRATIONS,
             {
                 "id": "sentry.integrations.opsgenie.notify_action.OpsgenieNotifyTeamAction",
-                "account": org_integration.id,
+                "account": self.integration.id,
                 "team": id2,
             },
         ]
@@ -242,7 +245,7 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
             self.installation.schedule_migrate_opsgenie_plugin()
 
         org_integration = OrganizationIntegration.objects.get(integration_id=self.integration.id)
-        id1 = str(self.organization.id) + "-thonk"
+        id1 = str(self.organization_integration.id) + "-thonk"
 
         assert org_integration.config == {
             "team_table": [
@@ -258,7 +261,7 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
         org_integration.config = {
             "team_table": [
                 {
-                    "id": str(self.organization.id) + "-pikachu",
+                    "id": str(self.organization_integration.id) + "-pikachu",
                     "team": "pikachu",
                     "integration_key": "123-key",
                 },
@@ -278,7 +281,7 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
         assert org_integration.config == {
             "team_table": [
                 {
-                    "id": str(self.organization.id) + "-pikachu",
+                    "id": str(self.organization_integration.id) + "-pikachu",
                     "team": "pikachu",
                     "integration_key": "123-key",
                 },
@@ -294,8 +297,8 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
             ALERT_LEGACY_INTEGRATIONS,
             {
                 "id": "sentry.integrations.opsgenie.notify_action.OpsgenieNotifyTeamAction",
-                "account": org_integration.id,
-                "team": str(self.organization.id) + "-pikachu",
+                "account": self.integration.id,
+                "team": str(self.organization_integration.id) + "-pikachu",
             },
         ]
 
@@ -323,7 +326,7 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
             self.installation.schedule_migrate_opsgenie_plugin()
 
         org_integration = OrganizationIntegration.objects.get(integration_id=self.integration.id)
-        id1 = str(self.organization.id) + "-thonk"
+        id1 = str(self.organization_integration.id) + "-thonk"
         rule_updated = Rule.objects.get(
             label="rule",
             project=self.project,
@@ -333,7 +336,7 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
             ALERT_LEGACY_INTEGRATIONS,
             {
                 "id": "sentry.integrations.opsgenie.notify_action.OpsgenieNotifyTeamAction",
-                "account": org_integration.id,
+                "account": self.integration.id,
                 "team": id1,
             },
         ]
@@ -353,7 +356,7 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
         org_integration.config = {
             "team_table": [
                 {
-                    "id": str(self.organization.id) + "-pikachu",
+                    "id": str(self.organization_integration.id) + "-pikachu",
                     "team": "pikachu",
                     "integration_key": "123-key",
                 },
@@ -370,8 +373,8 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
                     ALERT_LEGACY_INTEGRATIONS,
                     {
                         "id": "sentry.integrations.opsgenie.notify_action.OpsgenieNotifyTeamAction",
-                        "account": org_integration.id,
-                        "team": str(self.organization.id) + "-pikachu",
+                        "account": self.integration.id,
+                        "team": str(self.organization_integration.id) + "-pikachu",
                     },
                 ],
             },
@@ -383,7 +386,7 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
         assert org_integration.config == {
             "team_table": [
                 {
-                    "id": str(self.organization.id) + "-pikachu",
+                    "id": str(self.organization_integration.id) + "-pikachu",
                     "team": "pikachu",
                     "integration_key": "123-key",
                 },
@@ -399,7 +402,49 @@ class OpsgenieMigrationIntegrationTest(APITestCase):
             ALERT_LEGACY_INTEGRATIONS,
             {
                 "id": "sentry.integrations.opsgenie.notify_action.OpsgenieNotifyTeamAction",
-                "account": org_integration.id,
-                "team": str(self.organization.id) + "-pikachu",
+                "account": self.integration.id,
+                "team": str(self.organization_integration.id) + "-pikachu",
             },
         ]
+
+    def test_migrate_plugin_with_name(self):
+        """
+        Test that the Opsgenie plugin is migrated correctly if the legacy alert action has a name field.
+        """
+        org_integration = OrganizationIntegration.objects.get(integration_id=self.integration.id)
+        org_integration.config = {"team_table": []}
+        org_integration.save()
+
+        Rule.objects.create(
+            label="rule",
+            project=self.project,
+            data={"match": "all", "actions": [ALERT_LEGACY_INTEGRATIONS_WITH_NAME]},
+        )
+
+        with self.tasks():
+            self.installation.schedule_migrate_opsgenie_plugin()
+
+        org_integration = OrganizationIntegration.objects.get(integration_id=self.integration.id)
+        id1 = str(self.organization_integration.id) + "-thonk"
+        assert org_integration.config == {
+            "team_table": [
+                {"id": id1, "team": "thonk [MIGRATED]", "integration_key": "123-key"},
+            ]
+        }
+
+        rule_updated = Rule.objects.get(
+            label="rule",
+            project=self.project,
+        )
+
+        assert rule_updated.data["actions"] == [
+            ALERT_LEGACY_INTEGRATIONS,
+            {
+                "id": "sentry.integrations.opsgenie.notify_action.OpsgenieNotifyTeamAction",
+                "account": self.integration.id,
+                "team": id1,
+            },
+        ]
+
+        assert self.plugin.is_enabled(self.project) is False
+        assert self.plugin.is_configured(self.project) is False