Merge pull request #22734 from netbox-community/22486-webhook

22486 - Add Configurable timeout for webhooks

docs/configuration/miscellaneous.md

@@ -306,3 +306,11 @@ The base unit for disk sizes. Set this to `1024` to use binary prefixes (MiB, Gi
 Default: `1000`
 
 The base unit for RAM sizes. Set this to `1024` to use binary prefixes (MiB, GiB, etc.) instead of decimal prefixes (MB, GB, etc.).
+
+---
+
+## WEBHOOK_DEFAULT_TIMEOUT
+
+Default: `30`
+
+The default maximum time (in seconds) to wait for a response when sending a webhook. This value is used for any webhook which does not define its own timeout. Setting a value lower than [`RQ_DEFAULT_TIMEOUT`](#rq_default_timeout) prevents an unresponsive receiver from holding a background worker open for the full duration of the job.

docs/models/extras/webhook.md

@@ -75,6 +75,10 @@ Controls whether validation of the receiver's SSL certificate is enforced when H
 
 The file path to a particular certificate authority (CA) file to use when validating the receiver's SSL certificate (if not using the system defaults).
 
+### Timeout
+
+The maximum time (in seconds) to wait for a response from the receiver before the request is considered failed. If left blank, the global [`WEBHOOK_DEFAULT_TIMEOUT`](../../configuration/miscellaneous.md#webhook_default_timeout) configuration value is used.
+
 ## Context Data
 
 The following context variables are available to the text and link templates.

netbox/extras/api/serializers_/events.py

@@ -48,6 +48,6 @@ class WebhookSerializer(OwnerMixin, NetBoxModelSerializer):
         fields = [
             'id', 'url', 'display_url', 'display', 'name', 'description', 'payload_url', 'http_method',
             'http_content_type', 'additional_headers', 'body_template', 'secret', 'ssl_verification', 'ca_file_path',
-            'custom_fields', 'owner', 'tags', 'created', 'last_updated',
+            'timeout', 'custom_fields', 'owner', 'tags', 'created', 'last_updated',
         ]
         brief_fields = ('id', 'url', 'display', 'name', 'description')

netbox/extras/filtersets.py

@@ -82,7 +82,7 @@ class WebhookFilterSet(OwnerFilterMixin, NetBoxModelFilterSet):
         model = Webhook
         fields = (
             'id', 'name', 'payload_url', 'http_method', 'http_content_type', 'secret', 'ssl_verification',
-            'ca_file_path', 'description',
+            'ca_file_path', 'timeout', 'description',
         )
 
     def search(self, queryset, name, value):

netbox/extras/forms/bulk_edit.py

@@ -279,8 +279,14 @@ class WebhookBulkEditForm(OwnerMixin, NetBoxModelBulkEditForm):
         required=False,
         label=_('CA file path')
     )
+    timeout = forms.IntegerField(
+        required=False,
+        min_value=1,
+        max_value=3600,
+        label=_('Timeout')
+    )
 
-    nullable_fields = ('secret', 'ca_file_path')
+    nullable_fields = ('secret', 'ca_file_path', 'timeout')
 
 
 class EventRuleBulkEditForm(OwnerMixin, NetBoxModelBulkEditForm):

netbox/extras/forms/bulk_import.py

@@ -254,7 +254,7 @@ class WebhookImportForm(OwnerCSVMixin, NetBoxModelImportForm):
         model = Webhook
         fields = (
             'name', 'payload_url', 'http_method', 'http_content_type', 'additional_headers', 'body_template',
-            'secret', 'ssl_verification', 'ca_file_path', 'description', 'owner', 'tags'
+            'secret', 'ssl_verification', 'ca_file_path', 'timeout', 'description', 'owner', 'tags'
         )
 
 

netbox/extras/forms/model_forms.py

@@ -584,7 +584,7 @@ class WebhookForm(OwnerMixin, NetBoxModelForm):
         FieldSet('name', 'description', 'tags', name=_('Webhook')),
         FieldSet(
             'payload_url', 'http_method', 'http_content_type', 'additional_headers', 'body_template', 'secret',
-            name=_('HTTP Request')
+            'timeout', name=_('HTTP Request')
         ),
         FieldSet('ssl_verification', 'ca_file_path', name=_('SSL')),
     )

netbox/extras/graphql/filters.py

@@ -396,6 +396,7 @@ class WebhookFilter(CustomFieldsFilterMixin, TagsFilterMixin, ChangeLoggedModelF
     secret: StrFilterLookup | None = strawberry_django.filter_field()
     ssl_verification: FilterLookup[bool] | None = strawberry_django.filter_field()
     ca_file_path: StrFilterLookup | None = strawberry_django.filter_field()
+    timeout: FilterLookup[int] | None = strawberry_django.filter_field()
     events: Annotated['EventRuleFilter', strawberry.lazy('extras.graphql.filters')] | None = (
         strawberry_django.filter_field()
     )

netbox/extras/migrations/0142_webhook_timeout.py

@@ -0,0 +1,24 @@
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("extras", "0141_custom_field_nulls_first"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="webhook",
+            name="timeout",
+            field=models.PositiveSmallIntegerField(
+                blank=True,
+                null=True,
+                validators=[
+                    django.core.validators.MinValueValidator(1),
+                    django.core.validators.MaxValueValidator(3600),
+                ],
+            ),
+        ),
+    ]

netbox/extras/models/models.py

@@ -5,7 +5,7 @@ from pathlib import Path
 from django.conf import settings
 from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
 from django.contrib.postgres.fields import ArrayField
-from django.core.validators import ValidationError
+from django.core.validators import MaxValueValidator, MinValueValidator, ValidationError
 from django.db import models
 from django.urls import reverse
 from django.utils import timezone
@@ -247,6 +247,19 @@ class Webhook(CustomFieldsMixin, ExportTemplatesMixin, TagsMixin, OwnerMixin, Ch
             "The specific CA certificate file to use for SSL verification. Leave blank to use the system defaults."
         )
     )
