ref(hc): Fold PagerDutyService objects into OrganizationIntegration config directly. (#53342)

1. Switch back PagerDutyService to control silo now that getsentry is
updated
2. On save, put pagerdutyservice configuration into the organization
integration config object
3. Change sentry read paths to use this
4. Migrate all PagerDutyService objects to backfill the org integrations

migrations_lockfile.txt

@@ -6,5 +6,5 @@ To resolve this, rebase against latest master and regenerate your migration. Thi
 will then be regenerated, and you should be able to merge without conflicts.
 
 nodestore: 0002_nodestore_no_dictfield
-sentry: 0516_switch_pagerduty_silo
+sentry: 0517_backfill_pagerdutyservices_into_org_integrations
 social_auth: 0002_default_auto_field

src/sentry/incidents/endpoints/organization_alert_rule_available_action_index.py

@@ -42,8 +42,8 @@ def build_action_response(
 
         if registered_type.type == AlertRuleTriggerAction.Type.PAGERDUTY:
             action_response["options"] = [
-                {"value": service["id"], "label": service["service_name"]}
-                for service in get_pagerduty_services(organization.id, integration.id)
+                {"value": id, "label": service_name}
+                for id, service_name in get_pagerduty_services(organization.id, integration.id)
             ]
 
     elif sentry_app_installation:

src/sentry/incidents/logic.py

@@ -4,7 +4,7 @@ import logging
 from copy import deepcopy
 from dataclasses import replace
 from datetime import datetime, timedelta
-from typing import Any, Dict, List, Mapping, Optional, Sequence, Union
+from typing import Any, Dict, List, Mapping, Optional, Sequence, Tuple, Union
 
 from django.db import router, transaction
 from django.db.models.signals import post_save
@@ -1311,14 +1311,13 @@ def get_alert_rule_trigger_action_pagerduty_service(
     input_channel_id=None,
     integrations=None,
 ):
-    try:
-        # TODO: query the org as well to make sure we don't allow
-        # cross org access
-        service = PagerDutyService.objects.get(id=target_value)
-    except PagerDutyService.DoesNotExist:
+    service = integration_service.find_pagerduty_service(
+        organization_id=organization.id, integration_id=integration_id, service_id=target_value
+    )
+    if not service:
         raise InvalidTriggerActionError("No PagerDuty service found.")
 
-    return (service.id, service.service_name)
+    return service["id"], service["service_name"]
 
 
 def get_alert_rule_trigger_action_sentry_app(organization, sentry_app_id, installations):
@@ -1361,11 +1360,14 @@ def get_available_action_integrations_for_org(organization):
     )
 
 
-def get_pagerduty_services(organization_id, integration_id):
-    return PagerDutyService.objects.filter(
-        organization_id=organization_id,
-        integration_id=integration_id,
-    ).values("id", "service_name")
+def get_pagerduty_services(organization_id, integration_id) -> List[Tuple[int, str]]:
+    org_int = integration_service.get_organization_integration(
+        organization_id=organization_id, integration_id=integration_id
+    )
+    if org_int is None:
+        return []
+    services = PagerDutyService.services_in(org_int.config)
+    return [(s["id"], s["service_name"]) for s in services]
 
 
 # TODO: This is temporarily needed to support back and forth translations for snuba / frontend.

src/sentry/integrations/pagerduty/utils.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import logging
 
 from django.http import Http404
@@ -5,6 +7,7 @@ from django.http import Http404
 from sentry.incidents.models import AlertRuleTriggerAction, Incident, IncidentStatus
 from sentry.integrations.metric_alerts import incident_attachment_info
 from sentry.models import PagerDutyService
+from sentry.models.integrations.pagerduty_service import PagerDutyServiceDict
 from sentry.services.hybrid_cloud.integration import integration_service
 from sentry.shared_integrations.client.proxy import infer_org_integration
 from sentry.shared_integrations.exceptions import ApiError
@@ -51,9 +54,27 @@ def send_incident_alert_notification(
     new_status: IncidentStatus,
 ) -> None:
     integration_id = action.integration_id
-    try:
-        service = PagerDutyService.objects.get(id=action.target_identifier)
-    except PagerDutyService.DoesNotExist:
+    organization_id = incident.organization_id
+
+    service: PagerDutyServiceDict | None = None
+    org_integration = integration_service.get_organization_integration(
+        integration_id=integration_id,
+        organization_id=organization_id,
+    )
+    if org_integration is None:
+        org_integration_id = infer_org_integration(integration_id=integration_id, ctx_logger=logger)
+        org_integrations = integration_service.get_organization_integrations(
+            org_integration_ids=[org_integration_id]
+        )
+        if org_integrations:
+            org_integration = org_integrations[0]
+    else:
+        org_integration_id = org_integration.id
+
+    if org_integration and action.target_identifier:
+        service = PagerDutyService.find_service(org_integration.config, action.target_identifier)
+
+    if service is None:
         # service has been removed after rule creation
         logger.info(
             "fetch.fail.pagerduty_metric_alert",
@@ -64,17 +85,8 @@ def send_incident_alert_notification(
             },
         )
         raise Http404
-    org_integration = integration_service.get_organization_integration(
-        integration_id=service.integration_id,
-        organization_id=service.organization_id,
-    )
-    if org_integration is None:
-        org_integration_id = infer_org_integration(
-            integration_id=service.integration_id, ctx_logger=logger
-        )
-    else:
-        org_integration_id = org_integration.id
-    integration_key = service.integration_key
+
+    integration_key = service["integration_key"]
     client = PagerDutyProxyClient(
         org_integration_id=org_integration_id, integration_key=integration_key
     )
@@ -86,8 +98,8 @@ def send_incident_alert_notification(
             "rule.fail.pagerduty_metric_alert",
             extra={
                 "error": str(e),
-                "service_name": service.service_name,
-                "service_id": service.id,
+                "service_name": service["service_name"],
+                "service_id": service["id"],
                 "integration_id": integration_id,
             },
         )

src/sentry/migrations/0517_backfill_pagerdutyservices_into_org_integrations.py

@@ -0,0 +1,60 @@
+from django.db import migrations, router, transaction
+
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.utils.query import RangeQuerySetWrapper
+
+
+def as_dict(pds):
+    return dict(
+        integration_id=pds.integration_id,
+        integration_key=pds.integration_key,
+        service_name=pds.service_name,
+        id=pds.id,
+    )
+
+
+def backfill_pagerdutyservices(apps, schema_editor):
+    PagerDutyService = apps.get_model("sentry", "PagerDutyService")
+    OrganizationIntegration = apps.get_model("sentry", "OrganizationIntegration")
+
+    for pds in RangeQuerySetWrapper(PagerDutyService.objects.all()):
+        try:
+            with transaction.atomic(router.db_for_write(OrganizationIntegration)):
+                org_integration = (
+                    OrganizationIntegration.objects.filter(id=pds.organization_integration_id)
+                    .select_for_update()
+                    .get()
+                )
+                existing = org_integration.config.get("pagerduty_services", [])
+                org_integration.config["pagerduty_services"] = [
+                    row for row in existing if row["id"] != pds.id
+                ] + [as_dict(pds)]
+                org_integration.save()
+        except OrganizationIntegration.DoesNotExist:
+            pass
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, this should only be used for operations where it's safe to run the migration
+    # after your code has deployed. So this should not be used for most operations that alter the
+    # schema of a table.
+    # Here are some things that make sense to mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that they can
+    #   be monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = True
+
+    dependencies = [
+        ("sentry", "0516_switch_pagerduty_silo"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            backfill_pagerdutyservices,
+            reverse_code=migrations.RunPython.noop,
+            hints={"tables": ["sentry_pagerdutyservice", "sentry_organizationintegration"]},
+        ),
+    ]

src/sentry/models/integrations/pagerduty_service.py

@@ -1,15 +1,17 @@
-from django.db import models
+from __future__ import annotations
+
+from typing import Any, List, Mapping, Optional, TypedDict
+
+from django.db import models, router, transaction
 from django.db.models import CASCADE
 from django.utils import timezone
 
 from sentry.db.models import BoundedBigIntegerField, DefaultFieldsModel, FlexibleForeignKey
-from sentry.db.models.base import ModelSiloLimit
+from sentry.db.models.base import control_silo_only_model
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
-from sentry.silo import SiloMode
 
 
-# Temporary -- this will become a control silo model again after a future getsentry merge.
-@ModelSiloLimit(SiloMode.CONTROL, SiloMode.REGION)
+@control_silo_only_model
 class PagerDutyService(DefaultFieldsModel):
     __include_in_export__ = False
 
@@ -28,3 +30,55 @@ class PagerDutyService(DefaultFieldsModel):
     class Meta:
         app_label = "sentry"
         db_table = "sentry_pagerdutyservice"
+
+    def save(self, *args: Any, **kwds: Any) -> None:
+        with transaction.atomic(router.db_for_write(PagerDutyService)):
+            super().save(*args, **kwds)
+            self.add_to_org_integration()
+
+    def add_to_org_integration(self):
+        from sentry.models import OrganizationIntegration
+
+        try:
+            org_integration = (
+                OrganizationIntegration.objects.filter(id=self.organization_integration_id)
+                .select_for_update()
+                .get()
+            )
+            existing: list[PagerDutyServiceDict] = PagerDutyService.services_in(
+                org_integration.config
+            )
+            org_integration.config["pagerduty_services"] = [
+                row for row in existing if row["id"] != self.id
+            ] + [self.as_dict()]
+            org_integration.save()
+        except OrganizationIntegration.DoesNotExist:
+            pass
+
+    def as_dict(self) -> PagerDutyServiceDict:
+        return dict(
+            integration_id=self.integration_id,
+            integration_key=self.integration_key,
+            service_name=self.service_name,
+            id=self.id,
+        )
+
+    @staticmethod
+    def services_in(config: Mapping[str, Any]) -> List[PagerDutyServiceDict]:
+        return config.get("pagerduty_services", [])
+
+    @staticmethod
+    def find_service(config: Mapping[str, Any], id: int | str) -> Optional[PagerDutyServiceDict]:
+        try:
+            return next(
+                pds for pds in PagerDutyService.services_in(config) if str(pds["id"]) == str(id)
+            )
+        except StopIteration:
+            return None
+
+
+class PagerDutyServiceDict(TypedDict):
+    integration_id: int
+    integration_key: str
+    service_name: str
+    id: int

src/sentry/services/hybrid_cloud/integration/serial.py

@@ -20,12 +20,7 @@ def serialize_organization_integration(oi: OrganizationIntegration) -> RpcOrgani
     config: Dict[str, Any] = dict(**oi.config)
     if oi.integration.provider == ExternalProviders.PAGERDUTY.name:
         config["pagerduty_services"] = [
-            dict(
-                integration_id=pds.integration_id,
-                integration_key=pds.integration_key,
-                service_name=pds.service_name,
-                id=pds.id,
-            )
+            pds.as_dict()
             for pds in PagerDutyService.objects.filter(organization_integration_id=oi.id)
         ]
 

src/sentry/services/hybrid_cloud/integration/service.py

@@ -13,6 +13,7 @@ from sentry.integrations.base import (
     IntegrationProvider,
 )
 from sentry.models.integrations import Integration
+from sentry.models.integrations.pagerduty_service import PagerDutyService, PagerDutyServiceDict
 from sentry.services.hybrid_cloud.integration import RpcIntegration, RpcOrganizationIntegration
 from sentry.services.hybrid_cloud.organization import RpcOrganizationSummary
 from sentry.services.hybrid_cloud.pagination import RpcPaginationArgs, RpcPaginationResult
@@ -131,6 +132,19 @@ class IntegrationService(RpcService):
         )
         return ois[0] if len(ois) > 0 else None
 
