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):