fix(netbox_device_interface): fail on read-only mac_address for NetBox v4.2+#1574
Draft
ldrozdz93 wants to merge 1 commit into
Draft
fix(netbox_device_interface): fail on read-only mac_address for NetBox v4.2+#1574ldrozdz93 wants to merge 1 commit into
ldrozdz93 wants to merge 1 commit into
Conversation
…x v4.2+ On NetBox v4.2 and later the interface serializer's legacy `mac_address` field is read-only, so values sent to it are silently dropped by the API. The module derives `changed` from a local diff, so the dropped value resurfaced as a phantom change on every run (including check mode) while NetBox was never modified. Fail loudly with a message pointing at `primary_mac_address` and the `netbox_mac_address` module instead of reporting a change that was never persisted. The check is version-gated, so NetBox < 4.2 keeps working. Fixes netbox-community#1502 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 a NetBox v4.2+ idempotency/accuracy issue where netbox_device_interface could report changed for the legacy mac_address field even though NetBox silently discards the write. The module now fails early with actionable guidance so the reported result matches what NetBox can actually persist.
Changes:
- Add a version-gated guard that fails when
mac_addressis set for device interfaces on NetBox v4.2+ (read-only field). - Update
netbox_device_interfacedocumentation to clarifymac_addresssupport (<4.2) and point users toprimary_mac_address+netbox_mac_address. - Add unit + integration tests validating failure behavior (including check mode) and add a changelog fragment.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
plugins/module_utils/netbox_utils.py |
Adds _fail_on_read_only_mac_address() helper to fail on NetBox v4.2+ when legacy mac_address is set. |
plugins/module_utils/netbox_dcim.py |
Invokes the new guard for the interfaces endpoint to prevent phantom changes. |
plugins/modules/netbox_device_interface.py |
Documents that mac_address is only supported on NetBox < 4.2 and points to replacements. |
tests/unit/module_utils/netbox_utils/test_netbox_module.py |
Adds unit coverage for the new read-only MAC guard across versions and edge cases. |
tests/integration/targets/v4.2/tasks/netbox_device_interface.yml |
Adds integration assertions that legacy mac_address fails (unchanged) on v4.2, including check mode. |
tests/integration/targets/v4.3/tasks/netbox_device_interface.yml |
Adds the same failure assertions for v4.3. |
tests/integration/targets/v4.4/tasks/netbox_device_interface.yml |
Adds the same failure assertions for v4.4. |
tests/integration/targets/v4.5/tasks/netbox_device_interface.yml |
Adds the same failure assertions for v4.5. |
changelogs/fragments/1502-device-interface-mac-address-readonly.yml |
Adds a bugfix changelog entry describing the new behavior and guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
What this fixes
On NetBox 4.2 and later, setting
mac_addressonnetbox_device_interfacereportedchangedwith a before/after diff on every run, but NetBox was never actually updated. Users believed the MAC address was stored when it wasn't.NetBox 4.2 moved MAC addresses to a standalone
MACAddressmodel and made the interface's legacymac_addressfield read-only, so the API silently drops any value sent to it. Because the module decideschangedby comparing the requested data against the serialized object locally, the dropped value reappeared as a phantom change on every run — including in check mode.Behaviour after this change
On NetBox 4.2+, a task that sets the legacy
mac_addressfield now fails with a clear, actionable message instead of reporting a change that never happened:The reported result now matches what NetBox actually stores, in both normal and check mode. NetBox versions before 4.2, where
mac_addressis still writable, are unaffected.Why fail rather than store the MAC under the hood
primary_mac_addressonly links an existingMACAddress; creating and assigning one automatically would be a larger change with its own idempotency surface. Failing loudly is the smallest fix that guarantees the reported state matches NetBox and gives the user an unambiguous next step. The module documentation formac_addressnow states it is for NetBox < 4.2 and points to the replacements.Tests
_fail_on_read_only_mac_addressis covered across NetBox 3.7 / 4.1 (allowed), 4.2 / 4.4.8 / 4.5 (fail), plus the no-MAC and null-MAC cases.netbox_device_interfaceplays for v4.2–v4.5 assert that a legacymac_addresstask fails with the replacement guidance and reports no change, in both normal and check mode.Fixes #1502