[Bug]: Netbox ansible custom_fields are not idempotent

[DevFinGitGovSecChatOps]

Ansible NetBox Collection version

3.7.1

Ansible version

ansible [core 2.13.1]
  config file = None
  configured module search path = ['/home/user/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /builds/<redacted-path>/.ve/lib64/python3.8/site-packages/ansible
  ansible collection location = /home/user/.ansible/collections:/usr/share/ansible/collections
  executable location = /builds/<redacted-path>/.ve/bin/ansible
  python version = 3.8.13 (default, Apr  5 2022, 17:15:15) [GCC 9.1.1 20190605 (Red Hat 9.1.1-2)]
  jinja version = 3.1.2
  libyaml = True

NetBox version

• v3.2.0
• v3.2.5

Python version

3.8

Steps to Reproduce

ansible-playbook main.playbook --diff

Note:
cf1 and cf2 custom_fields are defined in netbox but not used in this run

- name: "Test SoT stuff"
  connection: local
  hosts: localhost
  gather_facts: false
  tasks:
    - name: "Setup prefix"
      netbox.netbox.netbox_prefix:
        netbox_url: "{{ netbox_url }}"
        netbox_token: "{{ netbox_token }}"
        data:
          prefix: "1.2.3.0/24"
          status: "Active"
          custom_fields:
            cf3: "something"
            cf4: "something_else"

Expected Behavior

When running a playbook with custom_fields, I would expect the modules to be idempotent.
In this example I am defining two custom_fields in a prefix that has four custom_fields defined in netbox.

If nothing changed, I would expect the diff to be unchanged.

TASK [Setup prefix] ************************************************************
ok: [localhost] => (item={'prefix': '1.2.3.0/24', 'status': 'Active', 'custom_fields': {'cf3': 'something', 'cf4': 'something_else'}})
PLAY RECAP *********************************************************************

Observed Behavior

However, instead of returning an "OK" Status, the run suggests there have been changes.

TASK [Setup prefix] ************************************************************
--- before
+++ after
@@ -1,5 +1,3 @@
custom_fields:
-  cf1: null
-  cf2: null
   cf3: something
   cf4: something_else
changed: [localhost] => (item={'prefix': '1.2.3.0/24', 'status': 'Active', 'custom_fields': {'cf3': 'something', 'cf4': 'something_else'}})
PLAY RECAP *********************************************************************

This is suboptimal, since I want to run a pipeline that checks for changes in netbox. It is hard to focus on what really changed, since the custom_fields suggest a change where none happened.

[DevFinGitGovSecChatOps]

Also a change is logged without a diff. This shouldn't be triggered as well I think.

[DevFinGitGovSecChatOps]

Another observered behaviour:
When I change one value of a custom_field of an object, the changelog will show in the diff, that all of them changed.

In this example I only changed cf3

[sc68cal]

I can also +1 this bug, there are custom fields that I set in interfaces and it results in Ansible marking it as a changed item.

[rodvand]

I think there's another issue for this as well. Not sure how to tackle it - any suggestions on logic to handle this?

[sc68cal]

Another observered behaviour: When I change one value of a custom_field of an object, the changelog will show in the diff, that all of them changed.

In this example I only changed cf3

I think this behavior is correct, since custom_fields is a dictionary, and you changed one of the key/value pairs inside it, so the whole dictionary did change.

[DevFinGitGovSecChatOps]

Another observered behaviour: When I change one value of a custom_field of an object, the changelog will show in the diff, that all of them changed.
In this example I only changed cf3

I think this behavior is correct, since custom_fields is a dictionary, and you changed one of the key/value pairs inside it, so the whole dictionary did change.

I'd argue that the values outside of custom_fields are also dicts, yet they get validated correctly as seen in this example:

The difference seems to be that in the module fields the diff will check all key-value pairs seperately but in the custom_fields it only checks for any change in the dict.

Might a solution be to iterate through the values in a custom_field when looking for changed values instead of all custom_fields as a whole?

[sc68cal]

I'd argue that the values outside of custom_fields are also dicts,

Unfortunately, that is not correct. Fields like description are CharFields while CustomFields are much more complex.

[DevFinGitGovSecChatOps]

Sorry, I didn't express myself correctly. What I ment to say is (maybe I'm wrong?) that the content of data: is a dict:

