Browse Source

fix(hc): Ensure RPC args are serializable (#53831)

Fix various bugs, mostly originating from arguments to RPC services not
being of a correct type to be serialized into a remote call.
Ryan Skonnord 1 year ago
parent
commit
67e49c6bab

+ 1 - 1
src/sentry/api/serializers/models/alert_rule.py

@@ -75,7 +75,7 @@ class AlertRuleSerializer(Serializer):
         use_by_user_id: MutableMapping[int, RpcUser] = {
             user.id: user
             for user in user_service.get_many(
-                filter=dict(user_ids=[r.user_id for r in rule_activities])
+                filter=dict(user_ids=[r.user_id for r in rule_activities if r.user_id is not None])
             )
         }
         for rule_activity in rule_activities:

+ 10 - 3
src/sentry/api/serializers/models/group.py

@@ -69,6 +69,7 @@ from sentry.search.events.filter import convert_search_filter_to_snuba_query, fo
 from sentry.services.hybrid_cloud.auth import AuthenticatedToken, auth_service
 from sentry.services.hybrid_cloud.integration import integration_service
 from sentry.services.hybrid_cloud.notifications import notifications_service
+from sentry.services.hybrid_cloud.user.serial import serialize_generic_user
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.snuba.dataset import Dataset
 from sentry.tagstore.snuba.backend import fix_tag_value_data
@@ -238,12 +239,18 @@ class GroupSerializerBase(Serializer, ABC):
 
         release_resolutions, commit_resolutions = self._resolve_resolutions(item_list, user)
 
-        user_ids = {r[-1] for r in release_resolutions.values()}
-        user_ids.update(r.actor_id for r in ignore_items.values() if r.actor_id is not None)
+        user_ids = {
+            user_id
+            for user_id in itertools.chain(
+                (r[-1] for r in release_resolutions.values()),
+                (r.actor_id for r in ignore_items.values()),
+            )
+            if user_id is not None
+        }
         if user_ids:
             serialized_users = user_service.serialize_many(
                 filter={"user_ids": user_ids, "is_active": True},
-                as_user=user,
+                as_user=serialize_generic_user(user),
             )
             actors = {id: u for id, u in zip(user_ids, serialized_users)}
         else:

+ 8 - 9
src/sentry/api/serializers/models/incidentseen.py

@@ -1,20 +1,19 @@
 from sentry.api.serializers import Serializer, register
 from sentry.incidents.models import IncidentSeen
+from sentry.services.hybrid_cloud.user.serial import serialize_generic_user
 from sentry.services.hybrid_cloud.user.service import user_service
 
 
 @register(IncidentSeen)
 class IncidentSeenSerializer(Serializer):
     def get_attrs(self, item_list, user):
-        user_map = {
-            d["id"]: d
-            for d in user_service.serialize_many(
-                filter={
-                    "user_ids": [i.user_id for i in item_list],
-                },
-                as_user=user,
-            )
-        }
+        item_users = user_service.serialize_many(
+            filter={
+                "user_ids": [i.user_id for i in item_list],
+            },
+            as_user=serialize_generic_user(user),
+        )
+        user_map = {d["id"]: d for d in item_users}
 
         result = {}
         for item in item_list:

+ 2 - 1
src/sentry/api/serializers/models/release.py

@@ -21,6 +21,7 @@ from sentry.models import (
     ReleaseProjectEnvironment,
     ReleaseStatus,
 )
+from sentry.services.hybrid_cloud.user.serial import serialize_generic_user
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.utils import metrics
 from sentry.utils.hashlib import md5_text
@@ -262,7 +263,7 @@ def get_users_for_authors(organization_id, authors, user=None) -> Mapping[str, A
                 "organization_id": organization_id,
                 "is_active": True,
             },
-            as_user=user,
+            as_user=serialize_generic_user(user),
         )
         # Figure out which email address matches to a user
         users_by_email = {}

+ 7 - 3
src/sentry/api/serializers/models/rule.py

