Standardize validation of interface VLAN assignments

netbox/dcim/api/serializers.py

@@ -584,22 +584,15 @@ class InterfaceSerializer(TaggedObjectSerializer, CableTerminationSerializer, Co
             'count_ipaddresses',
         ]
 
-    # TODO: This validation should be handled by Interface.clean()
     def validate(self, data):
 
-        # All associated VLANs be global or assigned to the parent device's site.
+        # Validate many-to-many VLAN assignments
         device = self.instance.device if self.instance else data.get('device')
-        untagged_vlan = data.get('untagged_vlan')
-        if untagged_vlan and untagged_vlan.site not in [device.site, None]:
-            raise serializers.ValidationError({
-                'untagged_vlan': "VLAN {} must belong to the same site as the interface's parent device, or it must be "
-                                 "global.".format(untagged_vlan)
-            })
         for vlan in data.get('tagged_vlans', []):
             if vlan.site not in [device.site, None]:
                 raise serializers.ValidationError({
-                    'tagged_vlans': "VLAN {} must belong to the same site as the interface's parent device, or it must "
+                    'tagged_vlans': f"VLAN {vlan} must belong to the same site as the interface's parent device, or "
-                                    "be global.".format(vlan)
+                                    f"it must be global."
                 })
 
         return super().validate(data)

netbox/dcim/forms.py

@@ -89,13 +89,12 @@ class DeviceComponentFilterForm(BootstrapMixin, forms.Form):
     )
 
 
-class InterfaceCommonForm:
+class InterfaceCommonForm(forms.Form):
 
     def clean(self):
-
         super().clean()
 
-        # Validate VLAN assignments
+        parent_field = 'device' if 'device' in self.cleaned_data else 'virtual_machine'
         tagged_vlans = self.cleaned_data['tagged_vlans']
 
         # Untagged interfaces cannot be assigned tagged VLANs
@@ -110,13 +109,13 @@ class InterfaceCommonForm:
 
         # Validate tagged VLANs; must be a global VLAN or in the same site
         elif self.cleaned_data['mode'] == InterfaceModeChoices.MODE_TAGGED:
-            valid_sites = [None, self.cleaned_data['device'].site]
+            valid_sites = [None, self.cleaned_data[parent_field].site]
             invalid_vlans = [str(v) for v in tagged_vlans if v.site not in valid_sites]
 
             if invalid_vlans:
                 raise forms.ValidationError({
-                    'tagged_vlans': "The tagged VLANs ({}) must belong to the same site as the interface's parent "
+                    'tagged_vlans': f"The tagged VLANs ({', '.join(invalid_vlans)}) must belong to the same site as "
-                                    "device/VM, or they must be global".format(', '.join(invalid_vlans))
+                                    f"the interface's parent device/VM, or they must be global"
                 })
 
 
@@ -2682,7 +2681,7 @@ class InterfaceFilterForm(DeviceComponentFilterForm):
     tag = TagFilterField(model)
 
 
-class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm):
+class InterfaceForm(BootstrapMixin, InterfaceCommonForm, forms.ModelForm):
     untagged_vlan = DynamicModelChoiceField(
         queryset=VLAN.objects.all(),
         required=False,

netbox/virtualization/api/serializers.py

@@ -116,3 +116,16 @@ class VMInterfaceSerializer(TaggedObjectSerializer, ValidatedModelSerializer):
             'id', 'url', 'virtual_machine', 'name', 'enabled', 'mtu', 'mac_address', 'description', 'mode',
             'untagged_vlan', 'tagged_vlans', 'tags',
         ]
+
+    def validate(self, data):
+
+        # Validate many-to-many VLAN assignments
+        virtual_machine = self.instance.virtual_machine if self.instance else data.get('virtual_machine')
+        for vlan in data.get('tagged_vlans', []):
+            if vlan.site not in [virtual_machine.site, None]:
+                raise serializers.ValidationError({
+                    'tagged_vlans': f"VLAN {vlan} must belong to the same site as the interface's parent virtual "
+                                    f"machine, or it must be global."
+                })
+
+        return super().validate(data)

