From 3c2c2597214a31c157508ec9bcd58b1b4bb5484a Mon Sep 17 00:00:00 2001 From: Rares Mardare Date: Tue, 16 Jan 2024 17:17:42 +0200 Subject: [PATCH 1/8] Proper fix for GForm not resetting the hidden fields to undefined (#3691) # What this PR does ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/3690 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 4 +++ grafana-plugin/src/components/GForm/GForm.tsx | 14 +++++++--- .../src/components/GForm/GForm.types.ts | 2 +- .../TextEllipsisTooltip.tsx | 2 +- .../OutgoingWebhookForm.config.tsx | 26 +++++++++---------- .../OutgoingWebhookForm.module.css | 1 + .../outgoing_webhooks/OutgoingWebhooks.tsx | 9 +++---- 7 files changed, 35 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 865a83fd..75c35b02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +- Fixed Webhooks UI not allowing simple webhooks to be created ([#3691](https://github.com/grafana/oncall/pull/3691)) + ## v1.3.88 (2024-01-16) ### Fixed diff --git a/grafana-plugin/src/components/GForm/GForm.tsx b/grafana-plugin/src/components/GForm/GForm.tsx index 20c83e3a..96324f67 100644 --- a/grafana-plugin/src/components/GForm/GForm.tsx +++ b/grafana-plugin/src/components/GForm/GForm.tsx @@ -189,8 +189,8 @@ class GForm extends React.Component {
{({ register, errors, control, getValues, setValue }) => { const renderField = (formItem: FormItem, formIndex: number) => { - if (formItem.isVisible && !formItem.isVisible(getValues())) { - return null; + if (this.isFormItemHidden(formItem, getValues())) { + return null; // don't render the field } const disabled = formItem.disabled @@ -270,13 +270,21 @@ class GForm extends React.Component { this.forceUpdate(); }; + isFormItemHidden(formItem: FormItem, data) { + return formItem?.isHidden?.(data); + } + handleSubmit = (data) => { const { form, onSubmit } = this.props; const normalizedData = Object.keys(data).reduce((acc, key) => { const formItem = form.fields.find((formItem) => formItem.name === key); - const value = formItem?.normalize ? formItem.normalize(data[key]) : nullNormalizer(data[key]); + let value = formItem?.normalize ? formItem.normalize(data[key]) : nullNormalizer(data[key]); + + if (this.isFormItemHidden(formItem, data)) { + value = undefined; + } return { ...acc, diff --git a/grafana-plugin/src/components/GForm/GForm.types.ts b/grafana-plugin/src/components/GForm/GForm.types.ts index 710c8f2d..90d3f8ed 100644 --- a/grafana-plugin/src/components/GForm/GForm.types.ts +++ b/grafana-plugin/src/components/GForm/GForm.types.ts @@ -21,7 +21,7 @@ export interface FormItem { description?: ReactNode; placeholder?: string; normalize?: (value: any) => any; - isVisible?: (data: any) => any; + isHidden?: (data: any) => any; getDisabled?: (value: any) => any; validation?: { required?: boolean; diff --git a/grafana-plugin/src/components/TextEllipsisTooltip/TextEllipsisTooltip.tsx b/grafana-plugin/src/components/TextEllipsisTooltip/TextEllipsisTooltip.tsx index 80164980..cdab55f8 100644 --- a/grafana-plugin/src/components/TextEllipsisTooltip/TextEllipsisTooltip.tsx +++ b/grafana-plugin/src/components/TextEllipsisTooltip/TextEllipsisTooltip.tsx @@ -9,7 +9,7 @@ import { TEXT_ELLIPSIS_CLASS } from 'utils/consts'; const cx = cn.bind(styles); interface TextEllipsisTooltipProps { - content: string; + content?: string; queryClassName?: string; placement?: string; className?: string; diff --git a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.tsx b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.tsx index d421c748..eeb7bbfb 100644 --- a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.tsx +++ b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.tsx @@ -100,7 +100,7 @@ export function createForm( }, ], }, - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.TriggerType), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.TriggerType), normalize: (value) => value, }, { @@ -136,16 +136,16 @@ export function createForm( }, ], }, - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.HttpMethod), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.HttpMethod), normalize: (value) => value, }, { name: WebhookFormFieldName.IntegrationFilter, label: 'Integrations', type: FormItemType.MultiSelect, - isVisible: (data) => - isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.IntegrationFilter) && - data.trigger_type !== WebhookTriggerType.EscalationStep.key, + isHidden: (data) => + !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.IntegrationFilter) || + data.trigger_type === WebhookTriggerType.EscalationStep.key, extra: { placeholder: 'Choose (Optional)', modelName: 'alertReceiveChannelStore', @@ -170,7 +170,7 @@ export function createForm( extra: { height: 30, }, - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Url), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Url), }, { name: WebhookFormFieldName.Headers, @@ -180,24 +180,24 @@ export function createForm( extra: { rows: 3, }, - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Headers), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Headers), }, { name: WebhookFormFieldName.Username, type: FormItemType.Input, - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Username), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Username), }, { name: WebhookFormFieldName.Password, type: FormItemType.Password, - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Password), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Password), }, { name: WebhookFormFieldName.AuthorizationHeader, description: 'Value of the Authorization header, do not need to prefix with "Authorization:". For example: Bearer AbCdEf123456', type: FormItemType.Password, - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.AuthorizationHeader), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.AuthorizationHeader), }, { name: WebhookFormFieldName.TriggerTemplate, @@ -207,14 +207,14 @@ export function createForm( extra: { rows: 2, }, - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.TriggerTemplate), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.TriggerTemplate), }, { name: WebhookFormFieldName.ForwardAll, normalize: (value) => (value ? Boolean(value) : value), type: FormItemType.Switch, description: "Forwards whole payload of the alert group and context data to the webhook's url as POST/PUT data", - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.ForwardAll), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.ForwardAll), }, { name: WebhookFormFieldName.Data, @@ -224,7 +224,7 @@ export function createForm( hasLabelsFeature ? ' {{ webhook }}' : '' }`, extra: {}, - isVisible: (data) => isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Data), + isHidden: (data) => !isPresetFieldVisible(data.preset, presets, WebhookFormFieldName.Data), }, ], }; diff --git a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.module.css b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.module.css index 3a41fb24..5d2ed084 100644 --- a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.module.css +++ b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.module.css @@ -12,6 +12,7 @@ .tabs__content { padding-top: 16px; + padding-bottom: 16px; } .form-row { diff --git a/grafana-plugin/src/pages/outgoing_webhooks/OutgoingWebhooks.tsx b/grafana-plugin/src/pages/outgoing_webhooks/OutgoingWebhooks.tsx index 99afa405..3a6a79c3 100644 --- a/grafana-plugin/src/pages/outgoing_webhooks/OutgoingWebhooks.tsx +++ b/grafana-plugin/src/pages/outgoing_webhooks/OutgoingWebhooks.tsx @@ -269,11 +269,7 @@ class OutgoingWebhooks extends React.Component - - - ); + return ; } renderActionButtons = (record: OutgoingWebhook) => { @@ -435,6 +431,9 @@ class OutgoingWebhooks extends React.Component this.update()) From 6c248ed1c8ea1343716c4764bae8d49bbfb2068b Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Wed, 17 Jan 2024 13:00:25 +0000 Subject: [PATCH 2/8] Fix posting Slack message when route is deleted (#3702) # What this PR does Fixes https://github.com/grafana/oncall/issues/3646 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 1 + .../apps/slack/scenarios/distribute_alerts.py | 7 ++++- .../test_distribute_alerts.py | 28 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75c35b02..4025b42e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed Webhooks UI not allowing simple webhooks to be created ([#3691](https://github.com/grafana/oncall/pull/3691)) +- Fix posting Slack message when route is deleted by @vadimkerr ([#3702](https://github.com/grafana/oncall/pull/3702)) ## v1.3.88 (2024-01-16) diff --git a/engine/apps/slack/scenarios/distribute_alerts.py b/engine/apps/slack/scenarios/distribute_alerts.py index e6a30191..873410f9 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -76,7 +76,12 @@ class AlertShootingStep(scenario_step.ScenarioStep): if num_updated_rows == 1: try: - channel_id = alert.group.channel_filter.slack_channel_id_or_general_log_id + channel_id = ( + alert.group.channel_filter.slack_channel_id_or_general_log_id + if alert.group.channel_filter + # if channel filter is deleted mid escalation, use default Slack channel + else alert.group.channel.organization.general_log_channel_id + ) self._send_first_alert(alert, channel_id) except SlackAPIError: AlertGroup.objects.filter(pk=alert.group.pk).update(slack_message_sent=False) diff --git a/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py b/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py index bb8a39c5..1eba2b9a 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py @@ -5,6 +5,7 @@ import pytest from apps.alerts.models import AlertGroup from apps.slack.errors import SlackAPIRestrictedActionError from apps.slack.models import SlackMessage +from apps.slack.scenarios.distribute_alerts import AlertShootingStep from apps.slack.scenarios.scenario_step import ScenarioStep from apps.slack.tests.conftest import build_slack_response @@ -36,3 +37,30 @@ def test_restricted_action_error( assert alert_group.slack_message is None assert SlackMessage.objects.count() == 0 assert not alert.delivered + + +@patch.object(AlertShootingStep, "_post_alert_group_to_slack") +@pytest.mark.django_db +def test_alert_shooting_no_channel_filter( + mock_post_alert_group_to_slack, + make_slack_team_identity, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, +): + slack_team_identity = make_slack_team_identity() + organization = make_organization( + slack_team_identity=slack_team_identity, general_log_channel_id="DEFAULT_CHANNEL_ID" + ) + alert_receive_channel = make_alert_receive_channel(organization) + + # simulate an alert group with channel filter deleted in the middle of the escalation + alert_group = make_alert_group(alert_receive_channel, channel_filter=None) + alert = make_alert(alert_group, raw_request_data={}) + + step = AlertShootingStep(slack_team_identity, organization) + step.process_signal(alert) + + mock_post_alert_group_to_slack.assert_called_once() + assert mock_post_alert_group_to_slack.call_args[1]["channel_id"] == "DEFAULT_CHANNEL_ID" From c7895c23089818d72f421a69fcd536855b2f3233 Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Wed, 17 Jan 2024 14:05:36 +0100 Subject: [PATCH 3/8] Fix post message to slack channel (#3701) # What this PR does Extend list of exceptions to ignore on posting message to slack channel ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/3694 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- engine/apps/slack/tests/test_utils.py | 34 +++++++++++++++++++++++++++ engine/apps/slack/utils.py | 14 +++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 engine/apps/slack/tests/test_utils.py diff --git a/engine/apps/slack/tests/test_utils.py b/engine/apps/slack/tests/test_utils.py new file mode 100644 index 00000000..eea12ae9 --- /dev/null +++ b/engine/apps/slack/tests/test_utils.py @@ -0,0 +1,34 @@ +from unittest.mock import Mock, patch + +import pytest + +from apps.slack.client import SlackClient +from apps.slack.errors import ( + SlackAPIChannelArchivedError, + SlackAPIChannelNotFoundError, + SlackAPIError, + SlackAPIInvalidAuthError, + SlackAPITokenError, +) +from apps.slack.utils import post_message_to_channel + + +@pytest.mark.parametrize( + "error,raise_exception", + [ + (SlackAPITokenError, False), + (SlackAPIChannelNotFoundError, False), + (SlackAPIChannelArchivedError, False), + (SlackAPIInvalidAuthError, False), + (SlackAPIError, True), + ], +) +def test_post_message_to_channel(error, raise_exception): + organization = Mock() + with patch.object(SlackClient, "chat_postMessage", side_effect=error(Mock())) as mock_chat_postMessage: + if raise_exception: + with pytest.raises(SlackAPIError): + post_message_to_channel(organization, "test", "test") + else: + post_message_to_channel(organization, "test", "test") + mock_chat_postMessage.assert_called_once() diff --git a/engine/apps/slack/utils.py b/engine/apps/slack/utils.py index 0412f09a..5498c240 100644 --- a/engine/apps/slack/utils.py +++ b/engine/apps/slack/utils.py @@ -3,7 +3,12 @@ import typing from datetime import datetime from apps.slack.client import SlackClient -from apps.slack.errors import SlackAPIChannelNotFoundError +from apps.slack.errors import ( + SlackAPIChannelArchivedError, + SlackAPIChannelNotFoundError, + SlackAPIInvalidAuthError, + SlackAPITokenError, +) if typing.TYPE_CHECKING: from apps.user_management.models import Organization @@ -68,7 +73,12 @@ def post_message_to_channel(organization: "Organization", channel_id: str, text: slack_client = SlackClient(organization.slack_team_identity) try: slack_client.chat_postMessage(channel=channel_id, text=text) - except SlackAPIChannelNotFoundError: + except ( + SlackAPITokenError, + SlackAPIInvalidAuthError, + SlackAPIChannelNotFoundError, + SlackAPIChannelArchivedError, + ): pass From 36d2c3bdb77a67babaf88b26ed3c94a27bfbfbc1 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Wed, 17 Jan 2024 21:49:36 +0800 Subject: [PATCH 4/8] Adds new templates cheatsheats (#3643) Co-authored-by: Maxim Mordasov --- .../test_alert_receive_channel_template.py | 47 +++++- engine/common/api_helpers/mixins.py | 18 ++- .../CommonAlertTemplatesForm.config.ts | 38 ++--- .../CheatSheet/CheatSheet.config.ts | 145 +++++++++++++++++- .../IntegrationLabelsForm.tsx | 9 +- .../IntegrationTemplate.tsx | 42 ++--- .../OutgoingWebhookForm.tsx | 8 +- .../TemplatePreview.module.css | 4 + .../TemplatePreview/TemplatePreview.tsx | 39 ++++- .../WebhooksTemplateEditor.tsx | 15 +- .../pages/integration/Integration.config.ts | 50 ------ .../integration/IntegrationCommon.config.ts | 74 +-------- 12 files changed, 314 insertions(+), 175 deletions(-) delete mode 100644 grafana-plugin/src/pages/integration/Integration.config.ts diff --git a/engine/apps/api/tests/test_alert_receive_channel_template.py b/engine/apps/api/tests/test_alert_receive_channel_template.py index f494d776..25c706d6 100644 --- a/engine/apps/api/tests/test_alert_receive_channel_template.py +++ b/engine/apps/api/tests/test_alert_receive_channel_template.py @@ -334,7 +334,48 @@ def test_preview_alert_receive_channel_backend_templater( response = client.post(url, format="json", data=data, **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.json() == {"preview": "title: alert!"} + assert response.json() == {"preview": "title: alert!", "is_valid_json_object": False} + + +@pytest.mark.django_db +def test_alert_receive_channel_template_is_valid_json_check( + make_organization_and_user_with_plugin_token, + make_user_auth_headers, + make_alert_receive_channel, + make_channel_filter, + make_alert_group, + make_alert, +): + organization, user, token = make_organization_and_user_with_plugin_token() + alert_receive_channel = make_alert_receive_channel(organization) + default_channel_filter = make_channel_filter(alert_receive_channel, is_default=True) + alert_group = make_alert_group(alert_receive_channel, channel_filter=default_channel_filter) + make_alert(alert_group=alert_group, raw_request_data={"title": "alert!"}) + client = APIClient() + + url = reverse( + "api-internal:alert_receive_channel-preview-template", kwargs={"pk": alert_receive_channel.public_primary_key} + ) + + # template which should produce valid json string + data = { + "template_body": "{{ payload | tojson }}", + "template_name": "alert_group_multi_label", + } + response = client.post(url, format="json", data=data, **make_user_auth_headers(user, token)) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["is_valid_json_object"] is True + + # template which produce not avalid json string + data = { + "template_body": "{{ payload.title }}", + "template_name": "alert_group_multi_label", + } + response = client.post(url, format="json", data=data, **make_user_auth_headers(user, token)) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["is_valid_json_object"] is False @pytest.mark.django_db @@ -360,12 +401,12 @@ def test_preview_alert_group_labels( data = { "template_body": "{{ payload.labels | tojson }}", - "template_name": "alert_group_labels", + "template_name": "alert_group_multi_label", } response = client.post(url, format="json", data=data, **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.json() == {"preview": '{"1": "2"}'} + assert response.json() == {"preview": '{"1": "2"}', "is_valid_json_object": True} @pytest.mark.django_db diff --git a/engine/common/api_helpers/mixins.py b/engine/common/api_helpers/mixins.py index 7bce174e..9b06f537 100644 --- a/engine/common/api_helpers/mixins.py +++ b/engine/common/api_helpers/mixins.py @@ -250,6 +250,8 @@ GROUPING_ID = "grouping_id" SOURCE_LINK = "source_link" ROUTE = "route" ALERT_GROUP_LABELS = "alert_group_labels" +ALERT_GROUP_MULTI_LABEL = "alert_group_multi_label" +ALERT_GROUP_DYNAMIC_LABEL = "alert_group_dynamic_label" NOTIFICATION_CHANNEL_TO_TEMPLATER_MAP = { SLACK: AlertSlackTemplater, @@ -272,7 +274,8 @@ BEHAVIOUR_TEMPLATE_NAMES = [ GROUPING_ID, SOURCE_LINK, ROUTE, - ALERT_GROUP_LABELS, + ALERT_GROUP_MULTI_LABEL, + ALERT_GROUP_DYNAMIC_LABEL, ] ALL_TEMPLATE_NAMES = APPEARANCE_TEMPLATE_NAMES + BEHAVIOUR_TEMPLATE_NAMES @@ -294,7 +297,10 @@ class PreviewTemplateMixin: ), responses=inline_serializer( name="PreviewTemplateResponse", - fields={"preview": serializers.CharField(allow_null=True)}, + fields={ + "preview": serializers.CharField(allow_null=True), + "is_valid_json_object": serializers.BooleanField(), + }, ), ) @action(methods=["post"], detail=True) @@ -351,9 +357,15 @@ class PreviewTemplateMixin: return Response({"preview": e.fallback_message}, status.HTTP_200_OK) else: templated_attr = None - response = {"preview": templated_attr} + response = {"preview": templated_attr, "is_valid_json_object": self.is_valid_json_object(templated_attr)} return Response(response, status=status.HTTP_200_OK) + def is_valid_json_object(self, json_str): + try: + return isinstance(json.loads(json_str), dict) + except ValueError: + return False + def get_alert_to_template(self, payload=None): raise NotImplementedError diff --git a/grafana-plugin/src/components/AlertTemplates/CommonAlertTemplatesForm.config.ts b/grafana-plugin/src/components/AlertTemplates/CommonAlertTemplatesForm.config.ts index 9b87e9af..41999de7 100644 --- a/grafana-plugin/src/components/AlertTemplates/CommonAlertTemplatesForm.config.ts +++ b/grafana-plugin/src/components/AlertTemplates/CommonAlertTemplatesForm.config.ts @@ -1,4 +1,4 @@ -import { BaseTemplateOptions } from 'pages/integration/IntegrationCommon.config'; +import { IntegrationTemplateOptions } from 'pages/integration/IntegrationCommon.config'; export interface Template { name: string; @@ -22,19 +22,19 @@ export interface TemplateForEdit { export const commonTemplateForEdit: { [id: string]: TemplateForEdit } = { web_title_template: { displayName: 'Web title', - name: BaseTemplateOptions.WebTitle.key, + name: IntegrationTemplateOptions.WebTitle.key, description: '', type: 'html', }, web_message_template: { displayName: 'Web message', - name: BaseTemplateOptions.WebMessage.key, + name: IntegrationTemplateOptions.WebMessage.key, description: '', type: 'html', }, slack_title_template: { displayName: 'Slack title', - name: BaseTemplateOptions.SlackTitle.key, + name: IntegrationTemplateOptions.SlackTitle.key, description: '', additionalData: { chatOpsName: 'slack', @@ -44,26 +44,26 @@ export const commonTemplateForEdit: { [id: string]: TemplateForEdit } = { type: 'plain', }, sms_title_template: { - name: BaseTemplateOptions.SMS.key, + name: IntegrationTemplateOptions.SMS.key, displayName: 'Sms title', description: "Result of this template will be used as title of SMS message. Please don't include any urls, or phone numbers, to avoid SMS message being blocked by carriers.", type: 'plain', }, phone_call_title_template: { - name: BaseTemplateOptions.Phone.key, + name: IntegrationTemplateOptions.Phone.key, displayName: 'Phone Call title', description: '', type: 'plain', }, email_title_template: { - name: BaseTemplateOptions.EmailTitle.key, + name: IntegrationTemplateOptions.EmailTitle.key, displayName: 'Email title', description: '', type: 'plain', }, telegram_title_template: { - name: BaseTemplateOptions.TelegramTitle.key, + name: IntegrationTemplateOptions.TelegramTitle.key, displayName: 'Telegram title', description: '', additionalData: { @@ -73,7 +73,7 @@ export const commonTemplateForEdit: { [id: string]: TemplateForEdit } = { type: 'plain', }, slack_message_template: { - name: BaseTemplateOptions.SlackMessage.key, + name: IntegrationTemplateOptions.SlackMessage.key, displayName: 'Slack message', description: '', additionalData: { @@ -84,13 +84,13 @@ export const commonTemplateForEdit: { [id: string]: TemplateForEdit } = { type: 'plain', }, email_message_template: { - name: BaseTemplateOptions.EmailMessage.key, + name: IntegrationTemplateOptions.EmailMessage.key, displayName: 'Email message', description: '', type: 'plain', }, telegram_message_template: { - name: BaseTemplateOptions.TelegramMessage.key, + name: IntegrationTemplateOptions.TelegramMessage.key, displayName: 'Telegram message', description: '', additionalData: { @@ -100,7 +100,7 @@ export const commonTemplateForEdit: { [id: string]: TemplateForEdit } = { type: 'plain', }, slack_image_url_template: { - name: BaseTemplateOptions.SlackImage.key, + name: IntegrationTemplateOptions.SlackImage.key, displayName: 'Slack image url', description: '', additionalData: { @@ -111,13 +111,13 @@ export const commonTemplateForEdit: { [id: string]: TemplateForEdit } = { type: 'plain', }, web_image_url_template: { - name: BaseTemplateOptions.WebImage.key, + name: IntegrationTemplateOptions.WebImage.key, displayName: 'Web image url', description: '', type: 'image', }, telegram_image_url_template: { - name: BaseTemplateOptions.TelegramImage.key, + name: IntegrationTemplateOptions.TelegramImage.key, displayName: 'Telegram image url', description: '', additionalData: { @@ -127,33 +127,33 @@ export const commonTemplateForEdit: { [id: string]: TemplateForEdit } = { type: 'image', }, grouping_id_template: { - name: BaseTemplateOptions.Grouping.key, + name: IntegrationTemplateOptions.Grouping.key, displayName: 'Grouping', description: 'Reduce noise, minimize duplication with Alert Grouping, based on time, alert content, and even multiple features at the same time. Check the cheasheet to customize your template.', type: 'plain', }, acknowledge_condition_template: { - name: BaseTemplateOptions.Autoacknowledge.key, + name: IntegrationTemplateOptions.Autoacknowledge.key, displayName: 'Acknowledge condition', description: '', type: 'boolean', }, resolve_condition_template: { - name: BaseTemplateOptions.Resolve.key, + name: IntegrationTemplateOptions.Resolve.key, displayName: 'Resolve condition', description: 'When monitoring systems return to normal, they can send "resolve" alerts. OnCall can use these signals to resolve alert groups accordingly.', type: 'boolean', }, source_link_template: { - name: BaseTemplateOptions.SourceLink.key, + name: IntegrationTemplateOptions.SourceLink.key, displayName: 'Source link', description: '', type: 'plain', }, route_template: { - name: BaseTemplateOptions.Routing.key, + name: IntegrationTemplateOptions.Routing.key, displayName: 'Routing', description: 'Routes direct alerts to different escalation chains based on the content, such as severity or region.', diff --git a/grafana-plugin/src/components/CheatSheet/CheatSheet.config.ts b/grafana-plugin/src/components/CheatSheet/CheatSheet.config.ts index 478f70aa..b28380be 100644 --- a/grafana-plugin/src/components/CheatSheet/CheatSheet.config.ts +++ b/grafana-plugin/src/components/CheatSheet/CheatSheet.config.ts @@ -58,7 +58,10 @@ export const genericTemplateCheatSheet: CheatSheetInterface = { { name: 'Jinja2 refresher ', listItems: [ - { listItemName: ' {{ payload.labels.foo }} - extract field value' }, + { + listItemName: 'Extract field value', + codeExample: '{{ payload.labels.foo }}', + }, { listItemName: 'Conditions', codeExample: `{%- if "status" in payload %} @@ -120,7 +123,10 @@ export const slackMessageTemplateCheatSheet: CheatSheetInterface = { { name: 'Jinja2 refresher ', listItems: [ - { listItemName: ' {{ payload.labels.foo }} - extract field value' }, + { + listItemName: 'Extract field value', + codeExample: '{{ payload.labels.foo }}', + }, { listItemName: 'Conditions', codeExample: '{%- if "status" in payload %} \n {{ payload.status }} \n {% endif -%}', @@ -172,3 +178,138 @@ export const slackMessageTemplateCheatSheet: CheatSheetInterface = { }, ], }; + +export const alertGroupDynamicLabelCheatSheet: CheatSheetInterface = { + name: 'Dynamic Label cheatsheet', + description: 'Dynamic Label template is used to extract value for a specified key from the alert payload.', + fields: [ + { + name: 'Examples', + listItems: [ + { + listItemName: + 'Extracting the value associated with the "severity" key. If the key is not present, it defaults to "unknown."', + codeExample: `{{ payload.get("severity", "unknown") }}`, + }, + ], + }, + { + name: 'Jinja2 refresher ', + listItems: [ + { + listItemName: 'Extract field value', + codeExample: '{{ payload.labels.foo }}', + }, + { + listItemName: 'Conditions', + codeExample: '{%- if "status" in payload %} \n {{ payload.status }} \n {% endif -%}', + }, + { listItemName: 'Booleans', codeExample: '{{ payload.status == “resolved” }}' }, + { listItemName: 'Loops', codeExample: '{% for label in labels %} \n {{ label.title }} \n {% endfor %}' }, + ], + }, + { + name: 'Additional jinja2 variables', + listItems: [{ listItemName: 'payload - payload of the first alert in the group' }], + }, + ], +}; + +export const alertGroupMultiLabelExtractionCheatSheet: CheatSheetInterface = { + name: 'Multi-label extraction cheatsheet', + description: + 'Multi-label extraction template allows extracting and modifying multiple labels from an alert payload. The Jinja template must result in string, representing valid JSON dictionary. See Examples for getting familiar with the idea', + fields: [ + { + name: 'Examples', + listItems: [ + { + listItemName: 'Extracting all the labels from the specific payload field', + codeExample: `{{ payload.labels | tojson }}`, + }, + { + listItemName: 'Extract labels from different payload fields', + codeExample: `{%- set labels = {} -%} +{# add several labels #} +{%- set labels = dict(labels, **payload.commonLabels) -%} +{# add one label #} +{%- set labels = dict(labels, **{"status": payload.status}) -%} +{# add label not from payload #} +{%- set labels = dict(labels, **{"service": "oncall"}) -%} +{# dump labels dict to json string, so OnCall can parse it #} +{{ labels | tojson }} +`, + }, + ], + }, + { + name: 'Jinja2 refresher ', + listItems: [ + { listItemName: 'Dump a structure to JSON string', codeExample: '{{ payload.labels | tojson }}' }, + { + listItemName: 'Extract field value', + codeExample: '{{ payload.labels.foo }}', + }, + { + listItemName: 'Conditions', + codeExample: '{%- if "status" in payload %} \n {{ payload.status }} \n {% endif -%}', + }, + { listItemName: 'Booleans', codeExample: '{{ payload.status == “resolved” }}' }, + { listItemName: 'Loops', codeExample: '{% for label in labels %} \n {{ label.title }} \n {% endfor %}' }, + ], + }, + { + name: 'Additional jinja2 variables', + listItems: [{ listItemName: 'payload - payload of the first alert in the group' }], + }, + ], +}; + +export const webhookPayloadCheatSheet: CheatSheetInterface = { + name: 'Webhook Payload cheatsheet ', + description: + "Webhook payload template is powered by Jinja2. It's used to process webhook data and customize the output", + fields: [ + { + name: 'Examples', + listItems: [ + { + listItemName: + 'Construct a custom webhook payload from various webhook data fields and output it as a JSON string', + codeExample: `{%- set payload = {} -%} +{# add alert group labels #} +{%- set payload = dict(payload, **{"labels": alert_group.labels}) -%} +{# add some other fields from webhook data just for example #} +{%- set payload = dict(payload, **{"event": event.type, "integration": integration.name}) -%} +{# encode payload dict to json #} +{{ payload | tojson }} +`, + }, + ], + }, + { + name: 'Jinja2 refresher ', + listItems: [ + { + listItemName: 'Extract field value', + codeExample: '{{ payload.labels.foo }}', + }, + { + listItemName: 'Show field value or “N/A” is not exist', + codeExample: '{{ payload.labels.foo | default(“N/A”) }}', + }, + { + listItemName: 'Conditions', + codeExample: '{%- if "status" in payload %} \n {{ payload.status }} \n {% endif -%}', + }, + { listItemName: 'Booleans', codeExample: '{{ payload.status == “resolved” }}' }, + { listItemName: 'Loops', codeExample: '{% for label in labels %} \n {{ label.title }} \n {% endfor %}' }, + { listItemName: 'Dump a structure to JSON string', codeExample: '{{ payload.labels | tojson }}' }, + ], + }, + { + name: 'Additional jinja2 variables', + listItems: [{ listItemName: 'payload - payload of the first alert in the group' }], + }, + ], +}; diff --git a/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx b/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx index ec9024cd..73f54d1e 100644 --- a/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx +++ b/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx @@ -24,6 +24,7 @@ import IntegrationTemplate from 'containers/IntegrationTemplate/IntegrationTempl import { AlertReceiveChannel } from 'models/alert_receive_channel/alert_receive_channel.types'; import { LabelsErrors } from 'models/label/label.types'; import { ApiSchemas } from 'network/oncall-api/api.types'; +import { LabelTemplateOptions } from 'pages/integration/IntegrationCommon.config'; import { useStore } from 'state/useStore'; import { openErrorNotification } from 'utils'; import { DOCS_ROOT } from 'utils/consts'; @@ -199,8 +200,8 @@ const IntegrationLabelsForm = observer((props: IntegrationLabelsFormProps) => { { { const getCheatSheet = (templateKey: string) => { switch (templateKey) { - case BaseTemplateOptions.Grouping.key: - case BaseTemplateOptions.Resolve.key: + case IntegrationTemplateOptions.Grouping.key: + case IntegrationTemplateOptions.Resolve.key: return groupingTemplateCheatSheet; - case BaseTemplateOptions.WebTitle.key: - case BaseTemplateOptions.WebMessage.key: - case BaseTemplateOptions.WebImage.key: + case IntegrationTemplateOptions.WebTitle.key: + case IntegrationTemplateOptions.WebMessage.key: + case IntegrationTemplateOptions.WebImage.key: return genericTemplateCheatSheet; - case BaseTemplateOptions.Autoacknowledge.key: - case BaseTemplateOptions.SourceLink.key: - case BaseTemplateOptions.Phone.key: - case BaseTemplateOptions.SMS.key: - case BaseTemplateOptions.SlackTitle.key: - case BaseTemplateOptions.SlackMessage.key: - case BaseTemplateOptions.SlackImage.key: - case BaseTemplateOptions.TelegramTitle.key: - case BaseTemplateOptions.TelegramMessage.key: - case BaseTemplateOptions.TelegramImage.key: - case BaseTemplateOptions.EmailTitle.key: - case BaseTemplateOptions.EmailMessage.key: + case IntegrationTemplateOptions.Autoacknowledge.key: + case IntegrationTemplateOptions.SourceLink.key: + case IntegrationTemplateOptions.Phone.key: + case IntegrationTemplateOptions.SMS.key: + case IntegrationTemplateOptions.SlackTitle.key: + case IntegrationTemplateOptions.SlackMessage.key: + case IntegrationTemplateOptions.SlackImage.key: + case IntegrationTemplateOptions.TelegramTitle.key: + case IntegrationTemplateOptions.TelegramMessage.key: + case IntegrationTemplateOptions.TelegramImage.key: + case IntegrationTemplateOptions.EmailTitle.key: + case IntegrationTemplateOptions.EmailMessage.key: return slackMessageTemplateCheatSheet; + case LabelTemplateOptions.AlertGroupDynamicLabel.key: + return alertGroupDynamicLabelCheatSheet; + case LabelTemplateOptions.AlertGroupMultiLabel.key: + return alertGroupMultiLabelExtractionCheatSheet; default: return genericTemplateCheatSheet; } diff --git a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx index d5fae6bd..a4ca5403 100644 --- a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx +++ b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx @@ -12,6 +12,7 @@ import { TabsBar, VerticalGroup, } from '@grafana/ui'; +import { capitalCase } from 'change-case'; import cn from 'classnames/bind'; import { observer } from 'mobx-react'; import { useHistory } from 'react-router-dom'; @@ -120,7 +121,12 @@ const OutgoingWebhookForm = observer((props: OutgoingWebhookFormProps) => { const getTemplateEditClickHandler = (formItem: FormItem, values, setFormFieldValue) => { return () => { const formValue = values[formItem.name]; - setTemplateToEdit({ value: formValue, displayName: undefined, description: undefined, name: formItem.name }); + setTemplateToEdit({ + value: formValue, + displayName: `Webhook ${capitalCase(formItem.name)}`, + description: undefined, + name: formItem.name, + }); setOnFormChangeFn({ fn: (value) => setFormFieldValue(value) }); }; }; diff --git a/grafana-plugin/src/containers/TemplatePreview/TemplatePreview.module.css b/grafana-plugin/src/containers/TemplatePreview/TemplatePreview.module.css index e88192b9..3968c8d0 100644 --- a/grafana-plugin/src/containers/TemplatePreview/TemplatePreview.module.css +++ b/grafana-plugin/src/containers/TemplatePreview/TemplatePreview.module.css @@ -26,3 +26,7 @@ .display-linebreak { white-space: pre-line; } + +.extra-check { + margin-bottom: 10px; +} diff --git a/grafana-plugin/src/containers/TemplatePreview/TemplatePreview.tsx b/grafana-plugin/src/containers/TemplatePreview/TemplatePreview.tsx index 2f0d4d78..a7508229 100644 --- a/grafana-plugin/src/containers/TemplatePreview/TemplatePreview.tsx +++ b/grafana-plugin/src/containers/TemplatePreview/TemplatePreview.tsx @@ -1,6 +1,6 @@ import React, { useEffect, useState } from 'react'; -import { HorizontalGroup, Icon, LoadingPlaceholder, VerticalGroup } from '@grafana/ui'; +import { Badge, HorizontalGroup, Icon, LoadingPlaceholder, VerticalGroup } from '@grafana/ui'; import cn from 'classnames/bind'; import { observer } from 'mobx-react'; @@ -8,6 +8,7 @@ import Text from 'components/Text/Text'; import { AlertReceiveChannel } from 'models/alert_receive_channel/alert_receive_channel.types'; import { Alert } from 'models/alertgroup/alertgroup.types'; import { OutgoingWebhook } from 'models/outgoing_webhook/outgoing_webhook.types'; +import { LabelTemplateOptions } from 'pages/integration/IntegrationCommon.config'; import { useStore } from 'state/useStore'; import { openErrorNotification } from 'utils'; import { useDebouncedCallback } from 'utils/hooks'; @@ -51,7 +52,9 @@ const TemplatePreview = observer((props: TemplatePreviewProps) => { templatePage, } = props; - const [result, setResult] = useState<{ preview: string | null } | undefined>(undefined); + const [result, setResult] = useState<{ preview: string | null; is_valid_json_object?: boolean } | undefined>( + undefined + ); const [conditionalResult, setConditionalResult] = useState({}); const store = useStore(); @@ -102,6 +105,29 @@ const TemplatePreview = observer((props: TemplatePreviewProps) => { } }; + function renderExtraChecks() { + function getExtraCheckResult() { + switch (templateName) { + case LabelTemplateOptions.AlertGroupMultiLabel.key: + return result.is_valid_json_object ? ( + + ) : ( + + ); + default: + return null; + } + } + + const checkResult = getExtraCheckResult(); + + return checkResult ?
{checkResult}
: null; + } + function renderResult() { switch (templateType) { case 'html': { @@ -182,7 +208,14 @@ const TemplatePreview = observer((props: TemplatePreviewProps) => { ); } - return result ? <>{renderResult()} : ; + return result ? ( + <> + {renderExtraChecks()} + {renderResult()} + + ) : ( + + ); }); export default TemplatePreview; diff --git a/grafana-plugin/src/containers/WebhooksTemplateEditor/WebhooksTemplateEditor.tsx b/grafana-plugin/src/containers/WebhooksTemplateEditor/WebhooksTemplateEditor.tsx index 093de6db..3ae6e637 100644 --- a/grafana-plugin/src/containers/WebhooksTemplateEditor/WebhooksTemplateEditor.tsx +++ b/grafana-plugin/src/containers/WebhooksTemplateEditor/WebhooksTemplateEditor.tsx @@ -5,7 +5,7 @@ import cn from 'classnames/bind'; import { debounce } from 'lodash-es'; import CheatSheet from 'components/CheatSheet/CheatSheet'; -import { genericTemplateCheatSheet } from 'components/CheatSheet/CheatSheet.config'; +import { genericTemplateCheatSheet, webhookPayloadCheatSheet } from 'components/CheatSheet/CheatSheet.config'; import MonacoEditor from 'components/MonacoEditor/MonacoEditor'; import Text from 'components/Text/Text'; import styles from 'containers/IntegrationTemplate/IntegrationTemplate.module.scss'; @@ -70,6 +70,15 @@ const WebhooksTemplateEditor: React.FC = ({ templat setIsCheatSheetVisible(false); }, []); + const getCheatSheet = (templateKey: string) => { + switch (templateKey) { + case 'data': + return webhookPayloadCheatSheet; + default: + return genericTemplateCheatSheet; + } + }; + return ( = ({ templat {isCheatSheetVisible ? ( ) : ( diff --git a/grafana-plugin/src/pages/integration/Integration.config.ts b/grafana-plugin/src/pages/integration/Integration.config.ts deleted file mode 100644 index 62201a0f..00000000 --- a/grafana-plugin/src/pages/integration/Integration.config.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { AppFeature } from 'state/features'; -import { KeyValuePair } from 'utils'; - -import { BASE_INTEGRATION_TEMPLATES_LIST, BaseTemplateOptions } from './IntegrationCommon.config'; - -export const MsTeamsTemplateOptions = { - MSTeams: new KeyValuePair('Microsoft Teams', 'Microsoft Teams'), - MSTeamsTitle: new KeyValuePair('MSTeams Title', 'Title'), - MSTeamsMessage: new KeyValuePair('MSTeams Message', 'Message'), - MSTeamsImage: new KeyValuePair('MSTeams Image', 'Image'), -}; - -export const getTemplateOptions = (features: Record) => { - if (features[AppFeature.MsTeams]) { - return { - ...BaseTemplateOptions, - ...MsTeamsTemplateOptions, - }; - } - return BaseTemplateOptions; -}; - -export const getIntegrationTemplatesList = (features: Record) => { - if (features[AppFeature.MsTeams]) { - return [ - ...BASE_INTEGRATION_TEMPLATES_LIST, - - { - label: MsTeamsTemplateOptions.MSTeams.value, - value: MsTeamsTemplateOptions.MSTeams.key, - children: [ - { - label: MsTeamsTemplateOptions.MSTeamsTitle.value, - value: MsTeamsTemplateOptions.MSTeamsTitle.key, - }, - { - label: MsTeamsTemplateOptions.MSTeamsMessage.value, - value: MsTeamsTemplateOptions.MSTeamsMessage.key, - }, - { - label: MsTeamsTemplateOptions.MSTeamsImage.value, - value: MsTeamsTemplateOptions.MSTeamsImage.key, - }, - ], - }, - ]; - } - - return BASE_INTEGRATION_TEMPLATES_LIST; -}; diff --git a/grafana-plugin/src/pages/integration/IntegrationCommon.config.ts b/grafana-plugin/src/pages/integration/IntegrationCommon.config.ts index c4590765..cf7b1554 100644 --- a/grafana-plugin/src/pages/integration/IntegrationCommon.config.ts +++ b/grafana-plugin/src/pages/integration/IntegrationCommon.config.ts @@ -6,7 +6,7 @@ export const MAX_CHARACTERS_COUNT = 50; export const MONACO_INPUT_HEIGHT_SMALL = '32px'; export const MONACO_INPUT_HEIGHT_TALL = '120px'; -export const BaseTemplateOptions = { +export const IntegrationTemplateOptions = { WebTitle: new KeyValuePair('web_title_template', 'Web Title'), WebMessage: new KeyValuePair('web_message_template', 'Web Message'), WebImage: new KeyValuePair('web_image_url_template', 'Web Image'), @@ -33,71 +33,7 @@ export const BaseTemplateOptions = { Telegram: new KeyValuePair('Telegram', 'Telegram'), }; -export const BASE_INTEGRATION_TEMPLATES_LIST = [ - { - label: BaseTemplateOptions.SourceLink.value, - value: BaseTemplateOptions.SourceLink.key, - }, - { - label: BaseTemplateOptions.Autoacknowledge.value, - value: BaseTemplateOptions.Autoacknowledge.key, - }, - { - label: BaseTemplateOptions.Phone.value, - value: BaseTemplateOptions.Phone.key, - }, - { - label: BaseTemplateOptions.SMS.value, - value: BaseTemplateOptions.SMS.key, - }, - { - label: BaseTemplateOptions.Email.value, - value: BaseTemplateOptions.Email.key, - children: [ - { - label: BaseTemplateOptions.EmailTitle.value, - value: BaseTemplateOptions.EmailTitle.key, - }, - { - label: BaseTemplateOptions.EmailMessage.value, - value: BaseTemplateOptions.EmailMessage.key, - }, - ], - }, - { - label: BaseTemplateOptions.Slack.value, - value: BaseTemplateOptions.Slack.key, - children: [ - { - label: BaseTemplateOptions.SlackTitle.value, - value: BaseTemplateOptions.SlackTitle.key, - }, - { - label: BaseTemplateOptions.SlackMessage.value, - value: BaseTemplateOptions.SlackMessage.key, - }, - { - label: BaseTemplateOptions.SlackImage.value, - value: BaseTemplateOptions.SlackImage.key, - }, - ], - }, - { - label: BaseTemplateOptions.Telegram.value, - value: BaseTemplateOptions.Telegram.key, - children: [ - { - label: BaseTemplateOptions.TelegramTitle.value, - value: BaseTemplateOptions.TelegramTitle.key, - }, - { - label: BaseTemplateOptions.TelegramMessage.value, - value: BaseTemplateOptions.TelegramMessage.key, - }, - { - label: BaseTemplateOptions.TelegramImage.value, - value: BaseTemplateOptions.TelegramImage.key, - }, - ], - }, -]; +export const LabelTemplateOptions = { + AlertGroupDynamicLabel: new KeyValuePair('alert_group_dynamic_label', 'Alert Group Dynamic Label'), + AlertGroupMultiLabel: new KeyValuePair('alert_group_multi_label', 'Alert Group Multi Label'), +}; From 0a077ccfdbfe69f100b2b040b8430cf2fbe89e05 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 17 Jan 2024 12:18:08 -0300 Subject: [PATCH 5/8] Update and refactor users API team filter (#3703) This should hopefully fix the lint issue [here](https://drone.grafana.net/grafana/oncall/3361/1/7) --- engine/apps/api/tests/test_user.py | 7 +++++-- engine/apps/api/views/user.py | 2 +- engine/common/api_helpers/filters.py | 13 +++++++------ 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/engine/apps/api/tests/test_user.py b/engine/apps/api/tests/test_user.py index 5d7dc538..0d853861 100644 --- a/engine/apps/api/tests/test_user.py +++ b/engine/apps/api/tests/test_user.py @@ -298,6 +298,7 @@ def test_list_users_filtered_by_team( organization = make_organization() user1 = make_user_for_organization(organization) user2 = make_user_for_organization(organization) + user3 = make_user_for_organization(organization) team1 = make_team(organization) team2 = make_team(organization) @@ -314,7 +315,7 @@ def test_list_users_filtered_by_team( def _get_user_pks(teams): response = client.get( url, - data={"team": [team.public_primary_key for team in teams]}, # these are query params + data={"team": [team.public_primary_key if team else "null" for team in teams]}, # these are query params **make_user_auth_headers(user1, token), ) assert response.status_code == status.HTTP_200_OK @@ -323,9 +324,11 @@ def test_list_users_filtered_by_team( assert _get_user_pks([team1]) == [user1.public_primary_key] assert _get_user_pks([team1, team2]) == [user1.public_primary_key, user2.public_primary_key] assert _get_user_pks([team3]) == [] + assert _get_user_pks([team1, None]) == [user1.public_primary_key, user3.public_primary_key] + assert _get_user_pks([None]) == [user3.public_primary_key] # check non-existent team returns bad request - response = client.get(f"{url}?team=null", **make_user_auth_headers(user1, token)) + response = client.get(f"{url}?team=non-existing", **make_user_auth_headers(user1, token)) assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index 8cd517c1..fe8e9cb6 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -138,7 +138,7 @@ class UserFilter(ByTeamModelFieldFilterMixin, filters.FilterSet): # TODO: remove "roles" in next version roles = filters.MultipleChoiceFilter(field_name="role", choices=LegacyAccessControlRole.choices()) permission = filters.ChoiceFilter(method="filter_by_permission", choices=ALL_PERMISSION_CHOICES) - team = TeamModelMultipleChoiceFilter(field_name="teams", null_label=None, null_value=None) + team = TeamModelMultipleChoiceFilter(field_name="teams") class Meta: model = User diff --git a/engine/common/api_helpers/filters.py b/engine/common/api_helpers/filters.py index 12e1fe41..daa3523a 100644 --- a/engine/common/api_helpers/filters.py +++ b/engine/common/api_helpers/filters.py @@ -86,13 +86,14 @@ class ByTeamModelFieldFilterMixin: return queryset filter = self.filters[ByTeamModelFieldFilterMixin.FILTER_FIELD_NAME] null_team_lookup = None - for value in values: - if filter.null_value == value: - null_team_lookup = Q(**{f"{name}__isnull": True}) - values.remove(value) - teams_lookup = Q(**{f"{name}__in": values}) + if filter.null_value in values: + null_team_lookup = Q(**{f"{name}__isnull": True}) + values.remove(filter.null_value) + teams_lookup = None + if values: + teams_lookup = Q(**{f"{name}__in": values}) if null_team_lookup is not None: - teams_lookup = teams_lookup | null_team_lookup + teams_lookup = teams_lookup | null_team_lookup if teams_lookup else null_team_lookup return queryset.filter(teams_lookup).distinct() From 31ffa805cca0c31615b52356335de01a1e1e44ae Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Wed, 17 Jan 2024 10:47:18 -0500 Subject: [PATCH 6/8] update mobile app gateway jwt to include user email (#3704) # What this PR does Changes as requested by Grafana Incident team ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- engine/apps/mobile_app/tests/test_mobile_app_gateway.py | 5 +++-- engine/apps/mobile_app/views.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py index b23b5929..6da6dad4 100644 --- a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py +++ b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py @@ -14,7 +14,7 @@ from apps.mobile_app.views import MobileAppGatewayView DOWNSTREAM_BACKEND = "incident" MOCK_DOWNSTREAM_URL = "https://mockdownstream.com" MOCK_DOWNSTREAM_INCIDENT_API_URL = "https://mockdownstreamincidentapi.com" -MOCK_DOWNSTREAM_HEADERS = {"Authorization": "Bearer mock_jwt"} +MOCK_DOWNSTREAM_HEADERS = {"X-OnCall-Mobile-Proxy-Authorization": "Bearer mock_jwt"} MOCK_DOWNSTREAM_RESPONSE_DATA = {"foo": "bar"} MOCK_TIMEZONE_NOW = timezone.datetime(2021, 1, 1, 3, 4, 5, tzinfo=timezone.utc) @@ -311,7 +311,7 @@ def test_mobile_app_gateway_jwt_header( MOCK_DOWNSTREAM_URL, data={}, params={}, - headers={"Authorization": f"Bearer {MOCK_JWT}"}, + headers={"X-OnCall-Mobile-Proxy-Authorization": f"Bearer {MOCK_JWT}"}, ) @@ -343,6 +343,7 @@ def test_mobile_app_gateway_properly_generates_a_jwt( "iat": MOCK_TIMEZONE_NOW, "exp": MOCK_TIMEZONE_NOW + timezone.timedelta(minutes=1), "user_id": user.user_id, # grafana user ID + "user_email": user.email, "stack_id": organization.stack_id, "organization_id": organization.org_id, # grafana org ID "stack_slug": organization.stack_slug, diff --git a/engine/apps/mobile_app/views.py b/engine/apps/mobile_app/views.py index b0ba7328..30ba5010 100644 --- a/engine/apps/mobile_app/views.py +++ b/engine/apps/mobile_app/views.py @@ -158,6 +158,7 @@ class MobileAppGatewayView(APIView): "exp": now + timezone.timedelta(minutes=1), # jwt is short lived. expires in 1 minute. # custom data "user_id": user.user_id, # grafana user ID + "user_email": user.email, "stack_id": organization.stack_id, "organization_id": organization.org_id, # grafana org ID "stack_slug": organization.stack_slug, @@ -177,7 +178,7 @@ class MobileAppGatewayView(APIView): @classmethod def _get_downstream_headers(cls, user: "User") -> typing.Dict[str, str]: return { - "Authorization": f"Bearer {cls._construct_jwt(user)}", + "X-OnCall-Mobile-Proxy-Authorization": f"Bearer {cls._construct_jwt(user)}", } @classmethod From c99788e9d2a3d97e69f59c991a06a63c741f43c2 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 17 Jan 2024 13:30:11 -0300 Subject: [PATCH 7/8] Update schedule on-call cache on scheduled refresh tasks (#3699) Related to https://github.com/grafana/oncall/issues/3673 Keep cache up to date on every schedule refresh task run (which should keep cache populated every time), helping on any call using cached information (particularly the direct paging slack dialog building). --- CHANGELOG.md | 4 ++ engine/apps/schedules/constants.py | 3 ++ engine/apps/schedules/ical_utils.py | 31 +++++++----- .../schedules/tasks/refresh_ical_files.py | 5 +- .../tests/tasks/test_refresh_ical_files.py | 49 ++++++++++++++++++- 5 files changed, 79 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4025b42e..6f175681 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed Webhooks UI not allowing simple webhooks to be created ([#3691](https://github.com/grafana/oncall/pull/3691)) - Fix posting Slack message when route is deleted by @vadimkerr ([#3702](https://github.com/grafana/oncall/pull/3702)) +### Changed + +- Update schedules on-call users cache on every scheduled schedule refresh task ([#3699](https://github.com/grafana/oncall/pull/3699)). + ## v1.3.88 (2024-01-16) ### Fixed diff --git a/engine/apps/schedules/constants.py b/engine/apps/schedules/constants.py index 818ddeb1..880cf50d 100644 --- a/engine/apps/schedules/constants.py +++ b/engine/apps/schedules/constants.py @@ -26,3 +26,6 @@ CALENDAR_TYPE_FINAL = "final" EXPORT_WINDOW_DAYS_AFTER = 180 EXPORT_WINDOW_DAYS_BEFORE = 15 + +SCHEDULE_ONCALL_CACHE_KEY_PREFIX = "schedule_oncall_users_" +SCHEDULE_ONCALL_CACHE_TTL = 15 * 60 # 15 minutes in seconds diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index 9dede8fc..eb9840d5 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -33,6 +33,8 @@ from apps.schedules.constants import ( RE_EVENT_UID_V1, RE_EVENT_UID_V2, RE_PRIORITY, + SCHEDULE_ONCALL_CACHE_KEY_PREFIX, + SCHEDULE_ONCALL_CACHE_TTL, ) from apps.schedules.ical_events import ical_events from common.cache import ensure_cache_key_allocates_to_the_same_hash_slot @@ -387,6 +389,22 @@ def get_oncall_users_for_multiple_schedules( return oncall_users +def _generate_cache_key_for_schedule_oncall_users(schedule: "OnCallSchedule") -> str: + return ensure_cache_key_allocates_to_the_same_hash_slot( + f"{SCHEDULE_ONCALL_CACHE_KEY_PREFIX}{schedule.public_primary_key}", SCHEDULE_ONCALL_CACHE_KEY_PREFIX + ) + + +def update_cached_oncall_users_for_schedule(schedule: "OnCallSchedule"): + oncall_users = get_oncall_users_for_multiple_schedules([schedule]) + users = oncall_users.get(schedule, []) + cache.set( + _generate_cache_key_for_schedule_oncall_users(schedule), + [user.public_primary_key for user in users], + timeout=SCHEDULE_ONCALL_CACHE_TTL, + ) + + def get_cached_oncall_users_for_multiple_schedules(schedules: typing.List["OnCallSchedule"]) -> SchedulesOnCallUsers: """ More "performant" version of `apps.schedules.ical_utils.get_oncall_users_for_multiple_schedules` @@ -404,22 +422,13 @@ def get_cached_oncall_users_for_multiple_schedules(schedules: typing.List["OnCal from apps.schedules.models import OnCallSchedule from apps.user_management.models import User - CACHE_KEY_PREFIX = "schedule_oncall_users_" - - def _generate_cache_key_for_schedule_oncall_users(schedule: "OnCallSchedule") -> str: - return ensure_cache_key_allocates_to_the_same_hash_slot( - f"{CACHE_KEY_PREFIX}{schedule.public_primary_key}", CACHE_KEY_PREFIX - ) - def _get_schedule_public_primary_key_from_schedule_oncall_users_cache_key(cache_key: str) -> str: """ remove any brackets that might be included in the cache key (when redis cluster is active). See `_generate_cache_key_for_schedule_oncall_users` just above """ cache_key = cache_key.replace("{", "").replace("}", "") - return cache_key.replace(CACHE_KEY_PREFIX, "") - - CACHE_TTL = 15 * 60 # 15 minutes in seconds + return cache_key.replace(SCHEDULE_ONCALL_CACHE_KEY_PREFIX, "") cache_keys = [_generate_cache_key_for_schedule_oncall_users(schedule) for schedule in schedules] @@ -464,7 +473,7 @@ def get_cached_oncall_users_for_multiple_schedules(schedules: typing.List["OnCal _generate_cache_key_for_schedule_oncall_users(schedule) ] = oncall_user_public_primary_keys - cache.set_many(new_results_to_update_in_cache, timeout=CACHE_TTL) + cache.set_many(new_results_to_update_in_cache, timeout=SCHEDULE_ONCALL_CACHE_TTL) # make two queries to the database, one to fetch the schedule objects we need and the other to fetch # the user objects we need diff --git a/engine/apps/schedules/tasks/refresh_ical_files.py b/engine/apps/schedules/tasks/refresh_ical_files.py index d0ae3a4a..a022d464 100644 --- a/engine/apps/schedules/tasks/refresh_ical_files.py +++ b/engine/apps/schedules/tasks/refresh_ical_files.py @@ -1,7 +1,7 @@ from celery.utils.log import get_task_logger from apps.alerts.tasks import notify_ical_schedule_shift # type: ignore[no-redef] -from apps.schedules.ical_utils import is_icals_equal +from apps.schedules.ical_utils import is_icals_equal, update_cached_oncall_users_for_schedule from apps.schedules.tasks import notify_about_empty_shifts_in_schedule_task, notify_about_gaps_in_schedule_task from apps.slack.tasks import start_update_slack_user_group_for_schedules from common.custom_celery_tasks import shared_dedicated_queue_retry_task @@ -81,6 +81,9 @@ def refresh_ical_file(schedule_pk): task_logger.info(f"run_task_overrides {schedule_pk} {run_task_primary} icals not equal") run_task = run_task_primary or run_task_overrides + # update cached schedule on-call users + update_cached_oncall_users_for_schedule(schedule) + if run_task: notify_about_empty_shifts_in_schedule_task.apply_async((schedule_pk,)) notify_about_gaps_in_schedule_task.apply_async((schedule_pk,)) diff --git a/engine/apps/schedules/tests/tasks/test_refresh_ical_files.py b/engine/apps/schedules/tests/tasks/test_refresh_ical_files.py index c6c8ced7..fdbbec36 100644 --- a/engine/apps/schedules/tests/tasks/test_refresh_ical_files.py +++ b/engine/apps/schedules/tests/tasks/test_refresh_ical_files.py @@ -2,8 +2,10 @@ import datetime from unittest.mock import patch import pytest +from django.core.cache import cache +from django.utils import timezone -from apps.schedules.models import OnCallScheduleICal, OnCallScheduleWeb +from apps.schedules.models import CustomOnCallShift, OnCallScheduleICal, OnCallScheduleWeb from apps.schedules.tasks.refresh_ical_files import refresh_ical_file, start_refresh_ical_files @@ -79,3 +81,48 @@ def test_refresh_ical_files_filter_orgs( assert len(called_args) == 1 assert schedule.id in called_args[0].args[0] assert schedule_from_deleted_org.id not in called_args[0].args[0] + + +@pytest.mark.django_db +def test_refresh_ical_updates_oncall_cache( + make_organization, + make_user_for_organization, + make_schedule, + make_on_call_shift, +): + organization = make_organization() + users = [make_user_for_organization(organization) for _ in range(2)] + + today = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + shift_start_time = today - timezone.timedelta(hours=1) + on_call_shift = make_on_call_shift( + organization=organization, + shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, + start=shift_start_time, + rotation_start=shift_start_time, + duration=timezone.timedelta(seconds=(24 * 60 * 60)), + priority_level=1, + frequency=CustomOnCallShift.FREQUENCY_DAILY, + schedule=schedule, + ) + on_call_shift.add_rolling_users([users]) + + def _generate_cache_key(schedule): + return f"schedule_oncall_users_{schedule.public_primary_key}" + + # start with empty cache + cache.clear() + + # patch ical comparison to string compare + with patch("apps.schedules.tasks.refresh_ical_files.is_icals_equal", side_effect=lambda a, b: a == b): + # patch schedule refresh to avoid changing schedule status (keep as defined above) + with patch("apps.schedules.models.OnCallSchedule.refresh_ical_file", return_value=None): + # do not trigger tasks for real + with patch("apps.schedules.tasks.refresh_ical_files.notify_ical_schedule_shift"): + with patch("apps.schedules.tasks.refresh_ical_files.notify_about_empty_shifts_in_schedule_task"): + with patch("apps.schedules.tasks.refresh_ical_files.notify_about_gaps_in_schedule_task"): + refresh_ical_file(schedule.pk) + + cached_data = cache.get(_generate_cache_key(schedule)) + assert cached_data == [u.public_primary_key for u in users] From 78de298a2ec70866d3e6c24a3e472b7fec7f654e Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Wed, 17 Jan 2024 12:02:11 -0500 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f175681..e7cf1994 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +## v1.3.89 (2024-01-17) + ### Fixed - Fixed Webhooks UI not allowing simple webhooks to be created ([#3691](https://github.com/grafana/oncall/pull/3691))