From 0eb1fcc09c1df3ccee3f10026a1e0763b819b814 Mon Sep 17 00:00:00 2001 From: bctiemann Date: Sat, 18 Jul 2026 04:35:08 -0400 Subject: [PATCH] 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. --- netbox/dcim/models/mixins.py | 7 +- netbox/dcim/models/sites.py | 24 ++++++ ...prefix__region_alter_prefix__site_group.py | 27 +++++++ netbox/ipam/tests/test_models.py | 76 ++++++++++++++++++- ...uster__region_alter_cluster__site_group.py | 27 +++++++ netbox/virtualization/tests/test_models.py | 53 ++++++++++++- ...0020_alter_wirelesslan__region_and_more.py | 27 +++++++ netbox/wireless/tests/test_models.py | 51 ++++++++++++- 8 files changed, 286 insertions(+), 6 deletions(-) create mode 100644 netbox/ipam/migrations/0093_alter_prefix__region_alter_prefix__site_group.py create mode 100644 netbox/virtualization/migrations/0057_alter_cluster__region_alter_cluster__site_group.py create mode 100644 netbox/wireless/migrations/0020_alter_wirelesslan__region_and_more.py diff --git a/netbox/dcim/models/mixins.py b/netbox/dcim/models/mixins.py index 2098b8b3b..cf1378596 100644 --- a/netbox/dcim/models/mixins.py +++ b/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 ) diff --git a/netbox/dcim/models/sites.py b/netbox/dcim/models/sites.py index 00bc9a240..f9710b7f8 100644 --- a/netbox/dcim/models/sites.py +++ b/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 diff --git a/netbox/ipam/migrations/0093_alter_prefix__region_alter_prefix__site_group.py b/netbox/ipam/migrations/0093_alter_prefix__region_alter_prefix__site_group.py new file mode 100644 index 000000000..111b4079d --- /dev/null +++ b/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' + ), + ), + ] diff --git a/netbox/ipam/tests/test_models.py b/netbox/ipam/tests/test_models.py index b76a7205b..c20a99bc6 100644 --- a/netbox/ipam/tests/test_models.py +++ b/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): """ diff --git a/netbox/virtualization/migrations/0057_alter_cluster__region_alter_cluster__site_group.py b/netbox/virtualization/migrations/0057_alter_cluster__region_alter_cluster__site_group.py new file mode 100644 index 000000000..07a5f41e6 --- /dev/null +++ b/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' + ), + ), + ] diff --git a/netbox/virtualization/tests/test_models.py b/netbox/virtualization/tests/test_models.py index 1497135c0..38179ffb5 100644 --- a/netbox/virtualization/tests/test_models.py +++ b/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 diff --git a/netbox/wireless/migrations/0020_alter_wirelesslan__region_and_more.py b/netbox/wireless/migrations/0020_alter_wirelesslan__region_and_more.py new file mode 100644 index 000000000..aed582421 --- /dev/null +++ b/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' + ), + ), + ] diff --git a/netbox/wireless/tests/test_models.py b/netbox/wireless/tests/test_models.py index a7589700a..4af0b04b1 100644 --- a/netbox/wireless/tests/test_models.py +++ b/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):