From fb4efdb9252b965be5c2ceaa0c969c179cf3f7cd Mon Sep 17 00:00:00 2001 From: Yuriy Andamasov Date: Fri, 21 Aug 2026 12:26:57 +0300 Subject: T8516: add vyos_l3_interfaces unit tests; delete only L3 attributes in deleted/overridden (#458) * T8516: add unit tests for vyos_l3_interfaces module * T8516: fix copyright header to use VyOS Networks attribution Co-Authored-By: Claude Sonnet 4.6 * Changelog * Misc fixes * Misc changes * T8516: bugfixes * T8516: changelog - bugfixes added * T8516: Update changelog Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * T8516: Docstring fix Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * T8516: docstring fix Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * T8516: restore _state_deleted docstring, align test docstring and fixture - restore the _state_deleted docstring lost in the L2-safeguard change, now documenting the address-only delete semantics - overridden test docstring said "delete interface stanzas" but the module now emits address-only deletes; align wording - quote loopback interface name in fixture for consistency with vyos_interfaces_config.cfg * T8516: drop out-of-scope config and generated-docs changes Reviewer request (gaige): top-level instruction files (.coderabbit.yaml, AGENTS.md) belong in separate PRs. Also revert unrelated generated-docs drift in vyos_config / vyos_bgp_global rst files. All four files are now identical to main. --------- Co-authored-by: Claude Sonnet 4.6 Co-authored-by: omnom62 <75066712+omnom62@users.noreply.github.com> Co-authored-by: omnom62 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../fragments/T8516_unit_tests_l3_interface.yml | 5 + .../vyos/config/l3_interfaces/l3_interfaces.py | 39 ++- .../vyos_l3_interfaces/tests/cli/deleted.yaml | 4 +- .../targets/vyos_l3_interfaces/vars/main.yaml | 18 +- .../vyos/fixtures/vyos_l3_interfaces_config.cfg | 8 + .../network/vyos/test_vyos_l3_interfaces.py | 278 +++++++++++++++++++++ 6 files changed, 340 insertions(+), 12 deletions(-) create mode 100644 changelogs/fragments/T8516_unit_tests_l3_interface.yml create mode 100644 tests/unit/modules/network/vyos/fixtures/vyos_l3_interfaces_config.cfg create mode 100644 tests/unit/modules/network/vyos/test_vyos_l3_interfaces.py diff --git a/changelogs/fragments/T8516_unit_tests_l3_interface.yml b/changelogs/fragments/T8516_unit_tests_l3_interface.yml new file mode 100644 index 00000000..41b3218e --- /dev/null +++ b/changelogs/fragments/T8516_unit_tests_l3_interface.yml @@ -0,0 +1,5 @@ +--- +trivial: + - Add unit tests for vyos_l3_interfaces module. +bugfixes: + - Safeguard interface L2 configuration by deleting only L3 address attributes in vyos_l3_interfaces; update unit and integration tests accordingly. diff --git a/plugins/module_utils/network/vyos/config/l3_interfaces/l3_interfaces.py b/plugins/module_utils/network/vyos/config/l3_interfaces/l3_interfaces.py index a23494cd..f7b76af9 100644 --- a/plugins/module_utils/network/vyos/config/l3_interfaces/l3_interfaces.py +++ b/plugins/module_utils/network/vyos/config/l3_interfaces/l3_interfaces.py @@ -256,6 +256,10 @@ class L3_interfaces(ConfigBase): def _state_deleted(self, want, have): """The command generator when state is deleted + Deletes only the L3 address attributes (base interface and VIFs) + owned by this module, never the interface subtree, so L2 settings + are preserved. + :rtype: A list :returns: the commands necessary to remove the current configuration of the provided objects @@ -266,14 +270,33 @@ class L3_interfaces(ConfigBase): if have_copy is not None: if all(v in (None, {}, []) for k, v in want_copy.items() if k != "name"): - commands.append( - self._compute_commands( - key=None, - value=None, - interface=want_copy["name"], - remove=True, - ), - ) + # Only delete L3 attributes we own — do not touch L2 config + have_vifs = have_copy.pop("vifs", []) or [] + + for addr_family in ("ipv4", "ipv6"): + for addr in have_copy.get(addr_family) or []: + commands.append( + self._compute_commands( + key="address", + value=addr["address"], + interface=want_copy["name"], + remove=True, + ), + ) + + for have_vif in have_vifs: + for addr_family in ("ipv4", "ipv6"): + for addr in have_vif.get(addr_family) or []: + commands.append( + self._compute_commands( + key="address", + value=addr["address"], + interface=want_copy["name"], + vif=have_vif["vlan_id"], + remove=True, + ), + ) + return commands want_vifs = want_copy.pop("vifs", []) diff --git a/tests/integration/targets/vyos_l3_interfaces/tests/cli/deleted.yaml b/tests/integration/targets/vyos_l3_interfaces/tests/cli/deleted.yaml index 000d55e0..3af51209 100644 --- a/tests/integration/targets/vyos_l3_interfaces/tests/cli/deleted.yaml +++ b/tests/integration/targets/vyos_l3_interfaces/tests/cli/deleted.yaml @@ -71,12 +71,12 @@ - name: Assert that the correct set of commands were generated (SLAAC) assert: that: - - "{{ deleted['commands'] | symmetric_difference(result['commands']) |length == 0 }}" + - "{{ deleted['commands_slaac'] | symmetric_difference(result['commands']) |length == 0 }}" - name: Assert that the after dicts were correctly generated (SLAAC) assert: that: - - "{{ deleted['after'] | symmetric_difference(result['after']) |length == 0 }}" + - "{{ deleted['after_slaac'] | symmetric_difference(result['after']) |length == 0 }}" always: - include_tasks: _remove_config.yaml diff --git a/tests/integration/targets/vyos_l3_interfaces/vars/main.yaml b/tests/integration/targets/vyos_l3_interfaces/vars/main.yaml index 84f1c637..16a3d821 100644 --- a/tests/integration/targets/vyos_l3_interfaces/vars/main.yaml +++ b/tests/integration/targets/vyos_l3_interfaces/vars/main.yaml @@ -161,8 +161,12 @@ rendered: deleted: commands: - - delete interfaces ethernet eth1 - - delete interfaces ethernet eth2 + - delete interfaces ethernet eth1 address '192.0.2.14/24' + - delete interfaces ethernet eth2 address '192.0.2.10/24' + - delete interfaces ethernet eth2 address '192.0.2.11/24' + - delete interfaces ethernet eth2 address '2001:db8::10/32' + - delete interfaces ethernet eth2 address '2001:db8::12/32' + commands_slaac: - delete interfaces ethernet eth1 ipv6 address 'autoconf' - delete interfaces ethernet eth2 vif 101 ipv6 address 'autoconf' @@ -170,6 +174,8 @@ deleted: - name: eth0 ipv4: - address: dhcp + - name: eth1 + - name: eth2 before_slaac: - name: eth0 ipv4: @@ -182,3 +188,11 @@ deleted: - vlan_id: 101 ipv6: - address: auto-config + after_slaac: + - name: eth0 + ipv4: + - address: dhcp + - name: eth1 + - name: eth2 + vifs: + - vlan_id: 101 diff --git a/tests/unit/modules/network/vyos/fixtures/vyos_l3_interfaces_config.cfg b/tests/unit/modules/network/vyos/fixtures/vyos_l3_interfaces_config.cfg new file mode 100644 index 00000000..8f9609cd --- /dev/null +++ b/tests/unit/modules/network/vyos/fixtures/vyos_l3_interfaces_config.cfg @@ -0,0 +1,8 @@ +set interfaces ethernet eth0 address 'dhcp' +set interfaces ethernet eth1 address '192.0.2.14/24' +set interfaces ethernet eth2 address '192.0.2.10/24' +set interfaces ethernet eth2 address '2001:db8::10/32' +set interfaces ethernet eth3 address '198.51.100.10/24' +set interfaces ethernet eth3 vif 101 address '198.51.100.130/25' +set interfaces ethernet eth3 vif 102 address '2001:db8:4000::3/34' +set interfaces loopback 'lo' diff --git a/tests/unit/modules/network/vyos/test_vyos_l3_interfaces.py b/tests/unit/modules/network/vyos/test_vyos_l3_interfaces.py new file mode 100644 index 00000000..7932f63d --- /dev/null +++ b/tests/unit/modules/network/vyos/test_vyos_l3_interfaces.py @@ -0,0 +1,278 @@ +# (c) 2024 VyOS Networks +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +from __future__ import absolute_import, division, print_function + + +__metaclass__ = type + +from unittest.mock import patch + +from ansible_collections.vyos.vyos.plugins.modules import vyos_l3_interfaces +from ansible_collections.vyos.vyos.tests.unit.modules.utils import set_module_args + +from .vyos_module import TestVyosModule, load_fixture + + +class TestVyosL3InterfacesModule(TestVyosModule): + module = vyos_l3_interfaces + + def setUp(self): + super(TestVyosL3InterfacesModule, self).setUp() + self.mock_get_config = patch( + "ansible_collections.ansible.netcommon.plugins.module_utils.network.common.network.Config.get_config", + ) + self.get_config = self.mock_get_config.start() + + self.mock_load_config = patch( + "ansible_collections.ansible.netcommon.plugins.module_utils.network.common.network.Config.load_config", + ) + self.load_config = self.mock_load_config.start() + + self.mock_get_resource_connection_config = patch( + "ansible_collections.ansible.netcommon.plugins.module_utils.network.common.cfg.base.get_resource_connection", + ) + self.get_resource_connection_config = self.mock_get_resource_connection_config.start() + + self.mock_get_resource_connection_facts = patch( + "ansible_collections.ansible.netcommon.plugins.module_utils.network.common.facts.facts.get_resource_connection", + ) + self.get_resource_connection_facts = self.mock_get_resource_connection_facts.start() + + def tearDown(self): + super(TestVyosL3InterfacesModule, self).tearDown() + self.mock_get_resource_connection_config.stop() + self.mock_get_resource_connection_facts.stop() + self.mock_get_config.stop() + self.mock_load_config.stop() + + def load_fixtures(self, commands=None, filename=None): + fixture_data = load_fixture("vyos_l3_interfaces_config.cfg") + self.get_resource_connection_facts.return_value.get_config.return_value = fixture_data + + def test_vyos_l3_interfaces_merged(self): + """Merge a new IPv4 address onto eth1 (currently has 192.0.2.14/24).""" + set_module_args( + dict( + config=[ + dict( + name="eth1", + ipv4=[dict(address="192.0.2.1/24")], + ), + ], + state="merged", + ), + ) + commands = [ + "set interfaces ethernet eth1 address '192.0.2.1/24'", + ] + self.execute_module(changed=True, commands=commands) + + def test_vyos_l3_interfaces_merged_idempotent(self): + """No change when desired config already matches existing config.""" + set_module_args( + dict( + config=[ + dict( + name="eth1", + ipv4=[dict(address="192.0.2.14/24")], + ), + ], + state="merged", + ), + ) + self.execute_module(changed=False, commands=[]) + + def test_vyos_l3_interfaces_merged_ipv6(self): + """Merge a new IPv6 address onto eth1.""" + set_module_args( + dict( + config=[ + dict( + name="eth1", + ipv6=[dict(address="2001:db8::1/32")], + ), + ], + state="merged", + ), + ) + commands = [ + "set interfaces ethernet eth1 address '2001:db8::1/32'", + ] + self.execute_module(changed=True, commands=commands) + + def test_vyos_l3_interfaces_replaced(self): + """Replace eth1 addresses: remove existing, add new.""" + set_module_args( + dict( + config=[ + dict( + name="eth1", + ipv4=[dict(address="10.0.0.1/24")], + ), + ], + state="replaced", + ), + ) + commands = [ + "delete interfaces ethernet eth1 address '192.0.2.14/24'", + "set interfaces ethernet eth1 address '10.0.0.1/24'", + ] + self.execute_module(changed=True, commands=commands) + + def test_vyos_l3_interfaces_overridden(self): + """Override: keep only eth2; delete L3 addresses from all other interfaces.""" + set_module_args( + dict( + config=[ + dict( + name="eth2", + ipv4=[dict(address="192.0.2.10/24")], + ipv6=[dict(address="2001:db8::10/32")], + ), + ], + state="overridden", + ), + ) + commands = [ + "delete interfaces ethernet eth0 address 'dhcp'", + "delete interfaces ethernet eth1 address '192.0.2.14/24'", + "delete interfaces ethernet eth3 address '198.51.100.10/24'", + "delete interfaces ethernet eth3 vif 101 address '198.51.100.130/25'", + "delete interfaces ethernet eth3 vif 102 address '2001:db8:4000::3/34'", + ] + self.execute_module(changed=True, commands=commands) + + def test_vyos_l3_interfaces_deleted(self): + """Delete all L3 config from eth1.""" + set_module_args( + dict( + config=[ + dict(name="eth1"), + ], + state="deleted", + ), + ) + commands = [ + "delete interfaces ethernet eth1 address '192.0.2.14/24'", + ] + self.execute_module(changed=True, commands=commands) + + def test_vyos_l3_interfaces_rendered(self): + """Render set commands without connecting to the device.""" + set_module_args( + dict( + config=[ + dict( + name="eth0", + ipv4=[dict(address="dhcp")], + ), + dict( + name="eth1", + ipv4=[dict(address="192.0.2.14/24")], + ), + ], + state="rendered", + ), + ) + rendered_cmds = [ + "set interfaces ethernet eth0 address 'dhcp'", + "set interfaces ethernet eth1 address '192.0.2.14/24'", + ] + result = self.execute_module(changed=False) + self.assertEqual( + sorted(result["rendered"]), + sorted(rendered_cmds), + result["rendered"], + ) + + def test_vyos_l3_interfaces_gathered(self): + """Gather L3 interface facts from the device.""" + set_module_args(dict(state="gathered")) + result = self.execute_module(changed=False) + gathered = result["gathered"] + # Verify eth1 is present with its IPv4 address + eth1 = next((i for i in gathered if i["name"] == "eth1"), None) + self.assertIsNotNone(eth1) + self.assertIn({"address": "192.0.2.14/24"}, eth1["ipv4"]) + + def test_vyos_l3_interfaces_parsed(self): + """Parse a raw config string into structured data.""" + raw_config = ( + "set interfaces ethernet eth0 address 'dhcp'\n" + "set interfaces ethernet eth1 address '192.0.2.14/24'\n" + ) + set_module_args( + dict( + running_config=raw_config, + state="parsed", + ), + ) + result = self.execute_module(changed=False) + parsed = result["parsed"] + eth0 = next((i for i in parsed if i["name"] == "eth0"), None) + self.assertIsNotNone(eth0) + self.assertIn({"address": "dhcp"}, eth0["ipv4"]) + eth1 = next((i for i in parsed if i["name"] == "eth1"), None) + self.assertIsNotNone(eth1) + self.assertIn({"address": "192.0.2.14/24"}, eth1["ipv4"]) + + def test_vyos_l3_interfaces_vif_merged(self): + """Merge a new address onto a VIF sub-interface.""" + set_module_args( + dict( + config=[ + dict( + name="eth3", + vifs=[ + dict( + vlan_id=101, + ipv4=[dict(address="198.51.100.200/25")], + ), + ], + ), + ], + state="merged", + ), + ) + commands = [ + "set interfaces ethernet eth3 vif 101 address '198.51.100.200/25'", + ] + self.execute_module(changed=True, commands=commands) + + def test_vyos_l3_interfaces_vif_deleted(self): + """Delete all L3 addresses from eth3, including VIF sub-interfaces.""" + set_module_args( + dict( + config=[ + dict( + name="eth3", + vifs=[ + dict(vlan_id=101), + dict(vlan_id=102), + ], + ), + ], + state="deleted", + ), + ) + commands = [ + "delete interfaces ethernet eth3 vif 101 address '198.51.100.130/25'", + "delete interfaces ethernet eth3 vif 102 address '2001:db8:4000::3/34'", + "delete interfaces ethernet eth3 address '198.51.100.10/24'", + ] + self.execute_module(changed=True, commands=commands) -- cgit v1.2.3