Browse Source

Revert "ref(hc): Move PagerDutyService into control silo (#53084)"

This reverts commit 4840bb4352188740c0462b821f8e6aa2fc1bf932.

Co-authored-by: corps <593850+corps@users.noreply.github.com>
getsentry-bot 1 year ago
parent
commit
7e86a48078

+ 1 - 1
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: 0515_slugify_invalid_monitors
 social_auth: 0001_initial

+ 12 - 16
src/sentry/api/serializers/rest_framework/notification_action.py

@@ -7,6 +7,7 @@ from sentry.api.serializers.rest_framework.base import CamelSnakeModelSerializer
 from sentry.api.serializers.rest_framework.project import ProjectField
 from sentry.constants import SentryAppInstallationStatus
 from sentry.integrations.slack.utils.channel import get_channel_id, validate_channel_id
+from sentry.models.integrations.pagerduty_service import PagerDutyService
 from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
 from sentry.models.notificationaction import ActionService, ActionTarget, NotificationAction
 from sentry.models.project import Project
@@ -219,16 +220,14 @@ class NotificationActionSerializer(CamelSnakeModelSerializer):
             return data
 
         service_id = data.get("target_identifier")
-        ois = integration_service.get_organization_integrations(
-            organization_id=self.context["organization"].id,
-            integration_id=self.integration.id,
-        )
 
         if not service_id:
             pd_service_options = [
                 f"{pds['id']} ({pds['service_name']})"
-                for oi in ois
-                for pds in oi.config.get("pagerduty_services", [])
+                for pds in PagerDutyService.objects.filter(
+                    organization_id=self.context["organization"].id,
+                    integration_id=self.integration.id,
+                ).values("id", "service_name")
             ]
 
             raise serializers.ValidationError(
@@ -237,21 +236,18 @@ class NotificationActionSerializer(CamelSnakeModelSerializer):
                 }
             )
 
-        try:
-            pds = next(
-                pds
-                for oi in ois
-                for pds in oi.config.get("pagerduty_services", [])
-                if service_id == str(pds["id"])
-            )
-        except StopIteration:
+        pds = PagerDutyService.objects.filter(
+            organization_id=self.context["organization"].id,
+            integration_id=self.integration.id,
+        ).first()
+        if not pds or str(pds.id) != service_id:
             raise serializers.ValidationError(
                 {
                     "target_identifier": f"Could not find associated PagerDuty service for the '{self.integration.name}' account. If it exists, ensure Sentry has access."
                 }
             )
-        data["target_display"] = pds["service_name"]
-        data["target_identifier"] = pds["id"]
+        data["target_display"] = pds.service_name
+        data["target_identifier"] = pds.id
         return data
 
     def validate(self, data: NotificationActionInputData) -> NotificationActionInputData:

+ 14 - 12
src/sentry/integrations/pagerduty/actions/form.py

@@ -5,8 +5,8 @@ from typing import Any, Mapping
 from django import forms
 from django.utils.translation import gettext_lazy as _
 
+from sentry.models import PagerDutyService
 from sentry.services.hybrid_cloud.integration import integration_service
-from sentry.types.integrations import ExternalProviders
 
 
 def _validate_int_field(field: str, cleaned_data: Mapping[str, Any]) -> int | None:
@@ -43,23 +43,25 @@ class PagerDutyNotifyServiceForm(forms.Form):
         self.fields["service"].choices = services
         self.fields["service"].widget.choices = self.fields["service"].choices
 
-    def _validate_service(self, service_id: int, integration_id: int) -> None:
+    def _validate_service(self, service_id: int, integration_id: int | None) -> None:
         params = {
             "account": dict(self.fields["account"].choices).get(integration_id),
             "service": dict(self.fields["service"].choices).get(service_id),
         }
 
