summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authoromnom62 <75066712+omnom62@users.noreply.github.com>2026-10-07 21:21:04 +1000
committerGitHub <noreply@github.com>2026-10-07 14:21:04 +0300
commit41781e7a16e73fdca1f26c677ed3645ca2a8a3d0 (patch)
tree61cf5e168e55ffd81c8d79d607b4ba3d9d91ad57
parent7cfca28e5857b480920c22ae12557f55ca2a8a19 (diff)
downloadrest.vyos-41781e7a16e73fdca1f26c677ed3645ca2a8a3d0.tar.gz
rest.vyos-41781e7a16e73fdca1f26c677ed3645ca2a8a3d0.zip
T8989: vyos_interfaces dict_op refactor (#33)HEADmain
* T8989: vyos_interfaces dict_op refactor --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Daniil Baturin <daniil@vyos.io>
-rw-r--r--changelogs/fragments/t8989_interface_dict_op.yml3
-rw-r--r--docs/vyos.rest.vyos_interfaces_module.rst132
-rw-r--r--plugins/modules/vyos_interfaces.py439
-rw-r--r--tests/integration/targets/vyos_interfaces/tests/httpapi/overridden.yaml104
-rw-r--r--tests/integration/targets/vyos_interfaces/tests/httpapi/rtt.yaml107
-rw-r--r--tests/unit/fixtures/interfaces_running.json156
-rw-r--r--tests/unit/modules/test_vyos_interfaces.py812
7 files changed, 1400 insertions, 353 deletions
diff --git a/changelogs/fragments/t8989_interface_dict_op.yml b/changelogs/fragments/t8989_interface_dict_op.yml
new file mode 100644
index 0000000..ef73e23
--- /dev/null
+++ b/changelogs/fragments/t8989_interface_dict_op.yml
@@ -0,0 +1,3 @@
+---
+minor_changes:
+ - vyos_interfaces - Refactor the module to comply with the dict_op paradigm.
diff --git a/docs/vyos.rest.vyos_interfaces_module.rst b/docs/vyos.rest.vyos_interfaces_module.rst
index 8438d9d..3fd0736 100644
--- a/docs/vyos.rest.vyos_interfaces_module.rst
+++ b/docs/vyos.rest.vyos_interfaces_module.rst
@@ -17,8 +17,9 @@ Version added: 1.0.0
Synopsis
--------
-- Manages L2 interface configuration (description, MTU, speed, duplex, enabled) on VyOS devices using the HTTPS REST API.
+- Manages L2 interface configuration (description, MTU, speed, duplex, enabled, VRF assignment, VLAN sub-interfaces) on VyOS devices using the HTTPS REST API.
- IP address configuration is handled by :ref:`vyos.rest.vyos_l3_interfaces <vyos.rest.vyos_l3_interfaces_module>`.
+- Covers 11 interface types (ethernet, bonding, loopback, tunnel, wireguard, vti, dummy, openvpn, pppoe, wireless, bridge), resolved from the device response when present, otherwise guessed from the interface name. The current CLI collection module documents a narrower scope of 5 types (ethernet, bonding, vxlan, loopback, vti).
@@ -30,12 +31,12 @@ Parameters
<table border=0 cellpadding=0 class="documentation-table">
<tr>
- <th colspan="2">Parameter</th>
+ <th colspan="3">Parameter</th>
<th>Choices/<font color="blue">Defaults</font></th>
<th width="100%">Comments</th>
</tr>
<tr>
- <td colspan="2">
+ <td colspan="3">
<div class="ansibleOptionAnchor" id="parameter-"></div>
<b>config</b>
<a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
@@ -52,7 +53,7 @@ Parameters
</tr>
<tr>
<td class="elbow-placeholder"></td>
- <td colspan="1">
+ <td colspan="2">
<div class="ansibleOptionAnchor" id="parameter-"></div>
<b>description</b>
<a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
@@ -68,7 +69,7 @@ Parameters
</tr>
<tr>
<td class="elbow-placeholder"></td>
- <td colspan="1">
+ <td colspan="2">
<div class="ansibleOptionAnchor" id="parameter-"></div>
<b>duplex</b>
<a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
@@ -89,7 +90,7 @@ Parameters
</tr>
<tr>
<td class="elbow-placeholder"></td>
- <td colspan="1">
+ <td colspan="2">
<div class="ansibleOptionAnchor" id="parameter-"></div>
<b>enabled</b>
<a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
@@ -109,7 +110,7 @@ Parameters
</tr>
<tr>
<td class="elbow-placeholder"></td>
- <td colspan="1">
+ <td colspan="2">
<div class="ansibleOptionAnchor" id="parameter-"></div>
<b>mtu</b>
<a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
@@ -125,7 +126,7 @@ Parameters
</tr>
<tr>
<td class="elbow-placeholder"></td>
- <td colspan="1">
+ <td colspan="2">
<div class="ansibleOptionAnchor" id="parameter-"></div>
<b>name</b>
<a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
@@ -142,7 +143,7 @@ Parameters
</tr>
<tr>
<td class="elbow-placeholder"></td>
- <td colspan="1">
+ <td colspan="2">
<div class="ansibleOptionAnchor" id="parameter-"></div>
<b>speed</b>
<a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
@@ -164,10 +165,117 @@ Parameters
<div>Interface speed setting.</div>
</td>
</tr>
+ <tr>
+ <td class="elbow-placeholder"></td>
+ <td colspan="2">
+ <div class="ansibleOptionAnchor" id="parameter-"></div>
+ <b>vifs</b>
+ <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
+ <div style="font-size: small">
+ <span style="color: purple">list</span>
+ / <span style="color: purple">elements=dictionary</span>
+ </div>
+ </td>
+ <td>
+ </td>
+ <td>
+ <div>802.1Q VLAN sub-interfaces.</div>
+ </td>
+ </tr>
+ <tr>
+ <td class="elbow-placeholder"></td>
+ <td class="elbow-placeholder"></td>
+ <td colspan="1">
+ <div class="ansibleOptionAnchor" id="parameter-"></div>
+ <b>description</b>
+ <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
+ <div style="font-size: small">
+ <span style="color: purple">string</span>
+ </div>
+ </td>
+ <td>
+ </td>
+ <td>
+ <div>Sub-interface description.</div>
+ </td>
+ </tr>
+ <tr>
+ <td class="elbow-placeholder"></td>
+ <td class="elbow-placeholder"></td>
+ <td colspan="1">
+ <div class="ansibleOptionAnchor" id="parameter-"></div>
+ <b>enabled</b>
+ <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
+ <div style="font-size: small">
+ <span style="color: purple">boolean</span>
+ </div>
+ </td>
+ <td>
+ <ul style="margin: 0; padding: 0"><b>Choices:</b>
+ <li>no</li>
+ <li><div style="color: blue"><b>yes</b>&nbsp;&larr;</div></li>
+ </ul>
+ </td>
+ <td>
+ <div>Whether the sub-interface is enabled.</div>
+ </td>
+ </tr>
+ <tr>
+ <td class="elbow-placeholder"></td>
+ <td class="elbow-placeholder"></td>
+ <td colspan="1">
+ <div class="ansibleOptionAnchor" id="parameter-"></div>
+ <b>mtu</b>
+ <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
+ <div style="font-size: small">
+ <span style="color: purple">integer</span>
+ </div>
+ </td>
+ <td>
+ </td>
+ <td>
+ <div>Sub-interface MTU.</div>
+ </td>
+ </tr>
+ <tr>
+ <td class="elbow-placeholder"></td>
+ <td class="elbow-placeholder"></td>
+ <td colspan="1">
+ <div class="ansibleOptionAnchor" id="parameter-"></div>
+ <b>vlan_id</b>
+ <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
+ <div style="font-size: small">
+ <span style="color: purple">integer</span>
+ / <span style="color: red">required</span>
+ </div>
+ </td>
+ <td>
+ </td>
+ <td>
+ <div>VLAN ID for this sub-interface.</div>
+ </td>
+ </tr>
<tr>
+ <td class="elbow-placeholder"></td>
<td colspan="2">
<div class="ansibleOptionAnchor" id="parameter-"></div>
+ <b>vrf</b>
+ <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
+ <div style="font-size: small">
+ <span style="color: purple">string</span>
+ </div>
+ </td>
+ <td>
+ </td>
+ <td>
+ <div>VRF instance to bind this interface to.</div>
+ </td>
+ </tr>
+
+ <tr>
+ <td colspan="3">
+ <div class="ansibleOptionAnchor" id="parameter-"></div>
<b>state</b>
<a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
<div style="font-size: small">
@@ -219,6 +327,10 @@ Examples
description: Management interface
mtu: 1500
enabled: true
+ vrf: mgmt
+ vifs:
+ - vlan_id: 200
+ description: VIF 200
state: merged
- name: Disable an interface
@@ -228,7 +340,7 @@ Examples
enabled: false
state: merged
- - name: Delete interface description
+ - name: Delete interface config
vyos.rest.vyos_interfaces:
config:
- name: eth0
diff --git a/plugins/modules/vyos_interfaces.py b/plugins/modules/vyos_interfaces.py
index 6ed45d5..f5733ac 100644
--- a/plugins/modules/vyos_interfaces.py
+++ b/plugins/modules/vyos_interfaces.py
@@ -12,9 +12,17 @@ DOCUMENTATION = r"""
module: vyos_interfaces
short_description: Manage interface configuration on VyOS devices via REST API.
description:
- - Manages L2 interface configuration (description, MTU, speed, duplex, enabled)
- on VyOS devices using the HTTPS REST API.
+ - Manages L2 interface configuration (description, MTU, speed, duplex,
+ enabled, VRF assignment, VLAN sub-interfaces) on VyOS devices using the
+ HTTPS REST API.
- IP address configuration is handled by M(vyos.rest.vyos_l3_interfaces).
+ - >-
+ Covers 11 interface types (ethernet, bonding, loopback, tunnel,
+ wireguard, vti, dummy, openvpn, pppoe, wireless, bridge), resolved
+ from the device response when present, otherwise guessed from the
+ interface name. The current CLI collection module documents a
+ narrower scope of 5 types (ethernet, bonding, vxlan, loopback,
+ vti).
version_added: "1.0.0"
author:
- VyOS Community (@vyos)
@@ -46,6 +54,28 @@ options:
description: Interface speed setting.
type: str
choices: [auto, "10", "100", "1000", "2500", "10000"]
+ vrf:
+ description: VRF instance to bind this interface to.
+ type: str
+ vifs:
+ description: 802.1Q VLAN sub-interfaces.
+ type: list
+ elements: dict
+ suboptions:
+ vlan_id:
+ description: VLAN ID for this sub-interface.
+ type: int
+ required: true
+ description:
+ description: Sub-interface description.
+ type: str
+ enabled:
+ description: Whether the sub-interface is enabled.
+ type: bool
+ default: true
+ mtu:
+ description: Sub-interface MTU.
+ type: int
state:
description:
- C(merged) - Merge config with existing interface settings.
@@ -69,6 +99,10 @@ EXAMPLES = r"""
description: Management interface
mtu: 1500
enabled: true
+ vrf: mgmt
+ vifs:
+ - vlan_id: 200
+ description: VIF 200
state: merged
- name: Disable an interface
@@ -78,7 +112,7 @@ EXAMPLES = r"""
enabled: false
state: merged
-- name: Delete interface description
+- name: Delete interface config
vyos.rest.vyos_interfaces:
config:
- name: eth0
@@ -117,11 +151,18 @@ response:
"""
from ansible.module_utils.basic import AnsibleModule
-from ansible_collections.vyos.rest.plugins.module_utils.vyos import VyOSModule
+from ansible_collections.vyos.rest.plugins.module_utils.vyos import (
+ VyOSModule,
+ cast_by_spec,
+ dict_op,
+ to_tag_dict,
+)
+
+_BASE = ["interfaces"]
-# Interface name prefix → API type key
-_IFACE_TYPE = {
+
+_IFACE_TYPE_PREFIX = {
"eth": "ethernet",
"bond": "bonding",
"lo": "loopback",
@@ -135,146 +176,293 @@ _IFACE_TYPE = {
"br": "bridge",
}
-# L2 fields managed by this module — excludes address, hw-id etc.
-_L2_FIELDS = ["description", "mtu", "duplex", "speed"]
-
-def _iface_type(name):
- for prefix, itype in _IFACE_TYPE.items():
+def _guess_iface_type(name):
+ for prefix, itype in _IFACE_TYPE_PREFIX.items():
if name.startswith(prefix):
return itype
return "ethernet"
-def _iface_base(name):
- return ["interfaces", _iface_type(name), name]
+def _resolve_iface_type(name, raw_have):
+ """Prefer the real type from the device's own raw response
+ (organized by type at the top level) over a name-prefix guess --
+ only fall back to guessing for a brand-new interface that doesn't
+ exist on the device yet.
+ """
+ for itype, ifaces in (raw_have or {}).items():
+ if name in to_tag_dict(ifaces):
+ return itype
+ return _guess_iface_type(name)
-def get_running_config(vyos):
- raw = vyos.get_config(["interfaces"])
- if not raw or not isinstance(raw, dict):
- return []
+def _iface_base(name, raw_have):
+ return _BASE + [_resolve_iface_type(name, raw_have), name]
- result = []
- for itype, ifaces in sorted(raw.items()):
- if not isinstance(ifaces, dict):
- continue
- for iname, idata in sorted(ifaces.items()):
- idata = idata or {}
- entry = {"name": iname}
- if idata.get("description"):
- entry["description"] = idata["description"]
- if "mtu" in idata:
- entry["mtu"] = int(idata["mtu"])
- if "duplex" in idata:
- entry["duplex"] = idata["duplex"]
- if "speed" in idata:
- entry["speed"] = idata["speed"]
- entry["enabled"] = "disable" not in idata
- result.append(entry)
+_DEVICE_RENAMES = {
+ "vifs": "vif",
+}
+
+
+_ENABLED_FIELD = "enabled"
+_DISABLE_DEVICE_KEY = "disable"
+
+
+def _derive_key_field(options_spec):
+ """The field identifying each entry in a keyed-list section is
+ never inferable from a generic walk alone -- but it doesn't need
+ to be hand-declared either: every such section in this argspec
+ already marks exactly one suboption required=True (you can't
+ create a VIF without a vlan_id). Deriving it here means the key
+ field is asserted to exist by the argspec itself, not duplicated
+ in a place that could drift out of sync with it.
+ """
+ required = [k for k, spec in options_spec.items() if spec.get("required")]
+ if len(required) != 1:
+ raise ValueError(
+ "expected exactly one required suboption to serve as the key field, "
+ "found: {0}".format(required),
+ )
+ return required[0]
+
+
+def _keyed_list_to_device(items, key_field, entry_transform):
+ result = {}
+ for item in items or []:
+ if item.get(key_field) is None:
+ continue
+ rest = {k: v for k, v in item.items() if k != key_field}
+ result[str(item[key_field])] = entry_transform(rest)
return result
-def _normalize(config):
- """Convert argspec list to dict keyed by interface name."""
- return {entry["name"]: entry for entry in (config or [])}
+def _keyed_list_from_device(raw, key_field, entry_transform, key_cast=None):
+ key_cast = key_cast or (lambda k: k)
+ return [
+ {key_field: key_cast(key), **entry_transform(data or {})}
+ for key, data in sorted(to_tag_dict(raw).items())
+ ]
-def _iface_cmds(name, want, have):
- """Generate set/delete commands to bring have → want for one interface."""
- cmds = []
- base = _iface_base(name)
- have = have or {}
+def _spec_to_device(value, options_spec):
+ if not isinstance(value, dict):
+ return value
+ result = {}
+ for arg_key, sub_spec in options_spec.items():
+ if arg_key == _ENABLED_FIELD:
+ if value.get(arg_key) is False:
+ result[_DISABLE_DEVICE_KEY] = {}
+ continue
- # description
- want_desc = want.get("description")
- have_desc = have.get("description")
- if want_desc is not None and want_desc != have_desc:
- cmds.append(("set", base + ["description", want_desc]))
- elif want_desc is None and have_desc is not None:
- cmds.append(("delete", base + ["description"]))
+ val = value.get(arg_key)
+ if val is None or val is False:
+ continue
- # mtu
- want_mtu = want.get("mtu")
- have_mtu = have.get("mtu")
- if want_mtu is not None and want_mtu != have_mtu:
- cmds.append(("set", base + ["mtu", str(want_mtu)]))
+ device_key = _DEVICE_RENAMES.get(arg_key, arg_key.replace("_", "-"))
+ sub_type = sub_spec.get("type")
+ sub_options = sub_spec.get("options")
+
+ if sub_type == "dict" and sub_options:
+ converted = _spec_to_device(val, sub_options)
+ if converted:
+ result[device_key] = converted
+ elif sub_type == "list" and sub_options:
+ key_field = _derive_key_field(sub_options)
+ result[device_key] = _keyed_list_to_device(
+ val,
+ key_field,
+ lambda rest, spec=sub_options: _spec_to_device(rest, spec),
+ )
+ elif val is True:
+ result[device_key] = {}
+ elif sub_type == "list":
+ result[device_key] = list(val)
+ else:
+ result[device_key] = val
+ return result
+
+
+def _device_to_spec(raw, options_spec):
+ if not raw or not isinstance(raw, dict):
+ return {}
+ have_idx = {k.replace("-", "_"): k for k in raw}
+ result = {}
+
+ if _ENABLED_FIELD in options_spec and _DISABLE_DEVICE_KEY in raw:
+ result[_ENABLED_FIELD] = False
- # duplex
- want_duplex = want.get("duplex")
- have_duplex = have.get("duplex")
- if want_duplex is not None and want_duplex != have_duplex:
- cmds.append(("set", base + ["duplex", want_duplex]))
+ for arg_key, sub_spec in options_spec.items():
+ if arg_key == _ENABLED_FIELD:
+ continue
+ device_key = _DEVICE_RENAMES.get(arg_key, arg_key.replace("_", "-"))
+ orig_key = device_key if device_key in raw else have_idx.get(arg_key)
+ if orig_key is None:
+ continue
+ raw_val = raw[orig_key]
+ sub_type = sub_spec.get("type")
+ sub_options = sub_spec.get("options")
+
+ if sub_type == "dict" and sub_options:
+ converted = _device_to_spec(raw_val, sub_options)
+ if converted:
+ result[arg_key] = converted
+ elif sub_type == "list" and sub_options:
+ key_field = _derive_key_field(sub_options)
+ key_cast = int if sub_options[key_field].get("type") == "int" else None
+ entries = _keyed_list_from_device(
+ raw_val,
+ key_field,
+ lambda d, spec=sub_options: _device_to_spec(d, spec),
+ key_cast=key_cast,
+ )
+ if entries:
+ result[arg_key] = entries
+ elif sub_type == "list":
+ if raw_val:
+ result[arg_key] = sorted(to_tag_dict(raw_val).keys())
+ elif isinstance(raw_val, dict) and not raw_val:
+ result[arg_key] = True
+ else:
+ result[arg_key] = raw_val
+ return result
- # speed
- want_speed = want.get("speed")
- have_speed = have.get("speed")
- if want_speed is not None and want_speed != have_speed:
- cmds.append(("set", base + ["speed", want_speed]))
- # enabled / disable flag
- want_enabled = want.get("enabled", True)
- have_enabled = have.get("enabled", True)
- if not want_enabled and have_enabled:
- cmds.append(("set", base + ["disable"]))
- elif want_enabled and not have_enabled:
- cmds.append(("delete", base + ["disable"]))
+def get_running_config(vyos):
+ """VyOS's REST API collapses a single-child tag node to a plain
+ string (or a list for multiple) -- normalizing through to_tag_dict
+ unconditionally means callers always receive a genuine dict.
+ """
+ return to_tag_dict(vyos.get_config(_BASE) or {})
- return cmds
+def _device_to_argspec(raw):
+ result = []
+ for itype, ifaces in sorted((raw or {}).items()):
+ for name, data in sorted(to_tag_dict(ifaces).items()):
+ entry = {"name": name}
+ entry.update(_device_to_spec(data or {}, _ENTRY_OPTIONS))
+ result.append(entry)
+ return result
-def _delete_iface_config(name, have):
- """Generate delete commands to remove L2 config from an interface."""
- cmds = []
- base = _iface_base(name)
- have = have or {}
- for field in _L2_FIELDS:
- if field in have:
- cmds.append(("delete", base + [field]))
- if not have.get("enabled", True):
- cmds.append(("delete", base + ["disable"]))
+def _shadow_vif_entries(want_device, have_device):
+ """Ensure want_device has a (possibly empty) placeholder for every
+ VLAN ID present in have_device's own "vif" dict, so a dict_op
+ purge recurses into each VIF individually rather than treating the
+ whole "vif" key, or any single VLAN entry, as one unit.
+ """
+ have_vifs = have_device.get("vif")
+ if not have_vifs:
+ return want_device
+ shadowed = dict(want_device)
+ want_vifs = dict(shadowed.get("vif") or {})
+ for vlan_id in have_vifs:
+ want_vifs.setdefault(vlan_id, {})
+ shadowed["vif"] = want_vifs
+ return shadowed
+
+
+def _purge_commands(want_device, have_device, base):
+ return dict_op(_shadow_vif_entries(want_device, have_device), have_device, base, op="purge")
+
+
+def _entry_to_device(entry, options_spec):
+ """to-device conversion for a keyed entry whose own key field
+ (e.g. "name" for an interface, same role "vlan_id" plays for a
+ VIF) is present in the input but must never be treated as a
+ regular child leaf -- it identifies the entry itself and is
+ already expressed in the API path (_iface_base), not a field to
+ set/purge under it. _keyed_list_to_device already strips a VIF's
+ "vlan_id" the same way before conversion; interface entries need
+ the same treatment here since build_commands handles the top
+ level manually rather than through that helper.
+ """
+ key_field = _derive_key_field(options_spec)
+ rest = {k: v for k, v in (entry or {}).items() if k != key_field}
+ return _spec_to_device(rest, options_spec)
+
+
+def _scoped_purge_commands(name, have_entry, raw_have):
+ """Remove every field this module manages for one interface --
+ scoped to this module's own fields only, never a whole-subtree
+ delete for the VIF container shared with vyos_l3_interfaces.
+ """
+ have_device = _entry_to_device(have_entry, _ENTRY_OPTIONS)
+ base = _iface_base(name, raw_have)
+ return _purge_commands({}, have_device, base)
+
+
+def _enabled_leaves(device, allowed_vlan_ids=None):
+ """allowed_vlan_ids restricts the VIF portion to VLAN IDs the task
+ actually listed. The interface itself needs no such restriction --
+ it's always "listed" by virtue of appearing in want at all -- but
+ an unlisted VIF was never mentioned by the task, and merged must
+ never touch it. Confirmed real bug otherwise: a VIF the task
+ doesn't reference at all (or a listed interface whose vifs simply
+ omits it) would still have its stale "disable" leaf removed,
+ silently re-enabling a VLAN the administrator deliberately
+ disabled. Pass None (the want side, where every VIF present
+ already is one the task listed) to skip this restriction.
+ """
+ result = {}
+ if _DISABLE_DEVICE_KEY in device:
+ result[_DISABLE_DEVICE_KEY] = device[_DISABLE_DEVICE_KEY]
+ vif = device.get("vif")
+ if vif:
+ vif_result = {
+ vlan_id: {_DISABLE_DEVICE_KEY: v[_DISABLE_DEVICE_KEY]}
+ for vlan_id, v in vif.items()
+ if _DISABLE_DEVICE_KEY in v
+ and (allowed_vlan_ids is None or vlan_id in allowed_vlan_ids)
+ }
+ if vif_result:
+ result["vif"] = vif_result
+ return result
- return cmds
+def build_commands(config, raw_have, state):
+ raw_have = raw_have or {}
+ config = config or []
-def build_commands(config, have_raw, state):
- cmds = []
- have_map = _normalize(have_raw)
+ have_list = _device_to_argspec(raw_have)
+ have_by_name = {e["name"]: e for e in have_list}
+ want_by_name = {e["name"]: e for e in config if e.get("name")}
if state == "deleted":
- if not config:
- for name, have in have_map.items():
- cmds += _delete_iface_config(name, have)
- else:
- for entry in config:
- name = entry["name"]
- cmds += _delete_iface_config(name, have_map.get(name, {}))
+ cmds = []
+ targets = (
+ have_by_name
+ if not config
+ else {n: have_by_name[n] for n in want_by_name if n in have_by_name}
+ )
+ for name, have_entry in targets.items():
+ cmds += _scoped_purge_commands(name, have_entry, raw_have)
return cmds
- want_map = _normalize(config)
-
+ commands = []
if state == "overridden":
- # delete L2 config from interfaces not in want
- for name in set(have_map) - set(want_map):
- cmds += _delete_iface_config(name, have_map[name])
-
- for name, want in want_map.items():
- have = have_map.get(name, {})
+ for name in set(have_by_name) - set(want_by_name):
+ commands += _scoped_purge_commands(name, have_by_name[name], raw_have)
- if state == "replaced":
- # pre-check — only act if something differs
- test_cmds = _iface_cmds(name, want, have)
- if not test_cmds:
- continue
- # delete L2 fields then rebuild
- cmds += _delete_iface_config(name, have)
- have = {}
+ for name, want_entry in want_by_name.items():
+ have_entry = have_by_name.get(name, {})
+ want_device = _entry_to_device(want_entry, _ENTRY_OPTIONS)
+ have_device = _entry_to_device(have_entry, _ENTRY_OPTIONS)
+ base = _iface_base(name, raw_have)
- cmds += _iface_cmds(name, want, have if state != "replaced" else {})
+ if state in ("replaced", "overridden"):
+ commands += _purge_commands(want_device, have_device, base)
+ else:
+ want_enabled = _enabled_leaves(want_device)
+ have_enabled = _enabled_leaves(
+ have_device,
+ allowed_vlan_ids=(want_device.get("vif") or {}).keys(),
+ )
+ commands += _purge_commands(want_enabled, have_enabled, base)
+ commands += dict_op(want_device, have_device, base, op="set")
- return cmds
+ return commands
ARGUMENT_SPEC = dict(
@@ -288,6 +476,17 @@ ARGUMENT_SPEC = dict(
mtu=dict(type="int"),
duplex=dict(type="str", choices=["auto", "full", "half"]),
speed=dict(type="str", choices=["auto", "10", "100", "1000", "2500", "10000"]),
+ vrf=dict(type="str"),
+ vifs=dict(
+ type="list",
+ elements="dict",
+ options=dict(
+ vlan_id=dict(type="int", required=True),
+ description=dict(type="str"),
+ enabled=dict(type="bool", default=True),
+ mtu=dict(type="int"),
+ ),
+ ),
),
),
state=dict(
@@ -297,6 +496,9 @@ ARGUMENT_SPEC = dict(
),
)
+_ENTRY_OPTIONS = ARGUMENT_SPEC["config"]["options"]
+_VIF_OPTIONS = _ENTRY_OPTIONS["vifs"]["options"]
+
def main():
module = AnsibleModule(ARGUMENT_SPEC, supports_check_mode=True)
@@ -305,23 +507,30 @@ def main():
state = module.params["state"]
config = module.params.get("config") or []
- have = get_running_config(vyos)
+ raw_have = get_running_config(vyos)
+ have = _device_to_argspec(raw_have)
+ for entry in have:
+ cast_by_spec(entry, _ENTRY_OPTIONS)
if state == "gathered":
module.exit_json(changed=False, gathered=have)
- commands = build_commands(config, have, state)
+ commands = build_commands(config, raw_have, state)
if module.check_mode:
- module.exit_json(changed=bool(commands), commands=commands, before=have)
+ module.exit_json(changed=bool(commands), commands=commands, before=have, after=have)
if commands:
response = vyos.apply_commands(commands)
saved = vyos.save_config()
+ after_raw = get_running_config(vyos)
+ after = _device_to_argspec(after_raw)
+ for entry in after:
+ cast_by_spec(entry, _ENTRY_OPTIONS)
module.exit_json(
changed=True,
before=have,
- after=get_running_config(vyos),
+ after=after,
commands=commands,
saved=saved,
response=response,
diff --git a/tests/integration/targets/vyos_interfaces/tests/httpapi/overridden.yaml b/tests/integration/targets/vyos_interfaces/tests/httpapi/overridden.yaml
new file mode 100644
index 0000000..650ce1b
--- /dev/null
+++ b/tests/integration/targets/vyos_interfaces/tests/httpapi/overridden.yaml
@@ -0,0 +1,104 @@
+---
+- debug:
+ msg: START vyos_interfaces overridden integration tests on connection={{ ansible_connection }}
+
+- include_tasks: _remove_config.yaml
+- include_tasks: _populate_config.yaml
+
+- block:
+ - name: Override interface configuration
+ register: result
+ vyos.rest.vyos_interfaces: &id001
+ config:
+ - name: eth0
+ description: Ansible overridden interface
+ mtu: 1300
+ state: overridden
+
+ - assert:
+ that:
+ - result.changed == true
+
+ - name: Override interface configuration (IDEMPOTENT)
+ register: result
+ vyos.rest.vyos_interfaces: *id001
+
+ - name: Assert idempotent
+ assert:
+ that:
+ - result.changed == false
+ - result.commands == []
+
+ always:
+ - include_tasks: _remove_config.yaml
+
+# Regression test (CodeRabbit, PR #33): overriding while an interface's
+# VIF is left entirely unlisted (the interface itself isn't even named
+# in config) must never delete the whole VIF node -- only this
+# module's own managed leaves within it -- since vyos_l3_interfaces
+# may have an address configured on that same VIF.
+- block:
+ - name: Set up an interface with a VIF
+ vyos.rest.vyos_interfaces:
+ config:
+ - name: eth1
+ description: mgmt
+ vifs:
+ - vlan_id: 200
+ description: management vlan
+ state: merged
+
+ - name: Set an address on that VIF via vyos_l3_interfaces
+ vyos.rest.vyos_l3_interfaces:
+ config:
+ - name: eth1
+ vifs:
+ - vlan_id: 200
+ ipv4:
+ - address: 192.0.2.1/24
+ state: merged
+
+ - name: Override with eth1 entirely unlisted
+ register: result
+ vyos.rest.vyos_interfaces:
+ config:
+ - name: eth0
+ description: Ansible overridden interface
+ state: overridden
+
+ - assert:
+ that:
+ - result.changed == true
+ - "'interfaces,ethernet,eth1,vif' not in (result.commands | map('join', ',') | list | join('|'))"
+ - "'interfaces,ethernet,eth1,vif,200' not in (result.commands | map('join', ',') | list | join('|'))"
+
+ - name: Confirm the address survived, via vyos_l3_interfaces
+ register: l3_check
+ vyos.rest.vyos_l3_interfaces:
+ state: gathered
+
+ - assert:
+ that:
+ - >-
+ l3_check.gathered
+ | selectattr('name', 'equalto', 'eth1')
+ | map(attribute='vifs')
+ | first
+ | selectattr('vlan_id', 'equalto', 200)
+ | map(attribute='ipv4')
+ | first
+ | map(attribute='address')
+ | list == ['192.0.2.1/24']
+
+ always:
+ - vyos.rest.vyos_interfaces:
+ config:
+ - name: eth1
+ state: deleted
+ ignore_errors: true
+ - vyos.rest.vyos_l3_interfaces:
+ config:
+ - name: eth1
+ state: deleted
+ ignore_errors: true
+ - include_tasks: _remove_config.yaml
diff --git a/tests/integration/targets/vyos_interfaces/tests/httpapi/rtt.yaml b/tests/integration/targets/vyos_interfaces/tests/httpapi/rtt.yaml
new file mode 100644
index 0000000..e6783e3
--- /dev/null
+++ b/tests/integration/targets/vyos_interfaces/tests/httpapi/rtt.yaml
@@ -0,0 +1,107 @@
+---
+- debug:
+ msg: START vyos_interfaces round trip integration tests on connection={{ ansible_connection }}
+
+- include_tasks: _remove_config.yaml
+
+- block:
+ - name: RTT - Apply base configuration
+ vyos.rest.vyos_interfaces:
+ config:
+ - name: eth0
+ description: Ansible test interface
+ mtu: 1450
+ vifs:
+ - vlan_id: 200
+ description: management vlan
+ state: merged
+
+ - name: RTT - Gather
+ register: gathered
+ vyos.rest.vyos_interfaces:
+ state: gathered
+
+ - assert:
+ that:
+ - >-
+ gathered.gathered
+ | selectattr('name', 'equalto', 'eth0')
+ | map(attribute='description')
+ | first == 'Ansible test interface'
+ - >-
+ gathered.gathered
+ | selectattr('name', 'equalto', 'eth0')
+ | map(attribute='mtu')
+ | first == 1450
+ - >-
+ gathered.gathered
+ | selectattr('name', 'equalto', 'eth0')
+ | map(attribute='vifs')
+ | first
+ | selectattr('vlan_id', 'equalto', 200)
+ | map(attribute='description')
+ | first == 'management vlan'
+
+ - name: RTT - Modify description and disable the VIF
+ vyos.rest.vyos_interfaces:
+ config:
+ - name: eth0
+ description: Ansible test interface updated
+ mtu: 1450
+ vifs:
+ - vlan_id: 200
+ description: management vlan
+ enabled: false
+ state: replaced
+
+ - name: RTT - Gather after modify
+ register: gathered2
+ vyos.rest.vyos_interfaces:
+ state: gathered
+
+ - assert:
+ that:
+ - >-
+ gathered2.gathered
+ | selectattr('name', 'equalto', 'eth0')
+ | map(attribute='description')
+ | first == 'Ansible test interface updated'
+ - >-
+ gathered2.gathered
+ | selectattr('name', 'equalto', 'eth0')
+ | map(attribute='vifs')
+ | first
+ | selectattr('vlan_id', 'equalto', 200)
+ | map(attribute='enabled', default=true)
+ | first == false
+
+ - name: RTT - Re-enable the VIF
+ vyos.rest.vyos_interfaces:
+ config:
+ - name: eth0
+ description: Ansible test interface updated
+ mtu: 1450
+ vifs:
+ - vlan_id: 200
+ description: management vlan
+ enabled: true
+ state: replaced
+
+ - name: RTT - Gather after re-enable
+ register: gathered3
+ vyos.rest.vyos_interfaces:
+ state: gathered
+
+ - assert:
+ that:
+ - >-
+ gathered3.gathered
+ | selectattr('name', 'equalto', 'eth0')
+ | map(attribute='vifs')
+ | first
+ | selectattr('vlan_id', 'equalto', 200)
+ | map(attribute='enabled', default=true)
+ | first == true
+
+ always:
+ - include_tasks: _remove_config.yaml
diff --git a/tests/unit/fixtures/interfaces_running.json b/tests/unit/fixtures/interfaces_running.json
index ee67fa2..91b132c 100644
--- a/tests/unit/fixtures/interfaces_running.json
+++ b/tests/unit/fixtures/interfaces_running.json
@@ -1,15 +1,147 @@
{
- "ethernet": {
- "eth0": {
- "address": "dhcp",
- "description": "Management",
- "duplex": "auto",
- "hw-id": "52:54:00:65:5a:24",
- "mtu": "1500",
- "speed": "auto"
- }
- },
- "loopback": {
- "lo": {}
+ "default": {
+ "ethernet": {
+ "eth0": {
+ "description": "mgmt",
+ "mtu": "1500",
+ "duplex": "auto",
+ "speed": "auto",
+ "vrf": "mgmt",
+ "vif": { "200": { "description": "vif200", "mtu": "1400" } }
+ },
+ "eth1": {
+ "disable": {}
+ }
+ },
+ "bonding": {
+ "bond0": { "description": "lag" }
+ },
+ "loopback": {
+ "lo": {}
+ }
+ },
+
+ "iface_with_address_mtu_description": {
+ "ethernet": {
+ "eth0": {
+ "address": ["192.168.122.6/24"],
+ "description": "mgmt",
+ "mtu": "1500"
+ }
+ }
+ },
+ "iface_address_only": {
+ "ethernet": { "eth0": { "address": ["192.168.122.6/24"] } }
+ },
+ "two_ifaces_one_with_address": {
+ "ethernet": {
+ "eth0": {},
+ "eth1": { "address": ["10.0.0.1/24"], "description": "old" }
+ }
+ },
+ "iface_address_and_description": {
+ "ethernet": { "eth0": { "address": ["10.0.0.1/24"], "description": "old" } }
+ },
+
+ "vif_address_base": {
+ "ethernet": {
+ "eth0": {
+ "description": "mgmt",
+ "vif": {
+ "200": {
+ "address": { "192.0.2.1/24": {} },
+ "description": "management vlan"
+ }
+ }
+ }
+ }
+ },
+ "vif_stale_disable_kept": {
+ "ethernet": {
+ "eth0": {
+ "description": "x",
+ "vif": { "200": { "description": "management vlan", "disable": {} } }
+ }
+ }
+ },
+ "vif_multi_mixed": {
+ "ethernet": {
+ "eth0": {
+ "description": "mgmt",
+ "vif": {
+ "100": { "description": "kept-vlan" },
+ "200": {
+ "address": { "192.0.2.1/24": {} },
+ "description": "removed-vlan"
+ },
+ "300": { "address": { "10.0.0.1/24": {} } }
+ }
+ }
+ }
+ },
+ "vif_mtu_with_address": {
+ "ethernet": {
+ "eth0": {
+ "vif": { "200": { "address": { "192.0.2.1/24": {} }, "mtu": "1400" } }
+ }
+ }
+ },
+ "vif_new_vif_base": {
+ "ethernet": { "eth0": { "description": "mgmt" } }
+ },
+
+ "description_mtu_only": {
+ "ethernet": { "eth0": { "description": "x", "mtu": "1500" } }
+ },
+ "iface_disabled_with_fields": {
+ "ethernet": {
+ "eth0": { "disable": {}, "mtu": "1500", "description": "keep" }
+ }
+ },
+ "vif_disabled_on_device": {
+ "ethernet": { "eth0": { "vif": { "200": { "disable": {} } } } }
+ },
+ "description_mtu_old_1400": {
+ "ethernet": { "eth0": { "description": "old", "mtu": "1400" } }
+ },
+ "description_mtu_duplex": {
+ "ethernet": {
+ "eth0": { "description": "old", "mtu": "1500", "duplex": "auto" }
+ }
+ },
+ "eth0_old_eth1_keepme": {
+ "ethernet": {
+ "eth0": { "description": "old" },
+ "eth1": { "description": "keep-me" }
+ }
+ },
+ "eth0_empty_eth1_described": {
+ "ethernet": { "eth0": {}, "eth1": { "description": "old" } }
+ },
+ "two_described_ifaces": {
+ "ethernet": {
+ "eth0": { "description": "a" },
+ "eth1": { "description": "b" }
+ }
+ },
+ "one_described_iface": {
+ "ethernet": { "eth0": { "description": "a", "mtu": "1500" } }
+ },
+ "eth0_truly_empty": {
+ "ethernet": { "eth0": {} }
+ },
+ "eth1_truly_empty": {
+ "ethernet": { "eth1": {} }
+ },
+
+ "bridge_ethbr0": {
+ "bridge": { "ethbr0": {} }
+ },
+ "ethernet_and_bonding": {
+ "ethernet": { "eth0": {} },
+ "bonding": { "bond0": {} }
+ },
+ "bonding_bond0_only": {
+ "bonding": { "bond0": {} }
}
}
diff --git a/tests/unit/modules/test_vyos_interfaces.py b/tests/unit/modules/test_vyos_interfaces.py
index d522c81..56683df 100644
--- a/tests/unit/modules/test_vyos_interfaces.py
+++ b/tests/unit/modules/test_vyos_interfaces.py
@@ -4,312 +4,692 @@ from __future__ import absolute_import, division, print_function
__metaclass__ = type
-import json
-import os
import unittest
from unittest.mock import MagicMock
from ansible_collections.vyos.rest.plugins.modules.vyos_interfaces import (
- _delete_iface_config,
+ _DEVICE_RENAMES,
+ ARGUMENT_SPEC,
+ _derive_key_field,
+ _device_to_argspec,
+ _device_to_spec,
+ _entry_to_device,
+ _guess_iface_type,
_iface_base,
- _iface_cmds,
- _iface_type,
+ _keyed_list_from_device,
+ _keyed_list_to_device,
+ _resolve_iface_type,
+ _spec_to_device,
build_commands,
+ cast_by_spec,
get_running_config,
)
+from .base import load_fixture
-def load_fixture(filename):
- fixtures_dir = os.path.join(os.path.dirname(__file__), "..", "fixtures")
- path = os.path.join(fixtures_dir, filename)
- with open(path) as f:
- return json.load(f)
+
+_FIXTURES = load_fixture("interfaces_running.json")
class VyOSModuleTestCase(unittest.TestCase):
def setUp(self):
self.mock_vyos = MagicMock()
- self.mock_vyos.get_config = MagicMock(return_value={})
+ self.fixture = _FIXTURES["default"]
+ self.mock_vyos.get_config = MagicMock(return_value=self.fixture)
- def set_running_config(self, data):
- self.mock_vyos.get_config.return_value = data
+class TestGetRunningConfig(VyOSModuleTestCase):
+ def test_returns_config_directly(self):
+ result = get_running_config(self.mock_vyos)
+ self.assertIn("ethernet", result)
-class TestVyOSInterfacesIfaceType(unittest.TestCase):
+ def test_empty_config(self):
+ self.mock_vyos.get_config = MagicMock(return_value=None)
+ self.assertEqual(get_running_config(self.mock_vyos), {})
- def test_eth_is_ethernet(self):
- self.assertEqual(_iface_type("eth0"), "ethernet")
+ def test_collapsed_response_normalized(self):
+ """VyOS's REST API collapses a single-child tag node to a bare
+ string -- get_running_config must always return a genuine
+ dict regardless."""
+ self.mock_vyos.get_config = MagicMock(return_value="eth0")
+ result = get_running_config(self.mock_vyos)
+ self.assertEqual(result, {"eth0": {}})
+
+
+class TestDeviceRenames(unittest.TestCase):
+ """The one thing a purely structural walk can never infer: "vifs"
+ (argspec, plural) vs "vif" (device, singular) is a genuine word-
+ form rename, not a hyphen/underscore difference dict_op's own
+ normalization could fold on its own. Declared once here as a flat
+ value map, not embedded in ARGUMENT_SPEC."""
+
+ def test_confirmed_rename_present(self):
+ self.assertEqual(_DEVICE_RENAMES.get("vifs"), "vif")
+
+
+class TestSpecToDevice(unittest.TestCase):
+ """The generic recursive walker that replaced a hand-written to-
+ device/from-device function pair for every nesting level in this
+ module. Driven by the options spec's own structure (dict ->
+ recurse, list with options -> a named list keyed by
+ _derive_key_field) plus _DEVICE_RENAMES and the enabled/disable
+ exception for the handful of non-mechanical differences."""
+
+ def test_plain_scalar_passes_through_unrenamed(self):
+ spec = {"description": {"type": "str"}}
+ self.assertEqual(_spec_to_device({"description": "x"}, spec), {"description": "x"})
+
+ def test_rename_applied_via_device_renames(self):
+ spec = {"vifs": {"type": "list", "options": {"vlan_id": {"required": True}}}}
+ result = _spec_to_device({"vifs": [{"vlan_id": 200}]}, spec)
+ self.assertEqual(result, {"vif": {"200": {}}})
+
+ def test_enabled_true_omitted(self):
+ spec = {"enabled": {"type": "bool"}}
+ self.assertEqual(_spec_to_device({"enabled": True}, spec), {})
+
+ def test_enabled_false_becomes_disable_presence(self):
+ spec = {"enabled": {"type": "bool"}}
+ self.assertEqual(_spec_to_device({"enabled": False}, spec), {"disable": {}})
+
+ def test_named_list_keyed_by_required_field(self):
+ spec = {
+ "vifs": {
+ "type": "list",
+ "options": {
+ "vlan_id": {"type": "int", "required": True},
+ "description": {"type": "str"},
+ },
+ },
+ }
+ result = _spec_to_device(
+ {"vifs": [{"vlan_id": 200, "description": "v200"}]},
+ spec,
+ )
+ self.assertEqual(result, {"vif": {"200": {"description": "v200"}}})
- def test_bond_is_bonding(self):
- self.assertEqual(_iface_type("bond0"), "bonding")
+ def test_plain_scalar_list_passes_through(self):
+ spec = {"tags": {"type": "list"}}
+ result = _spec_to_device({"tags": ["a", "b"]}, spec)
+ self.assertEqual(result, {"tags": ["a", "b"]})
- def test_lo_is_loopback(self):
- self.assertEqual(_iface_type("lo"), "loopback")
+ def test_bool_true_is_presence_for_non_enabled_fields(self):
+ spec = {"disable": {"type": "bool"}}
+ self.assertEqual(_spec_to_device({"disable": True}, spec), {"disable": {}})
- def test_wg_is_wireguard(self):
- self.assertEqual(_iface_type("wg0"), "wireguard")
+ def test_bool_false_omitted_for_non_enabled_fields(self):
+ spec = {"disable": {"type": "bool"}}
+ self.assertEqual(_spec_to_device({"disable": False}, spec), {})
- def test_br_is_bridge(self):
- self.assertEqual(_iface_type("br0"), "bridge")
+ def test_non_dict_value_passes_through(self):
+ self.assertEqual(_spec_to_device("not-a-dict", {}), "not-a-dict")
- def test_unknown_defaults_to_ethernet(self):
- self.assertEqual(_iface_type("xyz0"), "ethernet")
- def test_iface_base_ethernet(self):
- self.assertEqual(
- _iface_base("eth0"),
- ["interfaces", "ethernet", "eth0"],
- )
+class TestDeviceToSpec(unittest.TestCase):
+ """The reverse of _spec_to_device -- same structural rules, same
+ single source of truth for renames and the enabled/disable
+ exception."""
- def test_iface_base_loopback(self):
- self.assertEqual(
- _iface_base("lo"),
- ["interfaces", "loopback", "lo"],
- )
+ def test_mechanical_field_matched_via_hyphen_normalization(self):
+ spec = {"mtu_size": {"type": "str"}}
+ result = _device_to_spec({"mtu-size": "1500"}, spec)
+ self.assertEqual(result, {"mtu_size": "1500"})
+ def test_renamed_field_matched_via_device_renames(self):
+ spec = {
+ "vifs": {
+ "type": "list",
+ "options": {"vlan_id": {"type": "int", "required": True}},
+ },
+ }
+ result = _device_to_spec({"vif": {"200": {}}}, spec)
+ self.assertEqual(result, {"vifs": [{"vlan_id": 200}]})
-class TestVyOSInterfacesGetRunningFixture(VyOSModuleTestCase):
+ def test_disable_presence_becomes_enabled_false(self):
+ spec = {"enabled": {"type": "bool"}}
+ self.assertEqual(_device_to_spec({"disable": {}}, spec), {"enabled": False})
- def setUp(self):
- super().setUp()
- self.fixture = load_fixture("interfaces_running.json")
+ def test_enabled_omitted_when_no_disable_leaf(self):
+ spec = {"enabled": {"type": "bool"}, "description": {"type": "str"}}
+ result = _device_to_spec({"description": "v1"}, spec)
+ self.assertNotIn("enabled", result)
- def test_fixture_parses_eth0(self):
- self.set_running_config(self.fixture)
- result = get_running_config(self.mock_vyos)
- eth0 = next(e for e in result if e["name"] == "eth0")
- self.assertEqual(eth0["description"], "Management")
- self.assertEqual(eth0["mtu"], 1500)
- self.assertEqual(eth0["duplex"], "auto")
- self.assertEqual(eth0["speed"], "auto")
- self.assertTrue(eth0["enabled"])
+ def test_plain_scalar_list_sorted_and_collapse_safe(self):
+ spec = {"tags": {"type": "list"}}
+ result = _device_to_spec({"tags": "a"}, spec)
+ self.assertEqual(result, {"tags": ["a"]})
- def test_fixture_parses_loopback(self):
- self.set_running_config(self.fixture)
- result = get_running_config(self.mock_vyos)
- lo = next(e for e in result if e["name"] == "lo")
- self.assertTrue(lo["enabled"])
- self.assertNotIn("description", lo)
-
- def test_fixture_disabled_interface(self):
- fixture = dict(self.fixture)
- fixture["ethernet"]["eth1"] = {"disable": {}, "description": "Unused"}
- self.set_running_config(fixture)
- result = get_running_config(self.mock_vyos)
- eth1 = next(e for e in result if e["name"] == "eth1")
- self.assertFalse(eth1["enabled"])
+ def test_empty_or_non_dict_raw(self):
+ self.assertEqual(_device_to_spec({}, {}), {})
+ self.assertEqual(_device_to_spec(None, {}), {})
+ self.assertEqual(_device_to_spec("not-a-dict", {}), {})
-class TestVyOSInterfacesGetRunning(VyOSModuleTestCase):
+class TestKeyedListHelper(unittest.TestCase):
+ """The generic mechanic the VIF section shares with any other
+ named-list section: a list of dicts identified by one field
+ becomes a device dict keyed by that field's value."""
- def test_empty_returns_empty_list(self):
- self.set_running_config({})
- result = get_running_config(self.mock_vyos)
- self.assertEqual(result, [])
+ def test_to_device_skips_entries_missing_key_field(self):
+ result = _keyed_list_to_device([{"description": "x"}], "vlan_id", lambda r: r)
+ self.assertEqual(result, {})
- def test_mtu_cast_to_int(self):
- self.set_running_config(
- {
- "ethernet": {
- "eth0": {"mtu": "1500"},
- },
- },
- )
- result = get_running_config(self.mock_vyos)
- eth0 = next(e for e in result if e["name"] == "eth0")
- self.assertIsInstance(eth0["mtu"], int)
- self.assertEqual(eth0["mtu"], 1500)
+ def test_to_device_key_field_stripped_from_rest(self):
+ seen = {}
- def test_hw_id_not_included(self):
- self.set_running_config(
- {
- "ethernet": {
- "eth0": {"hw-id": "52:54:00:65:5a:24"},
- },
- },
- )
- result = get_running_config(self.mock_vyos)
- eth0 = next(e for e in result if e["name"] == "eth0")
- self.assertNotIn("hw_id", eth0)
- self.assertNotIn("hw-id", eth0)
+ def transform(rest):
+ seen.update(rest)
+ return rest
- def test_enabled_true_when_no_disable(self):
- self.set_running_config(
- {
- "ethernet": {"eth0": {}},
- },
+ _keyed_list_to_device([{"vlan_id": 200, "description": "v200"}], "vlan_id", transform)
+ self.assertNotIn("vlan_id", seen)
+ self.assertEqual(seen, {"description": "v200"})
+
+ def test_from_device_key_cast_applied(self):
+ result = _keyed_list_from_device({"200": {}}, "vlan_id", lambda d: d, key_cast=int)
+ self.assertEqual(result, [{"vlan_id": 200}])
+
+ def test_from_device_bare_string_collapse(self):
+ result = _keyed_list_from_device("200", "vlan_id", lambda d: d, key_cast=int)
+ self.assertEqual(result, [{"vlan_id": 200}])
+
+ def test_empty(self):
+ self.assertEqual(_keyed_list_to_device([], "vlan_id", lambda r: r), {})
+ self.assertEqual(_keyed_list_to_device(None, "vlan_id", lambda r: r), {})
+ self.assertEqual(_keyed_list_from_device({}, "vlan_id", lambda d: d), [])
+ self.assertEqual(_keyed_list_from_device(None, "vlan_id", lambda d: d), [])
+
+
+class TestDeriveKeyField(unittest.TestCase):
+ """key_field is derived from each section's own options spec, not
+ hand-declared -- every named-list section marks exactly one
+ suboption required=True (you can't create a VIF without a
+ vlan_id), so that's the field identifying each entry."""
+
+ def test_derives_the_single_required_field(self):
+ self.assertEqual(
+ _derive_key_field({"vlan_id": {"required": True}, "mtu": {"type": "int"}}),
+ "vlan_id",
)
- result = get_running_config(self.mock_vyos)
- eth0 = next(e for e in result if e["name"] == "eth0")
- self.assertTrue(eth0["enabled"])
- def test_enabled_false_when_disable_present(self):
- self.set_running_config(
- {
- "ethernet": {"eth0": {"disable": {}}},
- },
+ def test_raises_if_none_required(self):
+ with self.assertRaises(ValueError):
+ _derive_key_field({"mtu": {"type": "int"}})
+
+ def test_raises_if_more_than_one_required(self):
+ with self.assertRaises(ValueError):
+ _derive_key_field({"a": {"required": True}, "b": {"required": True}})
+
+
+class TestEntryToDevice(unittest.TestCase):
+ """_entry_to_device strips an entry's own identifying key field
+ (the same role _keyed_list_to_device already strips "vlan_id" for
+ -- here, "name" for a top-level interface entry) before the
+ generic walk, since it identifies the entry itself (already
+ expressed in the API path via _iface_base) rather than a child
+ leaf to set/purge under it."""
+
+ def setUp(self):
+ self.entry_options = ARGUMENT_SPEC["config"]["options"]
+ self.vif_options = self.entry_options["vifs"]["options"]
+
+ def test_name_never_leaks_as_a_device_field(self):
+ result = _entry_to_device({"name": "eth0", "description": "x"}, self.entry_options)
+ self.assertNotIn("name", result)
+ self.assertEqual(result, {"description": "x"})
+
+ def test_vifs_keyed_by_vlan_id(self):
+ result = _entry_to_device(
+ {"name": "eth0", "vifs": [{"vlan_id": 200, "description": "v200"}]},
+ self.entry_options,
)
- result = get_running_config(self.mock_vyos)
- eth0 = next(e for e in result if e["name"] == "eth0")
- self.assertFalse(eth0["enabled"])
+ self.assertEqual(result, {"vif": {"200": {"description": "v200"}}})
+ def test_disabled_interface(self):
+ result = _entry_to_device({"name": "eth0", "enabled": False}, self.entry_options)
+ self.assertEqual(result, {"disable": {}})
-class TestVyOSInterfacesIfaceCmds(unittest.TestCase):
+ def test_vrf_only_no_stray_vif_key(self):
+ result = _entry_to_device({"name": "eth0", "vrf": "mgmt"}, self.entry_options)
+ self.assertEqual(result, {"vrf": "mgmt"})
- def test_set_description(self):
- want = {"description": "WAN", "enabled": True}
- cmds = _iface_cmds("eth0", want, {})
+ def test_vif_entry_to_device(self):
+ result = _entry_to_device(
+ {"vlan_id": 200, "description": "v1", "mtu": 1400},
+ self.vif_options,
+ )
+ self.assertEqual(result, {"description": "v1", "mtu": 1400})
+
+ def test_vif_disabled(self):
+ result = _entry_to_device({"vlan_id": 200, "enabled": False}, self.vif_options)
+ self.assertEqual(result, {"disable": {}})
+
+
+class TestTypeResolution(unittest.TestCase):
+ """Primary confirmed bug fix: the original applied the name-prefix
+ guess unconditionally, even for interfaces already known to the
+ device (where the real type is directly, reliably available)."""
+
+ def test_resolves_from_have_when_present(self):
+ raw_have = _FIXTURES["bridge_ethbr0"]
+ self.assertEqual(_resolve_iface_type("ethbr0", raw_have), "bridge")
+
+ def test_falls_back_to_guess_for_new_interface(self):
+ self.assertEqual(_resolve_iface_type("eth5", {}), "ethernet")
+
+ def test_multiple_existing_interfaces_resolved_correctly(self):
+ raw_have = _FIXTURES["ethernet_and_bonding"]
+ self.assertEqual(_resolve_iface_type("eth0", raw_have), "ethernet")
+ self.assertEqual(_resolve_iface_type("bond0", raw_have), "bonding")
+
+ def test_guess_covers_all_11_types(self):
+ expected = {
+ "eth0": "ethernet",
+ "bond0": "bonding",
+ "lo": "loopback",
+ "tun0": "tunnel",
+ "wg0": "wireguard",
+ "vti0": "vti",
+ "dum0": "dummy",
+ "vtun0": "openvpn",
+ "ppp0": "pppoe",
+ "wlan0": "wireless",
+ "br0": "bridge",
+ }
+ for name, itype in expected.items():
+ self.assertEqual(_guess_iface_type(name), itype)
+
+ def test_iface_base_uses_resolved_type(self):
+ raw_have = _FIXTURES["bonding_bond0_only"]
+ self.assertEqual(_iface_base("bond0", raw_have), ["interfaces", "bonding", "bond0"])
+
+
+class TestAddressNeverLeaksIntoManagedFields(unittest.TestCase):
+ """Regression tests for a confirmed severe bug, caught against
+ real hardware: this module's deleted/replaced/overridden states
+ all operate against a reconstructed "have" -- if that have ever
+ included an unmanaged field like "address" (owned by
+ vyos_l3_interfaces, not this module), it would be treated as
+ "present in have, absent from want" and get deleted right
+ alongside the L2 fields this module actually manages. Confirmed
+ on real hardware: this destroyed an interface's IP address,
+ including the one the REST API itself was reachable through
+ ("No route to host" after a deleted-state task).
+
+ The tests from test_deleted_vif_address_never_deletes_whole_vif
+ onward are the command-level companion CodeRabbit asked for: a VIF
+ carrying an address, confirming the purge never wholesale-deletes
+ that VIF's container -- the actual channel through which address
+ could be lost on a real device, which the earlier tests alone did
+ not cover.
+ """
+
+ def test_device_to_spec_does_not_leak_address(self):
+ entry = _device_to_spec(
+ {"address": ["192.168.122.6/24"], "description": "mgmt", "hw-id": "08:00:27"},
+ ARGUMENT_SPEC["config"]["options"],
+ )
+ self.assertNotIn("address", entry)
+ self.assertNotIn("hw_id", entry)
+ self.assertEqual(entry, {"description": "mgmt"})
+
+ def test_vif_device_to_spec_does_not_leak_address(self):
+ entry = _device_to_spec(
+ {"address": ["192.0.2.1/24"], "description": "v1"},
+ ARGUMENT_SPEC["config"]["options"]["vifs"]["options"],
+ )
+ self.assertNotIn("address", entry)
+
+ def test_deleted_named_never_touches_address(self):
+ raw_have = _FIXTURES["iface_with_address_mtu_description"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "deleted")
+ self.assertFalse(any("address" in str(c) for c in cmds))
+ self.assertFalse(any("name" in c[1] for c in cmds))
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "description"]), cmds)
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "mtu"]), cmds)
+
+ def test_deleted_all_never_touches_address(self):
+ raw_have = _FIXTURES["iface_address_only"]
+ cmds = build_commands([], raw_have, "deleted")
+ self.assertFalse(any("address" in str(c) for c in cmds))
+
+ def test_overridden_omitted_interface_never_touches_address(self):
+ raw_have = _FIXTURES["two_ifaces_one_with_address"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "overridden")
+ self.assertFalse(any("address" in str(c) for c in cmds))
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth1", "description"]), cmds)
+
+ def test_replaced_never_touches_address(self):
+ raw_have = _FIXTURES["iface_address_and_description"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "replaced")
+ self.assertFalse(any("address" in str(c) for c in cmds))
+
+ # -----------------------------------------------------------------
+ # Command-level VIF-address-preservation tests (CodeRabbit, PR #33)
+ # -----------------------------------------------------------------
+
+ def test_deleted_vif_address_never_deletes_whole_vif(self):
+ raw_have = _FIXTURES["vif_address_base"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "deleted")
+ paths = [c[1] for c in cmds]
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif"], paths)
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif", "200"], paths)
self.assertIn(
- ("set", ["interfaces", "ethernet", "eth0", "description", "WAN"]),
+ ("delete", ["interfaces", "ethernet", "eth0", "vif", "200", "description"]),
cmds,
)
- def test_set_mtu(self):
- want = {"mtu": 9000, "enabled": True}
- cmds = _iface_cmds("eth0", want, {})
+ def test_replaced_omitting_vifs_never_deletes_whole_vif(self):
+ raw_have = _FIXTURES["vif_address_base"]
+ config = [{"name": "eth0", "description": "mgmt-updated"}]
+ cmds = build_commands(config, raw_have, "replaced")
+ paths = [c[1] for c in cmds]
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif"], paths)
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif", "200"], paths)
self.assertIn(
- ("set", ["interfaces", "ethernet", "eth0", "mtu", "9000"]),
+ ("delete", ["interfaces", "ethernet", "eth0", "vif", "200", "description"]),
cmds,
)
- def test_set_duplex(self):
- want = {"duplex": "full", "enabled": True}
- cmds = _iface_cmds("eth0", want, {})
+ def test_overridden_unlisted_interface_never_deletes_whole_vif(self):
+ raw_have = _FIXTURES["vif_address_base"]
+ cmds = build_commands([], raw_have, "overridden")
+ paths = [c[1] for c in cmds]
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif"], paths)
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif", "200"], paths)
self.assertIn(
- ("set", ["interfaces", "ethernet", "eth0", "duplex", "full"]),
+ ("delete", ["interfaces", "ethernet", "eth0", "vif", "200", "description"]),
cmds,
)
- def test_set_speed(self):
- want = {"speed": "1000", "enabled": True}
- cmds = _iface_cmds("eth0", want, {})
+ def test_overridden_listed_interface_never_deletes_whole_vif(self):
+ """Companion to the unlisted case above: the VIF-bearing
+ interface here is explicitly listed in want (not omitted
+ entirely), which routes through the inline purge path rather
+ than _scoped_purge_commands -- both must be equally safe."""
+ raw_have = _FIXTURES["vif_address_base"]
+ cmds = build_commands([{"name": "eth0", "description": "mgmt"}], raw_have, "overridden")
+ paths = [c[1] for c in cmds]
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif"], paths)
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif", "200"], paths)
+
+ def test_vif_kept_in_want_still_updates_normally(self):
+ """Confirms the fix didn't also break the unrelated, working
+ path: a VIF present in both want and have must still go
+ through the normal op="set" field-by-field reconciliation."""
+ raw_have = _FIXTURES["vif_address_base"]
+ config = [
+ {
+ "name": "eth0",
+ "description": "mgmt",
+ "vifs": [{"vlan_id": 200, "description": "updated-vlan"}],
+ },
+ ]
+ cmds = build_commands(config, raw_have, "replaced")
self.assertIn(
- ("set", ["interfaces", "ethernet", "eth0", "speed", "1000"]),
+ (
+ "set",
+ ["interfaces", "ethernet", "eth0", "vif", "200", "description", "updated-vlan"],
+ ),
cmds,
)
- def test_disable_interface(self):
- want = {"enabled": False}
- have = {"enabled": True}
- cmds = _iface_cmds("eth0", want, have)
+ def test_replaced_still_removes_stale_disable_on_kept_vif(self):
+ """Regression test for a bug introduced by the fix itself and
+ caught by a real ansible-test network-integration run: a VIF
+ kept in both want and have, with enabled: true, must still
+ have its stale "disable" leaf removed under
+ replaced/overridden -- the fix must not also disable field-
+ level purging within a kept VIF."""
+ raw_have = _FIXTURES["vif_stale_disable_kept"]
+ config = [
+ {
+ "name": "eth0",
+ "description": "x",
+ "vifs": [{"vlan_id": 200, "description": "management vlan", "enabled": True}],
+ },
+ ]
+ cmds = build_commands(config, raw_have, "replaced")
self.assertIn(
- ("set", ["interfaces", "ethernet", "eth0", "disable"]),
+ ("delete", ["interfaces", "ethernet", "eth0", "vif", "200", "disable"]),
cmds,
)
- def test_enable_interface(self):
- want = {"enabled": True}
- have = {"enabled": False}
- cmds = _iface_cmds("eth0", want, have)
+ def test_multi_vif_mixed_kept_removed_and_address_only(self):
+ """Three VIFs in one interface, each in a different state: one
+ kept and updated, one removed (with an address that must
+ survive), one address-only (never touched at all)."""
+ raw_have = _FIXTURES["vif_multi_mixed"]
+ config = [
+ {
+ "name": "eth0",
+ "description": "mgmt",
+ "vifs": [{"vlan_id": 100, "description": "kept-vlan-updated"}],
+ },
+ ]
+ cmds = build_commands(config, raw_have, "replaced")
+ paths = [c[1] for c in cmds]
+
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif", "200"], paths)
self.assertIn(
- ("delete", ["interfaces", "ethernet", "eth0", "disable"]),
+ ("delete", ["interfaces", "ethernet", "eth0", "vif", "200", "description"]),
cmds,
)
-
- def test_idempotent_description(self):
- want = {"description": "WAN", "enabled": True}
- have = {"description": "WAN", "enabled": True}
- cmds = _iface_cmds("eth0", want, have)
- self.assertEqual(cmds, [])
-
- def test_delete_description_when_none(self):
- want = {"enabled": True}
- have = {"description": "Old", "enabled": True}
- cmds = _iface_cmds("eth0", want, have)
self.assertIn(
- ("delete", ["interfaces", "ethernet", "eth0", "description"]),
+ (
+ "set",
+ [
+ "interfaces",
+ "ethernet",
+ "eth0",
+ "vif",
+ "100",
+ "description",
+ "kept-vlan-updated",
+ ],
+ ),
cmds,
)
-
-
-class TestVyOSInterfacesDeleteIfaceConfig(unittest.TestCase):
-
- def test_deletes_all_l2_fields(self):
- have = {
- "description": "WAN",
- "mtu": 1500,
- "duplex": "auto",
- "speed": "auto",
- "enabled": True,
- }
- cmds = _delete_iface_config("eth0", have)
+ self.assertFalse(any("300" in p for p in paths))
+
+ def test_mtu_purged_individually_not_whole_vif(self):
+ """The managed-leaf purge must work for mtu specifically, not
+ just description/disable -- otherwise mtu's path stays
+ unconfirmed."""
+ raw_have = _FIXTURES["vif_mtu_with_address"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "deleted")
paths = [c[1] for c in cmds]
- self.assertIn(["interfaces", "ethernet", "eth0", "description"], paths)
- self.assertIn(["interfaces", "ethernet", "eth0", "mtu"], paths)
- self.assertIn(["interfaces", "ethernet", "eth0", "duplex"], paths)
- self.assertIn(["interfaces", "ethernet", "eth0", "speed"], paths)
-
- def test_deletes_disable_when_disabled(self):
- have = {"enabled": False}
- cmds = _delete_iface_config("eth0", have)
+ self.assertNotIn(["interfaces", "ethernet", "eth0", "vif", "200"], paths)
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "vif", "200", "mtu"]), cmds)
+
+ def test_brand_new_vif_still_added_via_set_path(self):
+ """A VIF present in want but entirely absent from have (a
+ genuinely new VIF, not a purge scenario at all) must still be
+ added correctly -- confirms the fix didn't collaterally affect
+ the unrelated add path."""
+ raw_have = _FIXTURES["vif_new_vif_base"]
+ config = [
+ {
+ "name": "eth0",
+ "description": "mgmt",
+ "vifs": [{"vlan_id": 400, "description": "new-vlan"}],
+ },
+ ]
+ cmds = build_commands(config, raw_have, "replaced")
self.assertIn(
- ("delete", ["interfaces", "ethernet", "eth0", "disable"]),
+ ("set", ["interfaces", "ethernet", "eth0", "vif", "400", "description", "new-vlan"]),
cmds,
)
- def test_empty_have_produces_no_commands(self):
- cmds = _delete_iface_config("eth0", {})
- self.assertEqual(cmds, [])
+class TestDeviceToArgspecFixture(VyOSModuleTestCase):
+ def test_all_interfaces_present(self):
+ have = _device_to_argspec(self.fixture)
+ names = {e["name"] for e in have}
+ self.assertEqual(names, {"eth0", "eth1", "bond0", "lo"})
+
+ def test_eth0_full_fields_parsed(self):
+ """main() applies cast_by_spec to _device_to_argspec's output
+ before returning it to the caller -- applied the same way
+ here to confirm mtu is reported as an int, not the raw device
+ string."""
+ have = _device_to_argspec(self.fixture)
+ eth0 = next(e for e in have if e["name"] == "eth0")
+ cast_by_spec(eth0, ARGUMENT_SPEC["config"]["options"])
+ self.assertEqual(eth0["mtu"], 1500)
+ self.assertEqual(eth0["vrf"], "mgmt")
+ self.assertEqual(eth0["vifs"][0]["vlan_id"], 200)
+ self.assertEqual(eth0["vifs"][0]["mtu"], 1400)
-class TestVyOSInterfacesBuildCommands(unittest.TestCase):
+ def test_eth1_disabled(self):
+ have = _device_to_argspec(self.fixture)
+ eth1 = next(e for e in have if e["name"] == "eth1")
+ self.assertFalse(eth1["enabled"])
+
+ def test_lo_minimal(self):
+ have = _device_to_argspec(self.fixture)
+ lo = next(e for e in have if e["name"] == "lo")
+ self.assertNotIn("mtu", lo)
+
+ def test_empty_config(self):
+ self.assertEqual(_device_to_argspec({}), [])
+ self.assertEqual(_device_to_argspec(None), [])
+
+
+class TestBuildCommands(VyOSModuleTestCase):
+ def test_merged_idempotent_against_own_fixture(self):
+ have = _device_to_argspec(self.fixture)
+ self.assertEqual(build_commands(have, self.fixture, "merged"), [])
+
+ def test_replaced_idempotent_against_own_fixture(self):
+ have = _device_to_argspec(self.fixture)
+ self.assertEqual(build_commands(have, self.fixture, "replaced"), [])
+
+ def test_overridden_idempotent_against_own_fixture(self):
+ have = _device_to_argspec(self.fixture)
+ self.assertEqual(build_commands(have, self.fixture, "overridden"), [])
+
+ def test_merged_basic_set(self):
+ cmds = build_commands([{"name": "eth0", "description": "x", "mtu": 1500}], {}, "merged")
+ self.assertIn(("set", ["interfaces", "ethernet", "eth0", "description", "x"]), cmds)
+ self.assertIn(("set", ["interfaces", "ethernet", "eth0", "mtu", "1500"]), cmds)
+
+ def test_merged_never_purges_unlisted_fields(self):
+ """merged only ever adds/updates fields present in want -- a
+ field present on the device but omitted from want must never
+ be purged, unlike replaced/overridden."""
+ raw_have = _FIXTURES["description_mtu_only"]
+ cmds = build_commands([{"name": "eth0", "description": "y"}], raw_have, "merged")
+ self.assertFalse(any("mtu" in c[1] for c in cmds))
+
+ def test_merged_enabled_true_removes_disable(self):
+ """Interface-level re-enable under merged: mtu/description
+ must stay untouched, only the stale disable leaf is removed."""
+ raw_have = _FIXTURES["iface_disabled_with_fields"]
+ cmds = build_commands([{"name": "eth0", "enabled": True}], raw_have, "merged")
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "disable"]), cmds)
+ self.assertFalse(any("mtu" in c[1] for c in cmds))
+ self.assertFalse(any("description" in c[1] for c in cmds))
+
+ def test_merged_vif_enabled_true_removes_disable(self):
+ raw_have = _FIXTURES["vif_disabled_on_device"]
+ config = [{"name": "eth0", "vifs": [{"vlan_id": 200, "enabled": True}]}]
+ cmds = build_commands(config, raw_have, "merged")
+ self.assertIn(
+ ("delete", ["interfaces", "ethernet", "eth0", "vif", "200", "disable"]),
+ cmds,
+ )
- def _have_eth0(self):
- return [
+ def test_merged_never_reenables_an_unlisted_vif(self):
+ """CodeRabbit regression: merged's re-enable logic previously
+ scanned every disabled VIF on the device, not just ones the
+ task actually lists -- so a VLAN the administrator
+ deliberately disabled would be silently re-enabled by a task
+ that never even mentions "vifs" at all. merged must leave an
+ unlisted VIF completely untouched, same as any other unlisted
+ field."""
+ raw_have = _FIXTURES["vif_disabled_on_device"]
+ cmds = build_commands([{"name": "eth0", "description": "x"}], raw_have, "merged")
+ self.assertFalse(any("200" in c[1] for c in cmds))
+
+ def test_merged_new_interface_with_vrf_and_vif(self):
+ config = [
{
"name": "eth0",
- "description": "Management",
- "mtu": 1500,
- "enabled": True,
+ "vrf": "mgmt",
+ "vifs": [{"vlan_id": 100, "description": "v100"}],
},
]
-
- def test_merged_adds_description(self):
- config = [{"name": "eth0", "description": "WAN", "enabled": True}]
- cmds = build_commands(config, [], "merged")
- paths = [c[1] for c in cmds]
+ cmds = build_commands(config, {}, "merged")
+ self.assertIn(("set", ["interfaces", "ethernet", "eth0", "vrf", "mgmt"]), cmds)
self.assertIn(
- ["interfaces", "ethernet", "eth0", "description", "WAN"],
- paths,
+ ("set", ["interfaces", "ethernet", "eth0", "vif", "100", "description", "v100"]),
+ cmds,
)
- def test_merged_idempotent(self):
- cmds = build_commands(self._have_eth0(), self._have_eth0(), "merged")
- self.assertEqual(cmds, [])
-
- def test_deleted_removes_l2_fields(self):
- config = [{"name": "eth0", "enabled": True}]
- cmds = build_commands(config, self._have_eth0(), "deleted")
- paths = [c[1] for c in cmds]
- self.assertIn(["interfaces", "ethernet", "eth0", "description"], paths)
- self.assertIn(["interfaces", "ethernet", "eth0", "mtu"], paths)
+ def test_merged_disabled_vif(self):
+ config = [{"name": "eth0", "vifs": [{"vlan_id": 100, "enabled": False}]}]
+ cmds = build_commands(config, {}, "merged")
+ self.assertIn(
+ ("set", ["interfaces", "ethernet", "eth0", "vif", "100", "disable"]),
+ cmds,
+ )
- def test_deleted_idempotent_when_no_l2(self):
- have = [{"name": "eth0", "enabled": True}]
- config = [{"name": "eth0", "enabled": True}]
- cmds = build_commands(config, have, "deleted")
+ def test_replaced_basic_purge_and_set(self):
+ raw_have = _FIXTURES["description_mtu_old_1400"]
+ cmds = build_commands([{"name": "eth0", "description": "new"}], raw_have, "replaced")
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "mtu"]), cmds)
+ self.assertIn(("set", ["interfaces", "ethernet", "eth0", "description", "new"]), cmds)
+
+ def test_clear_omitted_fields_on_replaced(self):
+ """Primary confirmed bug: description had explicit clear-on-
+ omit logic, but mtu/duplex/speed did not -- a genuine
+ inconsistency. dict_op's purge handles all fields uniformly."""
+ raw_have = _FIXTURES["description_mtu_duplex"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "replaced")
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "description"]), cmds)
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "mtu"]), cmds)
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "duplex"]), cmds)
+
+ def test_overridden_basic_unlisted_purged_listed_reconciled(self):
+ raw_have = _FIXTURES["eth0_old_eth1_keepme"]
+ cmds = build_commands([{"name": "eth0", "description": "new"}], raw_have, "overridden")
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth1", "description"]), cmds)
+ self.assertIn(("set", ["interfaces", "ethernet", "eth0", "description", "new"]), cmds)
+
+ def test_overridden_removes_omitted_interface(self):
+ raw_have = _FIXTURES["eth0_empty_eth1_described"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "overridden")
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth1", "description"]), cmds)
+
+ def test_deleted_all(self):
+ raw_have = _FIXTURES["two_described_ifaces"]
+ cmds = build_commands([], raw_have, "deleted")
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "description"]), cmds)
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth1", "description"]), cmds)
+
+ def test_deleted_named(self):
+ raw_have = _FIXTURES["one_described_iface"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "deleted")
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "description"]), cmds)
+ self.assertIn(("delete", ["interfaces", "ethernet", "eth0", "mtu"]), cmds)
+
+ def test_deleted_named_empty_interface_is_noop(self):
+ """A named interface with no L2 fields set has nothing for
+ this module to delete -- confirmed correct, not a bug: an
+ empty {} in have means an empty purge."""
+ raw_have = _FIXTURES["eth0_truly_empty"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "deleted")
self.assertEqual(cmds, [])
- def test_replaced_idempotent(self):
- cmds = build_commands(self._have_eth0(), self._have_eth0(), "replaced")
+ def test_deleted_idempotent_absent_interface(self):
+ """Deleting an interface not present on the device at all
+ must be a no-op -- nothing to purge against."""
+ raw_have = _FIXTURES["eth1_truly_empty"]
+ cmds = build_commands([{"name": "eth0"}], raw_have, "deleted")
self.assertEqual(cmds, [])
- def test_replaced_updates_description(self):
- config = [{"name": "eth0", "description": "NEW", "mtu": 1500, "enabled": True}]
- cmds = build_commands(config, self._have_eth0(), "replaced")
- self.assertTrue(len(cmds) > 0)
-
- def test_overridden_clears_interfaces_not_in_want(self):
- have = [
- {"name": "eth0", "description": "Management", "enabled": True},
- {"name": "eth1", "description": "LAN", "enabled": True},
- ]
- config = [{"name": "eth0", "description": "Management", "enabled": True}]
- cmds = build_commands(config, have, "overridden")
- paths = [c[1] for c in cmds]
- self.assertIn(["interfaces", "ethernet", "eth1", "description"], paths)
-
if __name__ == "__main__":
unittest.main()