diff --git a/docs/models/dcim/interface.md b/docs/models/dcim/interface.md index 42b570964..3667dabd5 100644 --- a/docs/models/dcim/interface.md +++ b/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. diff --git a/docs/models/virtualization/vminterface.md b/docs/models/virtualization/vminterface.md index 264fb95ba..d923bdd5d 100644 --- a/docs/models/virtualization/vminterface.md +++ b/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. diff --git a/netbox/dcim/api/views.py b/netbox/dcim/api/views.py index 80a991736..a3e532f0b 100644 --- a/netbox/dcim/api/views.py +++ b/netbox/dcim/api/views.py @@ -24,6 +24,7 @@ from netbox.api.viewsets import NetBoxModelViewSet, MPTTLockedMixin 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( diff --git a/netbox/dcim/migrations/0183_protect_child_interfaces.py b/netbox/dcim/migrations/0183_protect_child_interfaces.py new file mode 100644 index 000000000..ca695f4bd --- /dev/null +++ b/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'), + ), + ] diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py index 639f8aadb..94568459e 100644 --- a/netbox/dcim/models/device_components.py +++ b/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, diff --git a/netbox/dcim/tests/test_api.py b/netbox/dcim/tests/test_api.py index 1ce362963..d3211a75f 100644 --- a/netbox/dcim/tests/test_api.py +++ b/netbox/dcim/tests/test_api.py @@ -1607,6 +1607,33 @@ class InterfaceTest(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase }, ] + 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 diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index a6981451f..88e0d44f2 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -2531,6 +2531,36 @@ class InterfaceTestCase(ViewTestCases.DeviceComponentViewTestCase): 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 diff --git a/netbox/dcim/views.py b/netbox/dcim/views.py index 0f5768173..be0d6bcbb 100644 --- a/netbox/dcim/views.py +++ b/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 tenancy.views import ObjectContactsView 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 diff --git a/netbox/netbox/api/viewsets/__init__.py b/netbox/netbox/api/viewsets/__init__.py index c6794bb61..522bcf77b 100644 --- a/netbox/netbox/api/viewsets/__init__.py +++ b/netbox/netbox/api/viewsets/__init__.py @@ -2,7 +2,7 @@ import logging 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 @@ class NetBoxModelViewSet( 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) diff --git a/netbox/netbox/api/viewsets/mixins.py b/netbox/netbox/api/viewsets/mixins.py index fde486fe9..7b6c00843 100644 --- a/netbox/netbox/api/viewsets/mixins.py +++ b/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] ) diff --git a/netbox/netbox/views/generic/bulk_views.py b/netbox/netbox/views/generic/bulk_views.py index 676e3f5af..fbe3aa2ba 100644 --- a/netbox/netbox/views/generic/bulk_views.py +++ b/netbox/netbox/views/generic/bulk_views.py @@ -7,7 +7,7 @@ from django.contrib.contenttypes.fields import GenericRel 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 @@ class BulkDeleteView(GetReturnURLMixin, BaseMultiObjectView): 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() + with transaction.atomic(): + 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") + 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)) diff --git a/netbox/netbox/views/generic/object_views.py b/netbox/netbox/views/generic/object_views.py index 99d8ff540..7c737aaf0 100644 --- a/netbox/netbox/views/generic/object_views.py +++ b/netbox/netbox/views/generic/object_views.py @@ -3,7 +3,7 @@ from copy import deepcopy 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 @@ class ObjectDeleteView(GetReturnURLMixin, BaseObjectView): 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()) diff --git a/netbox/utilities/error_handlers.py b/netbox/utilities/error_handlers.py index 1d3bdbafd..9af12ac2e 100644 --- a/netbox/utilities/error_handlers.py +++ b/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 {', '.join(str(obj) for obj in obj_list)}. " \ - 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 {objects}. {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 = [] diff --git a/netbox/virtualization/api/views.py b/netbox/virtualization/api/views.py index 5b9cf4117..04e8f2167 100644 --- a/netbox/virtualization/api/views.py +++ b/netbox/virtualization/api/views.py @@ -3,6 +3,7 @@ from rest_framework.routers import APIRootView 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')) diff --git a/netbox/virtualization/migrations/0037_protect_child_interfaces.py b/netbox/virtualization/migrations/0037_protect_child_interfaces.py new file mode 100644 index 000000000..ab6cf0cb3 --- /dev/null +++ b/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'), + ), + ] diff --git a/netbox/virtualization/tests/test_api.py b/netbox/virtualization/tests/test_api.py index b2ae68860..3fb46fbb9 100644 --- a/netbox/virtualization/tests/test_api.py +++ b/netbox/virtualization/tests/test_api.py @@ -293,3 +293,29 @@ class VMInterfaceTest(APIViewTestCases.APIViewTestCase): '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 diff --git a/netbox/virtualization/tests/test_views.py b/netbox/virtualization/tests/test_views.py index a5d831d7e..f47c386e9 100644 --- a/netbox/virtualization/tests/test_views.py +++ b/netbox/virtualization/tests/test_views.py @@ -374,3 +374,32 @@ class VMInterfaceTestCase(ViewTestCases.DeviceComponentViewTestCase): 'untagged_vlan': vlans[0].pk, 'tagged_vlans': [v.pk for v in vlans[1:4]], } + + 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 + data = { + 'confirm': True, + } + self.client.post(self._get_url('delete', interface1), data) + self.assertEqual(virtual_machine.interfaces.count(), 4) # 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(virtual_machine.interfaces.count(), 2) # Child & parent were both deleted diff --git a/netbox/virtualization/views.py b/netbox/virtualization/views.py index 798d1fc4d..e8782243f 100644 --- a/netbox/virtualization/views.py +++ b/netbox/virtualization/views.py @@ -1,5 +1,4 @@ import traceback -from collections import defaultdict from django.contrib import messages from django.db import transaction @@ -19,6 +18,7 @@ from ipam.tables import InterfaceVLANTable from netbox.constants import DEFAULT_ACTION_PERMISSIONS from netbox.views import generic from tenancy.views import ObjectContactsView +from utilities.query_functions import CollateAsChar from utilities.utils import count_related from utilities.views import ViewTab, register_model_view from . import filtersets, forms, tables @@ -550,7 +550,8 @@ class VMInterfaceBulkRenameView(generic.BulkRenameView): class VMInterfaceBulkDeleteView(generic.BulkDeleteView): - queryset = VMInterface.objects.all() + # Ensure child interfaces are deleted prior to their parents + queryset = VMInterface.objects.order_by('virtual_machine', 'parent', CollateAsChar('_name')) filterset = filtersets.VMInterfaceFilterSet table = tables.VMInterfaceTable