Fixes #943: Partial update of custom fields is not idempotent (#944)

It is important that the object's custom fields dictionary contains only those keys that were given by the user. Otherwise the following dictionary comparison does not work and leads to the result, that there is a difference in state even when the entries already have the desired state. The issue is caused, because the comparison takes also those entries into account that were not given by the user.

Signed-off-by: Christoph Fiehe <c.fiehe@eurodata.de>

Author: cfiehe

plugins/module_utils/netbox_utils.py

@@ -1270,22 +1270,16 @@ def _update_netbox_object(self, data):
         NetBox object and the Ansible diff.
         """
         serialized_nb_obj = self.nb_object.serialize()
-        updated_obj = serialized_nb_obj.copy()
-
-        if serialized_nb_obj.get("custom_fields"):
+        if "custom_fields" in serialized_nb_obj:
+            custom_fields = serialized_nb_obj.get("custom_fields", {})
+            shared_keys = custom_fields.keys() & data.get("custom_fields", {}).keys()
             serialized_nb_obj["custom_fields"] = {
-                key: value
-                for key, value in serialized_nb_obj["custom_fields"].items()
-                if value is not None
-            }
-
-        if updated_obj.get("custom_fields"):
-            updated_obj["custom_fields"] = {
-                key: value
-                for key, value in updated_obj["custom_fields"].items()
-                if value is not None
+                key: custom_fields[key]
+                for key in shared_keys
+                if custom_fields[key] is not None
             }
 
+        updated_obj = serialized_nb_obj.copy()
         updated_obj.update(data)
 
         if serialized_nb_obj.get("tags") and data.get("tags"):

tests/unit/module_utils/test_netbox_base_class.py

@@ -54,6 +54,11 @@ def fixture_arg_spec():
             "manufacturer": "Cisco",
             "site": "Test Site",
             "asset_tag": "1001",
+            "custom_fields": {
+                "Key1": "Value1",
+                "Key2": "Value2",
+                "Key3": "Value3",
+            },
         },
         "state": "present",
         "validate_certs": False,
@@ -70,6 +75,11 @@ def normalized_data():
         "manufacturer": "cisco",
         "site": "test-site",
         "asset_tag": "1001",
+        "custom_fields": {
+            "Key1": "Value1",
+            "Key2": "Value2",
+            "Key3": "Value3",
+        },
     }
 
 
@@ -142,14 +152,28 @@ def mock_netbox_module(mocker, mock_ansible_module, find_ids_return):
 def changed_serialized_obj(nb_obj_mock):
     changed_serialized_obj = nb_obj_mock.serialize().copy()
     changed_serialized_obj["name"] += " (modified)"
+    changed_serialized_obj["custom_fields"] = {
+        "Key1": "NewValue1",
+    }
 
     return changed_serialized_obj
 
 
 @pytest.fixture
 def on_update_diff(mock_netbox_module, nb_obj_mock, changed_serialized_obj):
     return mock_netbox_module._build_diff(
-        before={"name": "Test Device1"}, after={"name": "Test Device1 (modified)"}
+        before={
+            "name": "Test Device1",
+            "custom_fields": {
+                "Key1": "Value1",
+            },
+        },
+        after={
+            "name": "Test Device1 (modified)",
+            "custom_fields": {
+                "Key1": "NewValue1",
+            },
+        },
     )