diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index 40e3cfef9..dcea30cbf 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -1063,11 +1063,32 @@ model: TEST-4000 slug: test-4000 u_height: 1 interfaces: - - name: Bridge + - name: Standalone 1 type: 1000base-t + - name: Bridge Interface 2 + type: 1000base-t + bridge: Bridge - name: Bridge Interface 1 type: 1000base-t bridge: Bridge + - name: Standalone 2 + type: 1000base-t + - name: Sub-Bridge Interface 2 + type: 1000base-t + bridge: Sub-Bridge + - name: Bridge + type: 1000base-t + - name: Sub-Bridge + type: 1000base-t + bridge: Bridge + - name: Bridge Interface 3 + type: 1000base-t + bridge: Bridge + - name: Sub-Bridge Interface 1 + type: 1000base-t + bridge: Sub-Bridge + - name: Standalone 3 + type: 1000base-t """ # Add all required permissions to the test user @@ -1096,13 +1117,74 @@ interfaces: self.assertContains(response, "Imported 1 device types") device_type = DeviceType.objects.get(model='TEST-4000') - self.assertEqual(device_type.interfacetemplates.count(), 2) + self.assertEqual(device_type.interfacetemplates.count(), 10) interfaces = InterfaceTemplate.objects.all().order_by('id') - self.assertEqual(interfaces[0].name, 'Bridge') + self.assertEqual(interfaces[0].name, 'Standalone 1') self.assertIsNone(interfaces[0].bridge) - self.assertEqual(interfaces[1].name, 'Bridge Interface 1') - self.assertEqual(interfaces[1].bridge.name, "Bridge") + self.assertEqual(interfaces[1].name, 'Standalone 2') + self.assertIsNone(interfaces[1].bridge) + self.assertEqual(interfaces[2].name, 'Bridge') + self.assertIsNone(interfaces[2].bridge) + self.assertEqual(interfaces[3].name, 'Standalone 3') + self.assertIsNone(interfaces[3].bridge) + self.assertEqual(interfaces[4].name, 'Bridge Interface 2') + self.assertEqual(interfaces[4].bridge.name, "Bridge") + self.assertEqual(interfaces[5].name, 'Bridge Interface 1') + self.assertEqual(interfaces[5].bridge.name, "Bridge") + self.assertEqual(interfaces[6].name, 'Sub-Bridge') + self.assertEqual(interfaces[6].bridge.name, "Bridge") + self.assertEqual(interfaces[7].name, 'Bridge Interface 3') + self.assertEqual(interfaces[7].bridge.name, "Bridge") + self.assertEqual(interfaces[8].name, 'Sub-Bridge Interface 2') + self.assertEqual(interfaces[8].bridge.name, "Sub-Bridge") + self.assertEqual(interfaces[9].name, 'Sub-Bridge Interface 1') + self.assertEqual(interfaces[9].bridge.name, "Sub-Bridge") + + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) + def test_import_interfacebridge_cycle(self): + IMPORT_DATA = """ +manufacturer: Manufacturer 1 +model: TEST-5000 +slug: test-5000 +u_height: 1 +interfaces: + - name: Interface 1 + type: 1000base-t + bridge: Interface 2 + - name: Interface 2 + type: 1000base-t + bridge: Interface 3 + - name: Interface 3 + type: 1000base-t + bridge: Interface 1 + """ + + # Add all required permissions to the test user + self.add_permissions( + 'dcim.view_devicetype', + 'dcim.add_devicetype', + 'dcim.add_consoleporttemplate', + 'dcim.add_consoleserverporttemplate', + 'dcim.add_powerporttemplate', + 'dcim.add_poweroutlettemplate', + 'dcim.add_interfacetemplate', + 'dcim.add_frontporttemplate', + 'dcim.add_rearporttemplate', + 'dcim.add_modulebaytemplate', + 'dcim.add_devicebaytemplate', + 'dcim.add_inventoryitemtemplate', + ) + + form_data = { + 'data': IMPORT_DATA, + 'format': 'yaml' + } + + response = self.client.post(reverse('dcim:devicetype_bulk_import'), data=form_data, follow=True) + self.assertHttpStatus(response, 200) + self.assertContains(response, "interfaces: Dependency cycle detected in subset " + "[Interface 1, Interface 2, Interface 3]") def test_export_objects(self): url = reverse('dcim:devicetype_list') diff --git a/netbox/dcim/views.py b/netbox/dcim/views.py index 304438698..e4c5bc814 100644 --- a/netbox/dcim/views.py +++ b/netbox/dcim/views.py @@ -1,9 +1,11 @@ +from collections import deque + from django.contrib import messages from django.contrib.contenttypes.models import ContentType from django.core.paginator import EmptyPage, PageNotAnInteger from django.db import router, transaction from django.db.models import Prefetch -from django.forms import ModelMultipleChoiceField, MultipleHiddenInput, modelformset_factory +from django.forms import ModelMultipleChoiceField, MultipleHiddenInput, ValidationError, modelformset_factory from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.utils.html import escape @@ -1283,6 +1285,79 @@ class DeviceTypeImportView(generic.BulkImportView): 'inventory-items': forms.InventoryItemTemplateImportForm, } + def _sort_interfaces(self, interfaces): + """Sort the given enumerated interface list to satisfy any dependencies.""" + if not interfaces: + return + + # build the dependency graph + all_interface_names = set() + sorting_required = False + required_by = dict() # ifname to list of dependant subinterfaces # TODO replace list with ordered set + requires = dict() # ifname to set of depended-on superinterfaces + for idx, interface in interfaces: + if not isinstance(interface, dict): # TODO isinstance(MutableMapping)? + # not a dict, will fail validation anyway + # but still sort if required, to prevent false "bridge is invalid" errors on any other valid interfaces + continue + if not interface.get('name'): + # no interface name, will fail validation anyway + continue + + ifname = interface['name'] + all_interface_names.add(ifname) + + requirements = set() + if bridge := interface.get('bridge'): + requirements.add(bridge) + + requires[ifname] = requirements + for requirement in list(requirements): + required_by.setdefault(requirement, list()).append(ifname) + + # if we haven't seen all requirements yet, sorting is needed + sorting_required |= not requirements.issubset(all_interface_names) + + if not sorting_required: + return + + # use Kahn's algorithm to build a topological sorting + workqueue = deque(ifname for ifname, requirements in requires.items() if not requirements) + ifname_ordering = list() + while workqueue: + ifname = workqueue.popleft() + ifname_ordering.append(ifname) + + for dependant in list(required_by.get(ifname, list())): + requires[dependant].remove(ifname) + if not requires[dependant]: + workqueue.append(dependant) + + if len(ifname_ordering) != len(set(ifname_ordering)): + # should never happen + raise ValueError("Interface ordering contains duplicates") + + unsatisfied_requirements = list(ifname for ifname, deps in requires.items() if deps) + if unsatisfied_requirements: + raise ValidationError( + _("Dependency cycle detected in subset [%(interfaces)s]"), + params={"interfaces": ", ".join(unsatisfied_requirements)}, + ) + + # apply the topological sorting to the actual list + def get_sort_key(interface): + try: + return ifname_ordering.index(interface['name']) + except Exception: + # Everything broken/invalid goes to the beginning, so validation fails fast + return -1 + + interfaces.sort(key=lambda entry_tuple: get_sort_key(entry_tuple[1])) + + def prep_related_object_list(self, field_name, enumerated_list): + if field_name == 'interfaces': + self._sort_interfaces(enumerated_list) + def prep_related_object_data(self, parent, data): data.update({'device_type': parent}) return data