fix: compare unordered m2m list fields as sets in the generic diff engine (INT-411)

changelogs/fragments/int-411-unordered-m2m-list-set-comparison.yml

@@ -0,0 +1,17 @@
+---
+bugfixes:
+  - >-
+    The generic update-comparison engine now compares known many-to-many list
+    fields as unordered sets, matching the behaviour previously special-cased
+    for ``tags`` only. NetBox returns these fields in a non-deterministic
+    order, so a task whose list contents already matched NetBox state was
+    reported as ``changed`` on every run and ``--check`` mode produced false
+    positives. The fields treated as sets are ``object_types``,
+    ``tagged_vlans``, the ``netbox_config_context`` assignment scopes
+    (``regions``, ``sites``, ``site_groups``, ``locations``, ``device_types``,
+    ``roles``, ``platforms``, ``cluster_types``, ``cluster_groups``,
+    ``clusters``, ``tenant_groups``, ``tenants``), the route-target lists
+    (``import_targets``, ``export_targets``), and the user/permission lists
+    (``permissions``, ``groups``, ``actions``). ``netbox_tag`` with
+    ``object_types`` is now idempotent
+    (https://github.com/netbox-community/ansible_modules/issues/1486).

plugins/module_utils/netbox_users.py

@@ -8,16 +8,14 @@
 from ansible_collections.netbox.netbox.plugins.module_utils.netbox_utils import (
     NetboxModule,
     ENDPOINT_NAME_MAPPING,
+    LIST_AS_SET_KEYS,
 )
 
 NB_GROUPS = "groups"
 NB_PERMISSIONS = "permissions"
 NB_TOKENS = "tokens"
 NB_USERS = "users"
 
-# These suboptions are lists, but need to be modeled as sets for comparison purposes.
-LIST_AS_SET_KEYS = set(["permissions", "groups", "actions", "object_types"])
-
 
 class NetboxUsersModule(NetboxModule):
     def __init__(self, module, endpoint):

plugins/module_utils/netbox_utils.py

@@ -737,6 +737,42 @@
     "virtualization.clustergroup": "cluster_groups",
 }
 
