From 06aa7eade43914d4af5c3fbcd927f6f60848115c Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Thu, 10 Apr 2025 14:24:38 -0500 Subject: [PATCH] Address PR feedback, ensure that parent GFK underlying fields are not nullable This changes an existing (within the branch) migration, so if you're testing this you'll need to back up migrations and re-run. --- netbox/ipam/forms/bulk_import.py | 19 +++++++++++------ ...ce_virtual_machine_add_parent_gfk_index.py | 12 +++++++++++ netbox/ipam/models/services.py | 7 +------ netbox/ipam/tests/test_filtersets.py | 21 ++++++++++++++++--- 4 files changed, 44 insertions(+), 15 deletions(-) diff --git a/netbox/ipam/forms/bulk_import.py b/netbox/ipam/forms/bulk_import.py index cdac06e77..d17944674 100644 --- a/netbox/ipam/forms/bulk_import.py +++ b/netbox/ipam/forms/bulk_import.py @@ -571,6 +571,10 @@ class ServiceImportForm(NetBoxModelImportForm): to_field_name='name', help_text=_('Parent object name') ) + parent_object_id = forms.IntegerField( + required=False, + help_text=_('Parent object ID'), + ) protocol = CSVChoiceField( label=_('Protocol'), choices=ServiceProtocolChoices, @@ -586,8 +590,7 @@ class ServiceImportForm(NetBoxModelImportForm): class Meta: model = Service fields = ( - 'parent_object_type', 'ipaddresses', 'name', 'protocol', 'ports', 'description', 'comments', - 'tags', 'parent_object_id', + 'ipaddresses', 'name', 'protocol', 'ports', 'description', 'comments', 'tags', ) def __init__(self, data=None, *args, **kwargs): @@ -619,16 +622,20 @@ class ServiceImportForm(NetBoxModelImportForm): parent = parent_ct.model_class().objects.filter(id=parent_id).first() self.cleaned_data['parent'] = parent else: - # If a parent object type is passed and we've made it to here, then raise a validation - # error since an associated parent object or parent object id has nto be passed + # If a parent object type is passed and we've made it here, then raise a validation + # error since an associated parent object or parent object id has not been passed raise forms.ValidationError( - _("One of parent or parent_object_id needs to be included with parent_object_type") + _("One of parent or parent_object_id must be included with parent_object_type") ) + # making sure parent is defined. In cases where an import is resulting in an update, the + # import data might not include the parent object and so the above logic might not be + # triggered + parent = self.cleaned_data.get('parent') for ip_address in self.cleaned_data.get('ipaddresses', []): if not ip_address.assigned_object or getattr(ip_address.assigned_object, 'parent_object') != parent: raise forms.ValidationError( - _("{ip} is not assigned to this device/VM.").format(ip=ip_address) + _("{ip} is not assigned to this parent.").format(ip=ip_address) ) return self.cleaned_data diff --git a/netbox/ipam/migrations/0080_remove_service_device_virtual_machine_add_parent_gfk_index.py b/netbox/ipam/migrations/0080_remove_service_device_virtual_machine_add_parent_gfk_index.py index a0d049d61..434e1317a 100644 --- a/netbox/ipam/migrations/0080_remove_service_device_virtual_machine_add_parent_gfk_index.py +++ b/netbox/ipam/migrations/0080_remove_service_device_virtual_machine_add_parent_gfk_index.py @@ -18,6 +18,18 @@ class Migration(migrations.Migration): model_name='service', name='virtual_machine', ), + migrations.AlterField( + model_name='service', + name='parent_object_id', + field=models.PositiveBigIntegerField(), + ), + migrations.AlterField( + model_name='service', + name='parent_object_type', + field=models.ForeignKey( + on_delete=models.deletion.PROTECT, related_name='+', to='contenttypes.contenttype' + ), + ), migrations.AddIndex( model_name='service', index=models.Index( diff --git a/netbox/ipam/models/services.py b/netbox/ipam/models/services.py index 39ad9d32a..2afd16076 100644 --- a/netbox/ipam/models/services.py +++ b/netbox/ipam/models/services.py @@ -68,13 +68,8 @@ class Service(ContactsMixin, ServiceBase, PrimaryModel): to='contenttypes.ContentType', on_delete=models.PROTECT, related_name='+', - blank=True, - null=True - ) - parent_object_id = models.PositiveBigIntegerField( - blank=True, - null=True ) + parent_object_id = models.PositiveBigIntegerField() parent = GenericForeignKey( ct_field='parent_object_type', fk_field='parent_object_id' diff --git a/netbox/ipam/tests/test_filtersets.py b/netbox/ipam/tests/test_filtersets.py index 8779741df..9c500e002 100644 --- a/netbox/ipam/tests/test_filtersets.py +++ b/netbox/ipam/tests/test_filtersets.py @@ -1258,9 +1258,24 @@ class IPAddressTestCase(TestCase, ChangeLoggedFilterSetTests): IPAddress.objects.bulk_create(ipaddresses) services = ( - Service(name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]), - Service(name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]), - Service(name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]), + Service( + parent=devices[0], + name='Service 1', + protocol=ServiceProtocolChoices.PROTOCOL_TCP, + ports=[1], + ), + Service( + parent=devices[1], + name='Service 2', + protocol=ServiceProtocolChoices.PROTOCOL_TCP, + ports=[1], + ), + Service( + parent=devices[2], + name='Service 3', + protocol=ServiceProtocolChoices.PROTOCOL_TCP, + ports=[1], + ), ) Service.objects.bulk_create(services) services[0].ipaddresses.add(ipaddresses[0])