2
0
Эх сурвалжийг харах

Fixes #21440: Avoid erroneously clearing primary/OOB IP assignments during bulk import/update

Jeremy Stretch 4 өдөр өмнө
parent
commit
c86210f024

+ 26 - 9
netbox/ipam/forms/bulk_import.py

@@ -424,19 +424,36 @@ class IPAddressImportForm(PrimaryModelImportForm):
         # Set as primary for device/VM
         # Set as primary for device/VM
         if self.cleaned_data.get('is_primary') is not None:
         if self.cleaned_data.get('is_primary') is not None:
             parent = self.cleaned_data.get('device') or self.cleaned_data.get('virtual_machine')
             parent = self.cleaned_data.get('device') or self.cleaned_data.get('virtual_machine')
-            parent.snapshot()
-            if self.instance.address.version == 4:
-                parent.primary_ip4 = ipaddress if self.cleaned_data.get('is_primary') else None
-            elif self.instance.address.version == 6:
-                parent.primary_ip6 = ipaddress if self.cleaned_data.get('is_primary') else None
-            parent.save()
+            if self.cleaned_data.get('is_primary'):
+                parent.snapshot()
+                if self.instance.address.version == 4:
+                    parent.primary_ip4 = ipaddress
+                elif self.instance.address.version == 6:
+                    parent.primary_ip6 = ipaddress
+                parent.save()
+            else:
+                # Only clear the primary IP if this IP is currently set as primary
+                if self.instance.address.version == 4 and parent.primary_ip4 == ipaddress:
+                    parent.snapshot()
+                    parent.primary_ip4 = None
+                    parent.save()
+                elif self.instance.address.version == 6 and parent.primary_ip6 == ipaddress:
+                    parent.snapshot()
+                    parent.primary_ip6 = None
+                    parent.save()
 
 
         # Set as OOB for device
         # Set as OOB for device
         if self.cleaned_data.get('is_oob') is not None:
         if self.cleaned_data.get('is_oob') is not None:
             parent = self.cleaned_data.get('device')
             parent = self.cleaned_data.get('device')
-            parent.snapshot()
-            parent.oob_ip = ipaddress if self.cleaned_data.get('is_oob') else None
-            parent.save()
+            if self.cleaned_data.get('is_oob'):
+                parent.snapshot()
+                parent.oob_ip = ipaddress
+                parent.save()
+            elif parent.oob_ip == ipaddress:
+                # Only clear OOB if this IP is currently set as the OOB IP
+                parent.snapshot()
+                parent.oob_ip = None
+                parent.save()
 
 
         return ipaddress
         return ipaddress
 
 

+ 56 - 1
netbox/ipam/tests/test_forms.py

@@ -1,8 +1,10 @@
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.contenttypes.models import ContentType
 from django.test import TestCase
 from django.test import TestCase
 
 
-from dcim.models import Location, Region, Site, SiteGroup
+from dcim.constants import InterfaceTypeChoices
+from dcim.models import Device, DeviceRole, DeviceType, Interface, Location, Manufacturer, Region, Site, SiteGroup
 from ipam.forms import PrefixForm
 from ipam.forms import PrefixForm
+from ipam.forms.bulk_import import IPAddressImportForm
 
 
 
 
 class PrefixFormTestCase(TestCase):
 class PrefixFormTestCase(TestCase):
@@ -41,3 +43,56 @@ class PrefixFormTestCase(TestCase):
             })
             })
 
 
             assert 'data-dynamic-params' not in form.fields['vlan'].widget.attrs
             assert 'data-dynamic-params' not in form.fields['vlan'].widget.attrs
+
+
+class IPAddressImportFormTestCase(TestCase):
+    """Tests for IPAddressImportForm bulk import behavior."""
+
+    @classmethod
+    def setUpTestData(cls):
+        site = Site.objects.create(name='Site 1', slug='site-1')
+        manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
+        device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Model 1', slug='model-1')
+        device_role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1')
+        cls.device = Device.objects.create(
+            name='Device 1',
+            site=site,
+            device_type=device_type,
+            role=device_role,
+        )
+        cls.interface = Interface.objects.create(
+            device=cls.device,
+            name='eth0',
+            type=InterfaceTypeChoices.TYPE_1GE_FIXED,
+        )
+
+    def test_oob_import_not_cleared_by_subsequent_non_oob_row(self):
+        """
+        Regression test for #21440: importing a second IP with is_oob=False should
+        not clear the OOB IP set by a previous row with is_oob=True.
+        """
+        form1 = IPAddressImportForm(data={
+            'address': '10.10.10.1/24',
+            'status': 'active',
+            'device': 'Device 1',
+            'interface': 'eth0',
+            'is_oob': True,
+        })
+        self.assertTrue(form1.is_valid(), form1.errors)
+        ip1 = form1.save()
+
+        self.device.refresh_from_db()
+        self.assertEqual(self.device.oob_ip, ip1)
+
+        form2 = IPAddressImportForm(data={
+            'address': '2001:db8::1/64',
+            'status': 'active',
+            'device': 'Device 1',
+            'interface': 'eth0',
+            'is_oob': False,
+        })
+        self.assertTrue(form2.is_valid(), form2.errors)
+        form2.save()
+
+        self.device.refresh_from_db()
+        self.assertEqual(self.device.oob_ip, ip1, "OOB IP was incorrectly cleared by a row with is_oob=False")