Browse Source

fix(hybridcloud) API gateway should be using internal addresses (#53953)

When control silo has to proxy requests to the region we should use the
internal network address for the region instead of the public address.
This saves us some hops and fixes errors found in the test-silo
environment.
Mark Story 1 year ago
parent
commit
6b6902df2a

+ 3 - 3
src/sentry/api_gateway/proxy.py

@@ -2,6 +2,7 @@
 Utilities related to proxying a request to a region silo
 """
 import logging
+from urllib.parse import urljoin
 from wsgiref.util import is_hop_by_hop
 
 from django.conf import settings
@@ -17,7 +18,7 @@ from sentry.silo.util import (
     clean_outbound_headers,
     clean_proxy_headers,
 )
-from sentry.types.region import RegionResolutionError
+from sentry.types.region import RegionResolutionError, get_region_for_organization
 
 logger = logging.getLogger(__name__)
 
@@ -49,7 +50,6 @@ def _parse_response(response: ExternalResponse, remote_url: str) -> StreamingHtt
 
 def proxy_request(request: HttpRequest, org_slug: str) -> StreamingHttpResponse:
     """Take a django request object and proxy it to a remote location given an org_slug"""
-    from sentry.types.region import get_region_for_organization
 
     try:
         region = get_region_for_organization(org_slug)
@@ -57,7 +57,7 @@ def proxy_request(request: HttpRequest, org_slug: str) -> StreamingHttpResponse:
         logger.info("region_resolution_error", extra={"org_slug": org_slug})
         raise NotFound from e
 
-    target_url = region.to_url(request.path)
+    target_url = urljoin(region.address, request.path)
     header_dict = clean_proxy_headers(request.headers)
     # TODO: use requests session for connection pooling capabilities
     assert request.method is not None

+ 2 - 2
src/sentry/testutils/helpers/api_gateway.py

@@ -110,9 +110,9 @@ def provision_middleware():
 @override_settings(ROOT_URLCONF=__name__)
 class ApiGatewayTestCase(APITestCase):
     _REGION = Region(
-        name="region1",
+        name="region",
         snowflake_id=1,
-        address="http://region1.testserver",
+        address="http://region.internal.sentry.io",
         category=RegionCategory.MULTI_TENANT,
     )
 

+ 3 - 3
tests/sentry/api_gateway/test_api_gateway.py

@@ -18,7 +18,7 @@ class ApiGatewayTest(ApiGatewayTestCase):
         headers = dict(example="this")
         responses.add_callback(
             responses.GET,
-            f"http://region1.testserver/organizations/{self.organization.slug}/region/",
+            f"http://region.internal.sentry.io/organizations/{self.organization.slug}/region/",
             verify_request_params(query_params, headers),
         )
 
@@ -36,12 +36,12 @@ class ApiGatewayTest(ApiGatewayTestCase):
         """Test the logic of when a request should be proxied"""
         responses.add(
             responses.GET,
-            f"http://region1.testserver/organizations/{self.organization.slug}/region/",
+            f"http://region.internal.sentry.io/organizations/{self.organization.slug}/region/",
             json={"proxy": True},
         )
         responses.add(
             responses.GET,
-            f"http://region1.testserver/organizations/{self.organization.slug}/control/",
+            f"http://region.internal.sentry.io/organizations/{self.organization.slug}/control/",
             json={"proxy": True},
         )
 

+ 15 - 15
tests/sentry/api_gateway/test_proxy.py

@@ -29,7 +29,7 @@ class ProxyTestCase(ApiGatewayTestCase):
         assert resp.has_header("test")
         assert resp["test"] == "header"
         assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER)
-        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region1.testserver/get"
+        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region.internal.sentry.io/get"
 
         request = RequestFactory().get("http://sentry.io/error")
         resp = proxy_request(request, self.organization.slug)
@@ -39,7 +39,7 @@ class ProxyTestCase(ApiGatewayTestCase):
         assert resp.has_header("test")
         assert resp["test"] == "header"
         assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER)
-        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region1.testserver/error"
+        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region.internal.sentry.io/error"
 
     @responses.activate
     def test_query_params(self):
@@ -67,7 +67,7 @@ class ProxyTestCase(ApiGatewayTestCase):
         request_body = {"foo": "bar", "nested": {"int_list": [1, 2, 3]}}
         responses.add_callback(
             responses.POST,
-            "http://region1.testserver/post",
+            "http://region.internal.sentry.io/post",
             verify_request_body(request_body, {"test": "header"}),
         )
 
@@ -80,14 +80,14 @@ class ProxyTestCase(ApiGatewayTestCase):
         assert resp.status_code == 200
         assert resp_json["proxy"]
         assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER)
-        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region1.testserver/post"
+        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region.internal.sentry.io/post"
 
     @responses.activate
     def test_put(self):
         request_body = {"foo": "bar", "nested": {"int_list": [1, 2, 3]}}
         responses.add_callback(
             responses.PUT,
-            "http://region1.testserver/put",
+            "http://region.internal.sentry.io/put",
             verify_request_body(request_body, {"test": "header"}),
         )
 
@@ -100,14 +100,14 @@ class ProxyTestCase(ApiGatewayTestCase):
         assert resp.status_code == 200
         assert resp_json["proxy"]
         assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER)
-        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region1.testserver/put"
+        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region.internal.sentry.io/put"
 
     @responses.activate
     def test_patch(self):
         request_body = {"foo": "bar", "nested": {"int_list": [1, 2, 3]}}
         responses.add_callback(
             responses.PATCH,
-            "http://region1.testserver/patch",
+            "http://region.internal.sentry.io/patch",
             verify_request_body(request_body, {"test": "header"}),
         )
 
@@ -120,14 +120,14 @@ class ProxyTestCase(ApiGatewayTestCase):
         assert resp.status_code == 200
         assert resp_json["proxy"]
         assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER)
-        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region1.testserver/patch"
+        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region.internal.sentry.io/patch"
 
     @responses.activate
     def test_head(self):
         request_body = {"foo": "bar", "nested": {"int_list": [1, 2, 3]}}
         responses.add_callback(
             responses.HEAD,
-            "http://region1.testserver/head",
+            "http://region.internal.sentry.io/head",
             verify_request_headers({"test": "header"}),
         )
 
@@ -140,14 +140,14 @@ class ProxyTestCase(ApiGatewayTestCase):
         assert resp.status_code == 200
         assert resp_json["proxy"]
         assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER)
-        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region1.testserver/head"
+        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region.internal.sentry.io/head"
 
     @responses.activate
     def test_delete(self):
         request_body = {"foo": "bar", "nested": {"int_list": [1, 2, 3]}}
         responses.add_callback(
             responses.DELETE,
-            "http://region1.testserver/delete",
+            "http://region.internal.sentry.io/delete",
             verify_request_body(request_body, {"test": "header"}),
         )
 
@@ -160,7 +160,7 @@ class ProxyTestCase(ApiGatewayTestCase):
         assert resp.status_code == 200
         assert resp_json["proxy"]
         assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER)
-        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region1.testserver/delete"
+        assert resp[PROXY_DIRECT_LOCATION_HEADER] == "http://region.internal.sentry.io/delete"
 
     @responses.activate
     def test_file_upload(self):
@@ -173,7 +173,7 @@ class ProxyTestCase(ApiGatewayTestCase):
 
         responses.add_callback(
             responses.POST,
-            "http://region1.testserver/post",
+            "http://region.internal.sentry.io/post",
             verify_file_body(contents, {"test": "header"}),
         )
         request = RequestFactory().post(
@@ -193,7 +193,7 @@ class ProxyTestCase(ApiGatewayTestCase):
         request_body = contents
         responses.add_callback(
             responses.POST,
-            "http://region1.testserver/post",
+            "http://region.internal.sentry.io/post",
             verify_request_body(contents, {"test": "header"}),
         )
         request = RequestFactory().post(
@@ -212,7 +212,7 @@ class ProxyTestCase(ApiGatewayTestCase):
         request_body = {"foo": "bar", "nested": {"int_list": [1, 2, 3]}}
         responses.add_callback(
             responses.POST,
-            "http://region1.testserver/post",
+            "http://region.internal.sentry.io/post",
             verify_request_body(request_body, {"test": "header"}),
         )