Skip to content

Commit

Permalink
fix(hybridcloud) Fix cross-silo query in project rule details (#61435)
Browse files Browse the repository at this point in the history
In project rule details we hydrate a detached SentryAppInstallation
record to shim up ui component behavior. Preparing ui components
requires going from the install to sentryapp which emits a cross silo
query in test silos. We haven't been able to catch this in tests before
because we don't patch association properties.

To get this working I've hydrated a detached SentryApp record. The
entire prepare_ui component flow needs to be rebuilt with Rpc models to
remove this cruft and that is a bigger undertaking than we have time for
today. I've ticketed the follow up work for us to complete later.

Fixes HC-1023
  • Loading branch information
markstory authored Dec 11, 2023
1 parent 8d3f2b6 commit 83c9e95
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 5 deletions.
23 changes: 22 additions & 1 deletion src/sentry/api/endpoints/project_rule_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@
)
from sentry.apidocs.examples.issue_alert_examples import IssueAlertExamples
from sentry.apidocs.parameters import GlobalParams, IssueAlertParams
from sentry.constants import ObjectStatus
from sentry.constants import ObjectStatus, SentryAppStatus
from sentry.integrations.jira.actions.create_ticket import JiraCreateTicketAction
from sentry.integrations.jira_server.actions.create_ticket import JiraServerCreateTicketAction
from sentry.integrations.slack.utils import RedisRuleStatus
from sentry.mediators.project_rules.updater import Updater
from sentry.models.integrations.sentry_app import SentryApp
from sentry.models.integrations.sentry_app_component import SentryAppComponent
from sentry.models.integrations.sentry_app_installation import (
SentryAppInstallation,
Expand Down Expand Up @@ -135,7 +136,27 @@ def get(self, request: Request, project, rule) -> Response:
# Prepare Rule Actions that are SentryApp components using the meta fields
for action in serialized_rule.get("actions", []):
if action.get("_sentry_app_installation") and action.get("_sentry_app_component"):
# TODO(hybridcloud) This is nasty and should be fixed.
# Because all of the prepare_* functions currently operate on ORM
# records we need to convert our RpcSentryApp and dict data into detached
# ORM models and stitch together relations used in preparing UI components.
installation = SentryAppInstallation(**action.get("_sentry_app_installation", {}))
rpc_app = action.get("_sentry_app")
installation.sentry_app = SentryApp(
id=rpc_app.id,
scope_list=rpc_app.scope_list,
application_id=rpc_app.application_id,
application=None,
proxy_user_id=rpc_app.proxy_user_id,
owner_id=rpc_app.owner_id,
name=rpc_app.name,
slug=rpc_app.slug,
uuid=rpc_app.uuid,
events=rpc_app.events,
webhook_url=rpc_app.webhook_url,
status=SentryAppStatus.as_int(rpc_app.status),
metadata=rpc_app.metadata,
)
component = prepare_ui_component(
installation,
SentryAppComponent(**action.get("_sentry_app_component")),
Expand Down
12 changes: 8 additions & 4 deletions src/sentry/api/serializers/models/rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,12 @@ def get_attrs(self, item_list, user, **kwargs):
)
if sentry_app_uuid is not None
]
sentry_app_installs = app_service.get_many(filter=dict(uuids=sentry_app_uuids))
sentry_app_map = {
install.sentry_app.id: install.sentry_app for install in sentry_app_installs
}
sentry_app_ids: List[int] = list(sentry_app_map.keys())

sentry_app_ids: List[int] = [
i.sentry_app.id for i in app_service.get_many(filter=dict(uuids=sentry_app_uuids))
]
sentry_app_installations_by_uuid = app_service.get_related_sentry_app_components(
organization_ids=[rule.project.organization_id for rule in rules.values()],
sentry_app_ids=sentry_app_ids,
Expand Down Expand Up @@ -152,8 +154,10 @@ def get_attrs(self, item_list, user, **kwargs):
str(action.get("sentryAppInstallationUuid"))
)
if install:
installation = install.get("sentry_app_installation")
action["_sentry_app_component"] = install.get("sentry_app_component")
action["_sentry_app_installation"] = install.get("sentry_app_installation")
action["_sentry_app_installation"] = installation
action["_sentry_app"] = sentry_app_map.get(installation.get("sentry_app_id"))

if "lastTriggered" in self.expand:
last_triggered_lookup = {
Expand Down
57 changes: 57 additions & 0 deletions tests/sentry/api/endpoints/test_project_rule_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,63 @@ def test_with_snooze_rule_everyone(self):
assert response.data["snoozeCreatedBy"] == user2.get_display_name()
assert response.data["snoozeForEveryone"]

@responses.activate
def test_with_sentryapp_action(self):
conditions = [
{"id": "sentry.rules.conditions.every_event.EveryEventCondition"},
{"id": "sentry.rules.filters.issue_occurrences.IssueOccurrencesFilter", "value": 10},
]
actions = [
{
"id": "sentry.rules.actions.notify_event_sentry_app.NotifyEventSentryAppAction",
"sentryAppInstallationUuid": self.sentry_app_installation.uuid,
"settings": [
{"name": "title", "value": "An alert"},
{"summary": "Something happened here..."},
{"name": "points", "value": "3"},
{"name": "assignee", "value": "Nisanthan"},
],
}
]
data = {
"conditions": conditions,
"actions": actions,
"filter_match": "all",
"action_match": "all",
"frequency": 30,
}
self.rule.update(data=data)
responses.add(
responses.GET,
"https://example.com/sentry/members",
json=[
{"value": "bob", "label": "Bob"},
{"value": "jess", "label": "Jess"},
],
status=200,
)

response = self.get_success_response(
self.organization.slug, self.project.slug, self.rule.id, status_code=200
)
# Request to external service made
assert len(responses.calls) == 1

assert response.status_code == 200
assert "errors" not in response.data
assert "actions" in response.data

# Check that the sentryapp action contains choices from the integration host
action = response.data["actions"][0]
assert (
action["id"]
== "sentry.rules.actions.notify_event_sentry_app.NotifyEventSentryAppAction"
)
assert action["formFields"]["optional_fields"][-1]
assert "select" == action["formFields"]["optional_fields"][-1]["type"]
assert "sentry/members" in action["formFields"]["optional_fields"][-1]["uri"]
assert "bob" == action["formFields"]["optional_fields"][-1]["choices"][0][0]

@responses.activate
def test_with_unresponsive_sentryapp(self):
conditions = [
Expand Down

0 comments on commit 83c9e95

Please sign in to comment.