diff --git a/netbox/core/signals.py b/netbox/core/signals.py index cdb787667..cfd10c988 100644 --- a/netbox/core/signals.py +++ b/netbox/core/signals.py @@ -225,7 +225,23 @@ def handle_deleted_object(sender, instance, **kwargs): # We only care about triggering the m2m_changed signal for models which support # change logging continue + related_object_type = ContentType.objects.get_for_model(related_model) for obj in related_model.objects.filter(**{related_field_name: instance.pk}): + # Skip any related object that is itself being deleted as part of this same + # operation (e.g. a sibling caught up in the same cascade). Its deletion has + # already been recorded, so nulling the FK and saving here would record an + # UPDATE ObjectChange *after* the object's DELETE, corrupting the changelog and + # breaking branch replay. (Ref: #22270) + # + # Note this is order-dependent: it only fires once the related object's own + # pre_delete has run (adding it to the set). If the cascade happens to delete + # this instance *before* the related object, the guard won't trigger and the + # related object still gets an UPDATE — but in the harmless UPDATE-then-DELETE + # order, not the corrupting DELETE-then-UPDATE order. Fully suppressing it in + # every ordering would require the complete deletion set, which isn't available + # from a pre_delete signal. + if (related_object_type, obj.pk) in _signals_received.pre_delete: + continue obj.snapshot() # Ensure the change record includes the "before" state if type(relation) is ManyToManyRel: getattr(obj, related_field_name).remove(instance) diff --git a/netbox/core/tests/test_signals.py b/netbox/core/tests/test_signals.py index 9a78a04f2..d691e72e2 100644 --- a/netbox/core/tests/test_signals.py +++ b/netbox/core/tests/test_signals.py @@ -12,7 +12,8 @@ from core import signals from core.choices import DataSourceStatusChoices, JobStatusChoices, ObjectChangeActionChoices from core.models import ConfigRevision, DataSource, ObjectChange, ObjectType from core.signals import _signals_received, clear_events, post_sync -from dcim.models import Site, SiteGroup +from dcim.choices import InterfaceTypeChoices +from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site, SiteGroup from extras.models import Tag from extras.validators import CustomValidator from netbox.context import events_queue @@ -204,6 +205,47 @@ class HandleDeletedObjectSignalTestCase(TestCase): self.assertEqual(oc.prechange_data['group'], group_pk) self.assertIsNone(oc.postchange_data['group']) + def test_cascade_delete_does_not_record_update_after_delete(self): + # Regression test for #22270. Deleting a device cascades to all of its interfaces. + # When the LAG interface's pre_delete fires, it clears the `lag` FK (SET_NULL) on its + # member interfaces and records a change. If a member is itself being deleted in the + # same cascade, that update must not be written *after* the member's delete record — + # an UPDATE-after-DELETE corrupts the changelog and breaks branch replay. + manufacturer = Manufacturer.objects.create(name='Manufacturer', slug='manufacturer') + device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type', slug='device-type') + role = DeviceRole.objects.create(name='Role', slug='role') + site = Site.objects.create(name='Site', slug='site') + device = Device.objects.create(name='Device', site=site, device_type=device_type, role=role) + # Members are created before the LAG so their PKs are lower; the cascade fires their + # pre_delete (and records their DELETE) before the LAG's pre_delete runs. + member1 = Interface.objects.create(device=device, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED) + member2 = Interface.objects.create(device=device, name='eth1', type=InterfaceTypeChoices.TYPE_1GE_FIXED) + lag = Interface.objects.create(device=device, name='lag0', type=InterfaceTypeChoices.TYPE_LAG) + member1.lag = lag + member1.save() + member2.lag = lag + member2.save() + member_pks = (member1.pk, member2.pk) + interface_type = ContentType.objects.get_for_model(Interface) + + request = _build_request(self.user) + with event_tracking(request): + device.delete() + + for member_pk in member_pks: + actions = list( + ObjectChange.objects.filter( + request_id=request.id, + changed_object_type=interface_type, + changed_object_id=member_pk, + ).order_by('time', 'pk').values_list('action', flat=True) + ) + # In this ordering the member's pre_delete fires before the LAG's, so the LAG + # skips it: the member's only change record for the request is its own deletion. + # Pre-fix, a spurious UPDATE was appended *after* this DELETE — assert the exact + # sequence so any trailing (or leading) update fails the test. + self.assertEqual(actions, [ObjectChangeActionChoices.ACTION_DELETE]) + class ClearSignalHistorySignalTestCase(TestCase): """