-        org_integrations = integration_service.get_organization_integrations(
-            integration_id=integration_id,
-            providers=[ExternalProviders.PAGERDUTY.name],
-        )
+        try:
+            service = PagerDutyService.objects.get(id=service_id)
+        except PagerDutyService.DoesNotExist:
+            raise forms.ValidationError(
+                _('The service "%(service)s" does not exist in the %(account)s Pagerduty account.'),
+                code="invalid",
+                params=params,
+            )
 
-        if not any(
-            pds
-            for oi in org_integrations
-            for pds in oi.config.get("pagerduty_services", [])
-            if pds["id"] == service_id
-        ):
+        integration = integration_service.get_integration(
+            organization_integration_id=service.organization_integration_id
+        )
+        if not integration or integration.id != integration_id:
             # We need to make sure that the service actually belongs to that integration,
             # meaning that it belongs under the appropriate account in PagerDuty.
             raise forms.ValidationError(

+ 23 - 25
src/sentry/integrations/pagerduty/actions/notification.py

@@ -1,10 +1,11 @@
 from __future__ import annotations
 
 import logging
-from typing import Sequence, Tuple
+from typing import Sequence
 
 from sentry.integrations.pagerduty.actions import PagerDutyNotifyServiceForm
 from sentry.integrations.pagerduty.client import PagerDutyProxyClient
+from sentry.models import PagerDutyService
 from sentry.rules.actions import IntegrationEventAction
 from sentry.shared_integrations.client.proxy import infer_org_integration
 from sentry.shared_integrations.exceptions import ApiError
@@ -31,13 +32,7 @@ class PagerDutyNotifyServiceAction(IntegrationEventAction):
         }
 
     def _get_service(self):
-        oi = self.get_organization_integration()
-        if not oi:
-            return None
-        for pds in oi.config.get("pagerduty_services", []):
-            if pds["id"] == self.get_option("service"):
-                return pds
-        return None
+        return PagerDutyService.objects.get(id=self.get_option("service"))
 
     def after(self, event, state):
         integration = self.get_integration()
@@ -46,8 +41,9 @@ class PagerDutyNotifyServiceAction(IntegrationEventAction):
             # integration removed but rule still exists
             return
 
-        service = self._get_service()
-        if not service:
+        try:
+            service = self._get_service()
+        except PagerDutyService.DoesNotExist:
             logger.exception("The PagerDuty does not exist anymore while integration does.")
             return
 
@@ -58,11 +54,11 @@ class PagerDutyNotifyServiceAction(IntegrationEventAction):
                 org_integration_id = org_integration.id
             else:
                 org_integration_id = infer_org_integration(
-                    integration_id=service["integration_id"], ctx_logger=logger
+                    integration_id=service.integration_id, ctx_logger=logger
                 )
             client = PagerDutyProxyClient(
                 org_integration_id=org_integration_id,
-                integration_key=service["integration_key"],
+                integration_key=service.integration_key,
             )
             try:
                 resp = client.send_trigger(event)
@@ -71,8 +67,8 @@ class PagerDutyNotifyServiceAction(IntegrationEventAction):
                     "rule.fail.pagerduty_trigger",
                     extra={
                         "error": str(e),
-                        "service_name": service["service_name"],
-                        "service_id": service["id"],
+                        "service_name": service.service_name,
+                        "service_id": service.id,
                         "project_id": event.project_id,
                         "event_id": event.event_id,
                     },
@@ -87,31 +83,33 @@ class PagerDutyNotifyServiceAction(IntegrationEventAction):
                     "status_code": resp.status_code,
                     "project_id": event.project_id,
                     "event_id": event.event_id,
-                    "service_name": service["service_name"],
-                    "service_id": service["id"],
+                    "service_name": service.service_name,
+                    "service_id": service.id,
                 },
             )
 
-        key = f"pagerduty:{integration.id}:{service['id']}"
+        key = f"pagerduty:{integration.id}:{service.id}"
         yield self.future(send_notification, key=key)
 
