Closes #12135: Prevent the deletion of interfaces with children

docs/models/dcim/interface.md

@@ -77,6 +77,9 @@ If selected, this component will be treated as if a cable has been connected.
 
 Virtual interfaces can be bound to a physical parent interface. This is helpful for modeling virtual interfaces which employ encapsulation on a physical interface, such as an 802.1Q VLAN-tagged subinterface.
 
+!!! note
+    An interface with one or more child interfaces assigned cannot be deleted until all its child interfaces have been deleted or reassigned.
+
 ### Bridged Interface
 
 Interfaces can be bridged to other interfaces on a device in two manners: symmetric or grouped.

docs/models/virtualization/vminterface.md

@@ -16,6 +16,9 @@ The interface's name. Must be unique to the assigned VM.
 
 Identifies the parent interface of a subinterface (e.g. used to employ encapsulation).
 
+!!! note
+    An interface with one or more child interfaces assigned cannot be deleted until all its child interfaces have been deleted or reassigned.
+
 ### Bridged Interface
 
 An interface on the same VM with which this interface is bridged.

netbox/dcim/api/views.py

@@ -24,6 +24,7 @@
 from netbox.api.viewsets.mixins import SequentialBulkCreatesMixin
 from netbox.constants import NESTED_SERIALIZER_PREFIX
 from utilities.api import get_serializer_for_model
+from utilities.query_functions import CollateAsChar
 from utilities.utils import count_related
 from virtualization.models import VirtualMachine
 from . import serializers
@@ -505,6 +506,10 @@ class InterfaceViewSet(PathEndpointMixin, NetBoxModelViewSet):
     filterset_class = filtersets.InterfaceFilterSet
     brief_prefetch_fields = ['device']
 
+    def get_bulk_destroy_queryset(self):
+        # Ensure child interfaces are deleted prior to their parents
+        return self.get_queryset().order_by('device', 'parent', CollateAsChar('_name'))
+
 
 class FrontPortViewSet(PassThroughPortMixin, NetBoxModelViewSet):
     queryset = FrontPort.objects.prefetch_related(

netbox/dcim/migrations/0183_protect_child_interfaces.py

@@ -0,0 +1,19 @@
+# Generated by Django 4.2.6 on 2023-10-20 11:48
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dcim', '0182_devicetype_exclude_from_utilization'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='interface',
+            name='parent',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='child_interfaces', to='dcim.interface'),
+        ),
+    ]

netbox/dcim/models/device_components.py

@@ -537,7 +537,7 @@ class BaseInterface(models.Model):
     )
     parent = models.ForeignKey(
         to='self',
-        on_delete=models.SET_NULL,
+        on_delete=models.RESTRICT,
         related_name='child_interfaces',
         null=True,
         blank=True,

netbox/dcim/tests/test_api.py

@@ -1607,6 +1607,33 @@ def setUpTestData(cls):
             },
         ]
 
+    def test_bulk_delete_child_interfaces(self):
+        interface1 = Interface.objects.get(name='Interface 1')
+        device = interface1.device
+        self.add_permissions('dcim.delete_interface')
+
+        # Create a child interface
+        child = Interface.objects.create(
+            device=device,
+            name='Interface 1A',
+            type=InterfaceTypeChoices.TYPE_VIRTUAL,
+            parent=interface1
+        )
+        self.assertEqual(device.interfaces.count(), 4)
+
+        # Attempt to delete only the parent interface
+        url = self._get_detail_url(interface1)
+        self.client.delete(url, **self.header)
+        self.assertEqual(device.interfaces.count(), 4)  # Parent was not deleted
+
+        # Attempt to bulk delete parent & child together
+        data = [
+            {"id": interface1.pk},
+            {"id": child.pk},
+        ]
+        self.client.delete(self._get_list_url(), data, format='json', **self.header)
+        self.assertEqual(device.interfaces.count(), 2)  # Child & parent were both deleted
+
 
 class FrontPortTest(APIViewTestCases.APIViewTestCase):
     model = FrontPort

netbox/dcim/tests/test_views.py

@@ -2531,6 +2531,36 @@ def test_trace(self):
         response = self.client.get(reverse('dcim:interface_trace', kwargs={'pk': interface1.pk}))
         self.assertHttpStatus(response, 200)
 
+    def test_bulk_delete_child_interfaces(self):
+        interface1 = Interface.objects.get(name='Interface 1')
+        device = interface1.device
+        self.add_permissions('dcim.delete_interface')
+
+        # Create a child interface
+        child = Interface.objects.create(
+            device=device,
+            name='Interface 1A',
+            type=InterfaceTypeChoices.TYPE_VIRTUAL,
+            parent=interface1
+        )
+        self.assertEqual(device.interfaces.count(), 6)
+
+        # Attempt to delete only the parent interface
+        data = {
+            'confirm': True,
+        }
+        self.client.post(self._get_url('delete', interface1), data)
+        self.assertEqual(device.interfaces.count(), 6)  # Parent was not deleted
+
+        # Attempt to bulk delete parent & child together
+        data = {
+            'pk': [interface1.pk, child.pk],
+            'confirm': True,
+            '_confirm': True,  # Form button
+        }
+        self.client.post(self._get_url('bulk_delete'), data)
+        self.assertEqual(device.interfaces.count(), 4)  # Child & parent were both deleted
+
 
 class FrontPortTestCase(ViewTestCases.DeviceComponentViewTestCase):
     model = FrontPort

