From 72e4f7e96f6a38660295a8dead820be4bd6452fe Mon Sep 17 00:00:00 2001 From: Aniket Das Date: Wed, 19 Oct 2022 14:49:58 -0700 Subject: [PATCH 1/2] DO NOT MERGE - Api Gateway hacks Shows what hacks i had to do to get a request to proxy --- src/sentry/api_gateway/api_gateway.py | 2 +- src/sentry/api_gateway/proxy.py | 9 ++++++++- src/sentry/conf/server.py | 1 + src/sentry/types/region.py | 15 ++++++++++++++- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/sentry/api_gateway/api_gateway.py b/src/sentry/api_gateway/api_gateway.py index ff570b839efd6..e18af1edccbfc 100644 --- a/src/sentry/api_gateway/api_gateway.py +++ b/src/sentry/api_gateway/api_gateway.py @@ -10,7 +10,7 @@ def _request_should_be_proxied(request: Request, view_func, view_kwargs) -> bool: view_class = getattr(view_func, "view_class", None) current_silo_mode = SiloMode.get_current_mode() - if view_class is not None: + if current_silo_mode != SiloMode.MONOLITH and view_class is not None: endpoint_silo_limit = getattr(view_class, "silo_limit", None) if endpoint_silo_limit is not None: endpoint_silo_set = endpoint_silo_limit.modes diff --git a/src/sentry/api_gateway/proxy.py b/src/sentry/api_gateway/proxy.py index bf787a6a06886..a763732fc35b7 100644 --- a/src/sentry/api_gateway/proxy.py +++ b/src/sentry/api_gateway/proxy.py @@ -2,6 +2,8 @@ Utilities related to proxying a request to a region silo """ +from wsgiref.util import is_hop_by_hop + from django.conf import settings from django.http import StreamingHttpResponse from requests import Response as ExternalResponse @@ -26,11 +28,16 @@ def stream_response(): # type: ignore streamed_response = StreamingHttpResponse( streaming_content=stream_response(), status=response.status_code, - content_type=response.headers.pop("Content-Type"), + content_type=response.headers.pop("Content-Type", None), ) # Add Headers to response + marked_for_deletion = [] for header, value in response.headers.items(): streamed_response[header] = value + if is_hop_by_hop(header): + marked_for_deletion.append(header) + for header in marked_for_deletion: + del streamed_response[header] return streamed_response diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 675ee185271b8..9fed04f7b93db 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -276,6 +276,7 @@ def env(key, default="", type=None): # response modifying middleware reset the Content-Length header. # This is because CommonMiddleware Sets the Content-Length header for non-streaming responses. MIDDLEWARE = ( + "sentry.middleware.api_gateway.ApiGatewayMiddleware", "sentry.middleware.health.HealthCheck", "sentry.middleware.security.SecurityHeadersMiddleware", "sentry.middleware.env.SentryEnvMiddleware", diff --git a/src/sentry/types/region.py b/src/sentry/types/region.py index 6049a096d6364..2bf7d4b06a4a6 100644 --- a/src/sentry/types/region.py +++ b/src/sentry/types/region.py @@ -108,6 +108,15 @@ def get_region_for_organization(organization: Organization) -> Region: Raises RegionContextError if this Sentry platform instance is configured to run only in monolith mode. """ + if organization is None: + # KLUDGE + return Region( + name="region1", + id=1, + address="http://127.0.0.1:9001", + category=RegionCategory.MULTI_TENANT, + ) + mapping = _load_global_regions() if not mapping.regions: @@ -116,7 +125,11 @@ def get_region_for_organization(organization: Organization) -> Region: # Backend representation to be determined. If you are working on code # that depends on this method, you can mock it out in unit tests or # temporarily hard-code a placeholder. - raise NotImplementedError + + # KLUDGE + return Region( + name="region1", id=1, address="127.0.0.1:9001", category=RegionCategory.MULTI_TENANT + ) def get_local_region() -> Region: From 1bbe92d1f7b60433683b5afa6511c23ac55f9a05 Mon Sep 17 00:00:00 2001 From: Aniket Das Date: Mon, 24 Oct 2022 14:18:03 -0700 Subject: [PATCH 2/2] Get this passing --- src/sentry/models/apitoken.py | 4 ++-- src/sentry/models/options/user_option.py | 4 ++-- src/sentry/models/user.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sentry/models/apitoken.py b/src/sentry/models/apitoken.py index 21e725c227f5c..0e24de0651275 100644 --- a/src/sentry/models/apitoken.py +++ b/src/sentry/models/apitoken.py @@ -9,7 +9,7 @@ BaseManager, FlexibleForeignKey, Model, - region_silo_only_model, + control_silo_only_model, sane_repr, ) from sentry.models.apiscopes import HasApiScopes @@ -25,7 +25,7 @@ def generate_token(): return uuid4().hex + uuid4().hex -@region_silo_only_model +@control_silo_only_model class ApiToken(Model, HasApiScopes): __include_in_export__ = True diff --git a/src/sentry/models/options/user_option.py b/src/sentry/models/options/user_option.py index 124dc6842a4c7..56c4aac2b68c5 100644 --- a/src/sentry/models/options/user_option.py +++ b/src/sentry/models/options/user_option.py @@ -5,7 +5,7 @@ from django.conf import settings from django.db import models -from sentry.db.models import FlexibleForeignKey, Model, region_silo_only_model, sane_repr +from sentry.db.models import FlexibleForeignKey, Model, control_silo_only_model, sane_repr from sentry.db.models.fields import PickledObjectField from sentry.db.models.manager import OptionManager, Value @@ -120,7 +120,7 @@ def post_delete(self, instance: UserOption, **kwargs: Any) -> None: # TODO(dcramer): the NULL UNIQUE constraint here isn't valid, and instead has to # be manually replaced in the database. We should restructure this model. -@region_silo_only_model +@control_silo_only_model class UserOption(Model): # type: ignore """ User options apply only to a user, and optionally a project OR an organization. diff --git a/src/sentry/models/user.py b/src/sentry/models/user.py index 3ae5a8f77f2ae..a727d0f855522 100644 --- a/src/sentry/models/user.py +++ b/src/sentry/models/user.py @@ -19,7 +19,7 @@ BaseModel, BoundedAutoField, FlexibleForeignKey, - region_silo_only_model, + control_silo_only_model, sane_repr, ) from sentry.models import LostPasswordHash @@ -115,7 +115,7 @@ def get_for_email(self, email: str, case_sensitive: bool = True) -> Sequence["Us return self.filter(emails__is_verified=True, is_active=True, **kwargs) -@region_silo_only_model +@control_silo_only_model class User(BaseModel, AbstractBaseUser): __include_in_export__ = True