Преглед изворни кода

Closes #22682: Fix CachedScopeMixin cache fields cascading on ancestor deletion (#22693)

CachedScopeMixin._region and ._site_group may cache ancestors of a
Site or Location scope. Change these relationships to SET_NULL so
deleting a Region or SiteGroup clears the cached value instead of
deleting the scoped Prefix, Cluster, or WirelessLAN.

Add reverse GenericRelation fields for Cluster and WirelessLAN on
Region and SiteGroup. This preserves the expected cascade when a
Region or SiteGroup is itself the direct scope, matching the existing
Prefix behavior.

Add migrations recording the ORM-level on_delete changes and regression
coverage for Site, Location, and direct Region/SiteGroup scopes.
bctiemann пре 1 недеља
родитељ
комит
0eb1fcc09c

+ 5 - 2
netbox/dcim/models/mixins.py

@@ -72,15 +72,18 @@ class CachedScopeMixin(models.Model):
         blank=True,
         null=True
     )
+    # SET_NULL, not CASCADE: these cache an ancestor of the actual scope, so deleting that
+    # ancestor must not delete this object. Deletion of a Region/SiteGroup that *is* the
+    # actual scope is handled independently via its GenericRelation to this model.
     _region = models.ForeignKey(
         to='dcim.Region',
-        on_delete=models.CASCADE,
+        on_delete=models.SET_NULL,
         blank=True,
         null=True
     )
     _site_group = models.ForeignKey(
         to='dcim.SiteGroup',
-        on_delete=models.CASCADE,
+        on_delete=models.SET_NULL,
         blank=True,
         null=True
     )

+ 24 - 0
netbox/dcim/models/sites.py

@@ -42,6 +42,18 @@ class Region(ContactsMixin, NestedGroupModel):
         object_id_field='scope_id',
         related_query_name='region'
     )
+    clusters = GenericRelation(
+        to='virtualization.Cluster',
+        content_type_field='scope_type',
+        object_id_field='scope_id',
+        related_query_name='region'
+    )
+    wireless_lans = GenericRelation(
+        to='wireless.WirelessLAN',
+        content_type_field='scope_type',
+        object_id_field='scope_id',
+        related_query_name='region'
+    )
 
     class Meta:
         # Empty tuple triggers Django migration detection for MPTT indexes
@@ -101,6 +113,18 @@ class SiteGroup(ContactsMixin, NestedGroupModel):
         object_id_field='scope_id',
         related_query_name='site_group'
     )
+    clusters = GenericRelation(
+        to='virtualization.Cluster',
+        content_type_field='scope_type',
+        object_id_field='scope_id',
+        related_query_name='site_group'
+    )
+    wireless_lans = GenericRelation(
+        to='wireless.WirelessLAN',
+        content_type_field='scope_type',
+        object_id_field='scope_id',
+        related_query_name='site_group'
+    )
 
     class Meta:
         # Empty tuple triggers Django migration detection for MPTT indexes

+ 27 - 0
netbox/ipam/migrations/0093_alter_prefix__region_alter_prefix__site_group.py

@@ -0,0 +1,27 @@
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dcim', '0239_add_portmapping_objectchange'),
+        ('ipam', '0092_iprange_host_indexes'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='prefix',
+            name='_region',
+            field=models.ForeignKey(
+                blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='dcim.region'
+            ),
+        ),
+        migrations.AlterField(
+            model_name='prefix',
+            name='_site_group',
+            field=models.ForeignKey(
+                blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='dcim.sitegroup'
+            ),
+        ),
+    ]

+ 75 - 1
netbox/ipam/tests/test_models.py

@@ -5,7 +5,7 @@ from django.db.backends.postgresql.psycopg_any import NumericRange
 from django.test import TestCase, override_settings
 from netaddr import IPNetwork, IPSet
 
-from dcim.models import Site, SiteGroup
+from dcim.models import Location, Region, Site, SiteGroup
 from ipam.choices import *
 from ipam.constants import SERVICE_PORT_MAX, SERVICE_PORT_MIN
 from ipam.models import *
@@ -1262,6 +1262,80 @@ class PrefixTestCase(TestCase):
         duplicate_prefix = Prefix(vrf=vrf, prefix=IPNetwork('192.0.2.0/24'))
         self.assertRaises(ValidationError, duplicate_prefix.clean)
 
