Explorar el Código

Closes #23463: Include ObjectChange IDs in event data and webhook payloads (#23497)

Jeremy Stretch hace 1 día
padre
commit
68d403c498

+ 5 - 1
docs/integrations/webhooks.md

@@ -35,6 +35,7 @@ The following data is available as context for Jinja2 templates:
     * `request.user` - The name of the authenticated user who made the request (if available)
 * `data` - A detailed representation of the object in its current state. This is typically equivalent to the model's representation in NetBox's REST API.
 * `snapshots` - Minimal "snapshots" of the object state both before and after the change was made; provided as a dictionary with keys named `prechange` and `postchange`. These are not as extensive as the fully serialized representation, but contain enough information to convey what has changed.
+* `object_change_ids` - The IDs of the [change log](../features/change-logging.md) records associated with the event (if any), in the order they were recorded. Multiple changes made to an object within a single request produce a single event, which may reference several change records.
 
 ### Sanitizing Header Values
 
@@ -87,7 +88,10 @@ If no body template is specified, the request body will be populated with a JSON
             "status": "active",
             ...
         }
-    }
+    },
+    "object_change_ids": [
+        52
+    ]
 }
 ```
 

+ 9 - 8
docs/models/extras/webhook.md

@@ -102,14 +102,15 @@ When a request does time out, the failure is recorded by the `netbox.webhooks` l
 
 The following context variables are available to the text and link templates.
 
-| Variable      | Description                                          |
-|---------------|------------------------------------------------------|
-| `event`       | The event type (`create`, `update`, or `delete`)     |
-| `timestamp`   | The time at which the event occurred                 |
-| `object_type` | The type of object impacted (`app_label.model_name`) |
-| `data`        | A complete serialized representation of the object   |
-| `snapshots`   | Pre- and post-change snapshots of the object         |
-| `request`     | Data about the triggering request (if available)     |
+| Variable            | Description                                                 |
+|---------------------|-------------------------------------------------------------|
+| `event`             | The event type (`create`, `update`, or `delete`)            |
+| `timestamp`         | The time at which the event occurred                        |
+| `object_type`       | The type of object impacted (`app_label.model_name`)        |
+| `data`              | A complete serialized representation of the object          |
+| `snapshots`         | Pre- and post-change snapshots of the object                |
+| `object_change_ids` | IDs of the change log records for the event (if available)  |
+| `request`           | Data about the triggering request (if available)            |
 
 !!! note
     The `request` variable is populated in the context only when the webhook is associated with a triggering request. It exposes `request.id` (the unique request ID) and `request.user` (the name of the user associated with the change), among other attributes.

+ 7 - 2
netbox/core/signals.py

@@ -135,6 +135,7 @@ def handle_changed_object(sender, instance, **kwargs):
         OBJECT_DELETED: ObjectChangeActionChoices.ACTION_DELETE,
     }[event_type]
     objectchange = instance.to_objectchange(action)
+    object_change_id = None
     # If this is a many-to-many field change, check for a previous ObjectChange instance recorded
     # for this object by this request and update it
     if m2m_changed and (
@@ -146,10 +147,12 @@ def handle_changed_object(sender, instance, **kwargs):
     ):
         prev_change.postchange_data = objectchange.postchange_data
         prev_change.save()
+        object_change_id = prev_change.pk
     elif objectchange and objectchange.has_changes:
         objectchange.user = request.user
         objectchange.request_id = request.id
         objectchange.save()
+        object_change_id = objectchange.pk
 
     # Ensure that we're working with fresh M2M assignments
     if m2m_changed:
@@ -157,7 +160,7 @@ def handle_changed_object(sender, instance, **kwargs):
 
     # Enqueue the object for event processing
     queue = events_queue.get()
-    enqueue_event(queue, instance, request, event_type)
+    enqueue_event(queue, instance, request, event_type, object_change_id=object_change_id)
     events_queue.set(queue)
 
     # Increment metric counters
@@ -200,6 +203,7 @@ def handle_deleted_object(sender, instance, **kwargs):
     _signals_received.pre_delete.add(signature)
 
     # Record an ObjectChange if applicable
+    object_change_id = None
     if hasattr(instance, 'to_objectchange'):
         if hasattr(instance, 'snapshot') and not getattr(instance, '_prechange_snapshot', None):
             instance.snapshot()
@@ -207,6 +211,7 @@ def handle_deleted_object(sender, instance, **kwargs):
         objectchange.user = request.user
         objectchange.request_id = request.id
         objectchange.save()
+        object_change_id = objectchange.pk
 
     # Django does not automatically send an m2m_changed signal for the reverse direction of a
     # many-to-many relationship (see https://code.djangoproject.com/ticket/17688), so we need to
@@ -257,7 +262,7 @@ def handle_deleted_object(sender, instance, **kwargs):
 
     # Enqueue the object for event processing
     queue = events_queue.get()
-    enqueue_event(queue, instance, request, OBJECT_DELETED)
+    enqueue_event(queue, instance, request, OBJECT_DELETED, object_change_id=object_change_id)
     events_queue.set(queue)
 
     # Increment metric counters

+ 2 - 0
netbox/extras/event_rules.py

@@ -45,6 +45,8 @@ class WebhookAction(EventRuleAction):
             'timestamp': timezone.now().isoformat(),
             'retry': get_rq_retry(),
         }
+        if object_change_ids := event_context.get('object_change_ids'):
+            params['object_change_ids'] = object_change_ids
         if 'request' in event_context:
             # Exclude FILES - webhooks don't need uploaded files,
             # which can cause pickle errors with Pillow.

+ 9 - 2
netbox/extras/events.py

@@ -106,9 +106,10 @@ def get_snapshots(instance, event_type):
     }
 
 
-def enqueue_event(queue, instance, request, event_type):
+def enqueue_event(queue, instance, request, event_type, object_change_id=None):
     """
-    Enqueue (or coalesce) an event for a created/updated/deleted object.
+    Enqueue (or coalesce) an event for a created/updated/deleted object. If provided, object_change_id identifies the
+    ObjectChange recorded for the change.
 
     Events are processed after the request completes.
     """
@@ -146,10 +147,16 @@ def enqueue_event(queue, instance, request, event_type):
             object=instance,
             event_type=event_type,
             snapshots=get_snapshots(instance, event_type),
+            object_change_ids=[],
             request=request,
             user=request.user,
         )
 
+    # A coalesced event may span multiple ObjectChanges
+    object_change_ids = queue[key].setdefault('object_change_ids', [])
+    if object_change_id is not None and object_change_id not in object_change_ids:
+        object_change_ids.append(object_change_id)
+
     # For delete events, eagerly serialize the payload before the row is gone.
     # This covers both first-time enqueues and coalesced update→delete promotions.
     if event_type == OBJECT_DELETED:

+ 71 - 1
netbox/extras/tests/test_event_rules.py

@@ -8,6 +8,7 @@ from unittest.mock import Mock, PropertyMock, patch
 import django_rq
 import requests
 from django.conf import settings
+from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ImproperlyConfigured, ValidationError
 from django.http import HttpResponse
 from django.test import RequestFactory, TestCase, override_settings, tag
@@ -18,7 +19,7 @@ from rest_framework import status
 
 from core.choices import JobNotificationChoices, ManagedFileRootPathChoices
 from core.events import *
-from core.models import Job, ObjectType
+from core.models import Job, ObjectChange, ObjectType
 from dcim.choices import DeviceStatusChoices, InterfaceTypeChoices, SiteStatusChoices
 from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site
 from extras.choices import EventRuleActionChoices
@@ -220,6 +221,10 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
         self.assertEqual(len(job.kwargs['data']['tags']), len(response.data['tags']))
         self.assertEqual(job.kwargs['snapshots']['postchange']['name'], 'Site 1')
         self.assertEqual(job.kwargs['snapshots']['postchange']['tags'], ['Bar', 'Foo'])
+        objectchange = ObjectChange.objects.get(
+            changed_object_type=ContentType.objects.get_for_model(Site), changed_object_id=response.data['id']
+        )
+        self.assertEqual(job.kwargs['object_change_ids'], [objectchange.pk])
 
     def test_single_create_rollback_discards_events(self):
         """
@@ -397,6 +402,10 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
         self.assertEqual(job.kwargs['snapshots']['prechange']['tags'], ['Bar', 'Foo'])
         self.assertEqual(job.kwargs['snapshots']['postchange']['name'], 'Site X')
         self.assertEqual(job.kwargs['snapshots']['postchange']['tags'], ['Baz'])
+        objectchange = ObjectChange.objects.get(
+            changed_object_type=ContentType.objects.get_for_model(Site), changed_object_id=site.pk
+        )
+        self.assertEqual(job.kwargs['object_change_ids'], [objectchange.pk])
 
     def test_single_update_rollback_discards_events(self):
         """
@@ -541,6 +550,10 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
         self.assertEqual(job.kwargs['data']['foo'], 3)
         self.assertEqual(job.kwargs['snapshots']['prechange']['name'], 'Site 1')
         self.assertEqual(job.kwargs['snapshots']['prechange']['tags'], ['Bar', 'Foo'])
+        objectchange = ObjectChange.objects.get(
+            changed_object_type=ContentType.objects.get_for_model(Site), changed_object_id=site.pk
+        )
+        self.assertEqual(job.kwargs['object_change_ids'], [objectchange.pk])
 
     def test_single_delete_rollback_discards_events(self):
         """
@@ -696,6 +709,7 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
             self.assertEqual(body['request']['method'], 'GET')
             self.assertEqual(body['request']['path'], url_path)
             self.assertEqual(body['request']['user'], 'testuser')
+            self.assertEqual(body['object_change_ids'], [123])
 
             return HttpResponse()
 
@@ -712,6 +726,7 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
             instance=site,
             request=request,
             event_type=OBJECT_CREATED,
+            object_change_id=123,
         )
         flush_events(list(webhooks_queue.values()))
 
@@ -847,6 +862,7 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
         self.assertEqual(job.kwargs['event_type'], JOB_COMPLETED)
         self.assertEqual(job.kwargs['object_type'], script_type)
         self.assertNotIn('request', job.kwargs)
+        self.assertNotIn('object_change_ids', job.kwargs)
 
     def _job_event_rule(self, conditions=None):
         webhook = Webhook.objects.get(name='Webhook 1')
@@ -1055,6 +1071,60 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
         self.assertEqual(event['data']['name'], 'Site 1')
         self.assertIsNone(event['snapshots']['postchange'])
 
+    def test_coalesced_event_includes_all_object_change_ids(self):
+        """
+        An event coalesced from multiple changes to an object within a single request should reference
+        each ObjectChange recorded for it, in order and without duplicates.
+        """
+        request = RequestFactory().get(reverse('dcim:site_add'))
+        request.id = uuid.uuid4()
+        request.user = self.user
+
+        with event_tracking(request):
+            site = Site(name='Site 1', slug='site-1')
+            site.save()
+            site.snapshot()
+            site.description = 'foo'
+            site.save()
+            # Updates the prior ObjectChange rather than recording a new one
+            site.tags.set(Tag.objects.filter(name='Foo'))
+            # Records no ObjectChange
+            site.snapshot()
+            site.save()
+
+        object_change_ids = list(
+            ObjectChange.objects.filter(
+                changed_object_type=ContentType.objects.get_for_model(Site), changed_object_id=site.pk
+            ).order_by('pk').values_list('pk', flat=True)
+        )
+        self.assertEqual(len(object_change_ids), 2)
+        self.assertEqual(self.queue.count, 1)
+        job = self.queue.jobs[0]
+        self.assertEqual(job.kwargs['event_type'], OBJECT_CREATED)
+        self.assertEqual(job.kwargs['object_change_ids'], object_change_ids)
+
+    def test_send_webhook_omits_empty_object_change_ids(self):
+        """
+        The webhook context should omit object_change_ids if no ObjectChanges were recorded for the event.
+        """
+        def dummy_send(_, request, **kwargs):
+            self.assertNotIn('object_change_ids', json.loads(request.body))
+            return HttpResponse()
+
+        request = RequestFactory().get(reverse('dcim:site_add'))
+        request.id = uuid.uuid4()
+        request.user = self.user
+
+        queue = {}
+        site = Site.objects.create(name='Site 1', slug='site-1')
+        enqueue_event(queue, site, request, OBJECT_CREATED)
+        flush_events(list(queue.values()))
+
+        job = self.queue.jobs[0]
+        self.assertNotIn('object_change_ids', job.kwargs)
+        with patch.object(Session, 'send', dummy_send):
+            send_webhook(**job.kwargs)
+
     @tag('regression')  # #21338
     def test_cable_creation_event_payload_includes_connected_endpoints(self):
         """

+ 5 - 1
netbox/extras/webhooks.py

@@ -45,7 +45,9 @@ def generate_signature(request_body, secret):
 
 
 @job('default')
-def send_webhook(event_rule, object_type, event_type, data, timestamp, request=None, snapshots=None):
+def send_webhook(
+    event_rule, object_type, event_type, data, timestamp, request=None, snapshots=None, object_change_ids=None
+):
     """
     Make a POST request to the defined Webhook
     """
@@ -62,6 +64,8 @@ def send_webhook(event_rule, object_type, event_type, data, timestamp, request=N
         context['request'] = get_safe_request_context(request)
     if snapshots:
         context['snapshots'] = snapshots
+    if object_change_ids:
+        context['object_change_ids'] = object_change_ids
 
     # Add any additional context from plugins
     callback_data = {}