18706 Fix VLAN Assignment checking (#19332)

* 18706 Fix VLAN assignment checking

* 18706 add tests

* 18706 review feedback

netbox/ipam/models/vlans.py

@@ -1,4 +1,5 @@
 from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
+from django.contrib.contenttypes.models import ContentType
 from django.contrib.postgres.fields import ArrayField, IntegerRangeField
 from django.core.exceptions import ValidationError
 from django.core.validators import MaxValueValidator, MinValueValidator
@@ -6,7 +7,7 @@ from django.db import models
 from django.db.backends.postgresql.psycopg_any import NumericRange
 from django.utils.translation import gettext_lazy as _
 
-from dcim.models import Interface
+from dcim.models import Interface, Site, SiteGroup
 from ipam.choices import *
 from ipam.constants import *
 from ipam.querysets import VLANQuerySet, VLANGroupQuerySet
@@ -279,12 +280,20 @@ class VLAN(PrimaryModel):
         super().clean()
 
         # Validate VLAN group (if assigned)
-        if self.group and self.site and self.group.scope != self.site:
+        if self.group and self.site and self.group.scope_type == ContentType.objects.get_for_model(Site):
-            raise ValidationError(
+            if self.site != self.group.scope:
-                _(
+                raise ValidationError(
-                    "VLAN is assigned to group {group} (scope: {scope}); cannot also assign to site {site}."
+                    _(
-                ).format(group=self.group, scope=self.group.scope, site=self.site)
+                        "VLAN is assigned to group {group} (scope: {scope}); cannot also assign to site {site}."
-            )
+                    ).format(group=self.group, scope=self.group.scope, site=self.site)
+                )
+        if self.group and self.site and self.group.scope_type == ContentType.objects.get_for_model(SiteGroup):
+            if self.site not in self.group.scope.sites.all():
+                raise ValidationError(
+                    _(
+                        "The assigned site {site} is not a member of the assigned group {group} (scope: {scope})."
+                    ).format(group=self.group, scope=self.group.scope, site=self.site)
+                )
 
         # Check that the VLAN ID is permitted in the assigned group (if any)
         if self.group:

netbox/ipam/tests/test_models.py

@@ -1,8 +1,10 @@
+from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ValidationError
 from django.test import TestCase, override_settings
 from netaddr import IPNetwork, IPSet
 from utilities.data import string_to_ranges
 
+from dcim.models import Site, SiteGroup
 from ipam.choices import *
 from ipam.models import *
 
@@ -645,3 +647,54 @@ class TestVLAN(TestCase):
         )
         with self.assertRaises(ValidationError):
             vlan.full_clean()
+
+    def test_vlan_group_site_validation(self):
+        sitegroup = SiteGroup.objects.create(
+            name='Site Group 1',
+            slug='site-group-1',
+        )
+        sites = Site.objects.bulk_create((
+            Site(
+                name='Site 1',
+                slug='site-1',
+            ),
+            Site(
+                name='Site 2',
+                slug='site-2',
+            ),
+        ))
+        sitegroup.sites.add(sites[0])
+        vlangroups = VLANGroup.objects.bulk_create((
+            VLANGroup(
+                name='VLAN Group 1',
+                slug='vlan-group-1',
+                scope=sitegroup,
+                scope_type=ContentType.objects.get_for_model(SiteGroup),
+            ),
+            VLANGroup(
+                name='VLAN Group 2',
+                slug='vlan-group-2',
+                scope=sites[0],
+                scope_type=ContentType.objects.get_for_model(Site),
+            ),
+            VLANGroup(
+                name='VLAN Group 2',
+                slug='vlan-group-2',
+                scope=sites[1],
+                scope_type=ContentType.objects.get_for_model(Site),
+            ),
+        ))
+        vlan = VLAN(
+            name='VLAN 1',
+            vid=1,
+            group=vlangroups[0],
+            site=sites[0],
+        )
+
+        # VLAN Group 1 and 2 should be valid
+        vlan.full_clean()
+        vlan.group = vlangroups[1]
+        vlan.full_clean()
+        vlan.group = vlangroups[2]
+        with self.assertRaises(ValidationError):
+            vlan.full_clean()