netbox/dcim/views.py

@@ -1,5 +1,4 @@
 import traceback
-from collections import defaultdict
 
 from django.contrib import messages
 from django.contrib.contenttypes.models import ContentType
@@ -26,6 +25,7 @@
 from utilities.forms import ConfirmationForm
 from utilities.paginator import EnhancedPaginator, get_paginate_count
 from utilities.permissions import get_permission_for_model
+from utilities.query_functions import CollateAsChar
 from utilities.utils import count_related
 from utilities.views import GetReturnURLMixin, ObjectPermissionRequiredMixin, ViewTab, register_model_view
 from virtualization.models import VirtualMachine
@@ -2562,7 +2562,8 @@ class InterfaceBulkDisconnectView(BulkDisconnectView):
 
 
 class InterfaceBulkDeleteView(generic.BulkDeleteView):
-    queryset = Interface.objects.all()
+    # Ensure child interfaces are deleted prior to their parents
+    queryset = Interface.objects.order_by('device', 'parent', CollateAsChar('_name'))
     filterset = filtersets.InterfaceFilterSet
     table = tables.InterfaceTable
 

netbox/netbox/api/viewsets/__init__.py

@@ -2,7 +2,7 @@
 
 from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
 from django.db import transaction
-from django.db.models import ProtectedError
+from django.db.models import ProtectedError, RestrictedError
 from django_pglocks import advisory_lock
 from netbox.constants import ADVISORY_LOCK_KEYS
 from rest_framework import mixins as drf_mixins
@@ -91,8 +91,11 @@ def dispatch(self, request, *args, **kwargs):
 
         try:
             return super().dispatch(request, *args, **kwargs)
-        except ProtectedError as e:
-            protected_objects = list(e.protected_objects)
+        except (ProtectedError, RestrictedError) as e:
+            if type(e) is ProtectedError:
+                protected_objects = list(e.protected_objects)
+            else:
+                protected_objects = list(e.restricted_objects)
             msg = f'Unable to delete object. {len(protected_objects)} dependent objects were found: '
             msg += ', '.join([f'{obj} ({obj.pk})' for obj in protected_objects])
             logger.warning(msg)

netbox/netbox/api/viewsets/mixins.py

@@ -137,11 +137,14 @@ class BulkUpdateModelMixin:
         }
     ]
     """
+    def get_bulk_update_queryset(self):
+        return self.get_queryset()
+
     def bulk_update(self, request, *args, **kwargs):
         partial = kwargs.pop('partial', False)
         serializer = BulkOperationSerializer(data=request.data, many=True)
         serializer.is_valid(raise_exception=True)
-        qs = self.get_queryset().filter(
+        qs = self.get_bulk_update_queryset().filter(
             pk__in=[o['id'] for o in serializer.data]
         )
 
@@ -184,10 +187,13 @@ class BulkDestroyModelMixin:
         {"id": 456}
     ]
     """
+    def get_bulk_destroy_queryset(self):
+        return self.get_queryset()
+
     def bulk_destroy(self, request, *args, **kwargs):
         serializer = BulkOperationSerializer(data=request.data, many=True)
         serializer.is_valid(raise_exception=True)
