Skip to content

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

Draft
ldrozdz93 wants to merge 1 commit into
develfrom
int-411-ansible-false-changed-for-unordered-m2m-list-fields-diff
Draft

fix: compare unordered m2m list fields as sets in the generic diff engine (INT-411)#1571
ldrozdz93 wants to merge 1 commit into
develfrom
int-411-ansible-false-changed-for-unordered-m2m-list-fields-diff

Conversation

@ldrozdz93

Copy link
Copy Markdown
Collaborator

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 changed on every run, and --check mode reports phantom changes.

The engine previously special-cased only tags for set comparison; every other list field was compared element-wise. A per-module fix shipped for netbox_permission (LIST_AS_SET_KEYS in netbox_users.py), but the bug class stayed live for fields flowing through the generic engine — most visibly netbox_tag with object_types, which uses the generic engine and exhibits the exact #1486 behaviour today.

What changed

  • netbox_utils.py — a single LIST_AS_SET_KEYS set now drives set comparison in the generic _update_netbox_object, replacing the tags-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.
  • The set covers: tags, 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 lists (permissions, groups, actions).
  • netbox_users.py — imports the shared LIST_AS_SET_KEYS instead of redefining its own subset, so there is one source of truth.
  • Unit tests — reordering an unordered list field reports no change; a genuine content difference is still detected; non-hashable list elements do not raise.
  • Integration test (v4.5/netbox_tag.yml) — create a tag with object_types, re-run with the same values in a different order, assert not changed.
  • Changelog fragment.

Resolves

Linear: INT-411

… (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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_KEYS in netbox_utils.py and uses it in the generic _update_netbox_object comparison to perform set-based comparisons for known unordered m2m list fields (with safe fallback for unhashable elements).
  • Updates netbox_users.py to import the shared LIST_AS_SET_KEYS rather 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.

@dhoffend

Copy link
Copy Markdown

I had similiar ideas and discussions in PR #1487
But with your integration tests it is even better 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants