Skip to content

Commit 037e9f1

Browse files
authored
ref(slack): Feature gate workspace app functionality (getsentry#22926)
* ref(slack): Feature gate workspace app functionality
1 parent 418177e commit 037e9f1

17 files changed

+497
-31
lines changed

src/sentry/api/endpoints/organization_integrations.py

+18
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
from __future__ import absolute_import
22

3+
from sentry import features
4+
35
from sentry.api.bases.organization import OrganizationEndpoint, OrganizationIntegrationsPermission
46
from sentry.api.paginator import OffsetPaginator
57
from sentry.api.serializers import serialize
68
from sentry.models import ObjectStatus, OrganizationIntegration
9+
from sentry.integrations.slack.utils import get_integration_type
10+
from sentry.utils.compat import filter
711

812

913
class OrganizationIntegrationsEndpoint(OrganizationEndpoint):
@@ -17,6 +21,20 @@ def get(self, request, organization):
1721
if "provider_key" in request.GET:
1822
integrations = integrations.filter(integration__provider=request.GET["provider_key"])
1923

24+
# XXX(meredith): Filter out workspace apps if there are any.
25+
if not features.has(
26+
"organizations:slack-allow-workspace", organization=organization, actor=request.user
27+
):
28+
slack_integrations = integrations.filter(integration__provider="slack")
29+
workspace_ids = [
30+
workspace_app.id
31+
for workspace_app in filter(
32+
lambda i: get_integration_type(i.integration) == "workspace_app",
33+
slack_integrations,
34+
)
35+
]
36+
integrations = integrations.exclude(id__in=workspace_ids)
37+
2038
# include the configurations by default if no param
2139
include_config = True
2240
if request.GET.get("includeConfig") == "0":

src/sentry/conf/server.py

+3
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,9 @@ def create_partitioned_queues(name):
883883
"organizations:integrations-stacktrace-link": False,
884884
# Enables aws lambda integration
885885
"organizations:integrations-aws_lambda": False,
886+
# Temporary safety measure, turned on for specific orgs only if
887+
# absolutely necessary, to be removed shortly
888+
"organizations:slack-allow-workspace": False,
886889
# Enable data forwarding functionality for organizations.
887890
"organizations:data-forwarding": True,
888891
# Enable custom dashboards (dashboards 2)

src/sentry/features/__init__.py

+1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
default_manager.add("organizations:integrations-vsts-limited-scopes", OrganizationFeature) # NOQA
8484
default_manager.add("organizations:integrations-stacktrace-link", OrganizationFeature) # NOQA
8585
default_manager.add("organizations:integrations-aws_lambda", OrganizationFeature) # NOQA
86+
default_manager.add("organizations:slack-allow-workspace", OrganizationFeature) # NOQA
8687
default_manager.add("organizations:internal-catchall", OrganizationFeature) # NOQA
8788
default_manager.add("organizations:invite-members", OrganizationFeature) # NOQA
8889
default_manager.add("organizations:images-loaded-v2", OrganizationFeature) # NOQA

src/sentry/features/manager.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@
1313

1414
class RegisteredFeatureManager(object):
1515
"""
16-
Feature functions that are built around the need to register feature
17-
handlers
16+
Feature functions that are built around the need to register feature
17+
handlers
1818
19-
TODO: Once features have been audited and migrated to the entity
20-
handler, remove this class entirely
19+
TODO: Once features have been audited and migrated to the entity
20+
handler, remove this class entirely
2121
"""
2222

2323
def __init__(self):

src/sentry/incidents/logic.py

+11-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@
5151
from sentry.utils.compat import zip
5252
from sentry.utils.dates import to_timestamp
5353
from sentry.utils.snuba import bulk_raw_query, is_measurement, SnubaQueryParams, SnubaTSResult
54-
from sentry.shared_integrations.exceptions import DuplicateDisplayNameError
54+
from sentry.shared_integrations.exceptions import (
55+
DuplicateDisplayNameError,
56+
DeprecatedIntegrationError,
57+
)
5558

5659
# We can return an incident as "windowed" which returns a range of points around the start of the incident
5760
# It attempts to center the start of the incident, only showing earlier data if there isn't enough time
@@ -1262,6 +1265,13 @@ def get_alert_rule_trigger_action_slack_channel_id(
12621265
_prefix, channel_id, timed_out = get_channel_id(
12631266
organization, integration, name, use_async_lookup
12641267
)
1268+
1269+
# XXX(meredith): Will be removed when we rip out workspace app support completely.
1270+
except DeprecatedIntegrationError:
1271+
raise InvalidTriggerActionError(
1272+
"This workspace is using the deprecated Slack integration. Please re-install your integration to enable Slack alerting again."
1273+
)
1274+
12651275
except DuplicateDisplayNameError as e:
12661276
domain = integration.metadata["domain_name"]
12671277

src/sentry/integrations/slack/integration.py

+12-12
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,12 @@ def post_install(self, integration, organization, extra=None):
238238

239239
class SlackReAuthIntro(PipelineView):
240240
"""
241-
This pipeline step handles rendering the migration
242-
intro with context about the migration.
241+
This pipeline step handles rendering the migration
242+
intro with context about the migration.
243243
244-
If the `integration_id` is not present in the request
245-
then we can fast forward through the pipeline to move
246-
on to installing the integration as normal.
244+
If the `integration_id` is not present in the request
245+
then we can fast forward through the pipeline to move
246+
on to installing the integration as normal.
247247
248248
"""
249249

@@ -296,15 +296,15 @@ def dispatch(self, request, pipeline):
296296

297297
class SlackReAuthChannels(PipelineView):
298298
"""
299-
This pipeline step handles making requests to Slack and
300-
displaying the channels (if any) that are problematic:
299+
This pipeline step handles making requests to Slack and
300+
displaying the channels (if any) that are problematic:
301301
302-
1. private
303-
2. removed
304-
3. unauthorized
302+
1. private
303+
2. removed
304+
3. unauthorized
305305
306-
Any private channels in alert rules will also be binded
307-
to the pipeline state to be used later.
306+
Any private channels in alert rules will also be binded
307+
to the pipeline state to be used later.
308308
309309
"""
310310

src/sentry/integrations/slack/notify_action.py

+26-2
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,15 @@
88
from django import forms
99
from django.utils.translation import ugettext_lazy as _
1010

11+
from sentry import features
12+
1113
from sentry.models import Integration
1214
from sentry.rules.actions.base import IntegrationEventAction
13-
from sentry.shared_integrations.exceptions import ApiError, DuplicateDisplayNameError
15+
from sentry.shared_integrations.exceptions import (
16+
ApiError,
17+
DeprecatedIntegrationError,
18+
DuplicateDisplayNameError,
19+
)
1420
from sentry.utils import metrics, json
1521

1622
from .client import SlackClient
@@ -83,6 +89,16 @@ def clean(self):
8389
channel_prefix, channel_id, timed_out = self.channel_transformer(
8490
integration, channel
8591
)
92+
except DeprecatedIntegrationError:
93+
raise forms.ValidationError(
94+
_(
95+
'Workspace "%(workspace)s" is using the deprecated Slack integration. Please re-install your integration to enable Slack alerting again.',
96+
),
97+
code="invalid",
98+
params={
99+
"workspace": dict(self.fields["workspace"].choices).get(int(workspace)),
100+
},
101+
)
86102
except DuplicateDisplayNameError:
87103
domain = integration.metadata["domain_name"]
88104

@@ -151,6 +167,13 @@ def after(self, event, state):
151167
# Integration removed, rule still active.
152168
return
153169

170+
# XXX:(meredith) No longer support sending workspace app notifications unless explicitly
171+
# flagged in. Flag is temporary and will be taken out shortly
172+
if get_integration_type(integration) == "workspace_app" and not features.has(
173+
"organizations:slack-allow-workspace", event.group.project.organization
174+
):
175+
return
176+
154177
def send_notification(event, futures):
155178
with sentry_sdk.start_transaction(
156179
op=u"slack.send_notification", name=u"SlackSendNotification", sampled=1.0
@@ -159,7 +182,8 @@ def send_notification(event, futures):
159182
attachments = [
160183
build_group_attachment(event.group, event=event, tags=tags, rules=rules)
161184
]
162-
# check if we should have the upgrade notice attachment
185+
# XXX(meredith): Needs to be ripped out along with above feature flag. This will
186+
# not be used unless someone was explicitly flagged in to continue workspace alerts
163187
integration_type = get_integration_type(integration)
164188
if integration_type == "workspace_app":
165189
# stick the upgrade attachment first

src/sentry/integrations/slack/utils.py

+25-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from django.core.cache import cache
88
from django.http import Http404
99

10-
from sentry import tagstore
10+
from sentry import tagstore, features
1111
from sentry.api.fields.actor import Actor
1212
from sentry.utils import json
1313
from sentry.utils.assets import get_asset_url
@@ -26,7 +26,11 @@
2626
Team,
2727
ReleaseProject,
2828
)
29-
from sentry.shared_integrations.exceptions import ApiError, DuplicateDisplayNameError
29+
from sentry.shared_integrations.exceptions import (
30+
ApiError,
31+
DuplicateDisplayNameError,
32+
DeprecatedIntegrationError,
33+
)
3034
from sentry.integrations.metric_alerts import incident_attachment_info
3135

3236
from .client import SlackClient
@@ -419,7 +423,20 @@ def get_channel_id_with_timeout(integration, name, timeout):
419423

420424
# workspace tokens are the only tokens that don't works with the conversations.list endpoint,
421425
# once eveyone is migrated we can remove this check and usages of channels.list
422-
if get_integration_type(integration) == "workspace_app":
426+
427+
# XXX(meredith): Prevent anyone from creating new rules or editing existing rules that
428+
# have workspace app integrations. Force them to either remove slack action or re-install
429+
# their integration.
430+
integration_type = get_integration_type(integration)
431+
if integration_type == "workspace_app" and not any(
432+
[
433+
features.has("organizations:slack-allow-workspace", org)
434+
for org in integration.organizations.all()
435+
]
436+
):
437+
raise DeprecatedIntegrationError
438+
439+
if integration_type == "workspace_app":
423440
list_types = LEGACY_LIST_TYPES
424441
else:
425442
list_types = LIST_TYPES
@@ -483,6 +500,11 @@ def send_incident_alert_notification(action, incident, metric_value):
483500
"attachments": json.dumps([attachment]),
484501
}
485502

503+
if get_integration_type(integration) == "workspace_app" and not features.has(
504+
"organizations:slack-allow-workspace", incident.organization
505+
):
506+
return
507+
486508
client = SlackClient()
487509
try:
488510
client.post("/chat.postMessage", data=payload, timeout=5)

src/sentry/shared_integrations/exceptions.py

+4
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ class DuplicateDisplayNameError(IntegrationError):
8989
pass
9090

9191

92+
class DeprecatedIntegrationError(IntegrationError):
93+
pass
94+
95+
9296
class IntegrationFormError(IntegrationError):
9397
def __init__(self, field_errors):
9498
super(IntegrationFormError, self).__init__("Invalid integration action")

tests/acceptance/test_organization_integration_detail_view.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def test_uninstallation(self):
6060
provider="slack",
6161
external_id="some_slack",
6262
name="Test Slack",
63-
metadata={"domain_name": "slack-test.slack.com"},
63+
metadata={"domain_name": "slack-test.slack.com", "installation_type": "born_as_bot"},
6464
)
6565

