Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(hybridcloud) Remove outbox based webhooks #66158

Merged
merged 2 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 14 additions & 34 deletions src/sentry/middleware/integrations/parsers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@
from django.urls import ResolverMatch, resolve
from rest_framework import status

from sentry import options
from sentry.hybridcloud.models.webhookpayload import WebhookPayload
from sentry.models.integrations import Integration
from sentry.models.integrations.organization_integration import OrganizationIntegration
from sentry.models.outbox import ControlOutbox, WebhookProviderIdentifier
from sentry.models.outbox import WebhookProviderIdentifier
from sentry.services.hybrid_cloud.integration.model import RpcIntegration
from sentry.services.hybrid_cloud.organization import RpcOrganizationSummary
from sentry.services.hybrid_cloud.organization_mapping import organization_mapping_service
Expand Down Expand Up @@ -145,24 +144,14 @@ def get_response_from_outbox_creation(
if len(regions) < 1:
return HttpResponse(status=status.HTTP_202_ACCEPTED)

# TODO(hybridcloud) Rename/remove this once webhookpayloads are stable.
shard_identifier = shard_identifier_override or self.webhook_identifier.value
rollout_rate = options.get("hybridcloud.webhookpayload.rollout")
if ((shard_identifier % 100000) / 100000) < rollout_rate:
for region in regions:
WebhookPayload.create_from_request(
region=region.name,
provider=self.provider,
identifier=shard_identifier,
request=self.request,
)
else:
for outbox in ControlOutbox.for_webhook_update(
shard_identifier=shard_identifier,
region_names=[region.name for region in regions],
for region in regions:
WebhookPayload.create_from_request(
region=region.name,
provider=self.provider,
identifier=shard_identifier,
request=self.request,
):
outbox.save()
)

return HttpResponse(status=status.HTTP_202_ACCEPTED)

Expand All @@ -177,23 +166,14 @@ def get_response_from_outbox_creation_for_integration(
return HttpResponse(status=status.HTTP_202_ACCEPTED)

identifier = integration.id
rollout_rate = options.get("hybridcloud.webhookpayload.rollout")
if ((identifier % 100000) / 100000) < rollout_rate:
for region in regions:
WebhookPayload.create_from_request(
region=region.name,
provider=self.provider,
identifier=identifier,
request=self.request,
integration_id=identifier,
)
else:
for outbox in ControlOutbox.for_webhook_update(
shard_identifier=integration.id,
for region in regions:
WebhookPayload.create_from_request(
region=region.name,
provider=self.provider,
identifier=identifier,
request=self.request,
region_names=[region.name for region in regions],
):
outbox.save()
integration_id=identifier,
)
return HttpResponse(status=status.HTTP_202_ACCEPTED)

def get_response_from_first_region(self):
Expand Down
5 changes: 0 additions & 5 deletions src/sentry/testutils/outbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
raise OutboxRecursionLimitError


def assert_no_webhook_outboxes():
outboxes = ControlOutbox.objects.filter(category=OutboxCategory.WEBHOOK_PROXY).count()
assert outboxes == 0, "No outboxes should be created"


def assert_webhook_outboxes_with_shard_id(
factory_request: WSGIRequest,
expected_shard_id: int,
Expand Down
28 changes: 11 additions & 17 deletions tests/sentry/middleware/integrations/parsers/test_base.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import dataclasses
from collections.abc import Iterable
from unittest.mock import MagicMock, patch

Expand All @@ -7,13 +6,9 @@
from pytest import raises
from rest_framework import status

from sentry.hybridcloud.models.webhookpayload import WebhookPayload
from sentry.middleware.integrations.parsers.base import BaseRequestParser
from sentry.models.outbox import (
ControlOutbox,
OutboxCategory,
OutboxScope,
WebhookProviderIdentifier,
)
from sentry.models.outbox import WebhookProviderIdentifier
from sentry.silo.base import SiloLimit, SiloMode
from sentry.testutils.cases import TestCase
from sentry.types.region import Region, RegionCategory
Expand Down Expand Up @@ -92,23 +87,22 @@ def test_get_responses_from_region_silos_with_complete_failure(self, mock__get_r
assert type(response_map[region.name].error) is SiloLimit.AvailabilityError

@override_settings(SILO_MODE=SiloMode.CONTROL)
def test_get_response_from_outbox_creation(self):
def test_get_response_from_webhookpayload_creation(self):
with pytest.raises(NotImplementedError):
self.parser.get_response_from_outbox_creation(regions=self.region_config)

class MockParser(BaseRequestParser):
webhook_identifier = WebhookProviderIdentifier.SLACK
provider = "slack"

parser = MockParser(self.request, self.response_handler)

response = parser.get_response_from_outbox_creation(regions=self.region_config)
assert response.status_code == status.HTTP_202_ACCEPTED
new_outboxes = ControlOutbox.objects.all()
assert len(new_outboxes) == 2
for outbox in new_outboxes:
assert outbox.region_name in ["us", "eu"]
assert outbox.category == OutboxCategory.WEBHOOK_PROXY
assert outbox.shard_scope == OutboxScope.WEBHOOK_SCOPE
assert outbox.shard_identifier == WebhookProviderIdentifier.SLACK.value
payload = outbox.get_webhook_payload_from_request(self.request)
assert outbox.payload == dataclasses.asdict(payload)
payloads = WebhookPayload.objects.all()
assert len(payloads) == 2
for payload in payloads:
assert payload.region_name in ["us", "eu"]
assert payload.mailbox_name == "slack:0"
assert payload.request_path
assert payload.request_method
39 changes: 6 additions & 33 deletions tests/sentry/middleware/integrations/parsers/test_bitbucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,7 @@
from sentry.models.organizationmapping import OrganizationMapping
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_outboxes,
assert_no_webhook_payloads,
assert_webhook_outboxes_with_shard_id,
assert_webhook_payloads_for_mailbox,
)
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
from sentry.types.region import Region, RegionCategory
Expand Down Expand Up @@ -63,7 +57,7 @@ def test_routing_endpoints(self):
assert isinstance(response, HttpResponse)
assert response.status_code == 200
assert response.content == b"passthrough"
assert_no_webhook_outboxes()
assert_no_webhook_payloads()

@override_settings(SILO_MODE=SiloMode.CONTROL)
@override_regions(region_config)
Expand All @@ -83,7 +77,7 @@ def test_routing_webhook_no_regions(self):
assert isinstance(response, HttpResponse)
assert response.status_code == 200
assert response.content == b"passthrough"
assert_no_webhook_outboxes()
assert_no_webhook_payloads()

@override_settings(SILO_MODE=SiloMode.CONTROL)
@override_regions(region_config)
Expand All @@ -99,29 +93,8 @@ def test_routing_webhook_with_regions(self):
assert isinstance(response, HttpResponse)
assert response.status_code == 202
assert response.content == b""
assert_webhook_outboxes_with_shard_id(
factory_request=request,
expected_shard_id=self.organization.id,
region_names=[self.region.name],
)

@override_settings(SILO_MODE=SiloMode.CONTROL)
@override_regions(region_config)
@override_options({"hybridcloud.webhookpayload.rollout": 1.0})
def test_webhook_outbox_creation_webhookpayload(self):
self.get_integration()
path = reverse(
"sentry-extensions-bitbucket-webhook", kwargs={"organization_id": self.organization.id}
)
request = self.factory.post(path)
assert_no_webhook_payloads()
parser = BitbucketRequestParser(request=request, response_handler=self.get_response)

response = parser.get_response()
assert isinstance(response, HttpResponse)
assert response.status_code == 202
assert response.content == b""

assert_webhook_payloads_for_mailbox(
mailbox_name=f"bitbucket:{self.organization.id}", region_names=["us"], request=request
request=request,
mailbox_name=f"bitbucket:{self.organization.id}",
region_names=[self.region.name],
)
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,7 @@
from sentry.models.organizationmapping import OrganizationMapping
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_outboxes_with_shard_id,
assert_webhook_payloads_for_mailbox,
outbox_runner,
)
from sentry.testutils.outbox import assert_webhook_payloads_for_mailbox, outbox_runner
from sentry.testutils.region import override_regions
from sentry.testutils.silo import control_silo_test
from sentry.types.region import Region, RegionCategory
Expand Down Expand Up @@ -61,33 +55,13 @@ def test_routing_webhook(self):
OrganizationMapping.objects.get(organization_id=self.organization.id).update(
region_name="us"
)
parser.get_response()
assert_webhook_outboxes_with_shard_id(
factory_request=request,
expected_shard_id=self.organization.id,
region_names=[self.region.name],
)

@override_settings(SILO_MODE=SiloMode.CONTROL)
@override_regions(region_config)
@override_options({"hybridcloud.webhookpayload.rollout": 1.0})
def test_webhook_outbox_creation_webhookpayload(self):
integration = self.get_integration()
region_route = reverse(
"sentry-extensions-bitbucketserver-webhook",
kwargs={"organization_id": self.organization.id, "integration_id": integration.id},
)
request = self.factory.post(region_route)
assert_no_webhook_payloads()
parser = BitbucketServerRequestParser(request=request, response_handler=self.get_response)

response = parser.get_response()
assert isinstance(response, HttpResponse)
assert response.status_code == 202
assert response.content == b""

assert_webhook_payloads_for_mailbox(
mailbox_name=f"bitbucket_server:{self.organization.id}",
region_names=["us"],
request=request,
mailbox_name=f"bitbucket_server:{self.organization.id}",
region_names=[self.region.name],
)
16 changes: 8 additions & 8 deletions tests/sentry/middleware/integrations/parsers/test_discord.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from sentry.models.outbox import ControlOutbox
from sentry.silo.base import SiloMode
from sentry.testutils.cases import APITestCase, TestCase
from sentry.testutils.outbox import assert_no_webhook_outboxes
from sentry.testutils.outbox import assert_no_webhook_payloads
from sentry.testutils.silo import control_silo_test, create_test_regions
from sentry.utils import json
from sentry.utils.signing import sign
Expand Down Expand Up @@ -63,7 +63,7 @@ def test_interactions_endpoint_routing_ping(self, mock_verify_signature):
data = json.loads(response.content)
assert data == {"type": 1}
assert len(responses.calls) == 0
assert_no_webhook_outboxes()
assert_no_webhook_payloads()

@responses.activate
@patch(
Expand All @@ -77,7 +77,7 @@ def test_interactions_endpoint_validation_failure(self, mock_verify_signature):
response = parser.get_response()
assert response.status_code == 401
assert not response.content
assert_no_webhook_outboxes()
assert_no_webhook_payloads()
assert len(responses.calls) == 0

@responses.activate
Expand All @@ -92,7 +92,7 @@ def test_interactions_endpoint_routing_ping_no_integration(self, mock_verify_sig
assert response.status_code == 200
data = json.loads(response.content)
assert data == {"type": 1}
assert_no_webhook_outboxes()
assert_no_webhook_payloads()
assert len(responses.calls) == 0

@responses.activate
Expand All @@ -119,7 +119,7 @@ def test_interactions_endpoint_routing_command(self, mock_verify_signature):
assert response.status_code == 202
assert response.content == b"region_response"
assert len(responses.calls) == 1
assert_no_webhook_outboxes()
assert_no_webhook_payloads()

@responses.activate
@patch("sentry.integrations.discord.requests.base.verify_signature", return_value=None)
Expand All @@ -135,7 +135,7 @@ def test_interactions_endpoint_routing_command_no_integration(self, mock_verify_
assert isinstance(response, HttpResponse)
assert response.status_code == 400
assert len(responses.calls) == 0
assert_no_webhook_outboxes()
assert_no_webhook_payloads()

@responses.activate
@patch("sentry.integrations.discord.requests.base.verify_signature", return_value=None)
Expand All @@ -160,7 +160,7 @@ def test_interactions_endpoint_routing_message_component(self, mock_verify_signa
assert response.status_code == 201
assert response.content == b"region_response"
assert len(responses.calls) == 1
assert_no_webhook_outboxes()
assert_no_webhook_payloads()

@responses.activate
def test_control_classes(self):
Expand All @@ -184,7 +184,7 @@ def test_control_classes(self):
assert response.status_code == 200
assert response.content == b"passthrough"
assert len(responses.calls) == 0
assert_no_webhook_outboxes()
assert_no_webhook_payloads()

@responses.activate
@patch("sentry.middleware.integrations.parsers.discord.convert_to_async_discord_response")
Expand Down
Loading
Loading