Last active
November 11, 2020 21:11
-
-
Save caseyduquettesc/22f9b7ba401adc480e8ca739c23c79b4 to your computer and use it in GitHub Desktop.
Sentry On Premise backport of https://github.com/getsentry/sentry/pull/19446 to 9.1.2 to fix slack
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git a/src/sentry/integrations/slack/integration.py b/src/sentry/integrations/slack/integration.py | |
index 0926a1132d..0264d10c24 100644 | |
--- a/src/sentry/integrations/slack/integration.py | |
+++ b/src/sentry/integrations/slack/integration.py | |
@@ -75,13 +75,17 @@ class SlackIntegrationProvider(IntegrationProvider): | |
'links:read', | |
'links:write', | |
]) if not settings.SLACK_INTEGRATION_USE_WST else frozenset([ | |
- 'channels:read', | |
- 'groups:read', | |
- 'users:read', | |
- 'chat:write', | |
- 'links:read', | |
- 'links:write', | |
- 'team:read', | |
+ "channels:read", | |
+ "groups:read", | |
+ "users:read", | |
+ "chat:write", | |
+ "links:read", | |
+ "links:write", | |
+ "team:read", | |
+ "chat:write.public", | |
+ "chat:write.customize", | |
+ "im:read", | |
+ "mpim:read", | |
]) | |
setup_dialog_config = { | |
diff --git a/src/sentry/integrations/slack/notify_action.py b/src/sentry/integrations/slack/notify_action.py | |
index 1602c4e573..7e1e72a116 100644 | |
--- a/src/sentry/integrations/slack/notify_action.py | |
+++ b/src/sentry/integrations/slack/notify_action.py | |
@@ -1,9 +1,13 @@ | |
from __future__ import absolute_import | |
+import time | |
+import datetime | |
+ | |
from django import forms | |
from django.utils.translation import ugettext_lazy as _ | |
from sentry import http | |
+from sentry import options | |
from sentry.rules.actions.base import EventAction | |
from sentry.utils import metrics, json | |
from sentry.models import Integration | |
@@ -12,6 +16,7 @@ from .utils import build_attachment | |
MEMBER_PREFIX = '@' | |
CHANNEL_PREFIX = '#' | |
+SLACK_DEFAULT_TIMEOUT = 30 | |
strip_channel_chars = ''.join([MEMBER_PREFIX, CHANNEL_PREFIX]) | |
@@ -41,7 +46,10 @@ class SlackNotifyServiceForm(forms.Form): | |
workspace = cleaned_data.get('workspace') | |
channel = cleaned_data.get('channel', '').lstrip(strip_channel_chars) | |
+ channel_prefix = None | |
channel_id = self.channel_transformer(workspace, channel) | |
+ if channel_id: | |
+ channel_prefix, channel_id = channel_id | |
if channel_id is None and workspace is not None: | |
params = { | |
@@ -55,7 +63,6 @@ class SlackNotifyServiceForm(forms.Form): | |
params=params, | |
) | |
- channel_prefix, channel_id = channel_id | |
cleaned_data['channel'] = channel_prefix + channel | |
cleaned_data['channel_id'] = channel_id | |
@@ -104,6 +111,14 @@ class SlackNotifyServiceAction(EventAction): | |
# Integration removed, rule still active. | |
return | |
+ if not channel: | |
+ self.logger.warning('rule.fail.slack_post', extra={ | |
+ 'error': 'channel is none', | |
+ 'project_id': self.project.id, | |
+ 'project_name': self.project.name, | |
+ }) | |
+ return | |
+ | |
def send_notification(event, futures): | |
rules = [f.rule for f in futures] | |
attachment = build_attachment(event.group, event=event, tags=tags, rules=rules) | |
@@ -119,7 +134,12 @@ class SlackNotifyServiceAction(EventAction): | |
resp.raise_for_status() | |
resp = resp.json() | |
if not resp.get('ok'): | |
- self.logger.info('rule.fail.slack_post', extra={'error': resp.get('error')}) | |
+ self.logger.error('rule.fail.slack_post', extra={ | |
+ 'error': resp.get('error'), | |
+ 'channel': channel, | |
+ 'project_id': self.project.id, | |
+ 'project_name': self.project.name, | |
+ }) | |
key = u'slack:{}:{}'.format(integration_id, channel) | |
@@ -170,6 +190,25 @@ class SlackNotifyServiceAction(EventAction): | |
except Integration.DoesNotExist: | |
return None | |
+ return self.get_channel_id_with_timeout(integration, name, SLACK_DEFAULT_TIMEOUT) | |
+ | |
+ def get_channel_id_with_timeout(self, integration, name, timeout): | |
+ """ | |
+ Fetches the internal slack id of a channel. | |
+ :param organization: The organization that is using this integration | |
+ :param integration_id: The integration id of this slack integration | |
+ :param name: The name of the channel | |
+ :return: a tuple of three values | |
+ 1. prefix: string (`"#"` or `"@"`) | |
+ 2. channel_id: string or `None` | |
+ """ | |
+ LEGACY_LIST_TYPES = [ | |
+ ("channels", "channels", CHANNEL_PREFIX), | |
+ ("groups", "groups", CHANNEL_PREFIX), | |
+ ("users", "members", MEMBER_PREFIX), | |
+ ] | |
+ LIST_TYPES = [("conversations", "channels", CHANNEL_PREFIX), ("users", "members", MEMBER_PREFIX)] | |
+ | |
session = http.build_session() | |
token_payload = { | |
@@ -177,44 +216,64 @@ class SlackNotifyServiceAction(EventAction): | |
} | |
# Look for channel ID | |
- channels_payload = dict(token_payload, **{ | |
- 'exclude_archived': False, | |
- 'exclude_members': True, | |
- }) | |
- | |
- resp = session.get('https://slack.com/api/channels.list', params=channels_payload) | |
- resp = resp.json() | |
- if not resp.get('ok'): | |
- self.logger.info('rule.slack.channel_list_failed', extra={'error': resp.get('error')}) | |
- return None | |
- | |
- channel_id = {c['name']: c['id'] for c in resp['channels']}.get(name) | |
- | |
- if channel_id: | |
- return (CHANNEL_PREFIX, channel_id) | |
- | |
- # Channel may be private | |
- resp = session.get('https://slack.com/api/groups.list', params=channels_payload) | |
- resp = resp.json() | |
- if not resp.get('ok'): | |
- self.logger.info('rule.slack.group_list_failed', extra={'error': resp.get('error')}) | |
- return None | |
- | |
- group_id = {c['name']: c['id'] for c in resp['groups']}.get(name) | |
- | |
- if group_id: | |
- return (CHANNEL_PREFIX, group_id) | |
- | |
- # Channel may actually be a user | |
- resp = session.get('https://slack.com/api/users.list', params=token_payload) | |
- resp = resp.json() | |
- if not resp.get('ok'): | |
- self.logger.info('rule.slack.user_list_failed', extra={'error': resp.get('error')}) | |
- return None | |
- | |
- member_id = {c['name']: c['id'] for c in resp['members']}.get(name) | |
- | |
- if member_id: | |
- return (MEMBER_PREFIX, member_id) | |
- | |
- return None | |
+ channels_payload = dict(token_payload, **{"exclude_archived": True, "exclude_members": True}) | |
+ | |
+ if options.get("slack.legacy-app") is True: | |
+ list_types = LEGACY_LIST_TYPES | |
+ else: | |
+ list_types = LIST_TYPES | |
+ channels_payload = dict(channels_payload, **{"types": "public_channel,private_channel"}) | |
+ | |
+ time_to_quit = time.time() + timeout | |
+ | |
+ id_data = None | |
+ found_duplicate = False | |
+ scanned_item_count = 0 | |
+ for list_type, result_name, prefix in list_types: | |
+ cursor = "" | |
+ while True: | |
+ endpoint = "/%s.list" % list_type | |
+ | |
+ # Slack limits the response of `<list_type>.list` to 1000 channels | |
+ resp = session.get('https://slack.com/api%s' % endpoint, params=dict(channels_payload, cursor=cursor, limit=100)) | |
+ items = resp.json() | |
+ if not items.get('ok'): | |
+ retry_after = resp.headers.get('retry-after') | |
+ if retry_after: | |
+ self.logger.info('rule.slack', extra={'scanned_item_count': scanned_item_count, 'time_until_timeout': datetime.timedelta(seconds=time_to_quit - time.time())}) | |
+ self.logger.info('rule.slack.%s_list_rate_limited' % list_type, extra={'retry_after': retry_after}) | |
+ time.sleep(float(retry_after)) | |
+ continue | |
+ else: | |
+ self.logger.error('rule.slack.%s_list_failed' % list_type, extra={'error': items.get('error')}) | |
+ return (prefix, None) | |
+ | |
+ for c in items[result_name]: | |
+ scanned_item_count += 1 | |
+ # The "name" field is unique (this is the username for users) | |
+ # so we return immediately if we find a match. | |
+ if c["name"] == name: | |
+ return (prefix, c["id"]) | |
+ # If we don't get a match on a unique identifier, we look through | |
+ # the users' display names, and error if there is a repeat. | |
+ if list_type == "users": | |
+ profile = c.get("profile") | |
+ if profile and profile.get("display_name") == name: | |
+ if id_data: | |
+ found_duplicate = True | |
+ else: | |
+ id_data = (prefix, c["id"]) | |
+ | |
+ cursor = items.get("response_metadata", {}).get("next_cursor", None) | |
+ if time.time() > time_to_quit: | |
+ self.logger.error('rule.slack.%s_list_timed_out' % list_type) | |
+ return (prefix, None) | |
+ | |
+ if not cursor: | |
+ break | |
+ if found_duplicate: | |
+ return (prefix, None) | |
+ elif id_data: | |
+ return id_data | |
+ | |
+ return (prefix, None) | |
diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py | |
index 88ee554e1f..5f25288300 100644 | |
--- a/src/sentry/options/defaults.py | |
+++ b/src/sentry/options/defaults.py | |
@@ -135,6 +135,8 @@ register('tagstore.multi-sampling', default=0.0) | |
register('slack.client-id', flags=FLAG_PRIORITIZE_DISK) | |
register('slack.client-secret', flags=FLAG_PRIORITIZE_DISK) | |
register('slack.verification-token', flags=FLAG_PRIORITIZE_DISK) | |
+register("slack.signing-secret", flags=FLAG_PRIORITIZE_DISK) | |
+register("slack.legacy-app", flags=FLAG_PRIORITIZE_DISK, type=Bool, default=True) | |
# GitHub Integration | |
register('github-app.id', default=0) | |
diff --git a/src/sentry/utils/pytest/sentry.py b/src/sentry/utils/pytest/sentry.py | |
index bdc9f888ac..eac334a164 100644 | |
--- a/src/sentry/utils/pytest/sentry.py | |
+++ b/src/sentry/utils/pytest/sentry.py | |
@@ -150,6 +150,7 @@ def pytest_configure(config): | |
'slack.client-id': 'slack-client-id', | |
'slack.client-secret': 'slack-client-secret', | |
'slack.verification-token': 'slack-verification-token', | |
+ "slack.legacy-app": True, | |
'github-app.name': 'sentry-test-app', | |
'github-app.client-id': 'github-client-id', |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
You can apply it if you are building Sentry from source with