fix: compare unordered m2m list fields as sets in the generic diff engine (INT-411)#1571
Draft
ldrozdz93 wants to merge 1 commit into
Draft
Conversation
… (INT-411) NetBox returns many-to-many list fields in a non-deterministic order. The generic update-comparison engine special-cased only `tags` for set comparison and compared every other list field element-wise, so a task whose list contents already matched NetBox state was reported as `changed` on every run and `--check` mode produced false positives (GitHub #1486). Generalize the per-field fix that landed in netbox_users into the shared engine: a single `LIST_AS_SET_KEYS` set in netbox_utils now drives set comparison for all known unordered m2m fields (object_types, tagged_vlans, config-context scopes, route targets, and the user/permission lists). The conversion only applies when both sides carry the key and falls back to ordered comparison for non-hashable values. netbox_users imports the shared set instead of redefining it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes idempotency issues in the NetBox collection’s generic update-comparison logic by treating known unordered many-to-many (m2m) list fields as sets (instead of ordered lists). This prevents “phantom changes” when NetBox returns the same values in a different order (notably object_types, addressing INT-411 / #1486).
Changes:
- Introduces a shared
LIST_AS_SET_KEYSinnetbox_utils.pyand uses it in the generic_update_netbox_objectcomparison to perform set-based comparisons for known unordered m2m list fields (with safe fallback for unhashable elements). - Updates
netbox_users.pyto import the sharedLIST_AS_SET_KEYSrather than maintaining its own duplicate list. - Adds unit tests and an integration test to verify idempotency on reordered unordered-list inputs (including
netbox_tag.object_types), plus a changelog fragment.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
plugins/module_utils/netbox_utils.py |
Adds LIST_AS_SET_KEYS and applies set-based comparison for unordered m2m list fields in the generic diff/update engine. |
plugins/module_utils/netbox_users.py |
Switches to the shared LIST_AS_SET_KEYS constant to avoid duplicated key lists. |
tests/unit/module_utils/netbox_utils/test_netbox_module.py |
Adds unit coverage for unordered-list idempotency, real-change detection, and unhashable-element fallback behavior. |
tests/integration/targets/v4.5/tasks/netbox_tag.yml |
Adds an integration scenario verifying netbox_tag idempotency when object_types ordering changes. |
changelogs/fragments/int-411-unordered-m2m-list-set-comparison.yml |
Documents the bugfix and affected fields/endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I had similiar ideas and discussions in PR #1487 |
|
When fixing the ansible collection. It would make sense to sync the set fields with the ‘pynetbox’ modules as well |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes idempotency failures caused by the update-comparison engine treating unordered many-to-many (m2m) list fields as ordered lists. NetBox returns m2m fields in a non-deterministic order, so a task whose list contents already match NetBox state is reported as
changedon every run, and--checkmode reports phantom changes.The engine previously special-cased only
tagsfor set comparison; every other list field was compared element-wise. A per-module fix shipped fornetbox_permission(LIST_AS_SET_KEYSinnetbox_users.py), but the bug class stayed live for fields flowing through the generic engine — most visiblynetbox_tagwithobject_types, which uses the generic engine and exhibits the exact #1486 behaviour today.What changed
netbox_utils.py— a singleLIST_AS_SET_KEYSset now drives set comparison in the generic_update_netbox_object, replacing thetags-only special case. The conversion only applies when both the existing object and the incoming data carry the key, and falls back to ordered comparison when elements aren't hashable.tags,object_types,tagged_vlans, thenetbox_config_contextassignment 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 lists (permissions,groups,actions).netbox_users.py— imports the sharedLIST_AS_SET_KEYSinstead of redefining its own subset, so there is one source of truth.v4.5/netbox_tag.yml) — create a tag withobject_types, re-run with the same values in a different order, assertnot changed.Resolves
netbox_permission.object_typesreordering) — verify & close.netbox_tag.object_typescase.Linear: INT-411