From 1d4f828b93550a09b4842c7a985f5394df6ef2a9 Mon Sep 17 00:00:00 2001 From: jeremystretch Date: Tue, 27 Sep 2022 16:19:39 -0400 Subject: [PATCH] Device/VM unique constraints ignore case for name field --- .../migrations/0162_unique_constraints.py | 5 ++-- netbox/dcim/models/devices.py | 5 ++-- netbox/dcim/tests/test_models.py | 21 +++++++++++++++ .../migrations/0033_unique_constraints.py | 5 ++-- netbox/virtualization/models.py | 7 ++--- netbox/virtualization/tests/test_models.py | 26 ++++++++++++++++--- 6 files changed, 56 insertions(+), 13 deletions(-) diff --git a/netbox/dcim/migrations/0162_unique_constraints.py b/netbox/dcim/migrations/0162_unique_constraints.py index 5dac7039c..d52dbb6c9 100644 --- a/netbox/dcim/migrations/0162_unique_constraints.py +++ b/netbox/dcim/migrations/0162_unique_constraints.py @@ -1,4 +1,5 @@ from django.db import migrations, models +import django.db.models.functions.text class Migration(migrations.Migration): @@ -170,11 +171,11 @@ class Migration(migrations.Migration): ), migrations.AddConstraint( model_name='device', - constraint=models.UniqueConstraint(fields=('name', 'site', 'tenant'), name='dcim_device_unique_name_site_tenant'), + constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('name'), models.F('site'), models.F('tenant'), name='dcim_device_unique_name_site_tenant'), ), migrations.AddConstraint( model_name='device', - constraint=models.UniqueConstraint(condition=models.Q(('tenant__isnull', True)), fields=('name', 'site'), name='dcim_device_unique_name_site', violation_error_message='Device name must be unique per site.'), + constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('name'), models.F('site'), condition=models.Q(('tenant__isnull', True)), name='dcim_device_unique_name_site', violation_error_message='Device name must be unique per site.'), ), migrations.AddConstraint( model_name='device', diff --git a/netbox/dcim/models/devices.py b/netbox/dcim/models/devices.py index 79cc8c86b..d0d9001ad 100644 --- a/netbox/dcim/models/devices.py +++ b/netbox/dcim/models/devices.py @@ -8,6 +8,7 @@ from django.core.exceptions import ValidationError from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models from django.db.models import F, ProtectedError +from django.db.models.functions import Lower from django.urls import reverse from django.utils.safestring import mark_safe @@ -662,11 +663,11 @@ class Device(NetBoxModel, ConfigContextModel): ordering = ('_name', 'pk') # Name may be null constraints = ( models.UniqueConstraint( - fields=('name', 'site', 'tenant'), + Lower('name'), 'site', 'tenant', name='%(app_label)s_%(class)s_unique_name_site_tenant' ), models.UniqueConstraint( - fields=('name', 'site'), + Lower('name'), 'site', name='%(app_label)s_%(class)s_unique_name_site', condition=Q(tenant__isnull=True), violation_error_message="Device name must be unique per site." diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index acde02ecd..460a5e252 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -399,6 +399,27 @@ class DeviceTestCase(TestCase): self.assertEqual(Device.objects.filter(name__isnull=True).count(), 2) + def test_device_name_case_sensitivity(self): + + device1 = Device( + site=self.site, + device_type=self.device_type, + device_role=self.device_role, + name='device 1' + ) + device1.save() + + device2 = Device( + site=device1.site, + device_type=device1.device_type, + device_role=device1.device_role, + name='DEVICE 1' + ) + + # Uniqueness validation for name should ignore case + with self.assertRaises(ValidationError): + device2.full_clean() + def test_device_duplicate_names(self): device1 = Device( diff --git a/netbox/virtualization/migrations/0033_unique_constraints.py b/netbox/virtualization/migrations/0033_unique_constraints.py index 4667dcbd3..0624d3607 100644 --- a/netbox/virtualization/migrations/0033_unique_constraints.py +++ b/netbox/virtualization/migrations/0033_unique_constraints.py @@ -1,4 +1,5 @@ from django.db import migrations, models +import django.db.models.functions.text class Migration(migrations.Migration): @@ -30,11 +31,11 @@ class Migration(migrations.Migration): ), migrations.AddConstraint( model_name='virtualmachine', - constraint=models.UniqueConstraint(fields=('name', 'cluster', 'tenant'), name='virtualization_virtualmachine_unique_name_cluster_tenant'), + constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('name'), models.F('cluster'), models.F('tenant'), name='virtualization_virtualmachine_unique_name_cluster_tenant'), ), migrations.AddConstraint( model_name='virtualmachine', - constraint=models.UniqueConstraint(condition=models.Q(('tenant__isnull', True)), fields=('name', 'cluster'), name='virtualization_virtualmachine_unique_name_cluster', violation_error_message='Virtual machine name must be unique per site.'), + constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('name'), models.F('cluster'), condition=models.Q(('tenant__isnull', True)), name='virtualization_virtualmachine_unique_name_cluster', violation_error_message='Virtual machine name must be unique per cluster.'), ), migrations.AddConstraint( model_name='vminterface', diff --git a/netbox/virtualization/models.py b/netbox/virtualization/models.py index 5a1c361c2..37fcd68ae 100644 --- a/netbox/virtualization/models.py +++ b/netbox/virtualization/models.py @@ -3,6 +3,7 @@ from django.core.exceptions import ValidationError from django.core.validators import MinValueValidator from django.db import models from django.db.models import Q +from django.db.models.functions import Lower from django.urls import reverse from dcim.models import BaseInterface, Device @@ -318,14 +319,14 @@ class VirtualMachine(NetBoxModel, ConfigContextModel): ordering = ('_name', 'pk') # Name may be non-unique constraints = ( models.UniqueConstraint( - fields=('name', 'cluster', 'tenant'), + Lower('name'), 'cluster', 'tenant', name='%(app_label)s_%(class)s_unique_name_cluster_tenant' ), models.UniqueConstraint( - fields=('name', 'cluster'), + Lower('name'), 'cluster', name='%(app_label)s_%(class)s_unique_name_cluster', condition=Q(tenant__isnull=True), - violation_error_message="Virtual machine name must be unique per site." + violation_error_message="Virtual machine name must be unique per cluster." ), ) diff --git a/netbox/virtualization/tests/test_models.py b/netbox/virtualization/tests/test_models.py index df5816efa..bf0571d3d 100644 --- a/netbox/virtualization/tests/test_models.py +++ b/netbox/virtualization/tests/test_models.py @@ -8,12 +8,14 @@ from tenancy.models import Tenant class VirtualMachineTestCase(TestCase): - def test_vm_duplicate_name_per_cluster(self): + @classmethod + def setUpTestData(cls): cluster_type = ClusterType.objects.create(name='Cluster Type 1', slug='cluster-type-1') - cluster = Cluster.objects.create(name='Cluster 1', type=cluster_type) + Cluster.objects.create(name='Cluster 1', type=cluster_type) + def test_vm_duplicate_name_per_cluster(self): vm1 = VirtualMachine( - cluster=cluster, + cluster=Cluster.objects.first(), name='Test VM 1' ) vm1.save() @@ -43,7 +45,7 @@ class VirtualMachineTestCase(TestCase): vm2.save() def test_vm_mismatched_site_cluster(self): - cluster_type = ClusterType.objects.create(name='Cluster Type 1', slug='cluster-type-1') + cluster_type = ClusterType.objects.first() sites = ( Site(name='Site 1', slug='site-1'), @@ -71,3 +73,19 @@ class VirtualMachineTestCase(TestCase): # VM with cluster site but no direct site should fail with self.assertRaises(ValidationError): VirtualMachine(name='vm1', site=None, cluster=clusters[0]).full_clean() + + def test_vm_name_case_sensitivity(self): + vm1 = VirtualMachine( + cluster=Cluster.objects.first(), + name='virtual machine 1' + ) + vm1.save() + + vm2 = VirtualMachine( + cluster=vm1.cluster, + name='VIRTUAL MACHINE 1' + ) + + # Uniqueness validation for name should ignore case + with self.assertRaises(ValidationError): + vm2.full_clean()