-    def get_services(self) -> Sequence[Tuple[int, str]]:
+    def get_services(self) -> Sequence[PagerDutyService]:
         from sentry.services.hybrid_cloud.integration import integration_service
 
         organization_integrations = integration_service.get_organization_integrations(
             providers=[self.provider], organization_id=self.project.organization_id
         )
+        integration_ids = [oi.integration_id for oi in organization_integrations]
+
         return [
-            (v["id"], v["service_name"])
-            for oi in organization_integrations
-            for v in oi.config.get("pagerduty_services", [])
+            service
+            for service in PagerDutyService.objects.filter(
+                organization_id=self.project.organization_id, integration_id__in=integration_ids
+            ).values_list("id", "service_name")
         ]
 
     def render_label(self):
-        s = self._get_service()
-        if s:
-            service_name = s["service_name"]
-        else:
+        try:
+            service_name = self._get_service().service_name
+        except PagerDutyService.DoesNotExist:
             service_name = "[removed]"
 
         return self.label.format(account=self.get_integration_name(), service=service_name)

+ 0 - 4
src/sentry/integrations/pagerduty/integration.py

@@ -122,8 +122,6 @@ class PagerDutyIntegration(IntegrationInstallation):
                         organization_integration_id=self.org_integration.id,
                         service_name=service_name,
                         integration_key=key,
-                        integration_id=self.model.id,
-                        organization_id=self.organization_id,
                     )
 
     def get_config_data(self):
@@ -176,8 +174,6 @@ class PagerDutyIntegrationProvider(IntegrationProvider):
                     organization_integration_id=org_integration.id,
                     integration_key=service["integration_key"],
                     service_name=service["name"],
-                    organization_id=organization.id,
-                    integration_id=integration.id,
                 )
 
     def build_integration(self, state):

+ 0 - 53
src/sentry/migrations/0516_switch_pagerduty_silo.py

@@ -1,53 +0,0 @@
-# Generated by Django 3.2.20 on 2023-07-18 16:07
-
-import django.db.models.deletion
-from django.db import migrations
-
-import sentry.db.models.fields.foreignkey
-import sentry.db.models.fields.hybrid_cloud_foreign_key
-from sentry.new_migrations.migrations import CheckedMigration
-
-
-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 = False
-
-    dependencies = [
-        ("sentry", "0515_slugify_invalid_monitors"),
-    ]
-
-    state_operations = [
-        migrations.RemoveField(
-            model_name="pagerdutyservice",
-            name="organization_integration_id",
-        ),
-        migrations.AddField(
-            model_name="pagerdutyservice",
-            name="organization_integration",
-            field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
-                default=-1,
-                on_delete=django.db.models.deletion.CASCADE,
-                to="sentry.organizationintegration",
-                db_constraint=False,
-            ),
-            preserve_default=False,
-        ),
-        migrations.AlterField(
-            model_name="pagerdutyservice",
-            name="organization_id",
-            field=sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey(
-                "sentry.Organization", db_index=True, on_delete="CASCADE"
-            ),
-        ),
-    ]
-
-    operations = [migrations.SeparateDatabaseAndState(state_operations=state_operations)]

+ 9 - 12
src/sentry/models/integrations/pagerduty_service.py

@@ -1,24 +1,21 @@
 from django.db import models
-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 import BoundedBigIntegerField, DefaultFieldsModel, region_silo_only_model
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
-from sentry.silo import SiloMode
+from sentry.models.integrations.organization_integrity_backfill_mixin import (
+    OrganizationIntegrityBackfillMixin,
+)
 
 
-# Temporary -- this will become a control silo model again after a future getsentry merge.
-@ModelSiloLimit(SiloMode.CONTROL, SiloMode.REGION)
-class PagerDutyService(DefaultFieldsModel):
+@region_silo_only_model
+class PagerDutyService(OrganizationIntegrityBackfillMixin, DefaultFieldsModel):
     __include_in_export__ = False
 
