Fixes #22848: Ensure deterministic ordering for duplicate IP addresses (#22849)

netbox/ipam/managers.py

@@ -13,5 +13,9 @@ class IPAddressManager(Manager.from_queryset(IPAddressQuerySet)):
         address. We can use HOST() to extract just the host portion of the address (ignoring its mask), but we must
         then re-cast this value to INET() so that records will be ordered properly. We are essentially re-casting each
         IP address as a /32 or /128.
+
+        Host addresses are not unique, so we must also order by primary key to guarantee a stable, total ordering.
+        Without this tiebreaker, PostgreSQL is free to return tied rows in a different order from one query to the
+        next, which causes objects to be duplicated or omitted across paginated requests.
         """
-        return super().get_queryset().order_by(Inet(Host('address')))
+        return super().get_queryset().order_by(Inet(Host('address')), 'pk')

netbox/ipam/migrations/0094_ipaddress_host_index.py

@@ -0,0 +1,33 @@
+import django.db.models.functions.comparison
+from django.db import migrations, models
+
+import ipam.fields
+import ipam.lookups
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('ipam', '0093_alter_prefix__region_alter_prefix__site_group'),
+    ]
+
+    operations = [
+        # Replace the host address index with a composite index which also covers the primary key, so that it can
+        # satisfy the default ordering of IPAddress outright. Note that the existing index must be dropped rather
+        # than retained alongside the new one: with both present, PostgreSQL continues to select the narrower index
+        # and applies an incremental sort atop it.
+        migrations.RemoveIndex(
+            model_name='ipaddress',
+            name='ipam_ipaddress_host',
+        ),
+        migrations.AddIndex(
+            model_name='ipaddress',
+            index=models.Index(
+                django.db.models.functions.comparison.Cast(
+                    ipam.lookups.Host('address'),
+                    output_field=ipam.fields.IPAddressField(),
+                ),
+                models.F('id'),
+                name='ipam_ipaddress_host',
+            ),
+        ),
+    ]

netbox/ipam/models/ip.py

@@ -1023,8 +1023,13 @@ class IPAddress(ContactsMixin, PrimaryModel):
     class Meta:
         ordering = ('address', 'pk')  # address may be non-unique
         indexes = (
-            models.Index(fields=('address', 'id')),  # Default ordering
-            models.Index(Cast(Host('address'), output_field=IPAddressField()), name='ipam_ipaddress_host'),
+            models.Index(fields=('address', 'id')),
+            # Default ordering (see IPAddressManager). The primary key must be included so that the index can
+            # satisfy the ordering outright; without it PostgreSQL falls back to an incremental sort, which
+            # measurably slows deep pagination.
+            models.Index(
+                Cast(Host('address'), output_field=IPAddressField()), F('id'), name='ipam_ipaddress_host'
+            ),
             models.Index(fields=('assigned_object_type', 'assigned_object_id')),
         )
         verbose_name = _('IP address')

netbox/ipam/tests/test_ordering.py

@@ -198,3 +198,46 @@ class IPAddressOrderingTestCase(OrderingTestBase):
 
         # Test
         self._compare(IPAddress.objects.all(), addresses)
+
+    def test_duplicate_address_ordering(self):
+        """
+        Host addresses are not unique, so tied addresses must be ordered by primary key to yield a stable, total
+        ordering. Without a tiebreaker the database may return tied rows in a different order from one query to the
+        next, duplicating or omitting objects across paginated requests.
+        """
+        # Create several duplicates of each address, interleaved so that primary key order does not follow
+        # address order.
+        addresses = [
+            IPAddress(status=IPAddressStatusChoices.STATUS_ACTIVE, address=netaddr.IPNetwork(f'10.0.{i}.1/24'))
+            for _ in range(4)
+            for i in range(100)
+        ]
+        IPAddress.objects.bulk_create(addresses)
+
+        pks = list(IPAddress.objects.values_list('pk', flat=True))
+        expected = [
+            ip.pk for ip in sorted(IPAddress.objects.all(), key=lambda ip: (ip.address.ip, ip.pk))
+        ]
+
+        self.assertEqual(pks, expected)
+
+    def test_duplicate_address_pagination(self):
+        """
+        Paginating over duplicate addresses must not return the same object on two pages, nor omit any object.
+        """
+        addresses = [
+            IPAddress(status=IPAddressStatusChoices.STATUS_ACTIVE, address=netaddr.IPNetwork(f'10.0.{i}.1/24'))
+            for _ in range(4)
+            for i in range(100)
+        ]
+        IPAddress.objects.bulk_create(addresses)
+
+        queryset = IPAddress.objects.values_list('pk', flat=True)
+        page_size = 37
+        paginated = []
+        for offset in range(0, len(addresses), page_size):
+            paginated.extend(queryset[offset:offset + page_size])
+
+        self.assertEqual(len(paginated), len(addresses))
+        self.assertEqual(len(set(paginated)), len(addresses))
+        self.assertEqual(paginated, list(queryset))