test(hc): Change assertions on tests to proxied integration API (#58446)

Introduce IntegratedApiTestCase as a test case superclass with a utility
method for checking whether proxying to the control silo is expected.

In unit tests for integration, resolve assertion errors (typically
`KeyError: 'authorization'`) on proxied requests to external APIs. The
error occurred because the `responses` utility captures only the proxy
request to the control silo, and the underlying request by the control
silo to the external API (which the test asserts for) never happens.

This solution depends on the assumption that the "proxy to control silo"
behavior is basically correct as-is, and is adequately tested elsewhere,
so that we only need to change the assertions to match the existing
behavior. If so, then the monolith-mode test should be enough to test
the correctness of the requests made to the external API. In region
mode, we test only the proxy request to the control silo, and treat the
control silo's behavior as a black box.

src/sentry/shared_integrations/client/proxy.py

@@ -84,11 +84,7 @@ class IntegrationProxyClient(ApiClient):
         )
         self.org_integration_id = org_integration_id
 
-        is_region_silo = SiloMode.get_current_mode() == SiloMode.REGION
-        subnet_secret = getattr(settings, "SENTRY_SUBNET_SECRET", None)
-        control_address = getattr(settings, "SENTRY_CONTROL_ADDRESS", None)
-
-        if is_region_silo and subnet_secret and control_address:
+        if self.determine_whether_should_proxy_to_control():
             self._should_proxy_to_control = True
             self.proxy_url = get_proxy_url()
 
@@ -96,6 +92,14 @@ class IntegrationProxyClient(ApiClient):
             logger.info("proxy_disabled_in_test_env")
             self.proxy_url = self.base_url
 