+    timeout = models.PositiveSmallIntegerField(
+        verbose_name=_('timeout'),
+        null=True,
+        blank=True,
+        validators=(
+            MinValueValidator(1),
+            MaxValueValidator(3600),
+        ),
+        help_text=_(
+            "The maximum time (in seconds) to wait for a response before failing the request. Leave blank to use "
+            "the system default (WEBHOOK_DEFAULT_TIMEOUT)."
+        )
+    )
     events = GenericRelation(
         EventRule,
         content_type_field='action_object_type',

netbox/extras/tables/tables.py

@@ -503,7 +503,7 @@ class WebhookTable(NetBoxTable):
         model = Webhook
         fields = (
             'pk', 'id', 'name', 'http_method', 'payload_url', 'http_content_type', 'secret', 'ssl_verification',
-            'ca_file_path', 'description', 'tags', 'created', 'last_updated',
+            'ca_file_path', 'timeout', 'description', 'tags', 'created', 'last_updated',
         )
         default_columns = (
             'pk', 'name', 'http_method', 'payload_url', 'description',

netbox/extras/tests/test_api.py

@@ -43,6 +43,7 @@ class WebhookTestCase(APIViewTestCases.APIViewTestCase):
         {
             'name': 'Webhook 4',
             'payload_url': 'http://example.com/?4',
+            'timeout': 15,
         },
         {
             'name': 'Webhook 5',

netbox/extras/tests/test_event_rules.py

@@ -427,6 +427,9 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
             self.assertEqual(request.headers['X-Hook-Signature'], signature)
             self.assertEqual(request.headers['X-Foo'], 'Bar')
 
+            # The webhook does not define its own timeout, so the global default should be used
+            self.assertEqual(kwargs['timeout'], settings.WEBHOOK_DEFAULT_TIMEOUT)
+
             # Validate the outgoing request body
             body = json.loads(request.body)
             self.assertEqual(body['event'], 'created')
@@ -465,6 +468,37 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
         with patch.object(Session, 'send', dummy_send):
             send_webhook(**job.kwargs)
 
+    def test_send_webhook_per_webhook_timeout(self):
+        """
+        A webhook which defines its own timeout should use that value in preference to the
+        global WEBHOOK_DEFAULT_TIMEOUT.
+        """
+        webhook = Webhook.objects.get(name='Webhook 1')
+        webhook.timeout = 5
+        webhook.save()
+
+        def dummy_send(_, request, **kwargs):
+            self.assertEqual(kwargs['timeout'], 5)
+            return HttpResponse()
+
+        request = RequestFactory().get(reverse('dcim:site_add'))
+        request.id = uuid.uuid4()
+        request.user = self.user
+
+        webhooks_queue = {}
+        site = Site.objects.create(name='Site 1', slug='site-1')
+        enqueue_event(
+            webhooks_queue,
+            instance=site,
+            request=request,
+            event_type=OBJECT_CREATED,
+        )
+        flush_events(list(webhooks_queue.values()))
+
+        job = self.queue.jobs[0]
+        with patch.object(Session, 'send', dummy_send):
+            send_webhook(**job.kwargs)
+
     def test_job_completed_webhook_without_request(self):
         """
         Ensure job_end event processing can enqueue a webhook even when the EventContext

netbox/extras/tests/test_filtersets.py

@@ -227,6 +227,7 @@ class WebhookTestCase(TestCase, BaseFilterSetTests):
                 payload_url='http://example.com/?1',
                 http_method='GET',
                 ssl_verification=True,
+                timeout=10,
                 description='foobar1'
             ),
             Webhook(
@@ -234,6 +235,7 @@ class WebhookTestCase(TestCase, BaseFilterSetTests):
                 payload_url='http://example.com/?2',
                 http_method='POST',
                 ssl_verification=True,
+                timeout=20,
                 description='foobar2'
             ),
             Webhook(
@@ -241,6 +243,7 @@ class WebhookTestCase(TestCase, BaseFilterSetTests):
                 payload_url='http://example.com/?3',
                 http_method='PATCH',
                 ssl_verification=False,
+                timeout=30,
                 description='foobar3'
             ),
             Webhook(
@@ -278,6 +281,10 @@ class WebhookTestCase(TestCase, BaseFilterSetTests):
         params = {'ssl_verification': True}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_timeout(self):
+        params = {'timeout': [10, 20]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
+
 
 class EventRuleTestCase(TestCase, BaseFilterSetTests):
     queryset = EventRule.objects.all()

netbox/extras/tests/test_views.py

@@ -626,14 +626,15 @@ class WebhookTestCase(ViewTestCases.PrimaryObjectViewTestCase):
             'payload_url': 'http://example.com/?x',
             'http_method': 'GET',
             'http_content_type': 'application/foo',
+            'timeout': 45,
             'description': 'My webhook',
         }
 
         cls.csv_data = (
-            "name,payload_url,http_method,http_content_type,description",
-            "Webhook 4,http://example.com/?4,GET,application/json,Foo",
-            "Webhook 5,http://example.com/?5,GET,application/json,Bar",
-            "Webhook 6,http://example.com/?6,GET,application/json,Baz",
+            "name,payload_url,http_method,http_content_type,timeout,description",
+            "Webhook 4,http://example.com/?4,GET,application/json,15,Foo",
+            "Webhook 5,http://example.com/?5,GET,application/json,,Bar",
+            "Webhook 6,http://example.com/?6,GET,application/json,,Baz",
         )
 
         cls.csv_update_data = (
@@ -645,6 +646,7 @@ class WebhookTestCase(ViewTestCases.PrimaryObjectViewTestCase):
 
         cls.bulk_edit_data = {
             'http_method': 'GET',
+            'timeout': 60,
         }
 
 

netbox/extras/ui/panels.py

@@ -340,6 +340,7 @@ class WebhookHTTPPanel(panels.ObjectAttributesPanel):
     payload_url = attrs.TextAttr('payload_url', label=_('Payload URL'), style='font-monospace')
     http_content_type = attrs.TextAttr('http_content_type', label=_('HTTP content type'))
     secret = attrs.TextAttr('secret')
+    timeout = attrs.TextAttr('timeout')
 
 
 class WebhookSSLPanel(panels.ObjectAttributesPanel):

netbox/extras/webhooks.py

@@ -3,6 +3,7 @@ import hmac
 import logging
 
 import requests
+from django.conf import settings
 from django_rq import job
 from jinja2.exceptions import TemplateError
 
@@ -112,13 +113,16 @@ def send_webhook(event_rule, object_type, event_type, data, timestamp, request=N
     if webhook.secret != '':
         prepared_request.headers['X-Hook-Signature'] = generate_signature(prepared_request.body, webhook.secret)
 
+    # Determine the request timeout, preferring the webhook-specific value over the global default
+    timeout = webhook.timeout if webhook.timeout is not None else settings.WEBHOOK_DEFAULT_TIMEOUT
+
     # Send the request
     with requests.Session() as session:
         session.verify = webhook.ssl_verification
         if webhook.ca_file_path:
             session.verify = webhook.ca_file_path
         proxies = resolve_proxies(url=url, context={'client': webhook})
-        response = session.send(prepared_request, proxies=proxies)
+        response = session.send(prepared_request, proxies=proxies, timeout=timeout)
 
     if 200 <= response.status_code <= 299:
         logger.info(f"Request succeeded; response status {response.status_code}")

netbox/netbox/configuration_example.py

@@ -223,6 +223,11 @@ RQ_DEFAULT_TIMEOUT = 300
 # this setting is derived from the installed location.
 # SCRIPTS_ROOT = '/path/to/netbox/scripts'
 
+# The default maximum time (in seconds) to wait for a response when sending a webhook, unless overridden on the
+# individual webhook. This prevents an unresponsive receiver from holding a background worker open for the full
+# RQ_DEFAULT_TIMEOUT duration.
+WEBHOOK_DEFAULT_TIMEOUT = 30
+
 # The name to use for the session cookie.
 SESSION_COOKIE_NAME = 'sessionid'
 

netbox/netbox/settings.py

@@ -220,6 +220,11 @@ STORAGE_CONFIG = getattr(configuration, 'STORAGE_CONFIG', None)
 STORAGES = getattr(configuration, 'STORAGES', {})
 TIME_ZONE = getattr(configuration, 'TIME_ZONE', 'UTC')
 TRANSLATION_ENABLED = getattr(configuration, 'TRANSLATION_ENABLED', True)
+WEBHOOK_DEFAULT_TIMEOUT = getattr(configuration, 'WEBHOOK_DEFAULT_TIMEOUT', 30)
+if not isinstance(WEBHOOK_DEFAULT_TIMEOUT, int) or not 1 <= WEBHOOK_DEFAULT_TIMEOUT <= 3600:
+    raise ImproperlyConfigured(
+        f"WEBHOOK_DEFAULT_TIMEOUT must be an integer between 1 and 3600 (found {WEBHOOK_DEFAULT_TIMEOUT!r})"
+    )
 DISK_BASE_UNIT = getattr(configuration, 'DISK_BASE_UNIT', 1000)
 if DISK_BASE_UNIT not in [1000, 1024]:
     raise ImproperlyConfigured(f"DISK_BASE_UNIT must be 1000 or 1024 (found {DISK_BASE_UNIT})")