netbox/virtualization/forms.py

@@ -4,7 +4,7 @@ from django.core.exceptions import ValidationError
 
 from dcim.choices import InterfaceModeChoices
 from dcim.constants import INTERFACE_MTU_MAX, INTERFACE_MTU_MIN
-from dcim.forms import INTERFACE_MODE_HELP_TEXT
+from dcim.forms import InterfaceCommonForm, INTERFACE_MODE_HELP_TEXT
 from dcim.models import Device, DeviceRole, Platform, Rack, Region, Site
 from extras.forms import (
     AddRemoveTagsForm, CustomFieldBulkEditForm, CustomFieldModelCSVForm, CustomFieldModelForm, CustomFieldFilterForm,
@@ -543,10 +543,11 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil
 # VM interfaces
 #
 
-class VMInterfaceForm(BootstrapMixin, forms.ModelForm):
+class VMInterfaceForm(BootstrapMixin, InterfaceCommonForm, forms.ModelForm):
     untagged_vlan = DynamicModelChoiceField(
         queryset=VLAN.objects.all(),
         required=False,
+        label='Untagged VLAN',
         display_field='display_name',
         brief_mode=False,
         query_params={
@@ -556,6 +557,7 @@ class VMInterfaceForm(BootstrapMixin, forms.ModelForm):
     tagged_vlans = DynamicModelMultipleChoiceField(
         queryset=VLAN.objects.all(),
         required=False,
+        label='Tagged VLANs',
         display_field='display_name',
         brief_mode=False,
         query_params={
@@ -597,24 +599,8 @@ class VMInterfaceForm(BootstrapMixin, forms.ModelForm):
             self.fields['untagged_vlan'].widget.add_query_param('site_id', site.pk)
             self.fields['tagged_vlans'].widget.add_query_param('site_id', site.pk)
 
-    def clean(self):
-        super().clean()
 
-        # Validate VLAN assignments
+class VMInterfaceCreateForm(BootstrapMixin, InterfaceCommonForm):
-        tagged_vlans = self.cleaned_data['tagged_vlans']
-
-        # Untagged interfaces cannot be assigned tagged VLANs
-        if self.cleaned_data['mode'] == InterfaceModeChoices.MODE_ACCESS and tagged_vlans:
-            raise forms.ValidationError({
-                'mode': "An access interface cannot have tagged VLANs assigned."
-            })
-
-        # Remove all tagged VLAN assignments from "tagged all" interfaces
-        elif self.cleaned_data['mode'] == InterfaceModeChoices.MODE_TAGGED_ALL:
-            self.cleaned_data['tagged_vlans'] = []
-
-
-class VMInterfaceCreateForm(BootstrapMixin, forms.Form):
     virtual_machine = DynamicModelChoiceField(
         queryset=VirtualMachine.objects.all()
     )

netbox/virtualization/models.py

@@ -5,7 +5,6 @@ from django.db import models
 from django.urls import reverse
 from taggit.managers import TaggableManager
 
-from dcim.choices import InterfaceModeChoices
 from dcim.models import BaseInterface, Device
 from extras.models import ChangeLoggedModel, ConfigContextModel, CustomFieldModel, ObjectChange, TaggedItem
 from extras.querysets import ConfigContextModelQuerySet
@@ -449,8 +448,8 @@ class VMInterface(BaseInterface):
         # Validate untagged VLAN
         if self.untagged_vlan and self.untagged_vlan.site not in [self.virtual_machine.site, None]:
             raise ValidationError({
-                'untagged_vlan': "The untagged VLAN ({}) must belong to the same site as the interface's parent "
+                'untagged_vlan': f"The untagged VLAN ({self.untagged_vlan}) must belong to the same site as the "
-                                 "virtual machine, or it must be global".format(self.untagged_vlan)
+                                 f"interface's parent virtual machine, or it must be global"
             })
 
     def to_objectchange(self, action):