diff --git a/src/sentry/notifications/notification_action/action_handler_registry/sentry_app_handler.py b/src/sentry/notifications/notification_action/action_handler_registry/sentry_app_handler.py index 4adbed04bb43f6..b3f1deb4e908ed 100644 --- a/src/sentry/notifications/notification_action/action_handler_registry/sentry_app_handler.py +++ b/src/sentry/notifications/notification_action/action_handler_registry/sentry_app_handler.py @@ -33,6 +33,7 @@ class SentryAppActionHandler(ActionHandler): data_schema = { "$schema": "https://json-schema.org/draft/2020-12/schema", + "description": "The data schema for a Sentry App Action", "type": "object", "properties": { "settings": {"type": ["array", "object"]}, diff --git a/src/sentry/notifications/notification_action/action_validation.py b/src/sentry/notifications/notification_action/action_validation.py index 7aa99675cdb6d5..e8e9d5e2bbfb76 100644 --- a/src/sentry/notifications/notification_action/action_validation.py +++ b/src/sentry/notifications/notification_action/action_validation.py @@ -15,7 +15,7 @@ from sentry.rules.actions.integrations.create_ticket.form import IntegrationNotifyServiceForm from sentry.rules.actions.notify_event_service import NotifyEventServiceForm from sentry.rules.actions.sentry_apps.utils import validate_sentry_app_action -from sentry.sentry_apps.services.app.service import app_service +from sentry.sentry_apps.services.app import RpcSentryAppInstallation, app_service from sentry.sentry_apps.utils.errors import SentryAppBaseError from sentry.utils import json from sentry.workflow_engine.models.action import Action @@ -206,31 +206,74 @@ def __init__(self, validated_data: dict[str, Any], organization: Organization) - self.validated_data = validated_data self.organization = organization + def _get_sentry_app_installation( + self, sentry_app_identifier: SentryAppIdentifier, target_identifier: str + ) -> RpcSentryAppInstallation | None: + """ + Get the sentry app installation based on whether the target identifier is an installation id or sentry app id + We do not want to accept SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID long term, this is temporary until we migrate the data over + """ + installations = None + installation = None + + if sentry_app_identifier == SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID: + installations = app_service.get_many( + filter=dict(uuids=[target_identifier], organization_id=self.organization.id) + ) + else: + installations = app_service.get_many( + filter=dict(app_ids=[int(target_identifier)], organization_id=self.organization.id) + ) + if installations: + installation = installations[0] + + return installation + def clean_data(self) -> dict[str, Any]: - is_sentry_app_installation = ( - SentryAppIdentifier(self.validated_data["config"]["sentry_app_identifier"]) - == SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID + sentry_app_identifier = SentryAppIdentifier( + self.validated_data["config"]["sentry_app_identifier"] ) + target_identifier = self.validated_data["config"]["target_identifier"] + installation = self._get_sentry_app_installation(sentry_app_identifier, target_identifier) + if not installation: + raise ValidationError("Sentry app installation not found.") + + if sentry_app_identifier == SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID: + # convert to use sentry_app_id until we can migrate all the data + self.validated_data["config"][ + "sentry_app_identifier" + ] = SentryAppIdentifier.SENTRY_APP_ID + self.validated_data["config"]["target_identifier"] = str(installation.sentry_app.id) + + settings = self.validated_data["data"].get("settings", []) action = { - "settings": self.validated_data["data"]["settings"], - "sentryAppInstallationUuid": ( - self.validated_data["config"]["target_identifier"] - if is_sentry_app_installation - else None - ), + "settings": settings, + "sentryAppInstallationUuid": installation.uuid, } - try: - validate_sentry_app_action(action) + if not settings: + # XXX: it's only ok to not pass settings if there is no sentry app schema + # this means the app doesn't expect any settings + components = app_service.find_app_components(app_id=installation.sentry_app.id) + if any( + component.app_schema + for component in components + if component.type == "alert-rule-action" + ): + raise ValidationError("'settings' is a required property") + + else: # Sentry app config blob expects value to be a string - settings = self.validated_data["data"]["settings"] for setting in settings: if setting.get("value") is not None and not isinstance(setting["value"], str): setting["value"] = json.dumps(setting["value"]) + try: + # Only call creator for Sentry Apps with UI Components (settings) for actions + validate_sentry_app_action(action) + except SentryAppBaseError as e: + raise ValidationError(e.message) from e - return self.validated_data - except SentryAppBaseError as e: - raise ValidationError(e.message) from e + return self.validated_data @action_validator_registry.register(Action.Type.WEBHOOK) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py index af001f4af86742..df02e98782088a 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py @@ -1,5 +1,7 @@ from contextlib import AbstractContextManager +import responses + from sentry import audit_log from sentry.api.serializers import serialize from sentry.constants import ObjectStatus @@ -13,8 +15,20 @@ from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode, region_silo_test from sentry.workflow_engine.endpoints.validators.base.workflow import WorkflowValidator -from sentry.workflow_engine.models import Action, DataConditionGroup, Workflow +from sentry.workflow_engine.models import ( + Action, + Condition, + DataConditionGroup, + DataConditionGroupAction, + Workflow, + WorkflowDataConditionGroup, +) from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow +from sentry.workflow_engine.typings.notification_action import ( + ActionTarget, + ActionType, + SentryAppIdentifier, +) from tests.sentry.workflow_engine.test_base import BaseWorkflowTest @@ -54,7 +68,7 @@ def setUp(self) -> None: "enabled": True, "config": {}, "triggers": {"logicType": "any", "conditions": []}, - "action_filters": [], + "actionFilters": [], } validator = WorkflowValidator( data=self.valid_workflow, @@ -63,6 +77,15 @@ def setUp(self) -> None: validator.is_valid(raise_exception=True) self.workflow = validator.create(validator.validated_data) + self.sentry_app, self.sentry_app_installation = self.create_sentry_app_with_schema() + self.sentry_app_settings = [ + {"name": "alert_prefix", "value": "[Not Good]"}, + {"name": "channel", "value": "#ignored-errors"}, + {"name": "best_emoji", "value": ":fire:"}, + {"name": "teamId", "value": "1"}, + {"name": "assigneeId", "value": "3"}, + ] + def test_simple(self) -> None: self.valid_workflow["name"] = "Updated Workflow" response = self.get_success_response( @@ -73,6 +96,116 @@ def test_simple(self) -> None: assert response.status_code == 200 assert updated_workflow.name == "Updated Workflow" + @responses.activate + def test_update_add_sentry_app_action(self) -> None: + """ + Test that adding a sentry app action to a workflow works as expected + """ + responses.add( + method=responses.POST, + url="https://example.com/sentry/alert-rule", + status=200, + ) + self.valid_workflow["actionFilters"] = [ + { + "logicType": "any", + "conditions": [ + { + "type": Condition.EQUAL.value, + "comparison": 1, + "conditionResult": True, + } + ], + "actions": [ + { + "config": { + "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_ID, + "targetIdentifier": str(self.sentry_app.id), + "targetType": ActionType.SENTRY_APP, + }, + "data": {"settings": self.sentry_app_settings}, + "type": Action.Type.SENTRY_APP, + }, + ], + } + ] + response = self.get_success_response( + self.organization.slug, self.workflow.id, raw_data=self.valid_workflow + ) + updated_workflow = Workflow.objects.get(id=response.data.get("id")) + action_filter = WorkflowDataConditionGroup.objects.get(workflow=updated_workflow) + dcga = DataConditionGroupAction.objects.get(condition_group=action_filter.condition_group) + action = dcga.action + + assert response.status_code == 200 + assert action.type == Action.Type.SENTRY_APP + assert action.config == { + "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID, + "target_identifier": str(self.sentry_app.id), + "target_type": ActionTarget.SENTRY_APP.value, + } + assert action.data["settings"] == self.sentry_app_settings + + @responses.activate + def test_update_add_sentry_app_installation_uuid_action(self) -> None: + """ + Test that adding a sentry app action to a workflow works as expected when we pass the installation UUID instead of the sentry app id. + We do not create new actions like this today, but we have data like this in the DB and need to make sure it still works until we can migrate away from it + to use the sentry app id + """ + responses.add( + method=responses.POST, + url="https://example.com/sentry/alert-rule", + status=200, + ) + # update the settings + updated_sentry_app_settings = [ + {"name": "alert_prefix", "value": "[Very Good]"}, + {"name": "channel", "value": "#pay-attention-to-errors"}, + {"name": "best_emoji", "value": ":ice:"}, + {"name": "teamId", "value": "1"}, + {"name": "assigneeId", "value": "3"}, + ] + self.valid_workflow["actionFilters"] = [ + { + "logicType": "any", + "conditions": [ + { + "type": Condition.EQUAL.value, + "comparison": 1, + "conditionResult": True, + } + ], + "actions": [ + { + "config": { + "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID, + "targetIdentifier": self.sentry_app_installation.uuid, + "targetType": ActionType.SENTRY_APP, + }, + "data": {"settings": updated_sentry_app_settings}, + "type": Action.Type.SENTRY_APP, + }, + ], + } + ] + response = self.get_success_response( + self.organization.slug, self.workflow.id, raw_data=self.valid_workflow + ) + updated_workflow = Workflow.objects.get(id=response.data.get("id")) + action_filter = WorkflowDataConditionGroup.objects.get(workflow=updated_workflow) + dcga = DataConditionGroupAction.objects.get(condition_group=action_filter.condition_group) + action = dcga.action + + assert response.status_code == 200 + assert action.type == Action.Type.SENTRY_APP + assert action.config == { + "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID, + "target_identifier": str(self.sentry_app.id), + "target_type": ActionTarget.SENTRY_APP.value, + } + assert action.data["settings"] == updated_sentry_app_settings + def test_update_triggers_with_empty_conditions(self) -> None: """Test that passing an empty list to triggers.conditions clears all conditions""" # Create a workflow with a trigger condition diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py index 6a93eee22b7724..639f2e0109020a 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py @@ -2,6 +2,8 @@ from typing import Any from unittest import mock +import responses + from sentry import audit_log from sentry.api.serializers import serialize from sentry.constants import ObjectStatus @@ -9,7 +11,6 @@ from sentry.deletions.tasks.scheduled import run_scheduled_deletions from sentry.grouping.grouptype import ErrorGroupType from sentry.incidents.grouptype import MetricIssue -from sentry.notifications.models.notificationaction import ActionTarget from sentry.notifications.types import FallthroughChoiceType from sentry.testutils.asserts import assert_org_audit_log_exists from sentry.testutils.cases import APITestCase @@ -24,7 +25,12 @@ WorkflowFireHistory, ) from sentry.workflow_engine.models.data_condition import Condition -from tests.sentry.workflow_engine.test_base import MockActionValidatorTranslator +from sentry.workflow_engine.typings.notification_action import ( + ActionTarget, + ActionType, + SentryAppIdentifier, +) +from tests.sentry.workflow_engine.test_base import BaseWorkflowTest, MockActionValidatorTranslator class OrganizationWorkflowAPITestCase(APITestCase): @@ -375,7 +381,7 @@ def test_compound_query(self) -> None: @region_silo_test -class OrganizationWorkflowCreateTest(OrganizationWorkflowAPITestCase): +class OrganizationWorkflowCreateTest(OrganizationWorkflowAPITestCase, BaseWorkflowTest): method = "POST" def setUp(self) -> None: @@ -404,6 +410,14 @@ def setUp(self) -> None: "conditionResult": True, } ] + self.sentry_app, _ = self.create_sentry_app_with_schema() + self.sentry_app_settings = [ + {"name": "alert_prefix", "value": "[Not Good]"}, + {"name": "channel", "value": "#ignored-errors"}, + {"name": "best_emoji", "value": ":fire:"}, + {"name": "teamId", "value": "1"}, + {"name": "assigneeId", "value": "3"}, + ] @mock.patch("sentry.workflow_engine.endpoints.validators.base.workflow.create_audit_entry") def test_create_workflow__basic(self, mock_audit: mock.MagicMock) -> None: @@ -539,6 +553,146 @@ def test_create_workflow__with_fallthrough_type_action( assert dcga.action.type == Action.Type.EMAIL assert dcga.action.data == {"fallthrough_type": "ActiveMembers"} + @responses.activate + def test_create_workflow_with_sentry_app_action(self) -> None: + """ + Test that you can add a sentry app with settings + (e.g. a sentry app that makes a ticket in some 3rd party system as opposed to one without settings) + """ + responses.add( + method=responses.POST, + url="https://example.com/sentry/alert-rule", + status=200, + ) + self.valid_workflow["actionFilters"] = [ + { + "logicType": "any", + "conditions": self.basic_condition, + "actions": [ + { + "config": { + "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_ID, + "targetIdentifier": str(self.sentry_app.id), + "targetType": ActionType.SENTRY_APP, + }, + "data": {"settings": self.sentry_app_settings}, + "type": Action.Type.SENTRY_APP, + }, + ], + } + ] + + response = self.get_success_response( + self.organization.slug, + raw_data=self.valid_workflow, + ) + updated_workflow = Workflow.objects.get(id=response.data["id"]) + new_action_filters = WorkflowDataConditionGroup.objects.filter(workflow=updated_workflow) + dcga = DataConditionGroupAction.objects.filter( + condition_group=new_action_filters[0].condition_group + ) + action = dcga[0].action + + assert action.type == Action.Type.SENTRY_APP + assert action.config == { + "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID, + "target_identifier": str(self.sentry_app.id), + "target_type": ActionTarget.SENTRY_APP.value, + } + assert action.data["settings"] == self.sentry_app_settings + + @responses.activate + def test_create_sentry_app_action_missing_settings(self) -> None: + """ + Test that if you forget to pass settings to your sentry app action it will fail and tell you why. + Settings are only required if the sentry app schema is not an empty dict + """ + responses.add( + method=responses.POST, + url="https://example.com/sentry/alert-rule", + status=200, + ) + + self.valid_workflow["actionFilters"] = [ + { + "logicType": "any", + "conditions": self.basic_condition, + "actions": [ + { + "config": { + "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_ID, + "targetIdentifier": str(self.sentry_app.id), + "targetType": ActionType.SENTRY_APP, + }, + "data": {}, + "type": Action.Type.SENTRY_APP, + }, + ], + } + ] + response = self.get_response( + self.organization.slug, + raw_data=self.valid_workflow, + ) + + assert response.status_code == 400 + assert "'settings' is a required property" in str(response.data).lower() + + @responses.activate + def test_create_sentry_app_action_no_settings(self) -> None: + """ + Test that if you are creating a sentry app action for a sentry app that has no schema it works as expected when settings are not passed + because settings are not expected + """ + sentry_app = self.create_sentry_app( + name="Moo Deng's Wind Sentry App", + organization=self.organization, + is_alertable=True, + ) + self.create_sentry_app_installation(slug=sentry_app.slug, organization=self.organization) + + responses.add( + method=responses.POST, + url="https://example.com/sentry/alert-rule", + status=200, + ) + + self.valid_workflow["actionFilters"] = [ + { + "logicType": "any", + "conditions": self.basic_condition, + "actions": [ + { + "config": { + "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_ID, + "targetIdentifier": str(sentry_app.id), + "targetType": ActionType.SENTRY_APP, + }, + "data": {}, + "type": Action.Type.SENTRY_APP, + }, + ], + } + ] + response = self.get_success_response( + self.organization.slug, + raw_data=self.valid_workflow, + ) + updated_workflow = Workflow.objects.get(id=response.data["id"]) + new_action_filters = WorkflowDataConditionGroup.objects.filter(workflow=updated_workflow) + dcga = DataConditionGroupAction.objects.filter( + condition_group=new_action_filters[0].condition_group + ) + action = dcga[0].action + + assert action.type == Action.Type.SENTRY_APP + assert action.config == { + "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID, + "target_identifier": str(sentry_app.id), + "target_type": ActionTarget.SENTRY_APP.value, + } + assert action.data == {} + def test_create_invalid_workflow(self) -> None: self.valid_workflow["name"] = "" response = self.get_response( diff --git a/tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py b/tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py index 0eab559ca6c562..aa56068a4b3fe4 100644 --- a/tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py +++ b/tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py @@ -4,24 +4,33 @@ from sentry.sentry_apps.services.app.model import RpcAlertRuleActionResult from sentry.sentry_apps.utils.errors import SentryAppErrorType -from sentry.testutils.cases import TestCase from sentry.workflow_engine.endpoints.validators.base import BaseActionValidator from sentry.workflow_engine.models import Action from sentry.workflow_engine.types import WorkflowEventData +from sentry.workflow_engine.typings.notification_action import ActionType, SentryAppIdentifier +from tests.sentry.workflow_engine.test_base import BaseWorkflowTest -class TestSentryAppActionValidator(TestCase): +class TestSentryAppActionValidator(BaseWorkflowTest): def setUp(self) -> None: super().setUp() + self.sentry_app, self.sentry_app_installation = self.create_sentry_app_with_schema() + self.sentry_app_settings = [ + {"name": "alert_prefix", "value": "[Not Good]"}, + {"name": "channel", "value": "#ignored-errors"}, + {"name": "best_emoji", "value": ":fire:"}, + {"name": "teamId", "value": "1"}, + {"name": "assigneeId", "value": "3"}, + ] self.valid_data = { "type": Action.Type.SENTRY_APP, "config": { - "sentry_app_identifier": "sentry_app_installation_uuid", - "targetType": "sentry_app", - "target_identifier": "123", + "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID, + "targetType": ActionType.SENTRY_APP, + "targetIdentifier": self.sentry_app_installation.uuid, }, - "data": {"settings": []}, + "data": {"settings": self.sentry_app_settings}, } group_event = self.event.for_group(self.group) @@ -52,6 +61,34 @@ def test_validate(self, mock_trigger_sentry_app_action_creators: mock.MagicMock) assert result is True validator.save() + @mock.patch( + "sentry.rules.actions.sentry_apps.utils.app_service.trigger_sentry_app_action_creators" + ) + def test_validate_sentry_app_id( + self, mock_trigger_sentry_app_action_creators: mock.MagicMock + ) -> None: + mock_trigger_sentry_app_action_creators.return_value = RpcAlertRuleActionResult( + success=True, message="success" + ) + valid_data = { + "type": Action.Type.SENTRY_APP, + "config": { + "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_ID, + "targetType": ActionType.SENTRY_APP, + "targetIdentifier": str(self.sentry_app.id), + }, + "data": {"settings": self.sentry_app_settings}, + } + + validator = BaseActionValidator( + data=valid_data, + context={"organization": self.organization}, + ) + + result = validator.is_valid() + assert result is True + validator.save() + @mock.patch( "sentry.rules.actions.sentry_apps.utils.app_service.trigger_sentry_app_action_creators" ) @@ -97,8 +134,8 @@ def test_validate_settings_action_trigger( self.valid_data = { "type": Action.Type.SENTRY_APP, "config": { - "sentry_app_identifier": "sentry_app_installation_uuid", - "targetType": "sentry_app", + "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID, + "targetType": ActionType.SENTRY_APP, "target_identifier": install.uuid, }, "data": { diff --git a/tests/sentry/workflow_engine/test_base.py b/tests/sentry/workflow_engine/test_base.py index 917329517bba0a..b77eff1e3cad82 100644 --- a/tests/sentry/workflow_engine/test_base.py +++ b/tests/sentry/workflow_engine/test_base.py @@ -16,6 +16,8 @@ from sentry.models.group import Group from sentry.models.project import Project from sentry.notifications.notification_action.types import BaseActionValidatorHandler +from sentry.sentry_apps.models.sentry_app import SentryApp +from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation from sentry.services.eventstore.models import Event, GroupEvent from sentry.snuba.dataset import Dataset from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType @@ -364,3 +366,20 @@ def create_group_event( ) return group, event, group_event + + def create_sentry_app_with_schema(self) -> tuple[SentryApp, SentryAppInstallation]: + sentry_app_settings_schema = self.create_alert_rule_action_schema() + sentry_app = self.create_sentry_app( + name="Moo Deng's Fire Sentry App", + organization=self.organization, + schema={ + "elements": [ + sentry_app_settings_schema, + ] + }, + is_alertable=True, + ) + installation = self.create_sentry_app_installation( + slug=sentry_app.slug, organization=self.organization + ) + return sentry_app, installation