From a99e9a5686c2f3ae8ed5a3c9d657c2776093f19c Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 18 Apr 2023 13:03:33 -0600 Subject: [PATCH] Add unacknowledge trigger for new webhooks (#1768) - Add trigger for unacknowledge events in new webhooks - Improve test coverage to include is_webhook_enabled and integration_filter logic --- .../migrations/0004_auto_20230418_0109.py | 23 +++++++ engine/apps/webhooks/models/webhook.py | 4 +- .../apps/webhooks/tasks/alert_group_status.py | 1 + engine/apps/webhooks/tasks/trigger_webhook.py | 5 +- .../tests/test_alert_group_status_change.py | 1 + .../webhooks/tests/test_trigger_webhook.py | 67 +++++++++++++++++++ .../OutgoingWebhook2Form.config.tsx | 5 ++ 7 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 engine/apps/webhooks/migrations/0004_auto_20230418_0109.py diff --git a/engine/apps/webhooks/migrations/0004_auto_20230418_0109.py b/engine/apps/webhooks/migrations/0004_auto_20230418_0109.py new file mode 100644 index 00000000..ff70b580 --- /dev/null +++ b/engine/apps/webhooks/migrations/0004_auto_20230418_0109.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.18 on 2023-04-18 01:09 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('webhooks', '0003_auto_20230412_0006'), + ] + + operations = [ + migrations.AlterField( + model_name='webhook', + name='trigger_type', + field=models.IntegerField(choices=[(0, 'Escalation step'), (1, 'Firing'), (2, 'Acknowledged'), (3, 'Resolved'), (4, 'Silenced'), (5, 'Unsilenced'), (6, 'Unresolved'), (7, 'Unacknowledged')], default=None, null=True), + ), + migrations.AlterField( + model_name='webhookresponse', + name='trigger_type', + field=models.IntegerField(choices=[(0, 'Escalation step'), (1, 'Firing'), (2, 'Acknowledged'), (3, 'Resolved'), (4, 'Silenced'), (5, 'Unsilenced'), (6, 'Unresolved'), (7, 'Unacknowledged')]), + ), + ] diff --git a/engine/apps/webhooks/models/webhook.py b/engine/apps/webhooks/models/webhook.py index 11713eaa..97c7bdcc 100644 --- a/engine/apps/webhooks/models/webhook.py +++ b/engine/apps/webhooks/models/webhook.py @@ -63,7 +63,8 @@ class Webhook(models.Model): TRIGGER_SILENCE, TRIGGER_UNSILENCE, TRIGGER_UNRESOLVE, - ) = range(7) + TRIGGER_UNACKNOWLEDGE, + ) = range(8) # Must be the same order as previous TRIGGER_TYPES = ( @@ -74,6 +75,7 @@ class Webhook(models.Model): (TRIGGER_SILENCE, "Silenced"), (TRIGGER_UNSILENCE, "Unsilenced"), (TRIGGER_UNRESOLVE, "Unresolved"), + (TRIGGER_UNACKNOWLEDGE, "Unacknowledged"), ) public_primary_key = models.CharField( diff --git a/engine/apps/webhooks/tasks/alert_group_status.py b/engine/apps/webhooks/tasks/alert_group_status.py index 5db24161..2538127a 100644 --- a/engine/apps/webhooks/tasks/alert_group_status.py +++ b/engine/apps/webhooks/tasks/alert_group_status.py @@ -21,6 +21,7 @@ ACTION_TO_TRIGGER_TYPE = { AlertGroupLogRecord.TYPE_SILENCE: Webhook.TRIGGER_SILENCE, AlertGroupLogRecord.TYPE_UN_SILENCE: Webhook.TRIGGER_UNSILENCE, AlertGroupLogRecord.TYPE_UN_RESOLVED: Webhook.TRIGGER_UNRESOLVE, + AlertGroupLogRecord.TYPE_UN_ACK: Webhook.TRIGGER_UNACKNOWLEDGE, } diff --git a/engine/apps/webhooks/tasks/trigger_webhook.py b/engine/apps/webhooks/tasks/trigger_webhook.py index 03e2e55c..a1dc542e 100644 --- a/engine/apps/webhooks/tasks/trigger_webhook.py +++ b/engine/apps/webhooks/tasks/trigger_webhook.py @@ -20,6 +20,8 @@ from apps.webhooks.utils import ( ) from common.custom_celery_tasks import shared_dedicated_queue_retry_task +NOT_FROM_SELECTED_INTEGRATION = "Alert group was not from a selected integration" + logger = get_task_logger(__name__) logger.setLevel(logging.DEBUG) @@ -32,6 +34,7 @@ TRIGGER_TYPE_TO_LABEL = { Webhook.TRIGGER_UNSILENCE: "unsilence", Webhook.TRIGGER_UNRESOLVE: "unresolve", Webhook.TRIGGER_ESCALATION_STEP: "escalation", + Webhook.TRIGGER_UNACKNOWLEDGE: "unacknowledge", } @@ -102,7 +105,7 @@ def make_request(webhook, alert_group, data): exception = error = None try: if not webhook.check_integration_filter(alert_group): - status["request_trigger"] = f"Alert group was not from a selected integration" + status["request_trigger"] = NOT_FROM_SELECTED_INTEGRATION return status, None, None triggered, status["request_trigger"] = webhook.check_trigger(data) diff --git a/engine/apps/webhooks/tests/test_alert_group_status_change.py b/engine/apps/webhooks/tests/test_alert_group_status_change.py index 3b7e8f2a..ae66f9ea 100644 --- a/engine/apps/webhooks/tests/test_alert_group_status_change.py +++ b/engine/apps/webhooks/tests/test_alert_group_status_change.py @@ -67,6 +67,7 @@ def test_alert_group_created_does_not_exist(make_organization, make_custom_webho (AlertGroupLogRecord.TYPE_SILENCE, Webhook.TRIGGER_SILENCE), (AlertGroupLogRecord.TYPE_UN_SILENCE, Webhook.TRIGGER_UNSILENCE), (AlertGroupLogRecord.TYPE_UN_RESOLVED, Webhook.TRIGGER_UNRESOLVE), + (AlertGroupLogRecord.TYPE_UN_ACK, Webhook.TRIGGER_UNACKNOWLEDGE), ], ) def test_alert_group_status_change( diff --git a/engine/apps/webhooks/tests/test_trigger_webhook.py b/engine/apps/webhooks/tests/test_trigger_webhook.py index 9380f560..7d948e75 100644 --- a/engine/apps/webhooks/tests/test_trigger_webhook.py +++ b/engine/apps/webhooks/tests/test_trigger_webhook.py @@ -9,6 +9,7 @@ from apps.base.models import UserNotificationPolicyLogRecord from apps.public_api.serializers import IncidentSerializer from apps.webhooks.models import Webhook from apps.webhooks.tasks import execute_webhook, send_webhook_event +from apps.webhooks.tasks.trigger_webhook import NOT_FROM_SELECTED_INTEGRATION class MockResponse: @@ -63,6 +64,72 @@ def test_send_webhook_event_filters( assert mock_execute.call_args == call((other_org_webhook.pk, alert_group.pk, None, None)) +@pytest.mark.django_db +def test_execute_webhook_disabled( + make_organization, make_team, make_alert_receive_channel, make_alert_group, make_custom_webhook +): + organization = make_organization() + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + make_custom_webhook(organization=organization, trigger_type=Webhook.TRIGGER_FIRING) + make_custom_webhook(organization=organization, trigger_type=Webhook.TRIGGER_FIRING, is_webhook_enabled=False) + + with patch("apps.webhooks.tasks.trigger_webhook.execute_webhook.apply_async") as mock_execute: + send_webhook_event(Webhook.TRIGGER_FIRING, alert_group.pk, organization_id=organization.pk) + mock_execute.assert_called_once() + + +@pytest.mark.django_db +def test_execute_webhook_integration_filter_not_matching( + make_organization, make_team, make_alert_receive_channel, make_alert_group, make_custom_webhook +): + organization = make_organization() + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + webhook = make_custom_webhook( + organization=organization, trigger_type=Webhook.TRIGGER_FIRING, integration_filter=["does-not-match"] + ) + + with patch("apps.webhooks.models.webhook.requests") as mock_requests: + execute_webhook(webhook.pk, alert_group.pk, None, None) + + assert not mock_requests.post.called + # check log should exist but have no status code + assert ( + webhook.responses.count() == 1 + and webhook.responses.first().status_code is None + and webhook.responses.first().request_trigger == NOT_FROM_SELECTED_INTEGRATION + ) + + +@pytest.mark.django_db +def test_execute_webhook_integration_filter_matching( + make_organization, make_team, make_alert_receive_channel, make_alert_group, make_custom_webhook +): + organization = make_organization() + alert_receive_channel = make_alert_receive_channel(organization, public_primary_key="test-integration-1") + alert_group = make_alert_group(alert_receive_channel) + webhook = make_custom_webhook( + organization=organization, + trigger_type=Webhook.TRIGGER_FIRING, + integration_filter=["test-integration-1"], + # Check we get past integration filter but exit early to keep test simple + trigger_template="False", + ) + + with patch("apps.webhooks.models.webhook.requests") as mock_requests: + execute_webhook(webhook.pk, alert_group.pk, None, None) + + assert not mock_requests.post.called + # check log should exist but have no status code + assert ( + webhook.responses.count() == 1 + and webhook.responses.first().status_code is None + # Matches evaluated trigger_template + and webhook.responses.first().request_trigger == "False" + ) + + @pytest.mark.django_db def test_execute_webhook_ok( make_organization, make_user_for_organization, make_alert_receive_channel, make_alert_group, make_custom_webhook diff --git a/grafana-plugin/src/containers/OutgoingWebhook2Form/OutgoingWebhook2Form.config.tsx b/grafana-plugin/src/containers/OutgoingWebhook2Form/OutgoingWebhook2Form.config.tsx index 2bf504c0..ad4d9bf3 100644 --- a/grafana-plugin/src/containers/OutgoingWebhook2Form/OutgoingWebhook2Form.config.tsx +++ b/grafana-plugin/src/containers/OutgoingWebhook2Form/OutgoingWebhook2Form.config.tsx @@ -14,6 +14,7 @@ export const WebhookTriggerType = { Silenced: new KeyValuePair('4', 'Silenced'), Unsilenced: new KeyValuePair('5', 'Unsilenced'), Unresolved: new KeyValuePair('6', 'Unresolved'), + Unacknowledged: new KeyValuePair('7', 'Unacknowledged'), }; export const form: { name: string; fields: FormItem[] } = { @@ -78,6 +79,10 @@ export const form: { name: string; fields: FormItem[] } = { value: WebhookTriggerType.Unresolved.key, label: WebhookTriggerType.Unresolved.value, }, + { + value: WebhookTriggerType.Unacknowledged.key, + label: WebhookTriggerType.Unacknowledged.value, + }, ], }, validation: { required: true },