+    # Regression test for #22682
+    def test_deleting_site_group_does_not_delete_prefix_scoped_to_member_site(self):
+        sitegroup = SiteGroup.objects.create(name='Site Group 1', slug='site-group-1')
+        site = Site.objects.create(name='Site 1', slug='site-1', group=sitegroup)
+        prefix = Prefix.objects.create(prefix=IPNetwork('10.0.0.0/24'), scope=site)
+
+        sitegroup.delete()
+
+        site.refresh_from_db()
+        prefix.refresh_from_db()
+        self.assertIsNone(site.group)
+        self.assertEqual(prefix.scope, site)
+        self.assertIsNone(prefix._site_group_id)
+
+    # Regression test for #22682
+    def test_deleting_region_does_not_delete_prefix_scoped_to_member_site(self):
+        region = Region.objects.create(name='Region 1', slug='region-1')
+        site = Site.objects.create(name='Site 2', slug='site-2', region=region)
+        prefix = Prefix.objects.create(prefix=IPNetwork('10.0.1.0/24'), scope=site)
+
+        region.delete()
+
+        site.refresh_from_db()
+        prefix.refresh_from_db()
+        self.assertIsNone(site.region)
+        self.assertEqual(prefix.scope, site)
+        self.assertIsNone(prefix._region_id)
+
+    # Regression test for #22682
+    def test_deleting_site_group_does_not_delete_prefix_scoped_to_member_location(self):
+        sitegroup = SiteGroup.objects.create(name='Site Group 3', slug='site-group-3')
+        site = Site.objects.create(name='Site 3', slug='site-3', group=sitegroup)
+        location = Location.objects.create(name='Location 1', slug='location-1', site=site)
+        prefix = Prefix.objects.create(prefix=IPNetwork('10.0.4.0/24'), scope=location)
+
+        sitegroup.delete()
+
+        site.refresh_from_db()
+        prefix.refresh_from_db()
+        self.assertIsNone(site.group)
+        self.assertEqual(prefix.scope, location)
+        self.assertIsNone(prefix._site_group_id)
+
+    # Regression test for #22682
+    def test_deleting_region_does_not_delete_prefix_scoped_to_member_location(self):
+        region = Region.objects.create(name='Region 3', slug='region-3')
+        site = Site.objects.create(name='Site 4', slug='site-4', region=region)
+        location = Location.objects.create(name='Location 2', slug='location-2', site=site)
+        prefix = Prefix.objects.create(prefix=IPNetwork('10.0.5.0/24'), scope=location)
+
+        region.delete()
+
+        site.refresh_from_db()
+        prefix.refresh_from_db()
+        self.assertIsNone(site.region)
+        self.assertEqual(prefix.scope, location)
+        self.assertIsNone(prefix._region_id)
+
+    def test_deleting_site_group_scoped_to_it_directly_still_deletes_prefix(self):
+        sitegroup = SiteGroup.objects.create(name='Site Group 2', slug='site-group-2')
+        prefix = Prefix.objects.create(prefix=IPNetwork('10.0.2.0/24'), scope=sitegroup)
+
+        sitegroup.delete()
+
+        self.assertFalse(Prefix.objects.filter(pk=prefix.pk).exists())
+
+    def test_deleting_region_scoped_to_it_directly_still_deletes_prefix(self):
+        region = Region.objects.create(name='Region 2', slug='region-2')
+        prefix = Prefix.objects.create(prefix=IPNetwork('10.0.3.0/24'), scope=region)
+
+        region.delete()
+
+        self.assertFalse(Prefix.objects.filter(pk=prefix.pk).exists())
+
 
 class PrefixHierarchyTestCase(TestCase):
     """

+ 27 - 0
netbox/virtualization/migrations/0057_alter_cluster__region_alter_cluster__site_group.py

@@ -0,0 +1,27 @@
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dcim', '0239_add_portmapping_objectchange'),
+        ('virtualization', '0056_virtualmachine_render_config_permission'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='cluster',
+            name='_region',
+            field=models.ForeignKey(
+                blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='dcim.region'
+            ),
+        ),
+        migrations.AlterField(
+            model_name='cluster',
+            name='_site_group',
+            field=models.ForeignKey(
+                blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='dcim.sitegroup'
+            ),
+        ),
+    ]

+ 52 - 1
netbox/virtualization/tests/test_models.py

@@ -3,12 +3,63 @@ from decimal import Decimal
 from django.core.exceptions import ValidationError
 from django.test import TestCase
 
-from dcim.models import Platform, Site
+from dcim.models import Platform, Region, Site, SiteGroup
 from tenancy.models import Tenant
 from utilities.testing import create_test_device
 from virtualization.models import *
 
 
+class ClusterTestCase(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.cluster_type = ClusterType.objects.create(name='Cluster Type 1', slug='cluster-type-1')
+
+    # Regression test for #22682
+    def test_deleting_site_group_does_not_delete_cluster_scoped_to_member_site(self):
+        sitegroup = SiteGroup.objects.create(name='Site Group 1', slug='site-group-1')
+        site = Site.objects.create(name='Site 1', slug='site-1', group=sitegroup)
+        cluster = Cluster.objects.create(name='Cluster 1', type=self.cluster_type, scope=site)
+
+        sitegroup.delete()
+
+        site.refresh_from_db()
+        cluster.refresh_from_db()
+        self.assertIsNone(site.group)
+        self.assertEqual(cluster.scope, site)
+        self.assertIsNone(cluster._site_group_id)
+
+    # Regression test for #22682
+    def test_deleting_region_does_not_delete_cluster_scoped_to_member_site(self):
+        region = Region.objects.create(name='Region 1', slug='region-1')
+        site = Site.objects.create(name='Site 2', slug='site-2', region=region)
+        cluster = Cluster.objects.create(name='Cluster 2', type=self.cluster_type, scope=site)
+
+        region.delete()
+
+        site.refresh_from_db()
+        cluster.refresh_from_db()
+        self.assertIsNone(site.region)
+        self.assertIsNone(cluster._region_id)
+        self.assertEqual(cluster.scope, site)
+
+    def test_deleting_site_group_scoped_to_it_directly_still_deletes_cluster(self):
+        sitegroup = SiteGroup.objects.create(name='Site Group 2', slug='site-group-2')
+        cluster = Cluster.objects.create(name='Cluster 3', type=self.cluster_type, scope=sitegroup)
+
+        sitegroup.delete()
+
+        self.assertFalse(Cluster.objects.filter(pk=cluster.pk).exists())
+
+    def test_deleting_region_scoped_to_it_directly_still_deletes_cluster(self):
+        region = Region.objects.create(name='Region 2', slug='region-2')
+        cluster = Cluster.objects.create(name='Cluster 4', type=self.cluster_type, scope=region)
+
+        region.delete()
+
+        self.assertFalse(Cluster.objects.filter(pk=cluster.pk).exists())
+
+
 class VirtualMachineTypeTestCase(TestCase):
 
     @classmethod

+ 27 - 0
netbox/wireless/migrations/0020_alter_wirelesslan__region_and_more.py

@@ -0,0 +1,27 @@
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dcim', '0239_add_portmapping_objectchange'),
+        ('wireless', '0019_default_ordering_indexes'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='wirelesslan',
+            name='_region',
+            field=models.ForeignKey(
+                blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='dcim.region'
+            ),
+        ),
+        migrations.AlterField(
+            model_name='wirelesslan',
+            name='_site_group',
+            field=models.ForeignKey(
+                blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='dcim.sitegroup'
+            ),
+        ),
+    ]

+ 49 - 2
netbox/wireless/tests/test_models.py

@@ -5,11 +5,58 @@ from django.test import RequestFactory, TestCase
 
 from core.models import ObjectChange
 from dcim.choices import InterfaceTypeChoices
-from dcim.models import Interface
+from dcim.models import Interface, Region, Site, SiteGroup
 from netbox.context_managers import event_tracking
 from users.models import User
 from utilities.testing import create_test_device
-from wireless.models import WirelessLink
+from wireless.models import WirelessLAN, WirelessLink
+
+
+class WirelessLANTestCase(TestCase):
+
+    # Regression test for #22682
+    def test_deleting_site_group_does_not_delete_wirelesslan_scoped_to_member_site(self):
+        sitegroup = SiteGroup.objects.create(name='Site Group 1', slug='site-group-1')
+        site = Site.objects.create(name='Site 1', slug='site-1', group=sitegroup)
+        wlan = WirelessLAN.objects.create(ssid='WLAN 1', scope=site)
+
+        sitegroup.delete()
+
+        site.refresh_from_db()
+        wlan.refresh_from_db()
+        self.assertIsNone(site.group)
+        self.assertEqual(wlan.scope, site)
+        self.assertIsNone(wlan._site_group_id)
+
+    # Regression test for #22682
+    def test_deleting_region_does_not_delete_wirelesslan_scoped_to_member_site(self):
+        region = Region.objects.create(name='Region 1', slug='region-1')
+        site = Site.objects.create(name='Site 2', slug='site-2', region=region)
+        wlan = WirelessLAN.objects.create(ssid='WLAN 2', scope=site)
+
+        region.delete()
+
+        site.refresh_from_db()
+        wlan.refresh_from_db()
+        self.assertIsNone(site.region)
+        self.assertEqual(wlan.scope, site)
+        self.assertIsNone(wlan._region_id)
+
+    def test_deleting_site_group_scoped_to_it_directly_still_deletes_wirelesslan(self):
+        sitegroup = SiteGroup.objects.create(name='Site Group 2', slug='site-group-2')
+        wlan = WirelessLAN.objects.create(ssid='WLAN 3', scope=sitegroup)
+
+        sitegroup.delete()
+
+        self.assertFalse(WirelessLAN.objects.filter(pk=wlan.pk).exists())
+
+    def test_deleting_region_scoped_to_it_directly_still_deletes_wirelesslan(self):
+        region = Region.objects.create(name='Region 2', slug='region-2')
+        wlan = WirelessLAN.objects.create(ssid='WLAN 4', scope=region)
+
+        region.delete()
+
+        self.assertFalse(WirelessLAN.objects.filter(pk=wlan.pk).exists())
 
 
 class WirelessLinkTestCase(TestCase):