From 26ff937e97fb87efd090bf5591f9ec92f4f60c1f Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 2 Dec 2024 10:51:13 -0500 Subject: [PATCH] fix: patch slack ID reference issue (#5314) ## Which issue(s) this PR closes Fixes https://github.com/grafana/irm/issues/469 ## 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] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- engine/apps/alerts/models/alert_group.py | 16 +- engine/apps/alerts/models/channel_filter.py | 10 +- engine/apps/alerts/tests/test_alert_group.py | 63 +- .../apps/alerts/tests/test_channel_filter.py | 64 ++ engine/apps/alerts/tests/test_notify_all.py | 10 +- .../apps/slack/scenarios/distribute_alerts.py | 187 ++--- engine/apps/slack/tasks.py | 4 +- .../scenario_steps/test_distribute_alerts.py | 649 +++++++++++++++--- engine/apps/slack/tests/test_installation.py | 17 +- 9 files changed, 796 insertions(+), 224 deletions(-) diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index 442ab61e68..3db1d9edfb 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -1980,12 +1980,16 @@ def is_presented_in_slack(self): @property def slack_channel_id(self) -> str | None: - if not self.channel.organization.slack_team_identity: - return None - elif self.slack_message: - return self.slack_message.channel.slack_id - elif self.channel_filter: - return self.channel_filter.slack_channel_id_or_org_default_id + channel_filter = self.channel_filter + + if self.slack_message: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 + # + # return self.slack_message.channel.slack_id + return self.slack_message._channel_id + elif channel_filter and channel_filter.slack_channel_or_org_default: + return channel_filter.slack_channel_or_org_default.slack_id return None @property diff --git a/engine/apps/alerts/models/channel_filter.py b/engine/apps/alerts/models/channel_filter.py index 3ea2ea8bcb..0f67f575c8 100644 --- a/engine/apps/alerts/models/channel_filter.py +++ b/engine/apps/alerts/models/channel_filter.py @@ -172,14 +172,8 @@ def slack_channel_slack_id(self) -> typing.Optional[str]: return self.slack_channel.slack_id if self.slack_channel else None @property - def slack_channel_id_or_org_default_id(self): - organization = self.alert_receive_channel.organization - - if organization.slack_team_identity is None: - return None - elif self.slack_channel_slack_id is None: - return organization.default_slack_channel_slack_id - return self.slack_channel_slack_id + def slack_channel_or_org_default(self) -> typing.Optional["SlackChannel"]: + return self.slack_channel or self.alert_receive_channel.organization.default_slack_channel @property def str_for_clients(self): diff --git a/engine/apps/alerts/tests/test_alert_group.py b/engine/apps/alerts/tests/test_alert_group.py index e837516f86..d4fdbf7710 100644 --- a/engine/apps/alerts/tests/test_alert_group.py +++ b/engine/apps/alerts/tests/test_alert_group.py @@ -707,7 +707,7 @@ def test_delete_by_user( @pytest.mark.django_db -def test_integration_config_on_alert_group_created(make_organization, make_alert_receive_channel, make_channel_filter): +def test_integration_config_on_alert_group_created(make_organization, make_alert_receive_channel): organization = make_organization() alert_receive_channel = make_alert_receive_channel(organization, grouping_id_template="group_to_one_group") @@ -806,3 +806,64 @@ def test_alert_group_created_if_resolve_condition_but_auto_resolving_disabled( # the alert will create a new alert group assert alert.group != resolved_alert_group + + +class TestAlertGroupSlackChannelID: + @pytest.mark.django_db + def test_slack_channel_id_with_slack_message( + self, + make_organization_with_slack_team_identity, + make_alert_receive_channel, + make_slack_channel, + make_slack_message, + make_alert_group, + ): + """ + Test that slack_channel_id returns the _channel_id from slack_message when slack_message exists. + """ + organization, slack_team_identity = make_organization_with_slack_team_identity() + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) + + # Assert that slack_channel_id returns the _channel_id from slack_message + assert alert_group.slack_channel_id == slack_message._channel_id + + @pytest.mark.django_db + def test_slack_channel_id_with_channel_filter( + self, + make_organization_with_slack_team_identity, + make_alert_receive_channel, + make_channel_filter, + make_slack_channel, + make_alert_group, + ): + """ + Test that slack_channel_id returns the slack_id from channel_filter.slack_channel_or_org_default. + """ + organization, slack_team_identity = make_organization_with_slack_team_identity() + alert_receive_channel = make_alert_receive_channel(organization) + slack_channel = make_slack_channel(slack_team_identity) + channel_filter = make_channel_filter(alert_receive_channel, slack_channel=slack_channel) + alert_group = make_alert_group(alert_receive_channel, channel_filter=channel_filter) + + # Assert that slack_channel_id returns the slack_id from the channel filter's Slack channel + assert alert_group.slack_channel_id == slack_channel.slack_id + + @pytest.mark.django_db + def test_slack_channel_id_no_slack_message_no_channel_filter( + self, + make_organization_with_slack_team_identity, + make_alert_receive_channel, + make_alert_group, + ): + """ + Test that slack_channel_id returns None when there is no slack_message and no channel_filter. + """ + organization, _ = make_organization_with_slack_team_identity() + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel, channel_filter=None) + + # Assert that slack_channel_id is None + assert alert_group.slack_channel_id is None diff --git a/engine/apps/alerts/tests/test_channel_filter.py b/engine/apps/alerts/tests/test_channel_filter.py index ba19e68894..b6079449c9 100644 --- a/engine/apps/alerts/tests/test_channel_filter.py +++ b/engine/apps/alerts/tests/test_channel_filter.py @@ -152,3 +152,67 @@ def test_channel_filter_using_filter_labels( assert ChannelFilter.select_filter(alert_receive_channel, {"title": "Test Title", "value": 5}, labels) == ( custom_channel_filter if should_match else default_channel_filter ) + + +class TestChannelFilterSlackChannelOrOrgDefault: + @pytest.mark.django_db + def test_slack_channel_or_org_default_with_slack_channel( + self, + make_organization_with_slack_team_identity, + make_alert_receive_channel, + make_channel_filter, + make_slack_channel, + ): + """ + Test that slack_channel_or_org_default returns self.slack_channel when it is set. + """ + organization, slack_team_identity = make_organization_with_slack_team_identity() + alert_receive_channel = make_alert_receive_channel(organization) + slack_channel = make_slack_channel(slack_team_identity) + channel_filter = make_channel_filter(alert_receive_channel=alert_receive_channel, slack_channel=slack_channel) + + # Assert that slack_channel_or_org_default returns slack_channel + assert channel_filter.slack_channel_or_org_default == slack_channel + + @pytest.mark.django_db + def test_slack_channel_or_org_default_with_org_default( + self, + make_slack_team_identity, + make_organization, + make_alert_receive_channel, + make_channel_filter, + make_slack_channel, + ): + """ + Test that slack_channel_or_org_default returns organization's default_slack_channel when slack_channel is None. + """ + slack_team_identity = make_slack_team_identity() + default_slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization( + slack_team_identity=slack_team_identity, + default_slack_channel=default_slack_channel, + ) + alert_receive_channel = make_alert_receive_channel(organization) + channel_filter = make_channel_filter(alert_receive_channel, slack_channel=None) + + # Assert that slack_channel_or_org_default returns organization's default_slack_channel + assert channel_filter.slack_channel_or_org_default == default_slack_channel + + @pytest.mark.django_db + def test_slack_channel_or_org_default_none( + self, + make_organization_with_slack_team_identity, + make_alert_receive_channel, + make_channel_filter, + ): + """ + Test that slack_channel_or_org_default returns None when both slack_channel and organization's default_slack_channel are None. + """ + organization, _ = make_organization_with_slack_team_identity() + assert organization.default_slack_channel is None + + alert_receive_channel = make_alert_receive_channel(organization) + channel_filter = make_channel_filter(alert_receive_channel=alert_receive_channel, slack_channel=None) + + # Assert that slack_channel_or_org_default returns None + assert channel_filter.slack_channel_or_org_default is None diff --git a/engine/apps/alerts/tests/test_notify_all.py b/engine/apps/alerts/tests/test_notify_all.py index 0649314cd4..b7a277860d 100644 --- a/engine/apps/alerts/tests/test_notify_all.py +++ b/engine/apps/alerts/tests/test_notify_all.py @@ -9,10 +9,8 @@ @pytest.mark.django_db def test_notify_all( - make_organization, - make_slack_team_identity, + make_organization_and_user_with_slack_identities, make_slack_channel, - make_user, make_user_notification_policy, make_escalation_chain, make_escalation_policy, @@ -20,13 +18,9 @@ def test_notify_all( make_alert_receive_channel, make_alert_group, ): - organization = make_organization() - slack_team_identity = make_slack_team_identity() + organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() slack_channel = make_slack_channel(slack_team_identity) - organization.slack_team_identity = slack_team_identity - organization.save() - user = make_user(organization=organization) make_user_notification_policy( user=user, step=UserNotificationPolicy.Step.NOTIFY, diff --git a/engine/apps/slack/scenarios/distribute_alerts.py b/engine/apps/slack/scenarios/distribute_alerts.py index ad327bdcd8..cd088bf7b5 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -22,7 +22,7 @@ SlackAPIRestrictedActionError, SlackAPITokenError, ) -from apps.slack.models import SlackChannel, SlackTeamIdentity, SlackUserIdentity +from apps.slack.models import SlackTeamIdentity, SlackUserIdentity from apps.slack.scenarios import scenario_step from apps.slack.slack_formatter import SlackFormatter from apps.slack.tasks import send_message_to_thread_if_bot_not_in_channel, update_incident_slack_message @@ -94,7 +94,6 @@ def process_signal(self, alert: Alert) -> None: See this conversation for more context https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732800180834819?thread_ts=1732748893.183939&cid=C06K1MQ07GS """ - alert_group = alert.group if not alert_group: @@ -108,6 +107,8 @@ def process_signal(self, alert: Alert) -> None: alert_group_pk = alert_group.pk alert_receive_channel = alert_group.channel + organization = alert_receive_channel.organization + channel_filter = alert_group.channel_filter should_skip_escalation_in_slack = alert_group.skip_escalation_in_slack slack_team_identity = self.slack_team_identity slack_team_identity_pk = slack_team_identity.pk @@ -128,26 +129,115 @@ def process_signal(self, alert: Alert) -> None: if num_updated_rows == 1: # this will be the case in the event that we haven't yet created a Slack message for this alert group + # if channel filter is deleted mid escalation, use the organization's default Slack channel slack_channel = ( - alert_group.channel_filter.slack_channel - if alert_group.channel_filter - # if channel filter is deleted mid escalation, use default Slack channel - else alert_receive_channel.organization.default_slack_channel + channel_filter.slack_channel_or_org_default if channel_filter else organization.default_slack_channel ) + + # slack_channel can be None if the channel filter is deleted mid escalation, OR the channel filter does + # not have a slack channel + # + # In these cases, we try falling back to the organization's default slack channel. If that is also None, + # slack_channel will be None, and we will skip posting to Slack + # (because we don't know where to post the message to). + if slack_channel is None: + logger.info( + f"Skipping posting message to Slack for alert_group {alert_group_pk} because we don't know which " + f"Slack channel to post to. channel_filter={channel_filter} " + f"organization.default_slack_channel={organization.default_slack_channel}" + ) + + alert_group.slack_message_sent = False + alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_NOT_SPECIFIED + alert_group.save(update_fields=["slack_message_sent", "reason_to_skip_escalation"]) + return + slack_channel_id = slack_channel.slack_id try: - self._post_alert_group_to_slack( - slack_team_identity=slack_team_identity, - alert_group=alert_group, - alert=alert, + result = self._slack_client.chat_postMessage( + channel=slack_channel.slack_id, attachments=alert_group.render_slack_attachments(), - slack_channel=slack_channel, blocks=alert_group.render_slack_blocks(), ) - except (SlackAPIError, TimeoutError): - AlertGroup.objects.filter(pk=alert_group_pk).update(slack_message_sent=False) - raise + + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 + alert_group.slack_messages.create( + slack_id=result["ts"], + organization=alert_group.channel.organization, + _channel_id=slack_channel.slack_id, + channel=slack_channel, + ) + + alert.delivered = True + alert.save() + except (SlackAPIError, TimeoutError) as e: + reason_to_skip_escalation: typing.Optional[int] = None + extra_log_msg: typing.Optional[str] = None + reraise_exception = False + + if isinstance(e, TimeoutError): + reraise_exception = True + elif isinstance(e, SlackAPIRatelimitError): + extra_log_msg = "not delivering alert due to slack rate limit" + if not alert_receive_channel.is_maintenace_integration: + # we do not want to rate limit maintenace alerts.. + reason_to_skip_escalation = AlertGroup.RATE_LIMITED + extra_log_msg += ( + f" integration is a maintenance integration alert_receive_channel={alert_receive_channel}" + ) + else: + reraise_exception = True + elif isinstance(e, SlackAPITokenError): + reason_to_skip_escalation = AlertGroup.ACCOUNT_INACTIVE + extra_log_msg = "not delivering alert due to account_inactive" + elif isinstance(e, SlackAPIInvalidAuthError): + reason_to_skip_escalation = AlertGroup.INVALID_AUTH + extra_log_msg = "not delivering alert due to invalid_auth" + elif isinstance(e, (SlackAPIChannelArchivedError, SlackAPIChannelNotFoundError)): + reason_to_skip_escalation = AlertGroup.CHANNEL_ARCHIVED + extra_log_msg = "not delivering alert due to channel is archived" + elif isinstance(e, SlackAPIRestrictedActionError): + reason_to_skip_escalation = AlertGroup.RESTRICTED_ACTION + extra_log_msg = "not delivering alert due to workspace restricted action" + else: + # this is some other SlackAPIError.. + reraise_exception = True + + log_msg = f"{e.__class__.__name__} while posting alert {alert.pk} for {alert_group_pk} to Slack" + if extra_log_msg: + log_msg += f" ({extra_log_msg})" + + logger.warning(log_msg) + + update_fields = [] + if reason_to_skip_escalation is not None: + alert_group.reason_to_skip_escalation = reason_to_skip_escalation + update_fields.append("reason_to_skip_escalation") + + # Only set slack_message_sent to False under certain circumstances - the idea here is to prevent + # attempts to post a message to Slack, ONLY in cases when we are sure it's not possible + # (e.g. slack token error; because we call this step with every new alert in the alert group) + # + # In these cases, with every next alert in the alert group, num_updated_rows (above) should return 0, + # and we should skip "post the first message" part for the new alerts + if reraise_exception is True: + alert_group.slack_message_sent = False + update_fields.append("slack_message_sent") + + alert_group.save(update_fields=update_fields) + + if reraise_exception: + raise e + + # don't reraise an Exception, but we must return early here because the AlertGroup does not have a + # SlackMessage associated with it (eg. the failed called to chat_postMessage above, means + # that we never called alert_group.slack_messages.create), + # + # Given that, it would be impossible to try posting a debug mode notice or + # send_message_to_thread_if_bot_not_in_channel without the SlackMessage's thread_ts + return if alert_receive_channel.maintenance_mode == AlertReceiveChannel.DEBUG_MAINTENANCE: # send debug mode notice @@ -191,75 +281,6 @@ def process_signal(self, alert: Alert) -> None: else: logger.info("Skip updating alert_group in Slack due to rate limit") - def _post_alert_group_to_slack( - self, - slack_team_identity: SlackTeamIdentity, - alert_group: AlertGroup, - alert: Alert, - attachments, - slack_channel: typing.Optional[SlackChannel], - blocks: Block.AnyBlocks, - ) -> None: - # slack_channel can be None if org default slack channel for slack_team_identity is not set - if slack_channel is None: - logger.info( - f"Failed to post message to Slack for alert_group {alert_group.pk} because slack_channel is None" - ) - - alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_NOT_SPECIFIED - alert_group.save(update_fields=["reason_to_skip_escalation"]) - - return - - try: - result = self._slack_client.chat_postMessage( - channel=slack_channel.slack_id, - attachments=attachments, - blocks=blocks, - ) - - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 - alert_group.slack_messages.create( - slack_id=result["ts"], - organization=alert_group.channel.organization, - _channel_id=slack_channel.slack_id, - channel=slack_channel, - ) - - alert.delivered = True - except SlackAPITokenError: - alert_group.reason_to_skip_escalation = AlertGroup.ACCOUNT_INACTIVE - alert_group.save(update_fields=["reason_to_skip_escalation"]) - logger.info("Not delivering alert due to account_inactive.") - except SlackAPIInvalidAuthError: - alert_group.reason_to_skip_escalation = AlertGroup.INVALID_AUTH - alert_group.save(update_fields=["reason_to_skip_escalation"]) - logger.info("Not delivering alert due to invalid_auth.") - except SlackAPIChannelArchivedError: - alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_ARCHIVED - alert_group.save(update_fields=["reason_to_skip_escalation"]) - logger.info("Not delivering alert due to channel is archived.") - except SlackAPIRatelimitError as e: - # don't rate limit maintenance alert - if not alert_group.channel.is_maintenace_integration: - alert_group.reason_to_skip_escalation = AlertGroup.RATE_LIMITED - alert_group.save(update_fields=["reason_to_skip_escalation"]) - alert_group.channel.start_send_rate_limit_message_task(e.retry_after) - logger.info("Not delivering alert due to slack rate limit.") - else: - raise e - except SlackAPIChannelNotFoundError: - alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_ARCHIVED - alert_group.save(update_fields=["reason_to_skip_escalation"]) - logger.info("Not delivering alert due to channel is archived.") - except SlackAPIRestrictedActionError: - alert_group.reason_to_skip_escalation = AlertGroup.RESTRICTED_ACTION - alert_group.save(update_fields=["reason_to_skip_escalation"]) - logger.info("Not delivering alert due to workspace restricted action.") - finally: - alert.save() - def process_scenario( self, slack_user_identity: SlackUserIdentity, diff --git a/engine/apps/slack/tasks.py b/engine/apps/slack/tasks.py index 13b26bbeda..bfd6f22fd5 100644 --- a/engine/apps/slack/tasks.py +++ b/engine/apps/slack/tasks.py @@ -303,14 +303,14 @@ def post_slack_rate_limit_message(integration_id): return default_route = integration.channel_filters.get(is_default=True) - if (slack_channel_id := default_route.slack_channel_id_or_org_default_id) is not None: + if (slack_channel := default_route.slack_channel_or_org_default) is not None: text = ( f"Delivering and updating alert groups of integration {integration.verbal_name} in Slack is " f"temporarily stopped due to rate limit. You could find new alert groups at " f"<{integration.new_incidents_web_link}|web page " '"Alert Groups">' ) - post_message_to_channel(integration.organization, slack_channel_id, text) + post_message_to_channel(integration.organization, slack_channel.slack_id, text) @shared_dedicated_queue_retry_task( diff --git a/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py b/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py index 12943f9682..e815d78036 100644 --- a/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py +++ b/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py @@ -1,115 +1,556 @@ +from datetime import timedelta from unittest.mock import patch import pytest +from django.core.cache import cache +from django.utils import timezone -from apps.alerts.models import AlertGroup -from apps.slack.errors import get_error_class +from apps.alerts.models import AlertGroup, AlertReceiveChannel +from apps.slack.errors import SlackAPIFetchMembersFailedError, SlackAPIRatelimitError, get_error_class from apps.slack.models import SlackMessage from apps.slack.scenarios.distribute_alerts import IncomingAlertStep -from apps.slack.scenarios.scenario_step import ScenarioStep from apps.slack.tests.conftest import build_slack_response +from apps.slack.utils import get_cache_key_update_incident_slack_message +SLACK_MESSAGE_TS = "1234567890.123456" +SLACK_POST_MESSAGE_SUCCESS_RESPONSE = {"ts": SLACK_MESSAGE_TS} -@pytest.mark.django_db -@pytest.mark.parametrize( - "reason,slack_error", - [ - (reason, slack_error) - for reason, slack_error in AlertGroup.REASONS_TO_SKIP_ESCALATIONS - if reason != AlertGroup.NO_REASON - ], -) -def test_skip_escalations_error( - make_organization_and_user_with_slack_identities, - make_alert_receive_channel, - make_alert_group, - make_alert, - make_slack_channel, - reason, - slack_error, -): - SlackIncomingAlertStep = ScenarioStep.get_step("distribute_alerts", "IncomingAlertStep") - organization, _, slack_team_identity, _ = make_organization_and_user_with_slack_identities() - alert_receive_channel = make_alert_receive_channel(organization) - alert_group = make_alert_group(alert_receive_channel) - alert = make_alert(alert_group, raw_request_data="{}") - - slack_channel = make_slack_channel(slack_team_identity) - - step = SlackIncomingAlertStep(slack_team_identity) - - with patch.object(step._slack_client, "api_call") as mock_slack_api_call: - error_response = build_slack_response({"error": slack_error}) - error_class = get_error_class(error_response) - mock_slack_api_call.side_effect = error_class(error_response) - - channel = slack_channel - if reason == AlertGroup.CHANNEL_NOT_SPECIFIED: - channel = None - - step._post_alert_group_to_slack(slack_team_identity, alert_group, alert, None, channel, []) - - alert_group.refresh_from_db() - alert.refresh_from_db() - assert alert_group.reason_to_skip_escalation == reason - assert alert_group.slack_message is None - assert SlackMessage.objects.count() == 0 - assert not alert.delivered - - -@pytest.mark.django_db -def test_timeout_error( - make_slack_team_identity, - make_slack_channel, - make_organization, - make_alert_receive_channel, - make_alert_group, - make_alert, -): - SlackIncomingAlertStep = ScenarioStep.get_step("distribute_alerts", "IncomingAlertStep") - slack_team_identity = make_slack_team_identity() - slack_channel = make_slack_channel(slack_team_identity) - organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) - alert_receive_channel = make_alert_receive_channel(organization) - alert_group = make_alert_group(alert_receive_channel) - alert = make_alert(alert_group, raw_request_data="{}") - - step = SlackIncomingAlertStep(slack_team_identity) - - with pytest.raises(TimeoutError): - with patch.object(step._slack_client, "api_call") as mock_slack_api_call: - mock_slack_api_call.side_effect = TimeoutError + +class TestIncomingAlertStep: + @patch("apps.slack.client.SlackClient.chat_postMessage", return_value=SLACK_POST_MESSAGE_SUCCESS_RESPONSE) + @pytest.mark.django_db + def test_process_signal_success_first_message( + self, + mock_chat_postMessage, + make_organization_with_slack_team_identity, + make_slack_channel, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + """ + Test the success case where process_signal posts the first Slack message for the alert group. + """ + organization, slack_team_identity = make_organization_with_slack_team_identity() + + slack_channel = make_slack_channel(slack_team_identity) + organization.default_slack_channel = slack_channel + organization.save() + + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel, slack_message_sent=False) + alert = make_alert(alert_group, raw_request_data={}) + + # Ensure slack_message_sent is False initially + assert not alert_group.slack_message_sent + + step = IncomingAlertStep(slack_team_identity) + step.process_signal(alert) + + mock_chat_postMessage.assert_called_once_with( + channel=slack_channel.slack_id, + attachments=alert_group.render_slack_attachments(), + blocks=alert_group.render_slack_blocks(), + ) + + alert_group.refresh_from_db() + alert.refresh_from_db() + + assert alert_group.slack_message_sent is True + + assert alert_group.slack_message is not None + assert SlackMessage.objects.count() == 1 + assert alert_group.slack_message.slack_id == SLACK_MESSAGE_TS + assert alert_group.slack_message.channel == slack_channel + + assert alert.delivered is True + + @patch("apps.slack.client.SlackClient.chat_postMessage", return_value=SLACK_POST_MESSAGE_SUCCESS_RESPONSE) + @pytest.mark.django_db + def test_incoming_alert_no_channel_filter( + self, + mock_chat_postMessage, + make_slack_team_identity, + make_slack_channel, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) + alert_receive_channel = make_alert_receive_channel(organization) + + # Simulate an alert group with channel filter deleted in the middle of the escalation + # it should use the org default Slack channel to post the message to + alert_group = make_alert_group(alert_receive_channel, channel_filter=None) + alert = make_alert(alert_group, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity, organization) + step.process_signal(alert) + + mock_chat_postMessage.assert_called_once_with( + channel=slack_channel.slack_id, + attachments=alert_group.render_slack_attachments(), + blocks=alert_group.render_slack_blocks(), + ) + + @patch("apps.slack.client.SlackClient.chat_postMessage") + @pytest.mark.django_db + def test_process_signal_no_alert_group( + self, + mock_chat_postMessage, + make_slack_team_identity, + make_alert, + ): + slack_team_identity = make_slack_team_identity() + alert = make_alert(alert_group=None, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity) + step.process_signal(alert) + + mock_chat_postMessage.assert_not_called() + + @patch("apps.slack.client.SlackClient.chat_postMessage") + @pytest.mark.django_db + def test_process_signal_channel_rate_limited( + self, + mock_chat_postMessage, + make_organization_with_slack_team_identity, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + organization, slack_team_identity = make_organization_with_slack_team_identity() + + # Set rate_limited_in_slack_at to a recent time to simulate rate limiting + alert_receive_channel = make_alert_receive_channel( + organization, + rate_limited_in_slack_at=timezone.now() - timedelta(seconds=10), + ) + alert_group = make_alert_group(alert_receive_channel) + alert = make_alert(alert_group, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity) + step.process_signal(alert) + + mock_chat_postMessage.assert_not_called() + + alert_group.refresh_from_db() + assert alert_group.slack_message_sent is True + assert alert_group.reason_to_skip_escalation == AlertGroup.RATE_LIMITED + + @patch("apps.slack.client.SlackClient.chat_postMessage") + @pytest.mark.django_db + def test_process_signal_no_slack_channel( + self, + mock_chat_postMessage, + 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(default_slack_channel=None) + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel, channel_filter=None) + alert = make_alert(alert_group, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity) + step.process_signal(alert) + + alert_group.refresh_from_db() + assert alert_group.slack_message_sent is False + assert alert_group.reason_to_skip_escalation == AlertGroup.CHANNEL_NOT_SPECIFIED + + mock_chat_postMessage.assert_not_called() + + @patch("apps.slack.client.SlackClient.chat_postMessage") + @pytest.mark.django_db + def test_process_signal_debug_maintenance_mode( + self, + mock_chat_postMessage, + make_slack_team_identity, + make_organization, + make_slack_channel, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + """ + Test the scenario where the alert receive channel is in DEBUG_MAINTENANCE mode. + It should post the initial message and then send a debug mode notice in the same thread. + """ + # Mock chat_postMessage to handle both calls + # Set side_effect to return different values for each call + mock_chat_postMessage.side_effect = [ + SLACK_POST_MESSAGE_SUCCESS_RESPONSE, # create alert group slack message call return value + {"ok": True}, # debug mode notice call return value + ] + + slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) + + alert_receive_channel = make_alert_receive_channel( + organization, + maintenance_mode=AlertReceiveChannel.DEBUG_MAINTENANCE, + ) + + alert_group = make_alert_group(alert_receive_channel) + alert = make_alert(alert_group, raw_request_data={}) + + # Ensure slack_message_sent is False initially + assert not alert_group.slack_message_sent + + step = IncomingAlertStep(slack_team_identity) + step.process_signal(alert) + + assert mock_chat_postMessage.call_count == 2 + + _, create_alert_group_slack_message_call_kwargs = mock_chat_postMessage.call_args_list[0] + _, debug_mode_notice_call_kwargs = mock_chat_postMessage.call_args_list[1] + + assert create_alert_group_slack_message_call_kwargs["channel"] == slack_channel.slack_id + + text = "Escalations are silenced due to Debug mode" + assert debug_mode_notice_call_kwargs == { + "channel": slack_channel.slack_id, + "text": text, + "attachments": [], + "thread_ts": SLACK_MESSAGE_TS, # ts from first call + "mrkdwn": True, + "blocks": [ + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": text, + }, + }, + ], + } + + alert_group.refresh_from_db() + alert.refresh_from_db() + + assert alert_group.slack_message_sent is True + + assert alert_group.slack_message is not None + assert SlackMessage.objects.count() == 1 + assert alert_group.slack_message.slack_id == SLACK_MESSAGE_TS + assert alert_group.slack_message.channel == slack_channel + + assert alert.delivered is True + + @patch("apps.slack.client.SlackClient.chat_postMessage", return_value=SLACK_POST_MESSAGE_SUCCESS_RESPONSE) + @patch("apps.slack.scenarios.distribute_alerts.send_message_to_thread_if_bot_not_in_channel") + @pytest.mark.django_db + def test_process_signal_send_message_to_thread_if_bot_not_in_channel( + self, + mock_send_message_to_thread_if_bot_not_in_channel, + mock_chat_postMessage, + make_slack_team_identity, + make_slack_channel, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + alert = make_alert(alert_group, raw_request_data={}) + + assert alert_group.is_maintenance_incident is False + assert alert_group.skip_escalation_in_slack is False + + step = IncomingAlertStep(slack_team_identity) + step.process_signal(alert) + + mock_chat_postMessage.assert_called_once_with( + channel=slack_channel.slack_id, + attachments=alert_group.render_slack_attachments(), + blocks=alert_group.render_slack_blocks(), + ) + + mock_send_message_to_thread_if_bot_not_in_channel.apply_async.assert_called_once_with( + (alert_group.pk, slack_team_identity.pk, slack_channel.slack_id), countdown=1 + ) + + @patch("apps.slack.client.SlackClient.chat_postMessage") + @patch("apps.slack.scenarios.distribute_alerts.update_incident_slack_message") + @pytest.mark.django_db + def test_process_signal_update_existing_message( + self, + mock_update_incident_slack_message, + mock_chat_postMessage, + make_slack_team_identity, + make_slack_channel, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + mocked_update_incident_task_id = "1234" + mock_update_incident_slack_message.apply_async.return_value = mocked_update_incident_task_id + + slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) + alert_receive_channel = make_alert_receive_channel(organization) + + # Simulate that slack_message_sent is already True and skip_escalation_in_slack is False + alert_group = make_alert_group( + alert_receive_channel, + slack_message_sent=True, + reason_to_skip_escalation=AlertGroup.NO_REASON, + ) + assert alert_group.skip_escalation_in_slack is False + + alert = make_alert(alert_group, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity) + step.process_signal(alert) + + # assert that the background task is scheduled + mock_update_incident_slack_message.apply_async.assert_called_once_with( + (slack_team_identity.pk, alert_group.pk), countdown=10 + ) + mock_chat_postMessage.assert_not_called() + + # Verify that the cache is set correctly + assert cache.get(get_cache_key_update_incident_slack_message(alert_group.pk)) == mocked_update_incident_task_id + + @patch("apps.slack.client.SlackClient.chat_postMessage") + @patch("apps.slack.scenarios.distribute_alerts.update_incident_slack_message") + @pytest.mark.django_db + def test_process_signal_do_not_update_due_to_skip_escalation( + self, + mock_update_incident_slack_message, + mock_chat_postMessage, + make_organization_with_slack_team_identity, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + """ + Test that when skip_escalation_in_slack is True, the update task is not scheduled. + """ + organization, slack_team_identity = make_organization_with_slack_team_identity() + alert_receive_channel = make_alert_receive_channel(organization) + + # Simulate that slack_message_sent is already True and skip escalation due to RATE_LIMITED + alert_group = make_alert_group( + alert_receive_channel, + slack_message_sent=True, + reason_to_skip_escalation=AlertGroup.RATE_LIMITED, # Ensures skip_escalation_in_slack is True + ) + alert = make_alert(alert_group, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity) + step.process_signal(alert) + + # assert that the background task is not scheduled + mock_update_incident_slack_message.apply_async.assert_not_called() + mock_chat_postMessage.assert_not_called() + + @patch("apps.slack.client.SlackClient.chat_postMessage", side_effect=TimeoutError) + @pytest.mark.django_db + def test_process_signal_timeout_error( + self, + mock_chat_postMessage, + make_slack_team_identity, + make_slack_channel, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + alert = make_alert(alert_group, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity) + with pytest.raises(TimeoutError): + step.process_signal(alert) + + mock_chat_postMessage.assert_called_once_with( + channel=slack_channel.slack_id, + attachments=alert_group.render_slack_attachments(), + blocks=alert_group.render_slack_blocks(), + ) + + alert_group.refresh_from_db() + alert.refresh_from_db() + + # Ensure that slack_message_sent is set back to False, this will allow us to retry.. a TimeoutError may have + # been a transient error that is "recoverable" + assert alert_group.slack_message_sent is False + + assert alert_group.slack_message is None + assert SlackMessage.objects.count() == 0 + assert not alert.delivered + + @pytest.mark.parametrize( + "reason,slack_error", + [ + (reason, slack_error) + for reason, slack_error in AlertGroup.REASONS_TO_SKIP_ESCALATIONS + # we can skip NO_REASON because well this means theres no reason to skip the escalation + # we can skip CHANNEL_NOT_SPECIFIED because this is handled "higher up" in process_signal + if reason not in [AlertGroup.NO_REASON, AlertGroup.CHANNEL_NOT_SPECIFIED] + ], + ) + @pytest.mark.django_db + def test_process_signal_slack_errors( + self, + make_slack_team_identity, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, + make_slack_channel, + reason, + slack_error, + ): + slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + alert = make_alert(alert_group, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity) + + with patch.object(step._slack_client, "chat_postMessage") as mock_chat_postMessage: + error_response = build_slack_response({"error": slack_error}) + error_class = get_error_class(error_response) + mock_chat_postMessage.side_effect = error_class(error_response) + + step.process_signal(alert) + + alert_group.refresh_from_db() + alert.refresh_from_db() + + mock_chat_postMessage.assert_called_once_with( + channel=slack_channel.slack_id, + attachments=alert_group.render_slack_attachments(), + blocks=alert_group.render_slack_blocks(), + ) + + # For these Slack errors, retrying won't really help, so we should not set slack_message_sent back to False + assert alert_group.slack_message_sent is True + + assert alert_group.reason_to_skip_escalation == reason + assert alert_group.slack_message is None + assert SlackMessage.objects.count() == 0 + assert not alert.delivered + + @patch( + "apps.slack.client.SlackClient.chat_postMessage", + side_effect=SlackAPIRatelimitError(build_slack_response({"error": "ratelimited"})), + ) + @pytest.mark.django_db + def test_process_signal_slack_api_ratelimit_for_maintenance_integration( + self, + mock_chat_postMessage, + make_slack_team_identity, + make_slack_channel, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + """ + Test that when a SlackAPIRatelimitError occurs for a maintenance integration, + the exception is re-raised and slack_message_sent is set back to False. + """ + slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) + alert_receive_channel = make_alert_receive_channel( + organization, integration=AlertReceiveChannel.INTEGRATION_MAINTENANCE + ) + + alert_group = make_alert_group(alert_receive_channel) + alert = make_alert(alert_group, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity) + + with pytest.raises(SlackAPIRatelimitError): + step.process_signal(alert) + + mock_chat_postMessage.assert_called_once_with( + channel=slack_channel.slack_id, + attachments=alert_group.render_slack_attachments(), + blocks=alert_group.render_slack_blocks(), + ) + + alert_group.refresh_from_db() + + # Ensure that slack_message_sent is set back to False, this will allow us to retry.. a SlackAPIRatelimitError, + # may have been a transient error that is "recoverable" + # + # NOTE: we only want to retry for maintenance integrations, for other integrations we should not retry (this + # case is tested above under test_process_signal_slack_errors) + assert alert_group.slack_message_sent is False + + assert alert_group.reason_to_skip_escalation == AlertGroup.NO_REASON # Should remain unchanged + assert SlackMessage.objects.count() == 0 + assert not alert.delivered + + @patch( + "apps.slack.client.SlackClient.chat_postMessage", + side_effect=SlackAPIFetchMembersFailedError(build_slack_response({"error": "fetch_members_failed"})), + ) + @pytest.mark.django_db + def test_process_signal_unhandled_slack_error( + self, + mock_chat_postMessage, + make_slack_team_identity, + make_slack_channel, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, + ): + """ + Test that when an unhandled SlackAPIError occurs, the exception is re-raised + and slack_message_sent is set back to False. + """ + slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) + alert_receive_channel = make_alert_receive_channel(organization) + + alert_group = make_alert_group(alert_receive_channel) + alert = make_alert(alert_group, raw_request_data={}) + + step = IncomingAlertStep(slack_team_identity) + + with pytest.raises(SlackAPIFetchMembersFailedError): step.process_signal(alert) - alert_group.refresh_from_db() - alert.refresh_from_db() - assert alert_group.slack_message is None - assert alert_group.slack_message_sent is False - assert SlackMessage.objects.count() == 0 - assert not alert.delivered - - -@patch.object(IncomingAlertStep, "_post_alert_group_to_slack") -@pytest.mark.django_db -def test_incoming_alert_no_channel_filter( - mock_post_alert_group_to_slack, - make_slack_team_identity, - make_slack_channel, - make_organization, - make_alert_receive_channel, - make_alert_group, - make_alert, -): - slack_team_identity = make_slack_team_identity() - slack_channel = make_slack_channel(slack_team_identity, slack_id="DEFAULT_CHANNEL_ID") - organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) - 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 = IncomingAlertStep(slack_team_identity, organization) - step.process_signal(alert) - - assert mock_post_alert_group_to_slack.call_args[1]["slack_channel"] == slack_channel + mock_chat_postMessage.assert_called_once_with( + channel=slack_channel.slack_id, + attachments=alert_group.render_slack_attachments(), + blocks=alert_group.render_slack_blocks(), + ) + + alert_group.refresh_from_db() + + # For these Slack errors that we don't explictly want to handle, retrying won't really help, so we should not + # set slack_message_sent back to False + assert alert_group.slack_message_sent is False + + assert alert_group.reason_to_skip_escalation == AlertGroup.NO_REASON # Should remain unchanged + assert SlackMessage.objects.count() == 0 + assert not alert.delivered diff --git a/engine/apps/slack/tests/test_installation.py b/engine/apps/slack/tests/test_installation.py index 23f313e1f1..3248abc847 100644 --- a/engine/apps/slack/tests/test_installation.py +++ b/engine/apps/slack/tests/test_installation.py @@ -119,25 +119,18 @@ def test_install_slack_integration_legacy(settings, make_organization_and_user, @pytest.mark.django_db def test_uninstall_slack_integration( mock_clean_slack_integration_leftovers, - make_organization_and_user, - make_slack_team_identity, - make_slack_user_identity, + make_organization_and_user_with_slack_identities, ): - slack_team_identity = make_slack_team_identity() - organization, user = make_organization_and_user() - organization.slack_team_identity = slack_team_identity - organization.save() - organization.refresh_from_db() + organization, user, _, _ = make_organization_and_user_with_slack_identities() - slack_user_identity = make_slack_user_identity(slack_team_identity=slack_team_identity) - user.slack_user_identity = slack_user_identity - user.save() - user.refresh_from_db() + assert organization.slack_team_identity is not None + assert user.slack_user_identity is not None uninstall_slack_integration(organization, user) organization.refresh_from_db() user.refresh_from_db() + assert organization.slack_team_identity is None assert user.slack_user_identity is None