Skip to content

Commit 014f98b

Browse files
authored
feat(workflow_engine): Add Validator for BaseActions (#87140)
## Description Create an ingress validator for Actions, this generic action can be used to validate a portion of an Action from the base Action model. As I was adding this, I noticed the data blob didn't have a schema, so I added one of those as well -- let me know if you'd like that split out from here to make it easier to review.
1 parent 21de049 commit 014f98b

File tree

8 files changed

+238
-10
lines changed

8 files changed

+238
-10
lines changed

src/sentry/incidents/metric_alert_detector.py

+2
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ def validate(self, attrs):
5151
raise serializers.ValidationError("Too many conditions")
5252
return attrs
5353

54+
# TODO - @saponifi3d - we can make this more generic and move it into the base Detector
5455
def update_data_conditions(self, instance: Detector, data_conditions: list[DataConditionType]):
5556
"""
5657
Update the data condition if it already exists, create one if it does not
@@ -116,6 +117,7 @@ def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSour
116117
event_types=data_source.get("event_types", [event_type for event_type in event_types]),
117118
)
118119

120+
# TODO - @saponifi3d - we can make this more generic and move it into the base Detector
119121
def update(self, instance: Detector, validated_data: dict[str, Any]):
120122
instance.name = validated_data.get("name", instance.name)
121123
instance.type = validated_data.get("detector_type", instance.group_type).slug

src/sentry/workflow_engine/endpoints/validators/base/__init__.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
__all__ = [
2-
"BaseDataConditionValidator",
2+
"BaseActionValidator",
33
"BaseDataConditionGroupValidator",
4+
"BaseDataConditionValidator",
45
"BaseDataSourceValidator",
56
"BaseDetectorTypeValidator",
67
"DataSourceCreator",
78
"NumericComparisonConditionValidator",
89
]
910

11+
from .action import BaseActionValidator
1012
from .data_condition import (
1113
BaseDataConditionGroupValidator,
1214
BaseDataConditionValidator,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
from typing import Any, Generic, TypeVar
2+
3+
from django.forms import ValidationError
4+
from jsonschema import ValidationError as JsonValidationError
5+
from jsonschema import validate
6+
from rest_framework import serializers
7+
8+
from sentry.api.serializers.rest_framework import CamelSnakeModelSerializer
9+
from sentry.db.models import Model
10+
from sentry.workflow_engine.models import Action
11+
from sentry.workflow_engine.registry import action_handler_registry
12+
from sentry.workflow_engine.types import ActionHandler
13+
14+
T = TypeVar("T", bound=Model)
15+
ActionData = dict[str, Any]
16+
ActionConfig = dict[str, Any]
17+
18+
19+
def validate_json_schema(value, schema):
20+
try:
21+
validate(value, schema)
22+
except JsonValidationError as e:
23+
raise ValidationError(str(e))
24+
25+
return value
26+
27+
28+
class BaseActionValidator(CamelSnakeModelSerializer[T], Generic[T]):
29+
data: Any = serializers.JSONField()
30+
config: Any = serializers.JSONField()
31+
32+
class Meta:
33+
model = T
34+
fields = ["config", "data", "integration_id", "type"]
35+
36+
def _get_action_handler(self) -> ActionHandler:
37+
initial_type = self.initial_data.get("type")
38+
action_type = self.validate_type(initial_type)
39+
return action_handler_registry.get(action_type)
40+
41+
def validate_type(self, value) -> Action.Type:
42+
if not value:
43+
raise ValidationError("Action type is required")
44+
45+
try:
46+
Action.Type(value)
47+
except ValueError:
48+
raise ValidationError("Invalid action type")
49+
50+
return value
51+
52+
def validate_data(self, value) -> ActionData:
53+
data_schema = self._get_action_handler().data_schema
54+
return validate_json_schema(value, data_schema)
55+
56+
def validate_config(self, value) -> ActionConfig:
57+
config_schema = self._get_action_handler().config_schema
58+
return validate_json_schema(value, config_schema)

src/sentry/workflow_engine/handlers/action/notification/handler.py

+2
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ class NotificationActionHandler(ActionHandler):
7575
},
7676
}
7777

78+
data_schema = {}
79+
7880
@staticmethod
7981
def execute(
8082
job: WorkflowJob,

src/sentry/workflow_engine/models/action.py

+12-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from django.db import models
77
from django.db.models.signals import pre_save
88
from django.dispatch import receiver
9+
from jsonschema import ValidationError, validate
910

1011
from sentry.backup.scopes import RelocationScope
1112
from sentry.db.models import DefaultFieldsModel, region_silo_model, sane_repr
@@ -74,7 +75,15 @@ def trigger(self, job: WorkflowJob, detector: Detector) -> None:
7475
@receiver(pre_save, sender=Action)
7576
def enforce_config_schema(sender, instance: Action, **kwargs):
7677
handler = instance.get_handler()
77-
schema = handler.config_schema
7878

79-
if schema is not None:
80-
instance.validate_config(schema)
79+
config_schema = handler.config_schema
80+
data_schema = handler.data_schema
81+
82+
if config_schema is not None:
83+
instance.validate_config(config_schema)
84+
85+
if data_schema is not None:
86+
try:
87+
validate(instance.data, data_schema)
88+
except ValidationError as e:
89+
raise ValidationError(f"Invalid config: {e.message}")

src/sentry/workflow_engine/types.py

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class WorkflowJob(EventJob, total=False):
4747

4848
class ActionHandler:
4949
config_schema: ClassVar[dict[str, Any]]
50+
data_schema: ClassVar[dict[str, Any]]
5051

5152
@staticmethod
5253
def execute(job: WorkflowJob, action: Action, detector: Detector) -> None:

tests/sentry/workflow_engine/endpoints/test_validators.py

+120-2
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,18 @@
2727
from sentry.snuba.snuba_query_validator import SnubaQueryValidator
2828
from sentry.testutils.cases import TestCase
2929
from sentry.workflow_engine.endpoints.validators.base import (
30+
BaseActionValidator,
3031
BaseDataConditionGroupValidator,
3132
BaseDataSourceValidator,
3233
BaseDetectorTypeValidator,
3334
DataSourceCreator,
3435
NumericComparisonConditionValidator,
3536
)
36-
from sentry.workflow_engine.models import DataCondition, DataConditionGroup, DataSource
37+
from sentry.workflow_engine.models import Action, DataCondition, DataConditionGroup, DataSource
3738
from sentry.workflow_engine.models.data_condition import Condition
3839
from sentry.workflow_engine.models.detector import Detector
3940
from sentry.workflow_engine.registry import data_source_type_registry
40-
from sentry.workflow_engine.types import DetectorPriorityLevel
41+
from sentry.workflow_engine.types import ActionHandler, DetectorPriorityLevel
4142

4243

4344
class BaseValidatorTest(TestCase):
@@ -546,3 +547,120 @@ class MockConditionGroupValidator(BaseDataConditionGroupValidator):
546547
class MockDetectorValidator(BaseDetectorTypeValidator):
547548
data_source = MockDataSourceValidator()
548549
condition_group = MockConditionGroupValidator()
550+
551+
552+
class MockActionHandler(ActionHandler):
553+
config_schema = {
554+
"$schema": "https://json-schema.org/draft/2020-12/schema",
555+
"description": "The configuration schema for a Action Configuration",
556+
"type": "object",
557+
"properties": {
558+
"foo": {
559+
"type": ["string"],
560+
},
561+
},
562+
"required": ["foo"],
563+
"additionalProperties": False,
564+
}
565+
566+
data_schema = {
567+
"$schema": "https://json-schema.org/draft/2020-12/schema",
568+
"description": "The configuration schema for a Action Data",
569+
"type": "object",
570+
"properties": {
571+
"baz": {
572+
"type": ["string"],
573+
},
574+
},
575+
"required": ["baz"],
576+
"additionalProperties": False,
577+
}
578+
579+
580+
class MockActionValidator(BaseActionValidator[MockModel]):
581+
field1 = serializers.CharField()
582+
583+
class Meta:
584+
model = MockModel
585+
fields = "__all__"
586+
587+
588+
@mock.patch(
589+
"sentry.workflow_engine.registry.action_handler_registry.get",
590+
return_value=MockActionHandler,
591+
)
592+
class TestBaseActionValidator(TestCase):
593+
def setUp(self):
594+
super().setUp()
595+
self.valid_data = {
596+
"field1": "test",
597+
"type": Action.Type.SLACK,
598+
"config": {"foo": "bar"},
599+
"data": {"baz": "bar"},
600+
}
601+
602+
def test_validate_type(self, mock_action_handler_get):
603+
validator = MockActionValidator(
604+
data={
605+
**self.valid_data,
606+
"type": Action.Type.SLACK,
607+
},
608+
)
609+
610+
result = validator.is_valid()
611+
assert result is True
612+
613+
def test_validate_type__invalid(self, mock_action_handler_get):
614+
validator = MockActionValidator(
615+
data={
616+
**self.valid_data,
617+
"type": "invalid_test",
618+
}
619+
)
620+
621+
result = validator.is_valid()
622+
assert result is False
623+
624+
def test_validate_config(self, mock_action_handler_get):
625+
validator = MockActionValidator(
626+
data={
627+
**self.valid_data,
628+
"config": {"foo": "bar"},
629+
},
630+
)
631+
632+
result = validator.is_valid()
633+
assert result is True
634+
635+
def test_validate_config__invalid(self, mock_action_handler_get):
636+
validator = MockActionValidator(
637+
data={
638+
**self.valid_data,
639+
"config": {"invalid": 1},
640+
},
641+
)
642+
643+
result = validator.is_valid()
644+
assert result is False
645+
646+
def test_validate_data(self, mock_action_handler_get):
647+
validator = MockActionValidator(
648+
data={
649+
**self.valid_data,
650+
"data": {"baz": "foo"},
651+
},
652+
)
653+
654+
result = validator.is_valid()
655+
assert result is True
656+
657+
def test_validate_data__invalid(self, mock_action_handler_get):
658+
validator = MockActionValidator(
659+
data={
660+
**self.valid_data,
661+
"data": {"invalid": 1},
662+
},
663+
)
664+
665+
result = validator.is_valid()
666+
assert result is False

tests/sentry/workflow_engine/models/test_action.py

+40-4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import pytest
44
from django.test import TestCase
5+
from jsonschema import ValidationError
56

67
from sentry.eventstore.models import GroupEvent
78
from sentry.utils.registry import NoRegistrationExistsError
@@ -19,10 +20,16 @@ def setUp(self):
1920
"$schema": "https://json-schema.org/draft/2020-12/schema",
2021
"description": "A representation of a user profile",
2122
"type": "object",
22-
"required": [],
2323
"properties": {
2424
"foo": {"type": "string"},
2525
},
26+
"additionalProperties": False,
27+
}
28+
29+
self.valid_params = {
30+
"type": Action.Type.SLACK,
31+
"config": {"foo": "bar"},
32+
"data": {"foo": "bar"},
2633
}
2734

2835
def test_get_handler_notification_type(self):
@@ -82,15 +89,44 @@ def test_trigger_with_failing_handler(self):
8289
def test_config_schema(self):
8390
mock_handler = Mock(spec=ActionHandler)
8491
mock_handler.config_schema = self.config_schema
92+
mock_handler.data_schema = self.config_schema
8593

8694
with patch.object(Action, "get_handler", return_value=mock_handler):
87-
result = Action.objects.create(type=Action.Type.SLACK, config={"foo": "bar"})
95+
params = self.valid_params.copy()
96+
params["config"] = {"foo": "bar"}
97+
result = Action.objects.create(**params)
8898
assert result is not None
8999

90100
def test_config_schema__invalid(self):
91101
mock_handler = Mock(spec=ActionHandler)
92102
mock_handler.config_schema = self.config_schema
103+
mock_handler.data_schema = self.config_schema
104+
105+
with patch.object(Action, "get_handler", return_value=mock_handler):
106+
with pytest.raises(ValidationError):
107+
params = self.valid_params.copy()
108+
params["config"] = {"baz": 42}
109+
Action.objects.create(**params)
110+
111+
def test_data_schema(self):
112+
mock_handler = Mock(spec=ActionHandler)
113+
mock_handler.config_schema = self.config_schema
114+
mock_handler.data_schema = self.config_schema
115+
116+
with patch.object(Action, "get_handler", return_value=mock_handler):
117+
params = self.valid_params.copy()
118+
params["data"] = {"foo": "bar"}
119+
result = Action.objects.create(**params)
120+
121+
assert result is not None
122+
123+
def test_data_schema__invalid(self):
124+
mock_handler = Mock(spec=ActionHandler)
125+
mock_handler.config_schema = self.config_schema
126+
mock_handler.data_schema = self.config_schema
93127

94128
with patch.object(Action, "get_handler", return_value=mock_handler):
95-
with pytest.raises(Exception):
96-
Action.objects.create(type=Action.Type.SLACK, config={"foo": 42})
129+
with pytest.raises(ValidationError):
130+
params = self.valid_params.copy()
131+
params["data"] = {"baz": 42}
132+
Action.objects.create(**params)

0 commit comments

Comments
 (0)