From 355910e182abe412b2a1b3ac46204dde69e1eb67 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 9 Sep 2019 15:50:10 -0400 Subject: [PATCH] Fixes #3489: Prevent exception triggered by webhook upon object deletion --- CHANGELOG.md | 4 ++++ netbox/extras/middleware.py | 34 ++++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4f700c78..0a035266d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ v2.6.4 (FUTURE) * [#3318](https://github.com/netbox-community/netbox/issues/3318) - Increase length of platform name and slug to 64 characters * [#3341](https://github.com/netbox-community/netbox/issues/3341) - Add Inline Vlan Editing +## Bug Fixes + +* [#3489](https://github.com/netbox-community/netbox/issues/3489) - Prevent exception triggered by webhook upon object deletion + v2.6.3 (2019-09-04) ## New Features diff --git a/netbox/extras/middleware.py b/netbox/extras/middleware.py index 35a6f96b0..5083af8ec 100644 --- a/netbox/extras/middleware.py +++ b/netbox/extras/middleware.py @@ -6,6 +6,7 @@ from datetime import timedelta from django.conf import settings from django.db.models.signals import post_delete, post_save from django.utils import timezone +from django.utils.functional import curry from django_prometheus.models import model_deletes, model_inserts, model_updates from .constants import ( @@ -18,10 +19,11 @@ from .webhooks import enqueue_webhooks _thread_locals = threading.local() -def cache_changed_object(sender, instance, **kwargs): +def handle_changed_object(sender, instance, **kwargs): """ - Cache an object being created or updated for the changelog. + Fires when an object is created or updated """ + # Queue the object and a new ObjectChange for processing once the request completes if hasattr(instance, 'to_objectchange'): action = OBJECTCHANGE_ACTION_CREATE if kwargs['created'] else OBJECTCHANGE_ACTION_UPDATE objectchange = instance.to_objectchange(action) @@ -30,15 +32,22 @@ def cache_changed_object(sender, instance, **kwargs): ) -def cache_deleted_object(sender, instance, **kwargs): +def _handle_deleted_object(request, sender, instance, **kwargs): """ - Cache an object being deleted for the changelog. + Fires when an object is deleted """ + # Record an Object Change if hasattr(instance, 'to_objectchange'): objectchange = instance.to_objectchange(OBJECTCHANGE_ACTION_DELETE) - _thread_locals.changed_objects.append( - (instance, objectchange) - ) + objectchange.user = request.user + objectchange.request_id = request.id + objectchange.save() + + # Enqueue webhooks + enqueue_webhooks(instance, request.user, request.id, OBJECTCHANGE_ACTION_DELETE) + + # Increment metric counters + model_deletes.labels(instance._meta.model_name).inc() def purge_objectchange_cache(sender, **kwargs): @@ -54,7 +63,7 @@ class ObjectChangeMiddleware(object): 1. Create an ObjectChange to reflect the modification to the object in the changelog. 2. Enqueue any relevant webhooks. - 3. Increment metric counter for the event type + 3. Increment the metric counter for the event type. The post_save and post_delete signals are employed to catch object modifications, however changes are recorded a bit differently for each. Objects being saved are cached into thread-local storage for action *after* the response has @@ -74,9 +83,12 @@ class ObjectChangeMiddleware(object): # the same request. request.id = uuid.uuid4() + # Signals don't include the request context, so we're currying it into the post_delete function ahead of time. + handle_deleted_object = curry(_handle_deleted_object, request) + # Connect our receivers to the post_save and post_delete signals. - post_save.connect(cache_changed_object, dispatch_uid='cache_changed_object') - post_delete.connect(cache_deleted_object, dispatch_uid='cache_deleted_object') + post_save.connect(handle_changed_object, dispatch_uid='cache_changed_object') + post_delete.connect(handle_deleted_object, dispatch_uid='cache_deleted_object') # Provide a hook for purging the change cache purge_changelog.connect(purge_objectchange_cache) @@ -104,8 +116,6 @@ class ObjectChangeMiddleware(object): model_inserts.labels(obj._meta.model_name).inc() elif objectchange.action == OBJECTCHANGE_ACTION_UPDATE: model_updates.labels(obj._meta.model_name).inc() - elif objectchange.action == OBJECTCHANGE_ACTION_DELETE: - model_deletes.labels(obj._meta.model_name).inc() # Housekeeping: 1% chance of clearing out expired ObjectChanges. This applies only to requests which result in # one or more changes being logged.