data:
  prefix: "1.2.3.0/24"
  status: "Active"
  description: "Something"

And the custom_fields are a dict in a dict:

data:
  prefix: "1.2.3.0/24"
  status: "Active"
  description: "Something"
  custom_fields:
    cf3: "something"
    cf4: "something_else"

I wanted to say that there already seems to be a mechanism to enumerate through a dict and correctly check for diffs. Maybe that mechanism can be applied to enumerate through the values of the custom_fields (dict in dict) as well?

[cfiehe]

I have created a pull request that should fix the issue described here. The main problem is, that fields, which are unset, are compared with the target state, which does not contain those fields. You can see the issue here:

TASK [netbox : vm | Ensure virtual machine 'Test Virtual Machine'] *************
--- before
+++ after
@@ -1,48 +1,9 @@
 {
     "custom_fields": {
-        "VM_NetworkAdapter_1": null,
-        "VM_NetworkAdapter_1_IP": null,
-        "VM_NetworkAdapter_2": null,
-        "VM_NetworkAdapter_2_IP": null,
-        "VM_NetworkAdapter_3": null,
-        "VM_NetworkAdapter_3_IP": null,
-        "VM_NetworkAdapter_4": null,
-        "VM_NetworkAdapter_4_IP": null,
-        "VM_NetworkAdapter_5": null,
-        "VM_NetworkAdapter_5_IP": null,
-        "VM_application": null,
-        "VM_costcentre": null,
-        "VM_costunit": null,
         "VM_critical": true,
-        "VM_folder": null,
-        "VM_guestosfullname": null,
         "VM_ha_type": "self",
-        "VM_id": null,
-        "VM_impact": null,
-        "VM_licence": null,
-        "VM_orga": null,
-        "VM_owner": "LOC",
         "VM_priority": "ASAP",
-        "VM_prodtype": "Prod",
-        "VM_site": null,
-        "VM_siteaffinity": null,
-        "VM_sla": "99.50%",
-        "VM_team": null,
-        "VM_uuid": null,
-        "VMupdater_add": null,
-        "VMupdater_off": null,
-        "VMupdater_upd": null,
-        "monitoring_hostgroups": null,
-        "monitoring_notification_mail": null,
-        "monitoring_notification_mattermost": null,
-        "monitoring_notification_sms": null,
-        "monitoring_notification_ticket": null,
-        "monitoring_serviceset_app_vm": null,
-        "monitoring_serviceset_app_vm_priority": null,
-        "monitoring_serviceset_os_vm": null,
-        "monitoring_serviceset_os_vm_priority": null,
-        "monitoring_zone": null,
-        "vm_osversion": null
+        "VM_prodtype": "Prod"
     }
 }

We have only one change here VM_prodtype: Prod, but Ansible says that all unset values of the current state have not equivalent counterpart in the target state and must be removed. That is actually true, so the idea is only to include values in the current state which have no null value. In this case we compare values that have a "real" value either in the current state or in the target state and the issue is gone.

With the fix we get only this:

TASK [netbox : vm | Ensure virtual machine 'Test Virtual Machine'] *************
--- before
+++ after
@@ -5,7 +5,7 @@
         "VM_ha_type": "self",
         "VM_owner": "LOC",
         "VM_priority": "ASAP",
+        "VM_prodtype": "Prod",
         "VM_sla": "99.50%",
     }

And all following runs give you:

TASK [netbox : vm | Ensure virtual machine 'Test Virtual Machine'] *************
ok: [localhost]