diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index fff64649f43de7..21defc011bc956 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -11,7 +11,7 @@ feedback: 0001_squashed_0004_index_together flags: 0001_squashed_0004_add_flag_audit_log_provider_column -hybridcloud: 0001_squashed_0021_django_arrayfield_scope_list +hybridcloud: 0022_webhook_payload_update insights: 0001_squashed_0001_add_starred_transactions_model diff --git a/src/sentry/codecov/__init__.py b/src/sentry/codecov/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/src/sentry/codecov/client.py b/src/sentry/codecov/client.py new file mode 100644 index 00000000000000..5f97606f0f6dba --- /dev/null +++ b/src/sentry/codecov/client.py @@ -0,0 +1,129 @@ +import datetime +import logging +from enum import StrEnum +from typing import TypeAlias + +import requests +from rest_framework import status + +from sentry import options +from sentry.api.exceptions import SentryAPIException +from sentry.utils import jwt + +GitProviderId: TypeAlias = str + + +class GitProvider(StrEnum): + """ + Enum representing the Git provider that hosts the user/org that a + `CodecovApiClient` instance is acting on behalf of. + + Codecov doesn't require this to be GitHub, but that's all that's implemented + for now. + """ + + GitHub = "github" + + +logger = logging.getLogger(__name__) + +TIMEOUT_SECONDS = 10 + + +class ConfigurationError(SentryAPIException): + status_code = status.HTTP_500_INTERNAL_SERVER_ERROR + code = "configuration-error" + + +class CodecovApiClient: + """ + Thin client for making JWT-authenticated requests to the Codecov API. + + For each request, Sentry creates and signs (HS256) a JWT with a key shared + with Codecov. This JWT contains information that Codecov needs to service + the request. + """ + + def _create_jwt(self): + now = int(datetime.datetime.now(datetime.UTC).timestamp()) + exp = now + 300 # 5 minutes + claims = { + "iss": "https://sentry.io", + "iat": now, + "exp": exp, + } + claims.update(self.custom_claims) + + return jwt.encode(claims, self.signing_secret, algorithm="HS256") + + def __init__( + self, + git_provider_user: GitProviderId | None, + git_provider: GitProvider = GitProvider.GitHub, + ): + """ + Creates a `CodecovApiClient`. + + :param git_provider_user: The ID of the current Sentry user's linked git + provider account, according to the git provider. + :param git_provider: The git provider that the above user's account is + hosted on. + """ + + if not (base_url := options.get("codecov.base-url")): + raise ConfigurationError() + + if not (signing_secret := options.get("codecov.api-bridge-signing-secret")): + raise ConfigurationError() + + self.base_url = base_url + self.signing_secret = signing_secret + self.custom_claims = { + "g_u": git_provider_user, + "g_p": git_provider, + } + + def get(self, endpoint: str, params=None, headers=None) -> requests.Response | None: + """ + Makes a GET request to the specified endpoint of the configured Codecov + API host with the provided params and headers. + + :param endpoint: The endpoint to request, without the host portion. For + examples: `/api/v2/gh/getsentry/users` or `/graphql` + :param params: Dictionary of query params. + :param headers: Dictionary of request headers. + """ + headers = headers or {} + token = self._create_jwt() + headers.update(jwt.authorization_header(token)) + + url = f"{self.base_url}{endpoint}" + try: + response = requests.get(url, params=params, headers=headers, timeout=TIMEOUT_SECONDS) + except Exception: + logger.exception("Error when making GET request") + return None + + return response + + def post(self, endpoint: str, data=None, headers=None) -> requests.Response | None: + """ + Makes a POST request to the specified endpoint of the configured Codecov + API host with the provided data and headers. + + :param endpoint: The endpoint to request, without the host portion. For + examples: `/api/v2/gh/getsentry/users` or `/graphql` + :param data: Dictionary of form data. + :param headers: Dictionary of request headers. + """ + headers = headers or {} + token = self._create_jwt() + headers.update(jwt.authorization_header(token)) + url = f"{self.base_url}{endpoint}" + try: + response = requests.post(url, data=data, headers=headers, timeout=TIMEOUT_SECONDS) + except Exception: + logger.exception("Error when making POST request") + return None + + return response diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 1ff339a0379aaf..8489ed233430d6 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -3381,7 +3381,6 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: SENTRY_REPLAYS_SERVICE_URL = "http://localhost:8090" - SENTRY_ISSUE_ALERT_HISTORY = "sentry.rules.history.backends.postgres.PostgresRuleHistoryBackend" SENTRY_ISSUE_ALERT_HISTORY_OPTIONS: dict[str, Any] = {} diff --git a/src/sentry/hybridcloud/migrations/0022_webhook_payload_update.py b/src/sentry/hybridcloud/migrations/0022_webhook_payload_update.py new file mode 100644 index 00000000000000..9e2bb8191f59b7 --- /dev/null +++ b/src/sentry/hybridcloud/migrations/0022_webhook_payload_update.py @@ -0,0 +1,38 @@ +# Generated by Django 5.2.1 on 2025-05-26 21:05 + +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("hybridcloud", "0001_squashed_0021_django_arrayfield_scope_list"), + ] + + operations = [ + migrations.AddField( + model_name="webhookpayload", + name="destination_type", + field=models.CharField(db_default="sentry_region"), + ), + migrations.AlterField( + model_name="webhookpayload", + name="region_name", + field=models.CharField(null=True), + ), + ] diff --git a/src/sentry/hybridcloud/models/webhookpayload.py b/src/sentry/hybridcloud/models/webhookpayload.py index 15767de3b64b47..ba7e6646a78f5f 100644 --- a/src/sentry/hybridcloud/models/webhookpayload.py +++ b/src/sentry/hybridcloud/models/webhookpayload.py @@ -4,7 +4,7 @@ from typing import Any, Self from django.db import models -from django.db.models import Case, ExpressionWrapper, F, IntegerField, Value, When +from django.db.models import Case, ExpressionWrapper, F, IntegerField, TextChoices, Value, When from django.http import HttpRequest from django.utils import timezone @@ -19,13 +19,25 @@ BACKOFF_RATE = 1.4 +class DestinationType(TextChoices): + SENTRY_REGION = "sentry_region" + CODECOV = "codecov" + + @control_silo_model class WebhookPayload(Model): __relocation_scope__ = RelocationScope.Excluded mailbox_name = models.CharField(null=False, blank=False) provider = models.CharField(null=True, blank=True) - region_name = models.CharField(null=False) + + # Destination attributes + # Table is constantly being deleted from so let's make this non-nullable with a default value, since the table should be small at any given point in time. + destination_type = models.CharField( + choices=DestinationType.choices, null=False, db_default=DestinationType.SENTRY_REGION + ) + region_name = models.CharField(null=True) + # May need to add organization_id in the future for debugging. integration_id = models.BigIntegerField(null=True) @@ -69,6 +81,7 @@ class Meta: __repr__ = sane_repr( "mailbox_name", + "destination_type", "region_name", "schedule_for", "attempts", @@ -93,7 +106,8 @@ def get_attributes_from_request( def create_from_request( cls, *, - region: str, + destination_type: DestinationType, + region: str | None, provider: str, identifier: int | str, request: HttpRequest, @@ -103,6 +117,7 @@ def create_from_request( return cls.objects.create( mailbox_name=f"{provider}:{identifier}", provider=provider, + destination_type=destination_type, region_name=region, integration_id=integration_id, **cls.get_attributes_from_request(request), diff --git a/src/sentry/hybridcloud/tasks/deliver_webhooks.py b/src/sentry/hybridcloud/tasks/deliver_webhooks.py index d2fc4e00dfde60..9ec65f367b28ec 100644 --- a/src/sentry/hybridcloud/tasks/deliver_webhooks.py +++ b/src/sentry/hybridcloud/tasks/deliver_webhooks.py @@ -11,8 +11,14 @@ from rest_framework import status from sentry import options +from sentry.codecov.client import CodecovApiClient, ConfigurationError from sentry.exceptions import RestrictedIPAddress -from sentry.hybridcloud.models.webhookpayload import BACKOFF_INTERVAL, MAX_ATTEMPTS, WebhookPayload +from sentry.hybridcloud.models.webhookpayload import ( + BACKOFF_INTERVAL, + MAX_ATTEMPTS, + DestinationType, + WebhookPayload, +) from sentry.shared_integrations.exceptions import ( ApiConflictError, ApiConnectionResetError, @@ -25,7 +31,7 @@ from sentry.tasks.base import instrumented_task from sentry.taskworker.config import TaskworkerConfig from sentry.taskworker.namespaces import hybridcloud_control_tasks -from sentry.types.region import get_region_by_name +from sentry.types.region import Region, get_region_by_name from sentry.utils import metrics logger = logging.getLogger(__name__) @@ -403,12 +409,25 @@ def deliver_message(payload: WebhookPayload) -> None: def perform_request(payload: WebhookPayload) -> None: + destination_type = payload.destination_type + + match destination_type: + case DestinationType.SENTRY_REGION: + assert payload.region_name is not None + region = get_region_by_name(name=payload.region_name) + perform_region_request(region, payload) + case DestinationType.CODECOV: + perform_codecov_request(payload) + case _: + raise ValueError(f"Unknown destination type: {destination_type!r}") + + +def perform_region_request(region: Region, payload: WebhookPayload) -> None: logging_context: dict[str, str | int] = { "payload_id": payload.id, "mailbox_name": payload.mailbox_name, "attempt": payload.attempts, } - region = get_region_by_name(name=payload.region_name) try: client = RegionSiloClient(region=region) @@ -521,3 +540,72 @@ def perform_request(payload: WebhookPayload) -> None: extra={"error": str(err), "response_code": response_code, **logging_context}, ) raise DeliveryFailed() from err + + +def perform_codecov_request(payload: WebhookPayload) -> None: + """ + We're dont retrying Codecov forwarding requests for now. We want to prove out that it would work. + """ + logging_context: dict[str, str | int] = { + "payload_id": payload.id, + "mailbox_name": payload.mailbox_name, + "attempt": payload.attempts, + "request_method": payload.request_method, + "request_path": payload.request_path, + } + + with metrics.timer( + "hybridcloud.deliver_webhooks.send_request_to_codecov", + ): + # transform request to match what codecov is expecting + if payload.request_path.strip("/") != "extensions/github/webhook": + metrics.incr( + "hybridcloud.deliver_webhooks.send_request_to_codecov.unexpected_path", + ) + return + + try: + endpoint = "/webhooks/sentry" + headers = orjson.loads(payload.request_headers) + data = { + "event": headers.get("HTTP_X_GITHUB_EVENT", "unknown"), + "payload": orjson.loads(payload.request_body), + } + client = CodecovApiClient(None) + response = client.post( + endpoint=endpoint, + data=data, + headers=headers, + ) + + if response is None: + metrics.incr( + "hybridcloud.deliver_webhooks.send_request_to_codecov.failure", + ) + return + + logger.debug( + "deliver_webhooks.send_request_to_codecov.success", + extra={ + "status": response.status_code, + **logging_context, + }, + ) + except ConfigurationError as err: + metrics.incr( + "hybridcloud.deliver_webhooks.send_request_to_codecov.codecov_configuration_error", + ) + logger.warning( + "deliver_webhooks.send_request_to_codecov.codecov_configuration_error", + extra={"error": str(err), **logging_context}, + ) + return + except orjson.JSONDecodeError as err: + metrics.incr( + "hybridcloud.deliver_webhooks.send_request_to_codecov.json_decode_error", + ) + logger.warning( + "deliver_webhooks.send_request_to_codecov.json_decode_error", + extra={"error": str(err), **logging_context}, + ) + return diff --git a/src/sentry/integrations/middleware/hybrid_cloud/parser.py b/src/sentry/integrations/middleware/hybrid_cloud/parser.py index 3b40945eee1972..740aa5722393d6 100644 --- a/src/sentry/integrations/middleware/hybrid_cloud/parser.py +++ b/src/sentry/integrations/middleware/hybrid_cloud/parser.py @@ -14,7 +14,7 @@ from sentry.api.base import ONE_DAY from sentry.constants import ObjectStatus -from sentry.hybridcloud.models.webhookpayload import WebhookPayload +from sentry.hybridcloud.models.webhookpayload import DestinationType, WebhookPayload from sentry.hybridcloud.outbox.category import WebhookProviderIdentifier from sentry.hybridcloud.services.organization_mapping import organization_mapping_service from sentry.integrations.middleware.metrics import ( @@ -178,6 +178,7 @@ def get_response_from_webhookpayload( shard_identifier = identifier or self.webhook_identifier.value for region in regions: WebhookPayload.create_from_request( + destination_type=DestinationType.SENTRY_REGION, region=region.name, provider=self.provider, identifier=shard_identifier, @@ -337,3 +338,24 @@ def get_regions_from_organizations( def get_default_missing_integration_response(self) -> HttpResponse: return HttpResponse(status=status.HTTP_400_BAD_REQUEST) + + # Forwarding Helpers + + def forward_to_codecov( + self, + identifier: int | str | None = None, + integration_id: int | None = None, + ): + shard_identifier = f"codecov:{identifier or self.webhook_identifier.value}" + + # create webhookpayloads for each service + WebhookPayload.create_from_request( + destination_type=DestinationType.CODECOV, + region=None, + provider=self.provider, + identifier=shard_identifier, + integration_id=integration_id, + request=self.request, + ) + + return HttpResponse(status=status.HTTP_202_ACCEPTED) diff --git a/src/sentry/middleware/integrations/parsers/github.py b/src/sentry/middleware/integrations/parsers/github.py index b7db6ecf1d2d19..f528a6d13ddec7 100644 --- a/src/sentry/middleware/integrations/parsers/github.py +++ b/src/sentry/middleware/integrations/parsers/github.py @@ -7,6 +7,7 @@ import orjson from django.http import HttpResponse +from sentry import options from sentry.hybridcloud.outbox.category import WebhookProviderIdentifier from sentry.integrations.github.webhook import ( GitHubIntegrationsWebhookEndpoint, @@ -53,6 +54,11 @@ def get_response(self): return HttpResponse(status=400) if event.get("installation") and event.get("action") in {"created", "deleted"}: + if options.get("codecov.base-url"): # check if codecov is enabled + self.forward_to_codecov( + identifier="installation", + integration_id=None, + ) return self.get_response_from_control_silo() try: @@ -67,6 +73,12 @@ def get_response(self): if len(regions) == 0: return self.get_default_missing_integration_response() + if options.get("codecov.base-url"): # check if codecov is enabled + self.forward_to_codecov( + identifier=integration.id, + integration_id=integration.id, + ) + return self.get_response_from_webhookpayload( regions=regions, identifier=integration.id, integration_id=integration.id ) diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index 21954cd56ec2b4..550f18ecf4bcae 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -602,6 +602,8 @@ # Codecov Integration register("codecov.client-secret", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK) +register("codecov.base-url", default=None) +register("codecov.api-bridge-signing-secret", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK) # GitHub Integration register("github-app.id", default=0, flags=FLAG_AUTOMATOR_MODIFIABLE) diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index 78c9f1633eebaa..46d50321c18977 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -1983,7 +1983,9 @@ def create_request_access( @staticmethod @assume_test_silo_mode(SiloMode.CONTROL) - def create_webhook_payload(mailbox_name: str, region_name: str, **kwargs) -> WebhookPayload: + def create_webhook_payload( + mailbox_name: str, region_name: str | None, **kwargs + ) -> WebhookPayload: payload_kwargs = { "request_method": "POST", "request_path": "/extensions/github/webhook/", diff --git a/src/sentry/testutils/outbox.py b/src/sentry/testutils/outbox.py index c4a289bc5c25d2..7f04c4f3230666 100644 --- a/src/sentry/testutils/outbox.py +++ b/src/sentry/testutils/outbox.py @@ -8,7 +8,7 @@ from django.core.handlers.wsgi import WSGIRequest from sentry.hybridcloud.models.outbox import OutboxBase -from sentry.hybridcloud.models.webhookpayload import THE_PAST, WebhookPayload +from sentry.hybridcloud.models.webhookpayload import THE_PAST, DestinationType, WebhookPayload from sentry.hybridcloud.tasks.deliver_from_outbox import ( enqueue_outbox_jobs, enqueue_outbox_jobs_control, @@ -65,6 +65,7 @@ def assert_webhook_payloads_for_mailbox( request: WSGIRequest, mailbox_name: str, region_names: list[str], + destination_types: dict[DestinationType, int] | None = None, ): """ A test method for asserting that a webhook payload is properly queued for @@ -72,12 +73,13 @@ def assert_webhook_payloads_for_mailbox( :param request: :param mailbox_name: The mailbox name that messages should be found in. - :param region_names: The regions each messages should be queued for + :param region_names: Optional list of regions each messages should be queued for + :param destination_type: The destination type of the messages """ expected_payload = WebhookPayload.get_attributes_from_request(request=request) region_names_set = set(region_names) messages = WebhookPayload.objects.filter(mailbox_name=mailbox_name) - message_count = messages.count() + message_count = messages.filter(region_name__isnull=False).count() if message_count != len(region_names_set): raise Exception( f"Mismatch: Found {message_count} WebhookPayload but {len(region_names_set)} region_names" @@ -89,11 +91,25 @@ def assert_webhook_payloads_for_mailbox( assert message.request_body == expected_payload["request_body"] assert message.schedule_for == THE_PAST assert message.attempts == 0 - try: - region_names_set.remove(message.region_name) - except KeyError: - raise Exception( - f"Found ControlOutbox for '{message.region_name}', which was not in region_names: {str(region_names_set)}" - ) + + if destination_types: + destination_type = DestinationType(message.destination_type) + assert destination_type in destination_types + destination_types[destination_type] -= 1 + if destination_types[destination_type] == 0: + del destination_types[destination_type] + + if message.region_name: + try: + region_names_set.remove(message.region_name) + except KeyError: + raise Exception( + f"Found ControlOutbox for '{message.region_name}', which was not in region_names: {str(region_names_set)}" + ) if len(region_names_set) != 0: raise Exception(f"WebhookPayload not found for some region_names: {str(region_names_set)}") + + if destination_types and len(destination_types) != 0: + raise Exception( + f"WebhookPayload not found for some destination_types: {str(destination_types)}" + ) diff --git a/tests/sentry/codecov/test_client.py b/tests/sentry/codecov/test_client.py new file mode 100644 index 00000000000000..185cb1e236b16b --- /dev/null +++ b/tests/sentry/codecov/test_client.py @@ -0,0 +1,129 @@ +import datetime +from unittest.mock import patch + +import pytest + +from sentry.codecov.client import CodecovApiClient, ConfigurationError, GitProvider +from sentry.testutils.cases import TestCase +from sentry.testutils.silo import all_silo_test +from sentry.utils import jwt + + +@all_silo_test +class TestCodecovApiClient(TestCase): + def setUp(self): + self.test_git_provider_user = "12345" + self.test_base_url = "http://example.com" + self.test_secret = "test-secret-" + "a" * 20 + + self.test_timestamp = datetime.datetime.now(datetime.UTC) + self._mock_now = patch("datetime.datetime.now", return_value=self.test_timestamp) + + with self.options( + { + "codecov.base-url": self.test_base_url, + "codecov.api-bridge-signing-secret": self.test_secret, + } + ): + self.codecov_client = CodecovApiClient(self.test_git_provider_user) + + def test_raises_configuration_error_without_base_url(self): + with self.options( + { + "codecov.base-url": None, + "codecov.api-bridge-signing-secret": self.test_secret, + } + ): + with pytest.raises(ConfigurationError): + CodecovApiClient(self.test_git_provider_user) + + def test_raises_configuration_error_without_signing_secret(self): + with self.options( + { + "codecov.base-url": self.test_base_url, + "codecov.api-bridge-signing-secret": None, + } + ): + with pytest.raises(ConfigurationError): + CodecovApiClient(self.test_git_provider_user) + + def test_creates_valid_jwt(self): + encoded_jwt = self.codecov_client._create_jwt() + + header = jwt.peek_header(encoded_jwt) + assert header == { + "typ": "JWT", + "alg": "HS256", + } + + # Ensure the claims are what we expect, separate from verifying the + # signature and standard claims + claims = jwt.peek_claims(encoded_jwt) + expected_iat = int(self.test_timestamp.timestamp()) + expected_exp = expected_iat + 300 + assert claims == { + "g_u": self.test_git_provider_user, + "g_p": GitProvider.GitHub.value, + "iss": "https://sentry.io", + "iat": expected_iat, + "exp": expected_exp, + } + + # Ensure we can verify the signature and whatall + jwt.decode(encoded_jwt, self.test_secret) + + @patch("requests.get") + def test_sends_get_request_with_jwt_auth_header(self, mock_get): + with patch.object(self.codecov_client, "_create_jwt", return_value="test"): + self.codecov_client.get( + "/example/endpoint", {"example-param": "foo"}, {"X_TEST_HEADER": "bar"} + ) + mock_get.assert_called_once_with( + "http://example.com/example/endpoint", + params={"example-param": "foo"}, + headers={ + "Authorization": "Bearer test", + "X_TEST_HEADER": "bar", + }, + timeout=10, + ) + + @patch("requests.post") + def test_sends_post_request_with_jwt_auth_header(self, mock_post): + with patch.object(self.codecov_client, "_create_jwt", return_value="test"): + self.codecov_client.post( + "/example/endpoint", {"example-param": "foo"}, {"X_TEST_HEADER": "bar"} + ) + mock_post.assert_called_once_with( + "http://example.com/example/endpoint", + data={"example-param": "foo"}, + headers={ + "Authorization": "Bearer test", + "X_TEST_HEADER": "bar", + }, + timeout=10, + ) + + @patch("requests.post") + def test_sends_post_request_with_jwt_auth_header_empty_git_provider_user(self, mock_post): + with self.options( + { + "codecov.base-url": self.test_base_url, + "codecov.api-bridge-signing-secret": self.test_secret, + } + ): + self.codecov_client = CodecovApiClient(None) + + with patch.object(self.codecov_client, "_create_jwt", return_value="test"): + self.codecov_client.post( + "/example/endpoint", {"example-param": "foo"}, {"X_TEST_HEADER": "bar"} + ) + mock_post.assert_called_once_with( + "http://example.com/example/endpoint", + data={"example-param": "foo"}, + headers={ + "Authorization": "Bearer test", + "X_TEST_HEADER": "bar", + }, + timeout=10, + ) diff --git a/tests/sentry/hybridcloud/models/test_webhookpayload.py b/tests/sentry/hybridcloud/models/test_webhookpayload.py index 0d746e981dad4b..765932ec525ec4 100644 --- a/tests/sentry/hybridcloud/models/test_webhookpayload.py +++ b/tests/sentry/hybridcloud/models/test_webhookpayload.py @@ -4,7 +4,12 @@ from django.utils import timezone from sentry.hybridcloud.models import WebhookPayload -from sentry.hybridcloud.models.webhookpayload import BACKOFF_INTERVAL, BACKOFF_RATE, MAX_ATTEMPTS +from sentry.hybridcloud.models.webhookpayload import ( + BACKOFF_INTERVAL, + BACKOFF_RATE, + MAX_ATTEMPTS, + DestinationType, +) from sentry.testutils.cases import TestCase from sentry.testutils.silo import control_silo_test @@ -19,6 +24,7 @@ def test_create_from_request(self) -> None: content_type="application/json", ) hook = WebhookPayload.create_from_request( + destination_type=DestinationType.SENTRY_REGION, region="us", provider="github", identifier=123, @@ -35,6 +41,31 @@ def test_create_from_request(self) -> None: ) assert hook.request_body == '{"installation": {"id": "github:1"}}' + def test_create_from_request_for_codeocv(self) -> None: + factory = RequestFactory() + request = factory.post( + "/extensions/github/webhook/", + data={"installation": {"id": "github:1"}}, + content_type="application/json", + ) + hook = WebhookPayload.create_from_request( + destination_type=DestinationType.CODECOV, + region=None, + provider="github", + identifier="installation", + request=request, + integration_id=None, + ) + assert hook.mailbox_name == "github:installation" + assert hook.provider == "github" + assert hook.request_method == request.method + assert hook.request_path == request.get_full_path() + assert ( + hook.request_headers + == '{"Cookie":"","Content-Length":"36","Content-Type":"application/json"}' + ) + assert hook.request_body == '{"installation": {"id": "github:1"}}' + def test_schedule_next_attempt_moves_forward(self) -> None: hook = self.create_webhook_payload("jira:123", "us") start = timezone.now() diff --git a/tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py b/tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py index 04b84d587edbb0..b329bc0eba89be 100644 --- a/tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py +++ b/tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py @@ -7,7 +7,7 @@ from requests.exceptions import ConnectionError, ReadTimeout from sentry import options -from sentry.hybridcloud.models.webhookpayload import MAX_ATTEMPTS, WebhookPayload +from sentry.hybridcloud.models.webhookpayload import MAX_ATTEMPTS, DestinationType, WebhookPayload from sentry.hybridcloud.tasks import deliver_webhooks from sentry.hybridcloud.tasks.deliver_webhooks import ( MAX_MAILBOX_DRAIN, @@ -17,6 +17,7 @@ ) from sentry.testutils.cases import TestCase from sentry.testutils.factories import Factories +from sentry.testutils.helpers.options import override_options from sentry.testutils.region import override_regions from sentry.testutils.silo import control_silo_test from sentry.types.region import Region, RegionCategory, RegionResolutionError @@ -266,6 +267,20 @@ def create_payloads(num: int, mailbox: str) -> list[WebhookPayload]: return created +def create_payloads_with_destination_type( + num: int, mailbox: str, destination_type: DestinationType +) -> list[WebhookPayload]: + created = [] + for _ in range(0, num): + hook = Factories.create_webhook_payload( + mailbox_name=mailbox, + region_name=None, + destination_type=destination_type, + ) + created.append(hook) + return created + + @control_silo_test class DrainMailboxTest(TestCase): @responses.activate @@ -330,6 +345,76 @@ def test_drain_success(self) -> None: # Mailbox should be empty assert not WebhookPayload.objects.filter().exists() + @responses.activate + @override_options( + { + "codecov.base-url": "https://codecov.io", + "codecov.api-bridge-signing-secret": "test", + } + ) + @override_regions(region_config) + def test_drain_success_codecov(self) -> None: + responses.add( + responses.POST, + "https://codecov.io/extensions/github/webhook/", + status=200, + body="", + ) + + records = create_payloads_with_destination_type( + 3, "github:codecov:123", DestinationType.CODECOV + ) + drain_mailbox(records[0].id) + + # Mailbox should be empty + assert not WebhookPayload.objects.filter().exists() + + @responses.activate + @override_regions(region_config) + def test_drain_codecov_configuration_error(self) -> None: + responses.add( + responses.POST, + "https://codecov.io/extensions/github/webhook/", + status=200, + body="", + ) + + records = create_payloads_with_destination_type( + 3, "github:codecov:123", DestinationType.CODECOV + ) + drain_mailbox(records[0].id) + + # We don't retry codecov requests no matter what + hook = WebhookPayload.objects.filter().first() + assert hook is None + assert len(responses.calls) == 0 + + @responses.activate + @override_options( + { + "codecov.base-url": "https://codecov.io", + "codecov.api-bridge-signing-secret": "test", + } + ) + @override_regions(region_config) + def test_drain_codecov_request_error(self) -> None: + responses.add( + responses.POST, + "https://codecov.io/webhooks/sentry", + status=400, + body="", + ) + + records = create_payloads_with_destination_type( + 3, "github:codecov:123", DestinationType.CODECOV + ) + drain_mailbox(records[0].id) + + # We don't retry codecov requests no matter what + hook = WebhookPayload.objects.filter().first() + assert hook is None + assert len(responses.calls) == 3 + @responses.activate @override_regions(region_config) def test_drain_time_limit(self) -> None: diff --git a/tests/sentry/integrations/middleware/hybrid_cloud/test_base.py b/tests/sentry/integrations/middleware/hybrid_cloud/test_base.py index 818faf5f296a8d..0e587cf9219000 100644 --- a/tests/sentry/integrations/middleware/hybrid_cloud/test_base.py +++ b/tests/sentry/integrations/middleware/hybrid_cloud/test_base.py @@ -7,7 +7,7 @@ from rest_framework import status from sentry.constants import ObjectStatus -from sentry.hybridcloud.models.webhookpayload import WebhookPayload +from sentry.hybridcloud.models.webhookpayload import DestinationType, WebhookPayload from sentry.hybridcloud.outbox.category import WebhookProviderIdentifier from sentry.integrations.middleware.hybrid_cloud.parser import BaseRequestParser from sentry.integrations.middleware.metrics import MiddlewareHaltReason @@ -118,6 +118,25 @@ class MockParser(BaseRequestParser): assert payload.mailbox_name == "slack:0" assert payload.request_path assert payload.request_method + assert payload.destination_type == DestinationType.SENTRY_REGION + + @override_settings(SILO_MODE=SiloMode.CONTROL) + def test_forward_to_codecov(self): + class MockParser(BaseRequestParser): + webhook_identifier = WebhookProviderIdentifier.GITHUB + provider = "github" + + parser = MockParser(self.request, self.response_handler) + + parser.forward_to_codecov() + payloads = WebhookPayload.objects.all() + assert len(payloads) == 1 + for payload in payloads: + assert payload.region_name is None + assert payload.mailbox_name == "github:codecov:1" + assert payload.request_path + assert payload.request_method + assert payload.destination_type == DestinationType.CODECOV @override_settings(SILO_MODE=SiloMode.CONTROL) def test_get_organizations_from_integration_success(self): diff --git a/tests/sentry/middleware/integrations/parsers/test_github.py b/tests/sentry/middleware/integrations/parsers/test_github.py index a24844b0986c28..5c6d261b8a4fac 100644 --- a/tests/sentry/middleware/integrations/parsers/test_github.py +++ b/tests/sentry/middleware/integrations/parsers/test_github.py @@ -6,11 +6,13 @@ from rest_framework import status from sentry.hybridcloud.models.outbox import outbox_context +from sentry.hybridcloud.models.webhookpayload import DestinationType from sentry.integrations.models.integration import Integration from sentry.integrations.models.organization_integration import OrganizationIntegration from sentry.middleware.integrations.parsers.github import GithubRequestParser from sentry.silo.base import SiloMode from sentry.testutils.cases import TestCase +from sentry.testutils.helpers.options import override_options from sentry.testutils.outbox import assert_no_webhook_payloads, assert_webhook_payloads_for_mailbox from sentry.testutils.region import override_regions from sentry.testutils.silo import control_silo_test @@ -135,6 +137,35 @@ def test_webhook_outbox_creation(self): region_names=[region.name], ) + @override_settings(SILO_MODE=SiloMode.CONTROL) + @override_options({"codecov.base-url": "https://api.codecov.io"}) + @override_regions(region_config) + def test_webhook_for_codecov(self): + integration = self.get_integration() + request = self.factory.post( + self.path, + data={"installation": {"id": "github:1"}}, + content_type="application/json", + ) + parser = GithubRequestParser(request=request, response_handler=self.get_response) + + response = parser.get_response() + assert isinstance(response, HttpResponse) + assert response.status_code == status.HTTP_202_ACCEPTED + assert response.content == b"" + assert_webhook_payloads_for_mailbox( + request=request, + mailbox_name=f"github:{integration.id}", + region_names=[region.name], + destination_types={DestinationType.SENTRY_REGION: 1}, + ) + assert_webhook_payloads_for_mailbox( + request=request, + mailbox_name=f"github:codecov:{integration.id}", + region_names=[], + destination_types={DestinationType.CODECOV: 1}, + ) + @override_settings(SILO_MODE=SiloMode.CONTROL) @override_regions(region_config) @responses.activate