Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
bugfixes:
- >-
netbox_device_interface - fail with a clear, actionable message instead of
reporting a phantom change on every run when the legacy ``mac_address`` field is
set on NetBox v4.2 and later, where that field is read-only and silently ignored
by the API. Use ``primary_mac_address`` and the ``netbox_mac_address`` module to
manage interface MAC addresses on NetBox v4.2+
(https://github.com/netbox-community/ansible_modules/issues/1502).
5 changes: 5 additions & 0 deletions plugins/module_utils/netbox_dcim.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ def run(self):

data = self.data

# The interface serializer's legacy `mac_address` field is read-only on NetBox
# v4.2+, so writes are silently dropped and would be reported as a phantom change.
if self.endpoint == "interfaces":
self._fail_on_read_only_mac_address(data)

# Handle rack and form_factor
if endpoint_name == "rack":
if self._version_check_greater(
Expand Down
25 changes: 25 additions & 0 deletions plugins/module_utils/netbox_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,31 @@ def _version_check_greater(

return t_greater > t_lesser if not greater_or_equal else t_greater >= t_lesser

def _fail_on_read_only_mac_address(self, data):
"""Fail when the legacy ``mac_address`` field is set on NetBox v4.2+.

NetBox v4.2 moved MAC addresses to a standalone ``MACAddress`` model and
declared the interface serializer's ``mac_address`` field read-only, so any
value sent to it is silently ignored by the API. Because the module derives
``changed`` from a local diff, the dropped value resurfaces as a phantom change
on every run (including check mode) while NetBox is never modified. Fail loudly
and point at the supported replacements instead of reporting a change that was
never persisted.
"""
if not data.get("mac_address"):
return
if self._version_check_greater(self.api_version, "4.2", greater_or_equal=True):
self.module.fail_json(
msg=(
"The 'mac_address' field is read-only on NetBox v4.2 and later, so "
"the value is silently dropped and never stored. Use the "
"'primary_mac_address' option of this module to set the interface's "
"primary MAC address, and the 'netbox_mac_address' module to manage "
"MAC address objects."
),
changed=False,
)

def _connect_netbox_api(self, url, token, ssl_verify, cert, headers=None):
try:
session = requests.Session()
Expand Down
5 changes: 5 additions & 0 deletions plugins/modules/netbox_device_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@
mac_address:
description:
- The MAC address of the interface
- |
Only supported on NetBox versions before 4.2. From NetBox 4.2 onward this
field is read-only and the module fails if it is set; use
C(primary_mac_address) together with the M(netbox.netbox.netbox_mac_address)
module instead.
required: false
type: str
primary_mac_address:
Expand Down
40 changes: 40 additions & 0 deletions tests/integration/targets/v4.2/tasks/netbox_device_interface.yml
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,43 @@
- test_thirteen['interface']['name'] == "GigabitEthernet2"
- test_thirteen['interface']['device'] == 1
- test_thirteen['interface']['primary_mac_address'] == 2

- name: "14 - Legacy mac_address fails loudly on NetBox v4.2+ (no phantom change)"
netbox.netbox.netbox_device_interface:
netbox_url: http://localhost:32768
netbox_token: "0123456789abcdef0123456789abcdef01234567"
data:
device: test100
name: GigabitEthernet2
mac_address: "00:11:22:33:44:55"
state: present
register: test_fourteen
ignore_errors: true

- name: "14 - ASSERT - failed, unchanged, points at the supported replacement"
ansible.builtin.assert:
that:
- test_fourteen is failed
- not test_fourteen['changed']
- "'read-only' in test_fourteen['msg']"
- "'primary_mac_address' in test_fourteen['msg']"

- name: "15 - Legacy mac_address fails in check mode too (no phantom changed)"
netbox.netbox.netbox_device_interface:
netbox_url: http://localhost:32768
netbox_token: "0123456789abcdef0123456789abcdef01234567"
data:
device: test100
name: GigabitEthernet2
mac_address: "00:11:22:33:44:55"
state: present
check_mode: true
register: test_fifteen
ignore_errors: true

- name: "15 - ASSERT - check mode also fails, no phantom changed"
ansible.builtin.assert:
that:
- test_fifteen is failed
- not test_fifteen['changed']
- "'read-only' in test_fifteen['msg']"
40 changes: 40 additions & 0 deletions tests/integration/targets/v4.3/tasks/netbox_device_interface.yml
Original file line number Diff line number Diff line change
Expand Up @@ -368,3 +368,43 @@
- test_sixteen is changed
- test_sixteen['interface']['name'] == "BridgeChild"
- test_sixteen['interface']['bridge'] == test_fourteen['interface']['id']

- name: "17 - Legacy mac_address fails loudly on NetBox v4.2+ (no phantom change)"
netbox.netbox.netbox_device_interface:
netbox_url: http://localhost:32768
netbox_token: "0123456789abcdef0123456789abcdef01234567"
data:
device: test100
name: GigabitEthernet2
mac_address: "00:11:22:33:44:55"
state: present
register: test_seventeen
ignore_errors: true

- name: "17 - ASSERT - failed, unchanged, points at the supported replacement"
ansible.builtin.assert:
that:
- test_seventeen is failed
- not test_seventeen['changed']
- "'read-only' in test_seventeen['msg']"
- "'primary_mac_address' in test_seventeen['msg']"

- name: "18 - Legacy mac_address fails in check mode too (no phantom changed)"
netbox.netbox.netbox_device_interface:
netbox_url: http://localhost:32768
netbox_token: "0123456789abcdef0123456789abcdef01234567"
data:
device: test100
name: GigabitEthernet2
mac_address: "00:11:22:33:44:55"
state: present
check_mode: true
register: test_eighteen
ignore_errors: true

- name: "18 - ASSERT - check mode also fails, no phantom changed"
ansible.builtin.assert:
that:
- test_eighteen is failed
- not test_eighteen['changed']
- "'read-only' in test_eighteen['msg']"
40 changes: 40 additions & 0 deletions tests/integration/targets/v4.4/tasks/netbox_device_interface.yml
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,43 @@
- test_thirteen['interface']['name'] == "GigabitEthernet2"
- test_thirteen['interface']['device'] == 1
- test_thirteen['interface']['primary_mac_address'] == 2

- name: "14 - Legacy mac_address fails loudly on NetBox v4.2+ (no phantom change)"
netbox.netbox.netbox_device_interface:
netbox_url: http://localhost:32768
netbox_token: "0123456789abcdef0123456789abcdef01234567"
data:
device: test100
name: GigabitEthernet2
mac_address: "00:11:22:33:44:55"
state: present
register: test_fourteen
ignore_errors: true

- name: "14 - ASSERT - failed, unchanged, points at the supported replacement"
ansible.builtin.assert:
that:
- test_fourteen is failed
- not test_fourteen['changed']
- "'read-only' in test_fourteen['msg']"
- "'primary_mac_address' in test_fourteen['msg']"

- name: "15 - Legacy mac_address fails in check mode too (no phantom changed)"
netbox.netbox.netbox_device_interface:
netbox_url: http://localhost:32768
netbox_token: "0123456789abcdef0123456789abcdef01234567"
data:
device: test100
name: GigabitEthernet2
mac_address: "00:11:22:33:44:55"
state: present
check_mode: true
register: test_fifteen
ignore_errors: true

- name: "15 - ASSERT - check mode also fails, no phantom changed"
ansible.builtin.assert:
that:
- test_fifteen is failed
- not test_fifteen['changed']
- "'read-only' in test_fifteen['msg']"
40 changes: 40 additions & 0 deletions tests/integration/targets/v4.5/tasks/netbox_device_interface.yml
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,43 @@
- test_thirteen['interface']['name'] == "GigabitEthernet2"
- test_thirteen['interface']['device'] == 1
- test_thirteen['interface']['primary_mac_address'] == 2

- name: "14 - Legacy mac_address fails loudly on NetBox v4.2+ (no phantom change)"
netbox.netbox.netbox_device_interface:
netbox_url: http://localhost:32768
netbox_token: "{{ lookup('file', '/tmp/.netbox_test_token', errors='ignore') | default(lookup('env', 'NETBOX_TOKEN', default='0123456789abcdef0123456789abcdef01234567'), true) }}"
data:
device: test100
name: GigabitEthernet2
mac_address: "00:11:22:33:44:55"
state: present
register: test_fourteen
ignore_errors: true

- name: "14 - ASSERT - failed, unchanged, points at the supported replacement"
ansible.builtin.assert:
that:
- test_fourteen is failed
- not test_fourteen['changed']
- "'read-only' in test_fourteen['msg']"
- "'primary_mac_address' in test_fourteen['msg']"

- name: "15 - Legacy mac_address fails in check mode too (no phantom changed)"
netbox.netbox.netbox_device_interface:
netbox_url: http://localhost:32768
netbox_token: "{{ lookup('file', '/tmp/.netbox_test_token', errors='ignore') | default(lookup('env', 'NETBOX_TOKEN', default='0123456789abcdef0123456789abcdef01234567'), true) }}"
data:
device: test100
name: GigabitEthernet2
mac_address: "00:11:22:33:44:55"
state: present
check_mode: true
register: test_fifteen
ignore_errors: true

- name: "15 - ASSERT - check mode also fails, no phantom changed"
ansible.builtin.assert:
that:
- test_fifteen is failed
- not test_fifteen['changed']
- "'read-only' in test_fifteen['msg']"
36 changes: 36 additions & 0 deletions tests/unit/module_utils/netbox_utils/test_netbox_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,3 +459,39 @@ def test_version_sanitize_value_error(mock_netbox_module, nb_obj_mock, version):
mock_netbox_module.nb_object = nb_obj_mock
with pytest.raises(ValueError):
mock_netbox_module._version_sanitize(version)


@pytest.mark.parametrize(
"version, data, should_fail",
[
pytest.param("4.2", {"mac_address": "00:11:22:33:44:55"}, True, id="4.2-fails"),
pytest.param(
"4.4.8", {"mac_address": "00:11:22:33:44:55"}, True, id="4.4.8-fails"
),
pytest.param("4.5", {"mac_address": "00:11:22:33:44:55"}, True, id="4.5-fails"),
pytest.param(
"4.1", {"mac_address": "00:11:22:33:44:55"}, False, id="4.1-allowed"
),
pytest.param(
"3.7", {"mac_address": "00:11:22:33:44:55"}, False, id="3.7-allowed"
),
pytest.param("4.2", {"name": "GigabitEthernet1"}, False, id="no-mac-allowed"),
pytest.param("4.2", {"mac_address": None}, False, id="null-mac-allowed"),
],
)
def test_fail_on_read_only_mac_address(mock_netbox_module, version, data, should_fail):
"""`mac_address` is read-only on NetBox v4.2+, so setting it must fail loudly
rather than report a phantom change."""
mock_netbox_module.api_version = version
mock_netbox_module.module.fail_json.reset_mock()

mock_netbox_module._fail_on_read_only_mac_address(data)

if should_fail:
mock_netbox_module.module.fail_json.assert_called_once()
_, kwargs = mock_netbox_module.module.fail_json.call_args
assert kwargs["changed"] is False
assert "read-only" in kwargs["msg"]
assert "primary_mac_address" in kwargs["msg"]
else:
mock_netbox_module.module.fail_json.assert_not_called()
Loading