Browse Source

ref(hc): RPC in tests uses test client and hits rpc endpoint (#53237)

RPC Services use the django test client in tests rather than falling
back to local service.
Several refactors and lots of code removal related to this in addition
to fixing several service issues.

---------

Co-authored-by: Ryan Skonnord <ryan.skonnord@sentry.io>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Zach Collins 1 year ago
parent
commit
80c5f777a2

+ 0 - 1
pyproject.toml

@@ -293,7 +293,6 @@ module = [
     "sentry.api.endpoints.project_transfer",
     "sentry.api.endpoints.project_user_stats",
     "sentry.api.endpoints.prompts_activity",
-    "sentry.api.endpoints.rpc",
     "sentry.api.endpoints.rule_snooze",
     "sentry.api.endpoints.setup_wizard",
     "sentry.api.endpoints.shared_group_details",

+ 0 - 20
src/sentry/api/base.py

@@ -611,26 +611,6 @@ class EndpointSiloLimit(SiloLimit):
         new_class.__module__ = decorated_class.__module__
         return new_class
 
-    def create_override(
-        self,
-        original_method: Callable[..., Any],
-    ) -> Callable[..., Any]:
-        limiting_override = super().create_override(original_method)
-
-        def single_process_silo_mode_wrapper(*args: Any, **kwargs: Any) -> Any:
-            if SiloMode.single_process_silo_mode():
-                entering_mode: SiloMode = SiloMode.MONOLITH
-                for mode in self.modes:
-                    # Select a mode, if available, from the target modes.
-                    entering_mode = mode
-                with SiloMode.enter_single_process_silo_context(entering_mode):
-                    return limiting_override(*args, **kwargs)
-            else:
-                return limiting_override(*args, **kwargs)
-
-        functools.update_wrapper(single_process_silo_mode_wrapper, limiting_override)
-        return single_process_silo_mode_wrapper
-
     def modify_endpoint_method(self, decorated_method: Callable[..., Any]) -> Callable[..., Any]:
         return self.create_override(decorated_method)
 

+ 2 - 1
src/sentry/api/endpoints/avatar/user.py

@@ -7,6 +7,7 @@ from sentry.api.base import control_silo_endpoint
 from sentry.api.bases.avatar import AvatarMixin
 from sentry.api.bases.user import UserEndpoint
 from sentry.models import UserAvatar
+from sentry.services.hybrid_cloud.user.serial import serialize_rpc_user
 from sentry.services.hybrid_cloud.user.service import user_service
 
 
@@ -19,7 +20,7 @@ class UserAvatarEndpoint(AvatarMixin, UserEndpoint):
         user = kwargs.pop(self.object_type, None)
         serialized_user = user_service.serialize_many(
             filter=dict(user_ids=[user.id]),
-            as_user=user,
+            as_user=serialize_rpc_user(user),
         )[0]
         return Response(serialized_user)
 

+ 30 - 2
src/sentry/api/endpoints/rpc.py

@@ -1,14 +1,19 @@
+from typing import Any, Dict
+
+import pydantic
 from rest_framework.exceptions import NotFound, ParseError, PermissionDenied, ValidationError
 from rest_framework.request import Request
 from rest_framework.response import Response
 
 from sentry.api.authentication import RpcSignatureAuthentication
 from sentry.api.base import Endpoint, all_silo_endpoint
+from sentry.services.hybrid_cloud.auth import AuthenticationContext
 from sentry.services.hybrid_cloud.rpc import (
     RpcArgumentException,
     RpcResolutionException,
     dispatch_to_local_service,
 )
+from sentry.utils.env import in_test_environment
 
 
 @all_silo_endpoint
@@ -28,14 +33,37 @@ class RpcServiceEndpoint(Endpoint):
             raise PermissionDenied
 
         metadata = request.data.get("meta")  # noqa
-        arguments = request.data.get("args")
+        try:
+            arguments: Dict[str, Any] = request.data["args"]
+        except KeyError as e:
+            raise ParseError from e
+        if not isinstance(arguments, dict):
+            raise ParseError
+
+        auth_context: AuthenticationContext = AuthenticationContext()
+        if auth_context_json := arguments.get("auth_context"):
+            try:
+                # Note -- generally, this is NOT set, but only in cases where an RPC needs to invoke code
+                # that depends on the `env.request.user` object.  In that case, the authentication context
+                # includes an authenticated user that will be injected into the global request context
+                # for compatibility.  Notably, this authentication context is *trusted* as the request comes
+                # from within the privileged RPC channel.
+                auth_context = AuthenticationContext.parse_obj(auth_context_json)
+            except pydantic.ValidationError as e:
+                raise ParseError from e
 
         try:
-            result = dispatch_to_local_service(service_name, method_name, arguments)
+            with auth_context.applied_to_request(request):
+                result = dispatch_to_local_service(service_name, method_name, arguments)
         except RpcResolutionException as e:
             raise NotFound from e
         except RpcArgumentException as e:
             raise ParseError from e
         except Exception as e:
+            # Produce more detailed log
+            if in_test_environment():
+                raise Exception(
+                    f"Problem processing rpc service endpoint {service_name}/{method_name}"
+                ) from e
             raise ValidationError from e
         return Response(data=result)

+ 4 - 1
src/sentry/api/serializers/models/activity.py

@@ -3,6 +3,7 @@ import functools
 from sentry.api.serializers import Serializer, register, serialize
 from sentry.api.serializers.models.commit import CommitWithReleaseSerializer
 from sentry.models import Activity, Commit, Group, PullRequest
+from sentry.services.hybrid_cloud.user.serial import serialize_generic_user
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.types.activity import ActivityType
 from sentry.utils.functional import apply_values
@@ -16,7 +17,9 @@ class ActivitySerializer(Serializer):
     def get_attrs(self, item_list, user):
         # TODO(dcramer); assert on relations
         user_ids = [i.user_id for i in item_list if i.user_id]
-        user_list = user_service.serialize_many(filter={"user_ids": user_ids}, as_user=user)
+        user_list = user_service.serialize_many(
+            filter={"user_ids": user_ids}, as_user=serialize_generic_user(user)
+        )
         users = {u["id"]: u for u in user_list}
 
         commit_ids = {

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

@@ -50,7 +50,7 @@ class AlertRuleSerializer(Serializer):
                 "triggers", []
             )
             for action in serialized.get("actions", []):
-                install = sentry_app_installations_by_sentry_app_id.get(action.get("sentryAppId"))
+                install = sentry_app_installations_by_sentry_app_id.get(str(action["sentryAppId"]))
                 if install:
                     action["_sentry_app_component"] = install.get("sentry_app_component")
                     action["_sentry_app_installation"] = install.get("sentry_app_installation")

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

@@ -21,7 +21,7 @@ class DiscoverSavedQuerySerializer(Serializer):
                     if discover_saved_query.created_by_id
                 ]
             },