+    def find_pagerduty_service(
+        self, *, organization_id: int, integration_id: int, service_id: Union[str, int]
+    ) -> Optional[PagerDutyServiceDict]:
+        org_integration = self.get_organization_integration(
+            integration_id=integration_id, organization_id=organization_id
+        )
+        if not org_integration:
+            return None
+        try:
+            return PagerDutyService.find_service(org_integration.config, service_id)
+        except StopIteration:
+            return None
+
     @rpc_method
     @abstractmethod
     def get_organization_context(

tests/sentry/db/test_silo_models.py

@@ -3,7 +3,6 @@ from __future__ import annotations
 from django.db.models import Model
 
 from sentry.api.serializers.base import registry
-from sentry.models import OrganizationIntegration, PagerDutyService
 from sentry.testutils.silo import (
     validate_models_have_silos,
     validate_no_cross_silo_deletions,
@@ -11,8 +10,7 @@ from sentry.testutils.silo import (
 )
 
 decorator_exemptions: set[type[Model]] = set()
-# Temporary, remove after finishing changes on getsentry side
-fk_exemptions: set[tuple[type[Model], type[Model]]] = {(PagerDutyService, OrganizationIntegration)}
+fk_exemptions: set[tuple[type[Model], type[Model]]] = set()
 
 
 def test_models_have_silos():

tests/sentry/hybrid_cloud/test_integration.py

@@ -258,20 +258,20 @@ class OrganizationIntegrationServiceTest(BaseIntegrationServiceTest):
             org_integration = OrganizationIntegration.objects.get(
                 organization_id=self.organization.id, integration_id=integration.id
             )
-            id1 = PagerDutyService.objects.create(
+            pds = PagerDutyService.objects.create(
                 organization_integration_id=org_integration.id,
                 organization_id=self.organization.id,
                 integration_id=integration.id,
                 integration_key="key1",
                 service_name="service1",
-            ).id
-            id2 = PagerDutyService.objects.create(
+            )
+            pds2 = PagerDutyService.objects.create(
                 organization_integration_id=org_integration.id,
                 organization_id=self.organization.id,
                 integration_id=integration.id,
                 integration_key="key2",
                 service_name="service2",
-            ).id
+            )
 
         result = integration_service.get_organization_integration(
             integration_id=integration.id,
@@ -279,18 +279,8 @@ class OrganizationIntegrationServiceTest(BaseIntegrationServiceTest):
         )
         assert result
         assert result.config["pagerduty_services"] == [
-            dict(
-                integration_key="key1",
-                service_name="service1",
-                id=id1,
-                integration_id=integration.id,
-            ),
-            dict(
-                integration_key="key2",
-                service_name="service2",
-                id=id2,
-                integration_id=integration.id,
-            ),
+            pds.as_dict(),
+            pds2.as_dict(),
         ]
 
     def test_get_organization_context(self):