@@ -80,9 +80,13 @@ class RuleSerializer(Serializer):
         owners_by_type = defaultdict(list)
 
         sentry_app_uuids = [
-            action.get("sentryAppInstallationUuid")
-            for rule in rules.values()
-            for action in rule.data.get("actions", [])
+            sentry_app_uuid
+            for sentry_app_uuid in (
+                action.get("sentryAppInstallationUuid")
+                for rule in rules.values()
+                for action in rule.data.get("actions", [])
+            )
+            if sentry_app_uuid is not None
         ]
 
         sentry_app_ids: List[int] = [

+ 18 - 12
src/sentry/models/authprovider.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import logging
 from typing import List
 
@@ -82,18 +84,7 @@ class AuthProvider(Model):
         return self.get_provider().name
 
     def get_scim_token(self):
-        from sentry.services.hybrid_cloud.app import app_service
-
-        if self.flags.scim_enabled:
-            return app_service.get_installation_token(
-                organization_id=self.organization_id, provider=f"{self.provider}_scim"
-            )
-        else:
-            logger.warning(
-                "SCIM disabled but tried to access token",
-                extra={"organization_id": self.organization_id},
-            )
-            return None
+        return get_scim_token(self.flags.scim_enabled, self.organization_id, self.provider)
 
     def enable_scim(self, user):
         from sentry.models import SentryAppInstallation, SentryAppInstallationForProvider
@@ -189,3 +180,18 @@ class AuthProvider(Model):
             )
             for region_name in find_regions_for_orgs([self.organization_id])
         ]
+
+
+def get_scim_token(scim_enabled: bool, organization_id: int, provider: str) -> str | None:
+    from sentry.services.hybrid_cloud.app import app_service
+
+    if scim_enabled:
+        return app_service.get_installation_token(
+            organization_id=organization_id, provider=f"{provider}_scim"
+        )
+    else:
+        logger.warning(
+            "SCIM disabled but tried to access token",
+            extra={"organization_id": organization_id},
+        )
+        return None

+ 5 - 1
src/sentry/notifications/notifications/activity/base.py

@@ -92,7 +92,11 @@ class GroupActivityNotification(ActivityNotification, abc.ABC):
 
     @cached_property
     def user(self) -> RpcUser | None:
-        return user_service.get_user(self.activity.user_id)
+        return (
+            user_service.get_user(self.activity.user_id)
+            if self.activity.user_id is not None
+            else None
+        )
 
     def get_participants_with_group_subscription_reason(self) -> ParticipantMap:
         """This is overridden by the activity subclasses."""

+ 5 - 2
src/sentry/notifications/utils/__init__.py

@@ -274,8 +274,11 @@ def has_alert_integration(project: Project) -> bool:
     org = project.organization
 
     # check integrations
-    providers = filter(is_alert_rule_integration, list(integrations.all()))
-    provider_keys = map(lambda x: cast(str, x.key), providers)
+    provider_keys = [
+        cast(str, provider.key)
+        for provider in integrations.all()
+        if is_alert_rule_integration(provider)
+    ]
     if integration_service.get_integrations(organization_id=org.id, providers=provider_keys):
         return True
 

+ 2 - 2
src/sentry/notifications/utils/participants.py

@@ -183,7 +183,7 @@ def get_participants_for_release(
     projects: Iterable[Project], organization: Organization, commited_user_ids: set[int]
 ) -> ParticipantMap:
     # Collect all users with verified emails on a team in the related projects.
-    user_ids = (
+    user_ids = list(
         OrganizationMember.objects.filter(
             teams__projectteam__project__in=projects,
             user_is_active=True,
@@ -256,7 +256,7 @@ def get_owners(
     elif owners == ProjectOwnership.Everyone:
         outcome = "everyone"
         users = user_service.get_many(
-            filter=dict(user_ids=project.member_set.values_list("user_id", flat=True))
+            filter=dict(user_ids=list(project.member_set.values_list("user_id", flat=True)))
         )
         recipients = RpcActor.many_from_object(users)
 

+ 1 - 5
src/sentry/receivers/sentry_apps.py

@@ -3,7 +3,6 @@ from __future__ import annotations
 from typing import Any, List, Mapping
 
 from sentry import features
-from sentry.api.serializers.models.user import UserSerializer
 from sentry.models import Group, GroupAssignee, Organization, SentryFunction, Team, User
 from sentry.services.hybrid_cloud import coerce_id_from
 from sentry.services.hybrid_cloud.app import RpcSentryAppInstallation, app_service
@@ -112,10 +111,7 @@ def send_workflow_webhooks(
         )
     if features.has("organizations:sentry-functions", organization, actor=user):
         if user:
-            data["user"] = user_service.serialize_many(
-                filter={"user_ids": [user.id]},
-                serializer=UserSerializer(),
-            )[0]
+            data["user"] = user_service.serialize_many(filter={"user_ids": [user.id]})[0]
         for fn in SentryFunction.objects.get_sentry_functions(organization, "issue"):
             send_sentry_function_webhook.delay(fn.external_id, event, issue.id, data)
 

Some files were not shown because too many files changed in this diff