Fixes #6776: Fix erroneous webhook dispatch on failure to save objects

docs/release-notes/version-2.11.md

@@ -14,6 +14,7 @@
 
 * [#5968](https://github.com/netbox-community/netbox/issues/5968) - Model forms should save empty custom field values as null
 * [#6686](https://github.com/netbox-community/netbox/issues/6686) - Force assignment of null custom field values to objects
+* [#6776](https://github.com/netbox-community/netbox/issues/6776) - Fix erroneous webhook dispatch on failure to save objects
 * [#6974](https://github.com/netbox-community/netbox/issues/6974) - Show contextual label for IP address role
 * [#7012](https://github.com/netbox-community/netbox/issues/7012) - Fix hidden "add components" dropdown on devices list
 

netbox/extras/context_managers.py

@@ -2,7 +2,7 @@ from contextlib import contextmanager
 
 from django.db.models.signals import m2m_changed, pre_delete, post_save
 
-from extras.signals import _handle_changed_object, _handle_deleted_object
+from extras.signals import clear_webhooks, _clear_webhook_queue, _handle_changed_object, _handle_deleted_object
 from utilities.utils import curry
 from .webhooks import flush_webhooks
 
@@ -20,11 +20,13 @@ def change_logging(request):
     # Curry signals receivers to pass the current request
     handle_changed_object = curry(_handle_changed_object, request, webhook_queue)
     handle_deleted_object = curry(_handle_deleted_object, request, webhook_queue)
+    clear_webhook_queue = curry(_clear_webhook_queue, webhook_queue)
 
     # Connect our receivers to the post_save and post_delete signals.
     post_save.connect(handle_changed_object, dispatch_uid='handle_changed_object')
     m2m_changed.connect(handle_changed_object, dispatch_uid='handle_changed_object')
     pre_delete.connect(handle_deleted_object, dispatch_uid='handle_deleted_object')
+    clear_webhooks.connect(clear_webhook_queue, dispatch_uid='clear_webhook_queue')
 
     yield
 
@@ -33,6 +35,7 @@ def change_logging(request):
     post_save.disconnect(handle_changed_object, dispatch_uid='handle_changed_object')
     m2m_changed.disconnect(handle_changed_object, dispatch_uid='handle_changed_object')
     pre_delete.disconnect(handle_deleted_object, dispatch_uid='handle_deleted_object')
+    clear_webhooks.disconnect(clear_webhook_queue, dispatch_uid='clear_webhook_queue')
 
     # Flush queued webhooks to RQ
     flush_webhooks(webhook_queue)

netbox/extras/signals.py

@@ -1,3 +1,4 @@
+import logging
 import random
 from datetime import timedelta
 
@@ -6,6 +7,7 @@ from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
 from django.db import DEFAULT_DB_ALIAS
 from django.db.models.signals import m2m_changed, post_save, pre_delete
+from django.dispatch import Signal
 from django.utils import timezone
 from django_prometheus.models import model_deletes, model_inserts, model_updates
 from prometheus_client import Counter
@@ -19,6 +21,10 @@ from .webhooks import enqueue_object, get_snapshots, serialize_for_webhook
 # Change logging/webhooks
 #
 
+# Define a custom signal that can be sent to clear any queued webhooks
+clear_webhooks = Signal()
+
+
 def _handle_changed_object(request, webhook_queue, sender, instance, **kwargs):
     """
     Fires when an object is created or updated.
@@ -104,6 +110,16 @@ def _handle_deleted_object(request, webhook_queue, sender, instance, **kwargs):
     model_deletes.labels(instance._meta.model_name).inc()
 
 
+def _clear_webhook_queue(webhook_queue, sender, **kwargs):
+    """
+    Delete any queued webhooks (e.g. because of an aborted bulk transaction)
+    """
+    logger = logging.getLogger('webhooks')
+    logger.info(f"Clearing {len(webhook_queue)} queued webhooks ({sender})")
+
+    webhook_queue.clear()
+
+
 #
 # Custom fields
 #

netbox/netbox/views/generic.py

@@ -17,6 +17,7 @@ from django.views.generic import View
 from django_tables2.export import TableExport
 
 from extras.models import CustomField, ExportTemplate
+from extras.signals import clear_webhooks
 from utilities.error_handlers import handle_protectederror
 from utilities.exceptions import AbortTransaction, PermissionsViolation
 from utilities.forms import (
@@ -325,6 +326,7 @@ class ObjectEditView(GetReturnURLMixin, ObjectPermissionRequiredMixin, View):
                 msg = "Object save failed due to object-level permissions violation"
                 logger.debug(msg)
                 form.add_error(None, msg)
+                clear_webhooks.send(sender=self)
 
         else:
             logger.debug("Form validation failed")
@@ -603,12 +605,13 @@ class ObjectImportView(GetReturnURLMixin, ObjectPermissionRequiredMixin, View):
                                 raise ObjectDoesNotExist
 
                 except AbortTransaction:
-                    pass
+                    clear_webhooks.send(sender=self)
 
                 except PermissionsViolation:
                     msg = "Object creation failed due to object-level permissions violation"
                     logger.debug(msg)
                     form.add_error(None, msg)
+                    clear_webhooks.send(sender=self)
 
             if not model_form.errors:
                 logger.info(f"Import object {obj} (PK: {obj.pk})")
@@ -751,12 +754,13 @@ class BulkImportView(GetReturnURLMixin, ObjectPermissionRequiredMixin, View):
                     })
 
             except ValidationError:
-                pass
+                clear_webhooks.send(sender=self)
 
             except PermissionsViolation:
                 msg = "Object import failed due to object-level permissions violation"
                 logger.debug(msg)
                 form.add_error(None, msg)
+                clear_webhooks.send(sender=self)
 
         else:
             logger.debug("Form validation failed")
@@ -879,11 +883,13 @@ class BulkEditView(GetReturnURLMixin, ObjectPermissionRequiredMixin, View):
 
                 except ValidationError as e:
                     messages.error(self.request, "{} failed validation: {}".format(obj, e))
+                    clear_webhooks.send(sender=self)
 
                 except PermissionsViolation:
                     msg = "Object update failed due to object-level permissions violation"
                     logger.debug(msg)
                     form.add_error(None, msg)
+                    clear_webhooks.send(sender=self)
 
             else:
                 logger.debug("Form validation failed")
@@ -987,6 +993,7 @@ class BulkRenameView(GetReturnURLMixin, ObjectPermissionRequiredMixin, View):
                     msg = "Object update failed due to object-level permissions violation"
                     logger.debug(msg)
                     form.add_error(None, msg)
+                    clear_webhooks.send(sender=self)
 
         else:
             form = self.form(initial={'pk': request.POST.getlist('pk')})
@@ -1183,6 +1190,7 @@ class ComponentCreateView(GetReturnURLMixin, ObjectPermissionRequiredMixin, View
                     msg = "Component creation failed due to object-level permissions violation"
                     logger.debug(msg)
                     form.add_error(None, msg)
+                    clear_webhooks.send(sender=self)
 
         return render(request, self.template_name, {
             'component_type': self.queryset.model._meta.verbose_name,
@@ -1264,12 +1272,13 @@ class BulkComponentCreateView(GetReturnURLMixin, ObjectPermissionRequiredMixin,
                             raise PermissionsViolation
 
                 except IntegrityError:
-                    pass
+                    clear_webhooks.send(sender=self)
 
                 except PermissionsViolation:
                     msg = "Component creation failed due to object-level permissions violation"
                     logger.debug(msg)
                     form.add_error(None, msg)
+                    clear_webhooks.send(sender=self)
 
                 if not form.errors:
                     msg = "Added {} {} to {} {}.".format(