-            as_user=user,
+            as_user=user if user.id else None,
         )
         serialized_users = {user["id"]: user for user in service_serialized}
 

+ 2 - 4
src/sentry/api/serializers/models/group.py

@@ -66,7 +66,7 @@ from sentry.notifications.types import NotificationSettingTypes
 from sentry.reprocessing2 import get_progress
 from sentry.search.events.constants import RELEASE_STAGE_ALIAS
 from sentry.search.events.filter import convert_search_filter_to_snuba_query, format_search_filter
-from sentry.services.hybrid_cloud.auth import AuthenticatedToken, auth_service
+from sentry.services.hybrid_cloud.auth import AuthenticatedToken
 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
@@ -713,9 +713,7 @@ class GroupSerializerBase(Serializer, ABC):
             and is_api_token_auth(request.auth)
         ):
 
-            if auth_service.token_has_org_access(
-                token=AuthenticatedToken.from_token(request.auth), organization_id=organization_id
-            ):
+            if AuthenticatedToken.from_token(request.auth).token_has_org_access(organization_id):
                 return True
 
         if (

+ 3 - 1
src/sentry/api/serializers/models/incidentactivity.py

@@ -2,6 +2,7 @@ from django.db.models import prefetch_related_objects
 
 from sentry.api.serializers import Serializer, register
 from sentry.incidents.models import IncidentActivity
+from sentry.services.hybrid_cloud.user.serial import serialize_generic_user
 from sentry.services.hybrid_cloud.user.service import user_service
 
 
@@ -10,7 +11,8 @@ class IncidentActivitySerializer(Serializer):
     def get_attrs(self, item_list, user, **kwargs):
         prefetch_related_objects(item_list, "incident__organization")
         serialized_users = user_service.serialize_many(
-            filter={"user_ids": [i.user_id for i in item_list if i.user_id]}, as_user=user
+            filter={"user_ids": [i.user_id for i in item_list if i.user_id]},
+            as_user=serialize_generic_user(user),
         )
         user_lookup = {user["id"]: user for user in serialized_users}
         return {item: {"user": user_lookup.get(str(item.user_id))} for item in item_list}

+ 5 - 5
src/sentry/api/serializers/models/organization_access_request.py

@@ -6,11 +6,11 @@ from sentry.services.hybrid_cloud.user.service import user_service
 @register(OrganizationAccessRequest)
 class OrganizationAccessRequestSerializer(Serializer):
     def serialize(self, obj, attrs, user):
-        serialized_users = user_service.serialize_many(filter=dict(user_ids=[obj.requester_id]))
-        if serialized_users:
-            serialized_user = serialized_users[0]
-        else:
-            serialized_user = None
+        serialized_user = None
+        if obj.requester_id:
+            serialized_users = user_service.serialize_many(filter=dict(user_ids=[obj.requester_id]))
+            if serialized_users:
+                serialized_user = serialized_users[0]
 
         d = {
             "id": str(obj.id),

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