+# Many-to-many list fields that NetBox returns in a non-deterministic order.
+# The update-comparison engine must treat these as unordered sets, otherwise a
+# payload whose contents are identical to NetBox state but listed in a different
+# order is wrongly reported as a change (breaking idempotency and --check mode).
+# Adding a field name here is safe even if it never appears on a given endpoint:
+# the comparison only converts a key when both the existing object and the
+# incoming data carry it, and non-hashable values fall back to ordered
+# comparison. See INT-411 and GitHub issue #1486.
+LIST_AS_SET_KEYS = set(
+    [
+        "tags",
+        "object_types",
+        "tagged_vlans",
+        # netbox_users (permissions / groups / tokens)
+        "permissions",
+        "groups",
+        "actions",
+        # netbox_config_context assignment scopes
+        "regions",
+        "site_groups",
+        "sites",
+        "locations",
+        "device_types",
+        "roles",
+        "platforms",
+        "cluster_types",
+        "cluster_groups",
+        "clusters",
+        "tenant_groups",
+        "tenants",
+        # route targets (netbox_vrf / netbox_l2vpn)
+        "import_targets",
+        "export_targets",
+    ]
+)
+
 NETBOX_ARG_SPEC = dict(
     netbox_url=dict(type="str", required=True),
     netbox_token=dict(type="str", required=True, no_log=True),
@@ -1540,9 +1576,19 @@ def _update_netbox_object(self, data):
         updated_obj = serialized_nb_obj.copy()
         updated_obj.update(data)
 
-        if serialized_nb_obj.get("tags") and data.get("tags"):
-            serialized_nb_obj["tags"] = set(serialized_nb_obj["tags"])
-            updated_obj["tags"] = set(data["tags"])
+        # Compare unordered many-to-many list fields as sets so a pure
+        # reordering of identical values is not reported as a change.
+        for key in LIST_AS_SET_KEYS:
+            if serialized_nb_obj.get(key) and data.get(key):
+                try:
+                    before_set = set(serialized_nb_obj[key])
+                    after_set = set(data[key])
+                except TypeError:
+                    # Elements aren't hashable (e.g. a list of dicts); keep the
+                    # ordered comparison rather than failing.
+                    continue
+                serialized_nb_obj[key] = before_set
+                updated_obj[key] = after_set
 
         # Ensure idempotency for site on older netbox versions
         version_pre_30 = self._version_check_greater("3.0", self.api_version)

tests/integration/targets/v4.5/tasks/netbox_tag.yml

@@ -108,3 +108,38 @@
       - test_five['tags']['name'] == "Test Tag 5"
       - test_five['tags']['slug'] == "test-tag-five"
       - test_five['msg'] == "tags Test Tag 5 created"
+
+- name: "TAG 6: Create tag with object_types"
+  netbox.netbox.netbox_tag:
+    netbox_url: http://localhost:32768
+    netbox_token: "{{ lookup('file', '/tmp/.netbox_test_token', errors='ignore') | default(lookup('env', 'NETBOX_TOKEN', default='0123456789abcdef0123456789abcdef01234567'), true) }}"
+    data:
+      name: Test Tag 6
+      object_types:
+        - dcim.device
+        - dcim.site
+    state: present
+  register: test_six
+
+- name: "TAG 6: ASSERT - Tag with object_types created"
+  ansible.builtin.assert:
+    that:
+      - test_six is changed
+      - test_six['tags']['name'] == "Test Tag 6"
+
+- name: "TAG 7: Re-run with object_types in a different order"
+  netbox.netbox.netbox_tag:
+    netbox_url: http://localhost:32768
+    netbox_token: "{{ lookup('file', '/tmp/.netbox_test_token', errors='ignore') | default(lookup('env', 'NETBOX_TOKEN', default='0123456789abcdef0123456789abcdef01234567'), true) }}"
+    data:
+      name: Test Tag 6
+      object_types:
+        - dcim.site
+        - dcim.device
+    state: present
+  register: test_seven
+
+- name: "TAG 7: ASSERT - The same object_types in a new order do not update the tag"
+  ansible.builtin.assert:
+    that:
+      - not test_seven['changed']

tests/unit/module_utils/netbox_utils/test_netbox_module.py

@@ -397,6 +397,66 @@ def test_update_netbox_object_with_changes_check_mode_true(
     assert diff == on_update_diff
 
 
+@pytest.mark.parametrize(
+    ("field", "existing", "incoming"),
+    [
+        pytest.param("tags", [1, 2, 3], [3, 1, 2], id="tags"),
+        pytest.param(
+            "object_types",
+            ["dcim.device", "dcim.site"],
+            ["dcim.site", "dcim.device"],
+            id="object_types",
+        ),
+        pytest.param("tagged_vlans", [10, 20, 30], [30, 20, 10], id="tagged_vlans"),
+    ],
+)
+def test_update_netbox_object_unordered_list_is_idempotent(
+    mocker, mock_netbox_module, field, existing, incoming
+):
+    """A reordering of an unordered m2m list field must not report a change."""
+    nb_obj = mocker.Mock(name="nb_obj")
+    nb_obj.serialize.return_value = {"name": "test", field: existing}
+    mock_netbox_module.nb_object = nb_obj
+
+    serialized_obj, diff = mock_netbox_module._update_netbox_object({field: incoming})
+
+    nb_obj.update.assert_not_called()
+    assert diff is None
+
+
+def test_update_netbox_object_unordered_list_detects_real_change(
+    mocker, mock_netbox_module
+):
+    """A genuine difference in an unordered m2m list field is still detected."""
+    nb_obj = mocker.Mock(name="nb_obj")
+    nb_obj.serialize.return_value = {"name": "test", "tags": [1, 2, 3]}
+    nb_obj.update.return_value = True
+    mock_netbox_module.nb_object = nb_obj
+
+    serialized_obj, diff = mock_netbox_module._update_netbox_object({"tags": [1, 2, 4]})
+
+    assert diff is not None
+    assert diff["before"]["tags"] == {1, 2, 3}
+    assert diff["after"]["tags"] == {1, 2, 4}
+
+
+def test_update_netbox_object_unhashable_list_falls_back_to_ordered(
+    mocker, mock_netbox_module
+):
+    """Unhashable list elements must not raise; comparison stays ordered."""
+    terminations = [{"object_id": 1, "object_type": "dcim.interface"}]
+    nb_obj = mocker.Mock(name="nb_obj")
+    nb_obj.serialize.return_value = {"name": "test", "import_targets": terminations}
+    mock_netbox_module.nb_object = nb_obj
+
+    serialized_obj, diff = mock_netbox_module._update_netbox_object(
+        {"import_targets": list(terminations)}
+    )
+
+    nb_obj.update.assert_not_called()
+    assert diff is None
+
+
 @pytest.mark.parametrize("version", ["2.13", "2.12", "2.11", "2.10.8", "2.10"])
 def test_version_check_greater_true(mock_netbox_module, nb_obj_mock, version):
     mock_netbox_module.nb_object = nb_obj_mock