+    @staticmethod
+    def determine_whether_should_proxy_to_control() -> bool:
+        return (
+            SiloMode.get_current_mode() == SiloMode.REGION
+            and getattr(settings, "SENTRY_SUBNET_SECRET", None) is not None
+            and getattr(settings, "SENTRY_CONTROL_ADDRESS", None) is not None
+        )
+
     @control_silo_function
     def authorize_request(self, prepared_request: PreparedRequest) -> PreparedRequest:
         """

src/sentry/testutils/cases.py

@@ -127,6 +127,7 @@ from sentry.utils.samples import load_data
 from sentry.utils.snuba import _snuba_pool
 
 from ..services.hybrid_cloud.organization.serial import serialize_rpc_organization
+from ..shared_integrations.client.proxy import IntegrationProxyClient
 from ..snuba.metrics import (
     MetricConditionField,
     MetricField,
@@ -2777,3 +2778,8 @@ class MonitorIngestTestCase(MonitorTestCase):
                 self.endpoint_with_org, args=[self.organization.slug, monitor_slug]
             ),
         )
+
+
+class IntegratedApiTestCase(BaseTestCase):
+    def should_call_api_without_proxying(self) -> bool:
+        return not IntegrationProxyClient.determine_whether_should_proxy_to_control()

tests/sentry/integrations/github/test_issues.py

@@ -10,7 +10,8 @@ from sentry.models.integrations.external_issue import ExternalIssue
 from sentry.models.integrations.integration import Integration
 from sentry.services.hybrid_cloud.integration import integration_service
 from sentry.silo import SiloMode
-from sentry.testutils.cases import PerformanceIssueTestCase, TestCase
+from sentry.silo.util import PROXY_BASE_URL_HEADER, PROXY_OI_HEADER, PROXY_SIGNATURE_HEADER
+from sentry.testutils.cases import IntegratedApiTestCase, PerformanceIssueTestCase, TestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE
 from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
@@ -20,8 +21,8 @@ from sentry.utils import json
 pytestmark = [requires_snuba]
 
 
-@region_silo_test
-class GitHubIssueBasicTest(TestCase, PerformanceIssueTestCase):
+@region_silo_test(stable=True)
+class GitHubIssueBasicTest(TestCase, PerformanceIssueTestCase, IntegratedApiTestCase):
     @cached_property
     def request(self):
         return RequestFactory()
@@ -37,6 +38,13 @@ class GitHubIssueBasicTest(TestCase, PerformanceIssueTestCase):
         self.integration = GitHubIntegration(self.model, self.organization.id)
         self.min_ago = iso_format(before_now(minutes=1))
 
+    def _check_proxying(self) -> None:
+        assert len(responses.calls) == 1
+        request = responses.calls[0].request
+        assert request.headers[PROXY_OI_HEADER] == str(self.integration.org_integration.id)
+        assert request.headers[PROXY_BASE_URL_HEADER] == "https://api.github.com"
+        assert PROXY_SIGNATURE_HEADER in request.headers
+
     @responses.activate
     @patch("sentry.integrations.github.client.get_jwt", return_value="jwt_token_1")
     def test_get_allowed_assignees(self, mock_get_jwt):
@@ -58,11 +66,16 @@ class GitHubIssueBasicTest(TestCase, PerformanceIssueTestCase):
             ("MeredithAnya", "MeredithAnya"),
         )
 
-        request = responses.calls[0].request
-        assert request.headers["Authorization"] == "Bearer jwt_token_1"
+        if self.should_call_api_without_proxying():
+            assert len(responses.calls) == 2
 
-        request = responses.calls[1].request
-        assert request.headers["Authorization"] == "Bearer token_1"
+            request = responses.calls[0].request
+            assert request.headers["Authorization"] == "Bearer jwt_token_1"
+
+            request = responses.calls[1].request
+            assert request.headers["Authorization"] == "Bearer token_1"
+        else:
+            self._check_proxying()
 
     @responses.activate
     @patch("sentry.integrations.github.client.get_jwt", return_value="jwt_token_1")
@@ -97,13 +110,23 @@ class GitHubIssueBasicTest(TestCase, PerformanceIssueTestCase):
             "url": "https://github.com/getsentry/sentry/issues/231",
             "repo": "getsentry/sentry",
         }
-        request = responses.calls[0].request
-        assert request.headers["Authorization"] == "Bearer jwt_token_1"
 
-        request = responses.calls[1].request
-        assert request.headers["Authorization"] == "Bearer token_1"
-        payload = json.loads(request.body)
-        assert payload == {"body": "This is the description", "assignee": None, "title": "hello"}
+        if self.should_call_api_without_proxying():
+            assert len(responses.calls) == 2
+
+            request = responses.calls[0].request
+            assert request.headers["Authorization"] == "Bearer jwt_token_1"
+
+            request = responses.calls[1].request
+            assert request.headers["Authorization"] == "Bearer token_1"
+            payload = json.loads(request.body)
+            assert payload == {
+                "body": "This is the description",
+                "assignee": None,
+                "title": "hello",
+            }
+        else:
+            self._check_proxying()
 
     def test_performance_issues_content(self):
         """Test that a GitHub issue created from a performance issue has the expected title and description"""
@@ -169,11 +192,16 @@ class GitHubIssueBasicTest(TestCase, PerformanceIssueTestCase):
         repo = "getsentry/sentry"
         assert self.integration.get_repo_issues(repo) == ((321, "#321 hello"),)
 
-        request = responses.calls[0].request
-        assert request.headers["Authorization"] == "Bearer jwt_token_1"
+        if self.should_call_api_without_proxying():
+            assert len(responses.calls) == 2
 
-        request = responses.calls[1].request
-        assert request.headers["Authorization"] == "Bearer token_1"
+            request = responses.calls[0].request
+            assert request.headers["Authorization"] == "Bearer jwt_token_1"
+
+            request = responses.calls[1].request
+            assert request.headers["Authorization"] == "Bearer token_1"
+        else:
+            self._check_proxying()
 
     @responses.activate
     @patch("sentry.integrations.github.client.get_jwt", return_value="jwt_token_1")
@@ -205,11 +233,17 @@ class GitHubIssueBasicTest(TestCase, PerformanceIssueTestCase):
             "url": "https://github.com/getsentry/sentry/issues/231",
             "repo": "getsentry/sentry",
         }
-        request = responses.calls[0].request
-        assert request.headers["Authorization"] == "Bearer jwt_token_1"
 
-        request = responses.calls[1].request
-        assert request.headers["Authorization"] == "Bearer token_1"
+        if self.should_call_api_without_proxying():
+            assert len(responses.calls) == 2
+
+            request = responses.calls[0].request
+            assert request.headers["Authorization"] == "Bearer jwt_token_1"
+
+            request = responses.calls[1].request
+            assert request.headers["Authorization"] == "Bearer token_1"
+        else:
+            self._check_proxying()
 
     @responses.activate
     @patch("sentry.integrations.github.client.get_jwt", return_value="jwt_token_1")