-    organization_integration = FlexibleForeignKey(
-        "sentry.OrganizationIntegration", on_delete=CASCADE, db_constraint=False
+    organization_integration_id = HybridCloudForeignKey(
+        "sentry.OrganizationIntegration", on_delete="CASCADE"
     )
-
-    organization_id = HybridCloudForeignKey("sentry.Organization", on_delete="cascade")
-
+    organization_id = BoundedBigIntegerField(db_index=True)
     # From a region point of view, you really only have per organization scoping.
     integration_id = BoundedBigIntegerField(db_index=False)
     integration_key = models.CharField(max_length=255)

+ 3 - 3
src/sentry/services/hybrid_cloud/integration/impl.py

@@ -187,7 +187,7 @@ class DatabaseBackedIntegrationService(IntegrationService):
         if not oi_kwargs:
             return []
 
-        ois = OrganizationIntegration.objects.filter(**oi_kwargs).select_related("integration")
+        ois = OrganizationIntegration.objects.filter(**oi_kwargs)
 
         if limit is not None:
             ois = ois[:limit]
@@ -233,7 +233,7 @@ class DatabaseBackedIntegrationService(IntegrationService):
         )
         return (
             serialize_integration(integration),
-            organization_integrations,
+            [serialize_organization_integration(oi) for oi in organization_integrations],
         )
 
     def update_integrations(
@@ -325,7 +325,7 @@ class DatabaseBackedIntegrationService(IntegrationService):
             grace_period_end=grace_period_end,
             set_grace_period_end_null=set_grace_period_end_null,
         )
-        return ois[0] if len(ois) > 0 else None
+        return serialize_organization_integration(ois[0]) if len(ois) > 0 else None
 
     def send_incident_alert_notification(
         self,

+ 2 - 17
src/sentry/services/hybrid_cloud/integration/serial.py

@@ -1,8 +1,5 @@
-from typing import Any, Dict
-
-from sentry.models import Integration, OrganizationIntegration, PagerDutyService
+from sentry.models import Integration, OrganizationIntegration
 from sentry.services.hybrid_cloud.integration import RpcIntegration, RpcOrganizationIntegration
-from sentry.types.integrations import ExternalProviders
 
 
 def serialize_integration(integration: Integration) -> RpcIntegration:
@@ -17,24 +14,12 @@ def serialize_integration(integration: Integration) -> RpcIntegration:
 
 
 def serialize_organization_integration(oi: OrganizationIntegration) -> RpcOrganizationIntegration:
-    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,
-            )
-            for pds in PagerDutyService.objects.filter(organization_integration_id=oi.id)
-        ]
-
     return RpcOrganizationIntegration(
         id=oi.id,
         default_auth_id=oi.default_auth_id,
         organization_id=oi.organization_id,
         integration_id=oi.integration_id,
-        config=config,
+        config=oi.config,
         status=oi.status,
         grace_period_end=oi.grace_period_end,
     )

+ 2 - 1
src/sentry/services/hybrid_cloud/integration/service.py

@@ -14,6 +14,7 @@ from sentry.integrations.base import (
 )
 from sentry.models.integrations import Integration
 from sentry.services.hybrid_cloud.integration import RpcIntegration, RpcOrganizationIntegration
+from sentry.services.hybrid_cloud.integration.serial import serialize_organization_integration
 from sentry.services.hybrid_cloud.organization import RpcOrganizationSummary
 from sentry.services.hybrid_cloud.pagination import RpcPaginationArgs, RpcPaginationResult
 from sentry.services.hybrid_cloud.rpc import RpcService, rpc_method
@@ -129,7 +130,7 @@ class IntegrationService(RpcService):
         ois = self.get_organization_integrations(
             integration_id=integration_id, organization_id=organization_id, limit=1
         )
-        return ois[0] if len(ois) > 0 else None
+        return serialize_organization_integration(ois[0]) if len(ois) > 0 else None
 
     @rpc_method
     @abstractmethod

Some files were not shown because too many files changed in this diff