Fixes #20584: Ensure consistent validation between Interface & InterfaceTemplate (#20589)

Author: jeremystretch

netbox/dcim/models/device_component_templates.py

@@ -7,6 +7,7 @@
 
 from dcim.choices import *
 from dcim.constants import *
+from dcim.models.mixins import InterfaceValidationMixin
 from netbox.models import ChangeLoggedModel
 from utilities.fields import ColorField, NaturalOrderingField
 from utilities.mptt import TreeManager
@@ -405,7 +406,7 @@ def to_yaml(self):
         }
 
 
-class InterfaceTemplate(ModularComponentTemplateModel):
+class InterfaceTemplate(InterfaceValidationMixin, ModularComponentTemplateModel):
     """
     A template for a physical data interface on a new Device.
     """
@@ -469,8 +470,6 @@ def clean(self):
         super().clean()
 
         if self.bridge:
-            if self.pk and self.bridge_id == self.pk:
-                raise ValidationError({'bridge': _("An interface cannot be bridged to itself.")})
             if self.device_type and self.device_type != self.bridge.device_type:
                 raise ValidationError({
                     'bridge': _(
@@ -484,11 +483,6 @@ def clean(self):
                     ).format(bridge=self.bridge)
                 })
 
-        if self.rf_role and self.type not in WIRELESS_IFACE_TYPES:
-            raise ValidationError({
-                'rf_role': "Wireless role may be set only on wireless interfaces."
-            })
-
     def instantiate(self, **kwargs):
         return self.component_model(
             name=self.resolve_name(kwargs.get('module')),

netbox/dcim/models/device_components.py

@@ -11,6 +11,7 @@
 from dcim.choices import *
 from dcim.constants import *
 from dcim.fields import WWNField
+from dcim.models.mixins import InterfaceValidationMixin
 from netbox.choices import ColorChoices
 from netbox.models import OrganizationalModel, NetBoxModel
 from utilities.fields import ColorField, NaturalOrderingField
@@ -676,7 +677,14 @@ def mac_address(self):
             return self.primary_mac_address.mac_address
 
 
-class Interface(ModularComponentModel, BaseInterface, CabledObjectModel, PathEndpoint, TrackingModelMixin):
+class Interface(
+    InterfaceValidationMixin,
+    ModularComponentModel,
+    BaseInterface,
+    CabledObjectModel,
+    PathEndpoint,
+    TrackingModelMixin,
+):
     """
     A network interface within a Device. A physical Interface can connect to exactly one other Interface.
     """
@@ -893,10 +901,6 @@ def clean(self):
 
         # Bridge validation
 
-        # An interface cannot be bridged to itself
-        if self.pk and self.bridge_id == self.pk:
-            raise ValidationError({'bridge': _("An interface cannot be bridged to itself.")})
-
         # A bridged interface belongs to the same device or virtual chassis
         if self.bridge and self.bridge.device != self.device:
             if self.device.virtual_chassis is None:
@@ -942,29 +946,9 @@ def clean(self):
                     )
                 })
 
-        # PoE validation
-
-        # Only physical interfaces may have a PoE mode/type assigned
-        if self.poe_mode and self.is_virtual:
-            raise ValidationError({
-                'poe_mode': _("Virtual interfaces cannot have a PoE mode.")
-            })
-        if self.poe_type and self.is_virtual:
-            raise ValidationError({
-                'poe_type': _("Virtual interfaces cannot have a PoE type.")
-            })
-
-        # An interface with a PoE type set must also specify a mode
-        if self.poe_type and not self.poe_mode:
-            raise ValidationError({
-                'poe_type': _("Must specify PoE mode when designating a PoE type.")
-            })
-
         # Wireless validation
 
-        # RF role & channel may only be set for wireless interfaces
-        if self.rf_role and not self.is_wireless:
-            raise ValidationError({'rf_role': _("Wireless role may be set only on wireless interfaces.")})
+        # RF channel may only be set for wireless interfaces
         if self.rf_channel and not self.is_wireless:
             raise ValidationError({'rf_channel': _("Channel may be set only on wireless interfaces.")})
 

netbox/dcim/models/mixins.py

@@ -4,8 +4,11 @@
 from django.db import models
 from django.utils.translation import gettext_lazy as _
 
+from dcim.constants import VIRTUAL_IFACE_TYPES, WIRELESS_IFACE_TYPES
+
 __all__ = (
     'CachedScopeMixin',
+    'InterfaceValidationMixin',
     'RenderConfigMixin',
 )
 
@@ -116,3 +119,33 @@ def cache_related_objects(self):
                 self._site = self.scope.site
                 self._location = self.scope
     cache_related_objects.alters_data = True
+
+
+class InterfaceValidationMixin:
+
+    def clean(self):
+        super().clean()
+
+        # An interface cannot be bridged to itself
+        if self.pk and self.bridge_id == self.pk:
+            raise ValidationError({'bridge': _("An interface cannot be bridged to itself.")})
+
+        # Only physical interfaces may have a PoE mode/type assigned
+        if self.poe_mode and self.type in VIRTUAL_IFACE_TYPES:
+            raise ValidationError({
+                'poe_mode': _("Virtual interfaces cannot have a PoE mode.")
+            })
+        if self.poe_type and self.type in VIRTUAL_IFACE_TYPES:
+            raise ValidationError({
+                'poe_type': _("Virtual interfaces cannot have a PoE type.")
+            })
+
+        # An interface with a PoE type set must also specify a mode
+        if self.poe_type and not self.poe_mode:
+            raise ValidationError({
+                'poe_type': _("Must specify PoE mode when designating a PoE type.")
+            })
+
+        # RF role may be set only for wireless interfaces
+        if self.rf_role and self.type not in WIRELESS_IFACE_TYPES:
+            raise ValidationError({'rf_role': _("Wireless role may be set only on wireless interfaces.")})