Natural key for IPAddressToInterface is incorrect

gsnider2195 commented 1 year ago

Environment

Nautobot version (Docker tag too if applicable): v2.0.0-rc.2
Python version:
Database platform, version:
Middleware(s):

Steps to Reproduce

>>> IPAddressToInterface.natural_key_field_lookups

Expected Behavior

The vm_interface field is included in the natural key because the model's unique together is:

        unique_together = [
            ["ip_address", "interface"],
            ["ip_address", "vm_interface"],
        ]

Observed Behavior

It's not included because our natural key field lookup only uses the first row of the Meta.unique_together:

>>> IPAddressToInterface.objects.filter(vm_interface__isnull=True).first().natural_key()
['Positive reflect.', '10.7.128.1', 'Ethernet1', 'rtr01', None, 'Aisle-13', 'Room-12', 'Floor-03', 'Building-02', 'Campus-01']
>>> IPAddressToInterface.objects.filter(vm_interface__isnull=False).first().natural_key()
['Positive reflect.', '10.7.0.3']

glennmatthews commented 1 year ago

@lampwins @bryanculver This feels to me like it's probably going to become a problem as we address #4249 and #4367. IMHO it should be done for 2.0.0.

Since Interface.natural_key is variable-length (as it includes device__location) and VMInterface.natural_key is fixed-length (['virtual_machine__cluster__name', 'virtual_machine__tenant__name', 'virtual_machine__name', 'name']) the "correct" natural_key for IPAddressToInterface is likely ip_address, vm_interface, interface (which would expand to 'ip_address__namespace', 'ip_address__host', 'vm_interface__virtual_machine__cluster__name', 'vm_interface__virtual_machine__tenant__name', 'vm_interface__virtual_machine__name', 'vm_interface__name', 'interface__name', 'interface__device__name', 'interface__device__tenant__name', 'interface__device__location__name', 'interface__device__location__parent__name', ...

Since this is mostly an internal implementation-detail model rather than something directly user-facing, it might be more convenient/correct to instead define IPAddressToInterface.natural_key as something like ip_address__id, interface__id, vm_interface__id?

bryanculver commented 1 year ago

@lampwins @bryanculver This feels to me like it's probably going to become a problem as we address #4249 and #4367. IMHO it should be done for 2.0.0.

Since Interface.natural_key is variable-length (as it includes device__location) and VMInterface.natural_key is fixed-length (['virtual_machine__cluster__name', 'virtual_machine__tenant__name', 'virtual_machine__name', 'name']) the "correct" natural_key for IPAddressToInterface is likely ip_address, vm_interface, interface (which would expand to 'ip_address__namespace', 'ip_address__host', 'vm_interface__virtual_machine__cluster__name', 'vm_interface__virtual_machine__tenant__name', 'vm_interface__virtual_machine__name', 'vm_interface__name', 'interface__name', 'interface__device__name', 'interface__device__tenant__name', 'interface__device__location__name', 'interface__device__location__parent__name', ...

Since this is mostly an internal implementation-detail model rather than something directly user-facing, it might be more convenient/correct to instead define IPAddressToInterface.natural_key as something like ip_address__id, interface__id, vm_interface__id?

I'm in favor of overloading the natural key for this.

nautobot / nautobot