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

fix(webhooks): prevent unnecessary organisation webhook tasks #3365

Merged
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
24 changes: 16 additions & 8 deletions api/audit/signals.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging

from django.conf import settings
from django.db.models.signals import post_save
from django.dispatch import receiver

Expand All @@ -9,28 +10,35 @@
from integrations.dynatrace.dynatrace import DynatraceWrapper
from integrations.new_relic.new_relic import NewRelicWrapper
from integrations.slack.slack import SlackWrapper
from organisations.models import OrganisationWebhook
from webhooks.webhooks import WebhookEventType, call_organisation_webhooks

logger = logging.getLogger(__name__)


@receiver(post_save, sender=AuditLog)
def call_webhooks(sender, instance, **kwargs):
data = AuditLogListSerializer(instance=instance).data
if settings.DISABLE_WEBHOOKS:
return

if not (instance.project or instance.environment):
if not (instance.project_id or instance.environment_id):
logger.warning("Audit log without project or environment. Not sending webhook.")
return

organisation = (
instance.project.organisation
organisation_id = (
instance.project.organisation_id
if instance.project
else instance.environment.project.organisation
)
call_organisation_webhooks.delay(
args=(organisation.id, data, WebhookEventType.AUDIT_LOG_CREATED.value)
else instance.environment.project.organisation_id
)

if OrganisationWebhook.objects.filter(
organisation_id=organisation_id, enabled=True
).exists():
data = AuditLogListSerializer(instance=instance).data
call_organisation_webhooks.delay(
args=(organisation_id, data, WebhookEventType.AUDIT_LOG_CREATED.value)
)


def _get_integration_config(instance, integration_name):
if hasattr(instance.project, integration_name):
Expand Down
15 changes: 14 additions & 1 deletion api/tests/unit/audit/test_unit_audit_models.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@
from pytest_mock import MockerFixture

from audit.models import AuditLog
from audit.related_object_type import RelatedObjectType
from audit.serializers import AuditLogListSerializer
from integrations.datadog.models import DataDogConfiguration
from organisations.models import Organisation, OrganisationWebhook
from projects.models import Project
from webhooks.webhooks import WebhookEventType


def test_organisation_webhooks_are_called_when_audit_log_saved(project, mocker):
def test_organisation_webhooks_are_called_when_audit_log_saved(
project: Project, mocker: MockerFixture, organisation: Organisation
) -> None:
# Given
mock_call_webhooks = mocker.patch("audit.signals.call_organisation_webhooks")

audit_log = AuditLog(project=project, log="Some audit log")

OrganisationWebhook.objects.create(
organisation=organisation,
url="http://example.com/webhook",
enabled=True,
name="example webhook",
)

# When
audit_log.save()

Expand Down
83 changes: 83 additions & 0 deletions api/tests/unit/audit/test_unit_audit_signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
from pytest_django.fixtures import SettingsWrapper
from pytest_mock import MockerFixture

from audit.models import AuditLog
from audit.signals import call_webhooks
from organisations.models import Organisation, OrganisationWebhook
from projects.models import Project
from webhooks.webhooks import WebhookEventType


def test_call_webhooks_does_not_create_task_if_webhooks_disabled(
organisation: Organisation,
project: Project,
settings: SettingsWrapper,
mocker: MockerFixture,
) -> None:
# Given
audit_log = AuditLog(project=project)
settings.DISABLE_WEBHOOKS = True

mocked_call_organisation_webhooks = mocker.patch(
"audit.signals.call_organisation_webhooks"
)

# When
call_webhooks(sender=AuditLog, instance=audit_log)

# Then
mocked_call_organisation_webhooks.delay.assert_not_called()


def test_call_webhooks_does_not_create_task_if_organisation_has_no_webhooks(
organisation: Organisation,
project: Project,
settings: SettingsWrapper,
mocker: MockerFixture,
) -> None:
# Given
audit_log = AuditLog(project=project)
settings.DISABLE_WEBHOOKS = False

assert organisation.webhooks.count() == 0

mocked_call_organisation_webhooks = mocker.patch(
"audit.signals.call_organisation_webhooks"
)

# When
call_webhooks(sender=AuditLog, instance=audit_log)

# Then
mocked_call_organisation_webhooks.delay.assert_not_called()


def test_call_webhooks_creates_task_if_organisation_has_webhooks(
organisation: Organisation,
project: Project,
settings: SettingsWrapper,
mocker: MockerFixture,
) -> None:
# Given
audit_log = AuditLog.objects.create(project=project)
OrganisationWebhook.objects.create(
organisation=organisation,
name="Example webhook",
url="http://example.com/webhook",
enabled=True,
)
settings.DISABLE_WEBHOOKS = False

mocked_call_organisation_webhooks = mocker.patch(
"audit.signals.call_organisation_webhooks"
)

# When
call_webhooks(sender=AuditLog, instance=audit_log)

# Then
mocked_call_organisation_webhooks.delay.assert_called_once()
mock_call = mocked_call_organisation_webhooks.delay.call_args.kwargs
assert mock_call["args"][0] == organisation.id
assert mock_call["args"][1]["id"] == audit_log.id
assert mock_call["args"][2] == WebhookEventType.AUDIT_LOG_CREATED.name
Loading