Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
Sentry On Premise backport of https://github.com/getsentry/sentry/pull/19446 to 9.1.2 to fix slack
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',
@caseyduquettesc

This comment has been minimized.

Copy link
Owner Author

@caseyduquettesc caseyduquettesc commented Oct 22, 2020

You can apply it if you are building Sentry from source with

git apply slack.patch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment