Add cycle identification

Use a depth-first search to identify any cycles, to be able to report
them as such to the user.

Author: m-hau

netbox/dcim/tests/test_views.py

@@ -1148,16 +1148,68 @@ def test_import_interfacebridge_cycle(self):
 model: TEST-5000
 slug: test-5000
 u_height: 1
+interfaces:
+  - name: Cycle Interface 1
+    type: 1000base-t
+    bridge: Cycle Interface 2
+  - name: Cycle Interface 2
+    type: 1000base-t
+    bridge: Cycle Interface 3
+  - name: Cycle Interface 3
+    type: 1000base-t
+    bridge: Cycle Interface 1
+
+  - name: Unrelated Interface 1
+    type: 1000base-t
+  - name: Unrelated Interface 2
+    type: 1000base-t
+    bridge: Cycle Interface 1
+  - name: Unrelated Interface 3
+    type: 1000base-t
+    bridge: Cycle Interface 3
+    """
+
+        # 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 [Cycle Interface 1, Cycle Interface 2, "
+                                      "Cycle Interface 3, Cycle Interface 1] detected")
+
+    @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'])
+    def test_import_interfacebridge_invalid(self):
+        IMPORT_DATA = """
+manufacturer: Manufacturer 1
+model: TEST-6000
+slug: test-6000
+u_height: 1
 interfaces:
   - name: Interface 1
     type: 1000base-t
-    bridge: Interface 2
   - name: Interface 2
     type: 1000base-t
-    bridge: Interface 3
+    bridge: Non-existent Bridge
   - name: Interface 3
     type: 1000base-t
-    bridge: Interface 1
     """
 
         # Add all required permissions to the test user
@@ -1183,8 +1235,8 @@ def test_import_interfacebridge_cycle(self):
 
         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]")
+        self.assertContains(response, "interfaces[1] bridge: Select a valid choice. "
+                                      "That choice is not one of the available choices.")
 
     def test_export_objects(self):
         url = reverse('dcim:devicetype_list')

netbox/dcim/views.py

@@ -1339,10 +1339,30 @@ def _sort_interfaces(self, interfaces):
 
         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)},
-            )
+            def find_cycle(visited_interfaces):
+                """Recursive depth-first search to identify cycles."""
+                for ifname in list(required_by.get(visited_interfaces[-1], list())):
+                    if ifname in visited_interfaces:
+                        # found a cycle and its start
+                        start_index = visited_interfaces.index(ifname)
+                        visited_interfaces.append(ifname)
+                        return list(reversed(visited_interfaces[start_index:]))
+                    result = find_cycle(visited_interfaces + [ifname])
+                    if result:
+                        return result
+                return None
+
+            # Check if there is a cycle
+            for ifname in unsatisfied_requirements:
+                cycle = find_cycle([ifname])
+                if cycle:
+                    # stop at the first one, finding all while avoiding duplicates would be hard
+                    raise ValidationError(
+                        _("Dependency cycle [%(interfaces)s] detected"),
+                        params={"interfaces": ", ".join(cycle)},
+                    )
+            # no cycle, so the unsatisfied requirements must be due to requirements on non-existent interfaces,
+            # which will cause a validation error later when checking the individual interface objects.
 
         # apply the topological sorting to the actual list
         def get_sort_key(interface):