6666
model.add_organization(self.organization, self.user)

tests/sentry/api/endpoints/test_organization_integrations.py

+32
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from sentry.models import Integration
66
from sentry.testutils import APITestCase
7+
from sentry.testutils.helpers import with_feature
78

89

910
class OrganizationIntegrationsListTest(APITestCase):
@@ -30,3 +31,34 @@ def test_no_config(self):
3031
response = self.client.get(path, format="json")
3132
assert response.status_code == 200, response.content
3233
assert "configOrganization" not in response.data[0]
34+
35+
def test_no_workspace_apps(self):
36+
integration = Integration.objects.create(
37+
provider="slack",
38+
name="Awesome Team",
39+
external_id="TXXXXXXX2",
40+
metadata={"access_token": "xoxa-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx"},
41+
)
42+
integration.add_organization(self.org, self.user)
43+
path = u"/api/0/organizations/{}/integrations/".format(self.org.slug)
44+
45+
response = self.client.get(path, format="json")
46+
assert response.status_code == 200, response.content
47+
assert len(response.data) == 1
48+
assert response.data[0]["id"] == six.text_type(self.integration.id)
49+
assert "configOrganization" in response.data[0]
50+
51+
@with_feature("organizations:slack-allow-workspace")
52+
def test_show_workspace_apps(self):
53+
integration = Integration.objects.create(
54+
provider="slack",
55+
name="Awesome Team",
56+
external_id="TXXXXXXX2",
57+
metadata={"access_token": "xoxa-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx"},
58+
)
59+
integration.add_organization(self.org, self.user)
60+
path = u"/api/0/organizations/{}/integrations/".format(self.org.slug)
61+
62+
response = self.client.get(path, format="json")
63+
assert response.status_code == 200, response.content
64+
assert len(response.data) == 2

0 commit comments

Comments
 (0)