-        qs = self.get_queryset().filter(
+        qs = self.get_bulk_destroy_queryset().filter(
             pk__in=[o['id'] for o in serializer.data]
         )
 

netbox/netbox/views/generic/bulk_views.py

@@ -7,7 +7,7 @@
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import FieldDoesNotExist, ObjectDoesNotExist, ValidationError
 from django.db import transaction, IntegrityError
-from django.db.models import ManyToManyField, ProtectedError
+from django.db.models import ManyToManyField, ProtectedError, RestrictedError
 from django.db.models.fields.reverse_related import ManyToManyRel
 from django.forms import HiddenInput, ModelMultipleChoiceField, MultipleHiddenInput
 from django.http import HttpResponse
@@ -798,14 +798,15 @@ def post(self, request, **kwargs):
                 queryset = self.queryset.filter(pk__in=pk_list)
                 deleted_count = queryset.count()
                 try:
-                    for obj in queryset:
-                        # Take a snapshot of change-logged models
-                        if hasattr(obj, 'snapshot'):
-                            obj.snapshot()
-                        obj.delete()
-
-                except ProtectedError as e:
-                    logger.info("Caught ProtectedError while attempting to delete objects")
+                    with transaction.atomic():
+                        for obj in queryset:
+                            # Take a snapshot of change-logged models
+                            if hasattr(obj, 'snapshot'):
+                                obj.snapshot()
+                            obj.delete()
+
+                except (ProtectedError, RestrictedError) as e:
+                    logger.info(f"Caught {type(e)} while attempting to delete objects")
                     handle_protectederror(queryset, request, e)
                     return redirect(self.get_return_url(request))
 

netbox/netbox/views/generic/object_views.py

@@ -3,7 +3,7 @@
 
 from django.contrib import messages
 from django.db import transaction
-from django.db.models import ProtectedError
+from django.db.models import ProtectedError, RestrictedError
 from django.shortcuts import redirect, render
 from django.urls import reverse
 from django.utils.html import escape
@@ -374,8 +374,8 @@ def post(self, request, *args, **kwargs):
             try:
                 obj.delete()
 
-            except ProtectedError as e:
-                logger.info("Caught ProtectedError while attempting to delete object")
+            except (ProtectedError, RestrictedError) as e:
+                logger.info(f"Caught {type(e)} while attempting to delete objects")
                 handle_protectederror([obj], request, e)
                 return redirect(obj.get_absolute_url())
 

netbox/utilities/error_handlers.py

@@ -1,16 +1,26 @@
 from django.contrib import messages
+from django.db.models import ProtectedError, RestrictedError
 from django.utils.html import escape
 from django.utils.safestring import mark_safe
+from django.utils.translation import gettext_lazy as _
 
 
 def handle_protectederror(obj_list, request, e):
     """
-    Generate a user-friendly error message in response to a ProtectedError exception.
+    Generate a user-friendly error message in response to a ProtectedError or RestrictedError exception.
     """
-    protected_objects = list(e.protected_objects)
-    protected_count = len(protected_objects) if len(protected_objects) <= 50 else 'More than 50'
-    err_message = f"Unable to delete <strong>{', '.join(str(obj) for obj in obj_list)}</strong>. " \
-                  f"{protected_count} dependent objects were found: "
+    if type(e) is ProtectedError:
+        protected_objects = list(e.protected_objects)
+    elif type(e) is RestrictedError:
+        protected_objects = list(e.restricted_objects)
+    else:
+        raise e
+
+    # Formulate the error message
+    err_message = _("Unable to delete <strong>{objects}</strong>. {count} dependent objects were found: ").format(
+        objects=', '.join(str(obj) for obj in obj_list),
+        count=len(protected_objects) if len(protected_objects) <= 50 else _('More than 50')
+    )
 
     # Append dependent objects to error message
     dependent_objects = []

netbox/virtualization/api/views.py

@@ -3,6 +3,7 @@
 from dcim.models import Device
 from extras.api.mixins import ConfigContextQuerySetMixin
 from netbox.api.viewsets import NetBoxModelViewSet
+from utilities.query_functions import CollateAsChar
 from utilities.utils import count_related
 from virtualization import filtersets
 from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine, VMInterface
@@ -87,3 +88,7 @@ class VMInterfaceViewSet(NetBoxModelViewSet):
     serializer_class = serializers.VMInterfaceSerializer
     filterset_class = filtersets.VMInterfaceFilterSet
     brief_prefetch_fields = ['virtual_machine']
+
+    def get_bulk_destroy_queryset(self):
+        # Ensure child interfaces are deleted prior to their parents
+        return self.get_queryset().order_by('virtual_machine', 'parent', CollateAsChar('_name'))

netbox/virtualization/migrations/0037_protect_child_interfaces.py

@@ -0,0 +1,19 @@
+# Generated by Django 4.2.6 on 2023-10-20 11:48
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('virtualization', '0036_virtualmachine_config_template'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='vminterface',
+            name='parent',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='child_interfaces', to='virtualization.vminterface'),
+        ),
+    ]

netbox/virtualization/tests/test_api.py

@@ -293,3 +293,29 @@ def setUpTestData(cls):
                 'vrf': vrfs[2].pk,
             },
         ]
+
+    def test_bulk_delete_child_interfaces(self):
+        interface1 = VMInterface.objects.get(name='Interface 1')
+        virtual_machine = interface1.virtual_machine
+        self.add_permissions('virtualization.delete_vminterface')
+
+        # Create a child interface
+        child = VMInterface.objects.create(
+            virtual_machine=virtual_machine,
+            name='Interface 1A',
+            parent=interface1
+        )
+        self.assertEqual(virtual_machine.interfaces.count(), 4)
+
+        # Attempt to delete only the parent interface
+        url = self._get_detail_url(interface1)
+        self.client.delete(url, **self.header)
+        self.assertEqual(virtual_machine.interfaces.count(), 4)  # Parent was not deleted
+
+        # Attempt to bulk delete parent & child together
+        data = [
+            {"id": interface1.pk},
+            {"id": child.pk},
+        ]
+        self.client.delete(self._get_list_url(), data, format='json', **self.header)
+        self.assertEqual(virtual_machine.interfaces.count(), 2)  # Child & parent were both deleted