T8989: lag_interfaces dict op refactor (#35)

* T8989: vyos_lag_interfaces dict_op refactor

* T8989: vyos_lag_interfaces dict_op refactor

* T8989: vyos_lag_interfaces dict_op refactor

---------

Co-authored-by: Daniil Baturin <daniil@vyos.io>

5 files changed, 363 insertions, 546 deletions

diff --git a/changelogs/fragments/t8989_lag_interfaces_dict_op.yml b/changelogs/fragments/t8989_lag_interfaces_dict_op.yml
new file mode 100644
index 0000000..1f3d97a
--- /dev/null
+++ b/changelogs/fragments/t8989_lag_interfaces_dict_op.yml
@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - vyos_lag_interfaces - Refactor the module to comply with dict_op paradigm.
diff --git a/docs/vyos.rest.vyos_lag_interfaces_module.rst b/docs/vyos.rest.vyos_lag_interfaces_module.rst
index 49753f0..95d5969 100644
--- a/docs/vyos.rest.vyos_lag_interfaces_module.rst
+++ b/docs/vyos.rest.vyos_lag_interfaces_module.rst
@@ -18,7 +18,7 @@ Version added: 1.0.0
 Synopsis
 --------
 - Manages Link Aggregation Group (LAG/bonding) interface configuration on VyOS devices using the HTTPS REST API.
-- Mirrors ``vyos.vyos.vyos_lag_interfaces`` but uses the HTTP API instead of CLI.
+- A bonding interface's device path (``interfaces bonding <name>``) is shared with :ref:`vyos.rest.vyos_interfaces <vyos.rest.vyos_interfaces_module>` (L2 attributes: description, mtu, vrf) and :ref:`vyos.rest.vyos_l3_interfaces <vyos.rest.vyos_l3_interfaces_module>` (addresses) -- every command this module generates is scoped to exactly ``mode``/ ``primary``/``hash-policy``/``member``/``arp-monitor``, never the bond's subtree as a whole, so it never touches those other modules' fields.
 
 
 
@@ -220,21 +220,6 @@ Parameters
             <tr>
                 <td colspan="3">
                     <div class="ansibleOptionAnchor" id="parameter-"></div>
-                    <b>running_config</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>Used only with state <code>parsed</code>.</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">
@@ -248,8 +233,6 @@ Parameters
                                     <li>overridden</li>
                                     <li>deleted</li>
                                     <li>gathered</li>
-                                    <li>rendered</li>
-                                    <li>parsed</li>
                         </ul>
                 </td>
                 <td>
@@ -258,8 +241,6 @@ Parameters
                         <div><code>overridden</code> - Replace config for all LAG interfaces.</div>
                         <div><code>deleted</code> - Remove listed or all LAG interface config.</div>
                         <div><code>gathered</code> - Read LAG config from device without changes.</div>
-                        <div><code>rendered</code> - Return commands for provided config without connecting.</div>
-                        <div><code>parsed</code> - Parse running_config into structured data.</div>
                 </td>
             </tr>
     </table>
@@ -274,6 +255,10 @@ See Also
 
    :ref:`vyos.vyos.vyos_lag_interfaces_module`
       The official documentation on the **vyos.vyos.vyos_lag_interfaces** module.
+   :ref:`vyos.rest.vyos_interfaces_module`
+      The official documentation on the **vyos.rest.vyos_interfaces** module.
+   :ref:`vyos.rest.vyos_l3_interfaces_module`
+      The official documentation on the **vyos.rest.vyos_l3_interfaces** module.
 
 
 Examples
@@ -377,36 +362,6 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
             <tr>
                 <td colspan="1">
                     <div class="ansibleOptionAnchor" id="return-"></div>
-                    <b>parsed</b>
-                    <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
-                    <div style="font-size: small">
-                      <span style="color: purple">list</span>
-                    </div>
-                </td>
-                <td>when state is parsed</td>
-                <td>
-                            <div>Structured data parsed from running_config (state=parsed).</div>
-                    <br/>
-                </td>
-            </tr>
-            <tr>
-                <td colspan="1">
-                    <div class="ansibleOptionAnchor" id="return-"></div>
-                    <b>rendered</b>
-                    <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
-                    <div style="font-size: small">
-                      <span style="color: purple">list</span>
-                    </div>
-                </td>
-                <td>when state is rendered</td>
-                <td>
-                            <div>Commands for provided config (state=rendered).</div>
-                    <br/>
-                </td>
-            </tr>
-            <tr>
-                <td colspan="1">
-                    <div class="ansibleOptionAnchor" id="return-"></div>
                     <b>response</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
diff --git a/plugins/modules/vyos_lag_interfaces.py b/plugins/modules/vyos_lag_interfaces.py
index 6855419..58cd852 100644
--- a/plugins/modules/vyos_lag_interfaces.py
+++ b/plugins/modules/vyos_lag_interfaces.py
@@ -14,7 +14,13 @@ short_description: Manage LAG interface configuration on VyOS devices via REST A
 description:
   - Manages Link Aggregation Group (LAG/bonding) interface configuration on VyOS
     devices using the HTTPS REST API.
-  - Mirrors C(vyos.vyos.vyos_lag_interfaces) but uses the HTTP API instead of CLI.
+  - >-
+    A bonding interface's device path (C(interfaces bonding <name>)) is
+    shared with M(vyos.rest.vyos_interfaces) (L2 attributes: description,
+    mtu, vrf) and M(vyos.rest.vyos_l3_interfaces) (addresses) -- every
+    command this module generates is scoped to exactly C(mode)/
+    C(primary)/C(hash-policy)/C(member)/C(arp-monitor), never the bond's
+    subtree as a whole, so it never touches those other modules' fields.
 version_added: "1.0.0"
 author:
   - VyOS Community (@vyos)
@@ -68,9 +74,6 @@ options:
             description: IP addresses to use for ARP monitoring.
             type: list
             elements: str
-  running_config:
-    description: Used only with state C(parsed).
-    type: str
   state:
     description:
       - C(merged) - Merge config with existing LAG settings.
@@ -78,13 +81,13 @@ options:
       - C(overridden) - Replace config for all LAG interfaces.
       - C(deleted) - Remove listed or all LAG interface config.
       - C(gathered) - Read LAG config from device without changes.
-      - C(rendered) - Return commands for provided config without connecting.
-      - C(parsed) - Parse running_config into structured data.
     type: str
-    choices: [merged, replaced, overridden, deleted, gathered, rendered, parsed]
+    choices: [merged, replaced, overridden, deleted, gathered]
     default: merged
 seealso:
   - module: vyos.vyos.vyos_lag_interfaces
+  - module: vyos.rest.vyos_interfaces
+  - module: vyos.rest.vyos_l3_interfaces
 """
 
 EXAMPLES = r"""
@@ -125,14 +128,6 @@ gathered:
   description: Current LAG configuration as structured data.
   returned: when state is gathered
   type: list
-rendered:
-  description: Commands for provided config (state=rendered).
-  returned: when state is rendered
-  type: list
-parsed:
-  description: Structured data parsed from running_config (state=parsed).
-  returned: when state is parsed
-  type: list
 saved:
   description: Whether the config was saved after changes.
   returned: when changes are applied
@@ -144,7 +139,15 @@ 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,
+    autoclean,
+    cast_by_spec,
+    dict_op,
+    from_device,
+    scope_to_spec,
+    to_tag_dict,
+)
 
 
 _BASE = ["interfaces", "bonding"]
@@ -154,271 +157,220 @@ def _bond_base(name):
     return _BASE + [name]
 
 
-def get_running_config(vyos):
-    raw = vyos.get_config(_BASE)
-    if not raw or not isinstance(raw, dict):
-        return []
+def _bond_entry_to_device(rest):
+    """Explicit allowlist -- confirmed critical given bond0's device
+    path is shared with vyos_interfaces and vyos_l3_interfaces. Only
+    mode/primary/hash_policy/members/arp_monitor are modeled here.
+
+    members and arp_monitor.target are built as keyed-presence
+    structures (name/address -> {}) separately from the simple scalar
+    fields, and merged in without going through autoclean: confirmed
+    real bug otherwise -- autoclean recursively drops nested
+    empty-dict values, treating a presence-only leaf like
+    {"eth1": {}} as "nothing to see here" and silently stripping it,
+    when it's actually a meaningful member-interface reference. Their
+    keys are opaque interface names/IP addresses needing no
+    underscore-to-kebab conversion regardless.
+    """
+    simple = autoclean({k: v for k, v in rest.items() if k not in ("members", "arp_monitor")})
+    device = {k.replace("_", "-"): v for k, v in simple.items()}
+
+    members = rest.get("members") or []
+    member_names = [m.get("member") for m in members if m.get("member")]
+    if member_names:
+        device["member"] = {"interface": {m: {} for m in member_names}}
+
+    arp = rest.get("arp_monitor") or {}
+    arp_simple = autoclean({k: v for k, v in arp.items() if k != "target"})
+    arp_device = {k.replace("_", "-"): v for k, v in arp_simple.items()}
+    targets = arp.get("target") or []
+    if targets:
+        arp_device["target"] = {t: {} for t in targets}
+    if arp_device:
+        device["arp-monitor"] = arp_device
+
+    return device
+
+
+def _bond_entry_from_device(data):
+    """scope_to_spec derives the allowlist directly from this
+    module's own argspec (mode/primary/hash_policy), so bond0's
+    device path being shared with vyos_interfaces (description/mtu/
+    vrf) and vyos_l3_interfaces (address) is never a leak risk --
+    confirmed existing utility in module_utils/vyos.py for exactly
+    this cross-module scope problem, used here instead of a second,
+    manually-maintained field list.
+
+    members and arp_monitor are excluded from that call and handled
+    separately: "members" (argspec) doesn't match the device's actual
+    key "member" (singular), and arp_monitor's nested target needs its
+    own keyed-dict-to-list conversion that scope_to_spec (a top-level
+    filter only) doesn't do.
+    """
+    scoped = scope_to_spec(data, _ENTRY_OPTIONS, exclude=("name", "members", "arp_monitor"))
+    entry = from_device(scoped)
+
+    iface_raw = (data.get("member") or {}).get("interface")
+    if iface_raw:
+        entry["members"] = [{"member": m} for m in sorted(to_tag_dict(iface_raw))]
+
+    arp = data.get("arp-monitor") or {}
+    arp_entry = {}
+    if "interval" in arp:
+        arp_entry["interval"] = int(arp["interval"])
+    target_raw = arp.get("target")
+    if target_raw:
+        arp_entry["target"] = sorted(to_tag_dict(target_raw))
+    if arp_entry:
+        entry["arp_monitor"] = arp_entry
+
+    return entry
 
-    if len(raw) == 1 and "bonding" in raw:
-        raw = raw["bonding"]
 
+def get_running_config(vyos):
+    """VyOS's REST API collapses a single-child tag node to a plain
+    string (or a list for multiple) -- confirmed as a real failure
+    mode during vyos_ospf_interfaces's build. Normalizing through
+    to_tag_dict unconditionally means callers always receive a
+    genuine dict.
+
+    The original module also defensively unwrapped a possible extra
+    "bonding" wrapper key around the response -- kept here rather
+    than dropped, since that defensive check was never independently
+    disproven, matching the same wrapper-key pattern confirmed real
+    elsewhere in this collection (e.g. vyos_route_maps).
+    """
+    raw = to_tag_dict(vyos.get_config(_BASE) or {})
+    if isinstance(raw, dict) and len(raw) == 1 and "bonding" in raw:
+        raw = to_tag_dict(raw["bonding"])
+    return raw
+
+
+def _device_to_argspec(raw):
     result = []
-    for name, data in sorted(raw.items()):
-        if name in ("bonding", "ethernet", "loopback"):
-            continue
-        data = data or {}
+    for name, data in sorted((raw or {}).items()):
         entry = {"name": name}
-
-        if data.get("mode"):
-            entry["mode"] = data["mode"]
-        if data.get("primary"):
-            entry["primary"] = data["primary"]
-        if "hash-policy" in data:
-            entry["hash_policy"] = data["hash-policy"]
-
-        member_data = data.get("member", {})
-        if isinstance(member_data, dict):
-            iface_data = member_data.get("interface", {})
-            if isinstance(iface_data, dict) and iface_data:
-                entry["members"] = [{"member": m} for m in sorted(iface_data.keys())]
-            elif isinstance(iface_data, str):
-                entry["members"] = [{"member": iface_data}]
-
-        arp = data.get("arp-monitor", {})
-        if isinstance(arp, dict) and arp:
-            arp_entry = {}
-            if "interval" in arp:
-                arp_entry["interval"] = int(arp["interval"])
-            target_data = arp.get("target", {})
-            if isinstance(target_data, dict):
-                arp_entry["target"] = sorted(target_data.keys())
-            elif isinstance(target_data, str):
-                arp_entry["target"] = [target_data]
-            if arp_entry:
-                entry["arp_monitor"] = arp_entry
-
+        entry.update(_bond_entry_from_device(data or {}))
         result.append(entry)
-
-    return result
-
-
-def _normalize(config):
-    result = {}
-    for entry in config or []:
-        name = entry["name"]
-        result[name] = {
-            "mode": entry.get("mode"),
-            "primary": entry.get("primary"),
-            "hash_policy": entry.get("hash_policy"),
-            "members": sorted([m["member"] for m in (entry.get("members") or [])]),
-            "arp_interval": (entry.get("arp_monitor") or {}).get("interval"),
-            "arp_targets": sorted((entry.get("arp_monitor") or {}).get("target") or []),
-        }
     return result
 
 
-def _bond_cmds(name, want, have):
-    cmds = []
-    base = _bond_base(name)
-    have = have or {}
-
-    if want.get("mode") and want["mode"] != have.get("mode"):
-        cmds.append(("set", base + ["mode", want["mode"]]))
-
-    if want.get("primary") and want["primary"] != have.get("primary"):
-        cmds.append(("set", base + ["primary", want["primary"]]))
-
-    if want.get("hash_policy") and want["hash_policy"] != have.get("hash_policy"):
-        cmds.append(("set", base + ["hash-policy", want["hash_policy"]]))
-
-    want_members = set(want.get("members") or [])
-    have_members = set(have.get("members") or [])
-    for m in want_members - have_members:
-        cmds.append(("set", base + ["member", "interface", m]))
-
-    want_interval = want.get("arp_interval")
-    have_interval = have.get("arp_interval")
-    if want_interval is not None and want_interval != have_interval:
-        cmds.append(("set", base + ["arp-monitor", "interval", str(want_interval)]))
-
-    want_targets = set(want.get("arp_targets") or [])
-    have_targets = set(have.get("arp_targets") or [])
-    for t in want_targets - have_targets:
-        cmds.append(("set", base + ["arp-monitor", "target", t]))
-
-    return cmds
-
+def build_commands(config, raw_have, state):
+    raw_have = raw_have or {}
+    config = config or []
 
-def _delete_bond_cmds(name, have, want=None):
-    """Generate delete commands for a bond — full delete or selective."""
-    cmds = []
-    base = _bond_base(name)
-    have = have or {}
-    want = want or {}
+    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 not want:
-        # full delete
-        cmds.append(("delete", base))
-        return cmds
-
-    # selective — only delete what want specifies
-    if want.get("mode") and have.get("mode"):
-        cmds.append(("delete", base + ["mode"]))
-    if want.get("primary") and have.get("primary"):
-        cmds.append(("delete", base + ["primary"]))
-    if want.get("hash_policy") and have.get("hash_policy"):
-        cmds.append(("delete", base + ["hash-policy"]))
-    for m in set(want.get("members") or []) & set(have.get("members") or []):
-        cmds.append(("delete", base + ["member", "interface", m]))
-    if want.get("arp_interval") and have.get("arp_interval"):
-        cmds.append(("delete", base + ["arp-monitor", "interval"]))
-    for t in set(want.get("arp_targets") or []) & set(have.get("arp_targets") or []):
-        cmds.append(("delete", base + ["arp-monitor", "target", t]))
-
-    return cmds
-
-
-def build_commands(config, have_raw, state):
-    cmds = []
-    have_map = _normalize(have_raw)
+    def _scoped_purge(name, have_entry):
+        have_device = _bond_entry_to_device(
+            {k: v for k, v in have_entry.items() if k != "name"},
+        )
+        return dict_op({}, have_device, _bond_base(name), op="purge")
 
     if state == "deleted":
+        cmds = []
         if not config:
-            for name in have_map:
-                cmds.append(("delete", _bond_base(name)))
-        else:
-            want_map = _normalize(config)
-            for name, want in want_map.items():
-                have = have_map.get(name, {})
-                if not any(
-                    [
-                        want.get("mode"),
-                        want.get("primary"),
-                        want.get("hash_policy"),
-                        want.get("members"),
-                        want.get("arp_interval"),
-                        want.get("arp_targets"),
-                    ],
-                ):
-                    # delete entire bond
-                    if name in have_map:
-                        cmds.append(("delete", _bond_base(name)))
-                else:
-                    cmds += _delete_bond_cmds(name, have, want)
+            for name, have_entry in have_by_name.items():
+                cmds += _scoped_purge(name, have_entry)
+            return cmds
+        for entry in config:
+            name = entry.get("name")
+            if name and name in have_by_name:
+                cmds += _scoped_purge(name, have_by_name[name])
         return cmds
 
-    want_map = _normalize(config)
-
+    commands = []
     if state == "overridden":
-        for name in set(have_map) - set(want_map):
-            cmds.append(("delete", _bond_base(name)))
+        for name in set(have_by_name) - set(want_by_name):
+            commands += _scoped_purge(name, have_by_name[name])
 
-    for name, want in want_map.items():
-        have = have_map.get(name, {})
+    for name, want_entry in want_by_name.items():
+        have_entry = have_by_name.get(name, {})
+        want_device = _bond_entry_to_device(
+            {k: v for k, v in want_entry.items() if k != "name"},
+        )
+        have_device = _bond_entry_to_device(
+            {k: v for k, v in have_entry.items() if k != "name"},
+        )
+        base = _bond_base(name)
 
-        if state == "replaced" and name in have_map:
-            test_cmds = _bond_cmds(name, want, have)
-            # check for extra members/targets in have not in want
-            extra_members = set(have.get("members") or []) - set(want.get("members") or [])
-            extra_targets = set(have.get("arp_targets") or []) - set(want.get("arp_targets") or [])
-            have_fields = {k: v for k, v in have.items() if v}
-            want_fields = {k: v for k, v in want.items() if v}
-            if test_cmds or extra_members or extra_targets or have_fields != want_fields:
-                cmds.append(("delete", _bond_base(name)))
-                have = {}
-            else:
-                continue
+        if state in ("replaced", "overridden"):
+            commands += dict_op(want_device, have_device, base, op="purge")
+        commands += dict_op(want_device, have_device, base, op="set")
 
-        cmds += _bond_cmds(name, want, have)
+    return commands
 
-    return cmds
 
+_MEMBER_OPTIONS = dict(member=dict(type="str"))
 
-ARGUMENT_SPEC = dict(
-    config=dict(
-        type="list",
-        elements="dict",
-        options=dict(
-            name=dict(type="str", required=True),
-            mode=dict(
-                type="str",
-                choices=[
-                    "802.3ad",
-                    "active-backup",
-                    "broadcast",
-                    "round-robin",
-                    "transmit-load-balance",
-                    "adaptive-load-balance",
-                    "xor-hash",
-                ],
-            ),
-            members=dict(
-                type="list",
-                elements="dict",
-                options=dict(member=dict(type="str")),
-            ),
-            primary=dict(type="str"),
-            hash_policy=dict(
-                type="str",
-                choices=["layer2", "layer2+3", "layer3+4"],
-            ),
-            arp_monitor=dict(
-                type="dict",
-                options=dict(
-                    interval=dict(type="int"),
-                    target=dict(type="list", elements="str"),
-                ),
-            ),
-        ),
+_ARP_MONITOR_OPTIONS = dict(
+    interval=dict(type="int"),
+    target=dict(type="list", elements="str"),
+)
+
+_ENTRY_OPTIONS = dict(
+    name=dict(type="str", required=True),
+    mode=dict(
+        type="str",
+        choices=[
+            "802.3ad",
+            "active-backup",
+            "broadcast",
+            "round-robin",
+            "transmit-load-balance",
+            "adaptive-load-balance",
+            "xor-hash",
+        ],
     ),
-    running_config=dict(type="str"),
+    members=dict(type="list", elements="dict", options=_MEMBER_OPTIONS),
+    primary=dict(type="str"),
+    hash_policy=dict(type="str", choices=["layer2", "layer2+3", "layer3+4"]),
+    arp_monitor=dict(type="dict", options=_ARP_MONITOR_OPTIONS),
+)
+
+ARGUMENT_SPEC = dict(
+    config=dict(type="list", elements="dict", options=_ENTRY_OPTIONS),
     state=dict(
         type="str",
         default="merged",
-        choices=["merged", "replaced", "overridden", "deleted", "gathered", "rendered", "parsed"],
+        choices=["merged", "replaced", "overridden", "deleted", "gathered"],
     ),
 )
 
 
 def main():
-    module = AnsibleModule(
-        argument_spec=ARGUMENT_SPEC,
-        mutually_exclusive=[["config", "running_config"]],
-        required_if=[
-            ("state", "rendered", ["config"]),
-            ("state", "parsed", ["running_config"]),
-        ],
-        supports_check_mode=True,
-    )
+    module = AnsibleModule(ARGUMENT_SPEC, supports_check_mode=True)
     vyos = VyOSModule(module)
 
     state = module.params["state"]
     config = module.params.get("config") or []
 
-    if state == "parsed":
-        module.exit_json(parsed=[])
-
-    if state == "rendered":
-        cmds = build_commands(config, [], "merged")
-        module.exit_json(rendered=cmds, commands=cmds)
-
-    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/unit/fixtures/lag_interfaces_running.json b/tests/unit/fixtures/lag_interfaces_running.json
index 3477b1d..c3cc646 100644
--- a/tests/unit/fixtures/lag_interfaces_running.json
+++ b/tests/unit/fixtures/lag_interfaces_running.json
@@ -1,12 +1,15 @@
 {
   "bond0": {
+    "mode": "802.3ad",
+    "hash-policy": "layer2",
+    "primary": "eth1",
+    "member": { "interface": { "eth1": {}, "eth2": {} } },
     "arp-monitor": {
       "interval": "100",
-      "target": {
-        "192.0.2.1": {}
-      }
-    },
-    "hash-policy": "layer2",
+      "target": { "192.0.2.1": {}, "192.0.2.2": {} }
+    }
+  },
+  "bond1": {
     "mode": "active-backup"
   }
 }
diff --git a/tests/unit/modules/test_vyos_lag_interfaces.py b/tests/unit/modules/test_vyos_lag_interfaces.py
index 7c6df87..abb94a4 100644
--- a/tests/unit/modules/test_vyos_lag_interfaces.py
+++ b/tests/unit/modules/test_vyos_lag_interfaces.py
@@ -4,289 +4,193 @@ 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_lag_interfaces import (
-    _bond_base,
-    _bond_cmds,
-    _normalize,
+    ARGUMENT_SPEC,
+    _bond_entry_from_device,
+    _bond_entry_to_device,
+    _device_to_argspec,
     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)
+
+_BASE = ["interfaces", "bonding"]
 
 
 class VyOSModuleTestCase(unittest.TestCase):
     def setUp(self):
         self.mock_vyos = MagicMock()
-        self.mock_vyos.get_config = MagicMock(return_value={})
-
-    def set_running_config(self, data):
-        self.mock_vyos.get_config.return_value = data
-
-
-class TestVyOSLagInterfacesBondBase(unittest.TestCase):
-
-    def test_bond_base(self):
-        self.assertEqual(
-            _bond_base("bond0"),
-            ["interfaces", "bonding", "bond0"],
-        )
-
-
-class TestVyOSLagInterfacesGetRunningFixture(VyOSModuleTestCase):
-
-    def setUp(self):
-        super().setUp()
         self.fixture = load_fixture("lag_interfaces_running.json")
+        self.mock_vyos.get_config = MagicMock(return_value=self.fixture)
 
-    def test_fixture_parses_bond0_mode(self):
-        self.set_running_config(self.fixture)
-        result = get_running_config(self.mock_vyos)
-        bond0 = next((e for e in result if e["name"] == "bond0"), None)
-        self.assertIsNotNone(bond0)
-        self.assertEqual(bond0["mode"], "active-backup")
-
-    def test_fixture_parses_hash_policy(self):
-        self.set_running_config(self.fixture)
-        result = get_running_config(self.mock_vyos)
-        bond0 = next(e for e in result if e["name"] == "bond0")
-        self.assertEqual(bond0["hash_policy"], "layer2")
-
-    def test_fixture_parses_arp_monitor(self):
-        self.set_running_config(self.fixture)
-        result = get_running_config(self.mock_vyos)
-        bond0 = next(e for e in result if e["name"] == "bond0")
-        self.assertIn("arp_monitor", bond0)
-        self.assertEqual(bond0["arp_monitor"]["interval"], 100)
-        self.assertIn("192.0.2.1", bond0["arp_monitor"]["target"])
-
-    def test_fixture_unwraps_bonding_key(self):
-        # wrap in extra "bonding" key as VyOS sometimes returns
-        wrapped = {"bonding": self.fixture}
-        self.set_running_config(wrapped)
-        result = get_running_config(self.mock_vyos)
-        self.assertTrue(len(result) > 0)
-        self.assertEqual(result[0]["name"], "bond0")
-
+    def gather(self):
+        have = _device_to_argspec(self.fixture)
+        for entry in have:
+            cast_by_spec(entry, ARGUMENT_SPEC["config"]["options"])
+        return have
 
-class TestVyOSLagInterfacesGetRunning(VyOSModuleTestCase):
 
-    def test_empty_returns_empty_list(self):
-        self.set_running_config({})
-        result = get_running_config(self.mock_vyos)
-        self.assertEqual(result, [])
-
-    def test_parses_mode(self):
-        self.set_running_config(
-            {
-                "bond0": {"mode": "802.3ad"},
-            },
-        )
-        result = get_running_config(self.mock_vyos)
-        self.assertEqual(result[0]["mode"], "802.3ad")
-
-    def test_parses_hash_policy(self):
-        self.set_running_config(
-            {
-                "bond0": {"hash-policy": "layer2+3"},
-            },
-        )
-        result = get_running_config(self.mock_vyos)
-        self.assertEqual(result[0]["hash_policy"], "layer2+3")
-
-    def test_parses_members(self):
-        self.set_running_config(
-            {
-                "bond0": {"member": {"interface": {"eth1": {}, "eth2": {}}}},
-            },
-        )
-        result = get_running_config(self.mock_vyos)
-        members = [m["member"] for m in result[0]["members"]]
-        self.assertIn("eth1", members)
-        self.assertIn("eth2", members)
-
-    def test_parses_arp_monitor_interval(self):
-        self.set_running_config(
-            {
-                "bond0": {"arp-monitor": {"interval": "100"}},
-            },
-        )
-        result = get_running_config(self.mock_vyos)
-        self.assertEqual(result[0]["arp_monitor"]["interval"], 100)
-
-    def test_parses_arp_monitor_target_dict(self):
-        self.set_running_config(
-            {
-                "bond0": {"arp-monitor": {"target": {"192.0.2.1": {}}}},
-            },
-        )
+class TestGetRunningConfig(VyOSModuleTestCase):
+    def test_returns_config_directly(self):
         result = get_running_config(self.mock_vyos)
-        self.assertIn("192.0.2.1", result[0]["arp_monitor"]["target"])
-
-    def test_skips_type_keys(self):
-        self.set_running_config(
-            {
-                "bonding": {"bond0": {"mode": "802.3ad"}},
-            },
-        )
-        result = get_running_config(self.mock_vyos)
-        # "bonding" key should be skipped
-        names = [e["name"] for e in result]
-        self.assertNotIn("bonding", names)
-
-
-class TestVyOSLagInterfacesNormalize(unittest.TestCase):
-
-    def test_normalize_basic(self):
-        config = [{"name": "bond0", "mode": "802.3ad", "hash_policy": "layer2"}]
-        result = _normalize(config)
         self.assertIn("bond0", result)
-        self.assertEqual(result["bond0"]["mode"], "802.3ad")
-        self.assertEqual(result["bond0"]["hash_policy"], "layer2")
-
-    def test_normalize_members_sorted(self):
-        config = [
-            {
-                "name": "bond0",
-                "members": [{"member": "eth2"}, {"member": "eth1"}],
-            },
-        ]
-        result = _normalize(config)
-        self.assertEqual(result["bond0"]["members"], ["eth1", "eth2"])
-
-    def test_normalize_arp_targets_sorted(self):
-        config = [
-            {
-                "name": "bond0",
-                "arp_monitor": {"interval": 100, "target": ["192.0.2.2", "192.0.2.1"]},
-            },
-        ]
-        result = _normalize(config)
-        self.assertEqual(result["bond0"]["arp_targets"], ["192.0.2.1", "192.0.2.2"])
-
-    def test_normalize_empty(self):
-        result = _normalize([])
-        self.assertEqual(result, {})
-
-
-class TestVyOSLagInterfacesBondCmds(unittest.TestCase):
-
-    def test_set_mode(self):
-        want = {"mode": "802.3ad"}
-        cmds = _bond_cmds("bond0", want, {})
-        self.assertIn(
-            ("set", ["interfaces", "bonding", "bond0", "mode", "802.3ad"]),
-            cmds,
-        )
-
-    def test_set_hash_policy(self):
-        want = {"hash_policy": "layer2"}
-        cmds = _bond_cmds("bond0", want, {})
-        self.assertIn(
-            ("set", ["interfaces", "bonding", "bond0", "hash-policy", "layer2"]),
-            cmds,
-        )
-
-    def test_set_member(self):
-        want = {"members": ["eth1"]}
-        cmds = _bond_cmds("bond0", want, {})
-        self.assertIn(
-            ("set", ["interfaces", "bonding", "bond0", "member", "interface", "eth1"]),
-            cmds,
-        )
-
-    def test_set_arp_interval(self):
-        want = {"arp_interval": 100}
-        cmds = _bond_cmds("bond0", want, {})
-        self.assertIn(
-            ("set", ["interfaces", "bonding", "bond0", "arp-monitor", "interval", "100"]),
-            cmds,
-        )
 
-    def test_set_arp_target(self):
-        want = {"arp_targets": ["192.0.2.1"]}
-        cmds = _bond_cmds("bond0", want, {})
-        self.assertIn(
-            ("set", ["interfaces", "bonding", "bond0", "arp-monitor", "target", "192.0.2.1"]),
-            cmds,
+    def test_unwraps_bonding_wrapper_key(self):
+        self.mock_vyos.get_config = MagicMock(
+            return_value={"bonding": {"bond0": {"mode": "802.3ad"}}},
         )
-
-    def test_idempotent_mode(self):
-        want = {"mode": "802.3ad"}
-        have = {"mode": "802.3ad"}
-        cmds = _bond_cmds("bond0", want, have)
-        self.assertEqual(cmds, [])
-
-    def test_no_commands_when_empty_want(self):
-        cmds = _bond_cmds("bond0", {}, {})
-        self.assertEqual(cmds, [])
-
-
-class TestVyOSLagInterfacesBuildCommands(unittest.TestCase):
-
-    def _have_bond0(self):
-        return [
-            {
-                "name": "bond0",
-                "mode": "active-backup",
-                "hash_policy": "layer2",
-                "arp_monitor": {"interval": 100, "target": ["192.0.2.1"]},
+        result = get_running_config(self.mock_vyos)
+        self.assertEqual(result, {"bond0": {"mode": "802.3ad"}})
+
+    def test_empty_config(self):
+        self.mock_vyos.get_config = MagicMock(return_value=None)
+        self.assertEqual(get_running_config(self.mock_vyos), {})
+
+    def test_collapsed_response_normalized(self):
+        self.mock_vyos.get_config = MagicMock(return_value="bond0")
+        self.assertEqual(get_running_config(self.mock_vyos), {"bond0": {}})
+
+
+class TestScopeIsolation(unittest.TestCase):
+    """Primary confirmed severe bug, same class as vyos_interfaces/
+    vyos_l3_interfaces: bond0's device path is shared with
+    vyos_interfaces (description/mtu/vrf) and vyos_l3_interfaces
+    (address). A blanket pass-through when parsing device data would
+    leak these into have, and a whole-subtree delete would destroy
+    them alongside this module's own fields."""
+
+    def test_from_device_does_not_leak_unmanaged_fields(self):
+        entry = _bond_entry_from_device(
+            {"description": "lag", "mtu": "1500", "address": ["10.0.0.1/24"], "mode": "802.3ad"},
+        )
+        self.assertNotIn("description", entry)
+        self.assertNotIn("mtu", entry)
+        self.assertNotIn("address", entry)
+        self.assertEqual(entry, {"mode": "802.3ad"})
+
+    def test_deleted_all_never_whole_bond_subtree(self):
+        raw_have = {
+            "bond0": {
+                "mode": "802.3ad",
+                "description": "lag interface",
+                "mtu": "1500",
+                "address": ["10.0.0.1/24"],
             },
-        ]
-
-    def test_merged_adds_bond(self):
-        config = [{"name": "bond0", "mode": "802.3ad"}]
-        cmds = build_commands(config, [], "merged")
-        self.assertIn(
-            ("set", ["interfaces", "bonding", "bond0", "mode", "802.3ad"]),
-            cmds,
-        )
-
-    def test_merged_idempotent(self):
-        cmds = build_commands(self._have_bond0(), self._have_bond0(), "merged")
-        self.assertEqual(cmds, [])
-
-    def test_deleted_no_config_removes_all(self):
-        cmds = build_commands([], self._have_bond0(), "deleted")
-        self.assertIn(
-            ("delete", ["interfaces", "bonding", "bond0"]),
-            cmds,
-        )
-
-    def test_deleted_with_name_removes_bond(self):
-        config = [{"name": "bond0"}]
-        cmds = build_commands(config, self._have_bond0(), "deleted")
-        self.assertIn(
-            ("delete", ["interfaces", "bonding", "bond0"]),
-            cmds,
-        )
-
-    def test_deleted_idempotent_when_empty(self):
-        cmds = build_commands([], [], "deleted")
-        self.assertEqual(cmds, [])
-
-    def test_replaced_idempotent(self):
-        cmds = build_commands(self._have_bond0(), self._have_bond0(), "replaced")
-        self.assertEqual(cmds, [])
-
-    def test_overridden_removes_unlisted_bond(self):
-        config = [{"name": "bond1", "mode": "802.3ad"}]
-        cmds = build_commands(config, self._have_bond0(), "overridden")
-        self.assertIn(
-            ("delete", ["interfaces", "bonding", "bond0"]),
-            cmds,
-        )
+        }
+        cmds = build_commands([], raw_have, "deleted")
+        self.assertNotIn(("delete", _BASE + ["bond0"]), cmds)
+        self.assertIn(("delete", _BASE + ["bond0", "mode"]), cmds)
+
+    def test_deleted_named_never_whole_bond_subtree(self):
+        raw_have = {"bond0": {"mode": "802.3ad", "description": "important"}}
+        cmds = build_commands([{"name": "bond0"}], raw_have, "deleted")
+        self.assertNotIn(("delete", _BASE + ["bond0"]), cmds)
+
+    def test_overridden_omitted_bond_never_whole_subtree(self):
+        raw_have = {
+            "bond0": {},
+            "bond1": {"mode": "802.3ad", "description": "important", "address": ["10.0.0.1/24"]},
+        }
+        cmds = build_commands([{"name": "bond0"}], raw_have, "overridden")
+        self.assertNotIn(("delete", _BASE + ["bond1"]), cmds)
+        self.assertIn(("delete", _BASE + ["bond1", "mode"]), cmds)
+
+    def test_replaced_never_whole_bond_subtree(self):
+        raw_have = {"bond0": {"mode": "802.3ad", "description": "important"}}
+        cmds = build_commands([{"name": "bond0"}], raw_have, "replaced")
+        self.assertNotIn(("delete", _BASE + ["bond0"]), cmds)
+
+    def test_allowlist_derived_from_argspec_not_a_separate_constant(self):
+        """Confirmed via scope_to_spec (module_utils/vyos.py), not a
+        second, manually-maintained field list that could drift out
+        of sync with the argspec -- already used by vyos_bgp_global
+        for exactly this cross-module scope problem."""
+        from ansible_collections.vyos.rest.plugins.modules.vyos_lag_interfaces import (
+            _ENTRY_OPTIONS,
+        )
+
+        entry = _bond_entry_from_device({k: "x" for k in _ENTRY_OPTIONS if k != "name"})
+        # every simple scalar field in the argspec should survive
+        for field in ("mode", "primary"):
+            self.assertIn(field, entry)
+
+
+class TestMemberAndArpMonitorPreservation(unittest.TestCase):
+    """Regression coverage for a bug caught during this module's own
+    build: autoclean recursively drops nested empty-dict values,
+    treating a presence-only leaf like {"eth1": {}} as "nothing to
+    see here" -- but that's a meaningful member-interface reference,
+    not nothing. Caught by testing the full round-trip, not assumed."""
+
+    def test_members_preserved_in_device_shape(self):
+        result = _bond_entry_to_device(
+            {"members": [{"member": "eth1"}, {"member": "eth2"}]},
+        )
+        self.assertEqual(result, {"member": {"interface": {"eth1": {}, "eth2": {}}}})
+
+    def test_arp_monitor_target_preserved_in_device_shape(self):
+        result = _bond_entry_to_device(
+            {"arp_monitor": {"interval": 100, "target": ["192.0.2.1"]}},
+        )
+        self.assertEqual(result, {"arp-monitor": {"interval": 100, "target": {"192.0.2.1": {}}}})
+
+    def test_partial_member_change_generates_per_interface_commands(self):
+        """Confirmed correct: dict_op generates per-interface set/
+        delete for a partial member list change, not a whole-node
+        delete -- that only happens when members is entirely absent
+        from want."""
+        raw_have = {"bond0": {"member": {"interface": {"eth1": {}, "eth2": {}}}}}
+        config = [{"name": "bond0", "members": [{"member": "eth1"}, {"member": "eth3"}]}]
+        cmds = build_commands(config, raw_have, "replaced")
+        self.assertIn(("delete", _BASE + ["bond0", "member", "interface", "eth2"]), cmds)
+        self.assertIn(("set", _BASE + ["bond0", "member", "interface", "eth3"]), cmds)
+        self.assertNotIn(("delete", _BASE + ["bond0", "member"]), cmds)
+
+
+class TestDeviceToArgspecFixture(VyOSModuleTestCase):
+    def test_both_bonds_present(self):
+        have = self.gather()
+        names = {e["name"] for e in have}
+        self.assertEqual(names, {"bond0", "bond1"})
+
+    def test_bond0_full_fields_parsed(self):
+        have = self.gather()
+        bond0 = next(e for e in have if e["name"] == "bond0")
+        self.assertEqual(bond0["mode"], "802.3ad")
+        self.assertEqual(bond0["hash_policy"], "layer2")
+        self.assertEqual({m["member"] for m in bond0["members"]}, {"eth1", "eth2"})
+        self.assertEqual(bond0["arp_monitor"]["interval"], 100)
+        self.assertEqual(set(bond0["arp_monitor"]["target"]), {"192.0.2.1", "192.0.2.2"})
+
+
+class TestBuildCommands(VyOSModuleTestCase):
+    def test_merged_idempotent_against_own_fixture(self):
+        have = self.gather()
+        self.assertEqual(build_commands(have, self.fixture, "merged"), [])
+
+    def test_replaced_idempotent_against_own_fixture(self):
+        have = self.gather()
+        self.assertEqual(build_commands(have, self.fixture, "replaced"), [])
+
+    def test_clear_all_omitted_fields_on_replaced(self):
+        cmds = build_commands([{"name": "bond0", "mode": "802.3ad"}], self.fixture, "replaced")
+        self.assertIn(("delete", _BASE + ["bond0", "primary"]), cmds)
+        self.assertIn(("delete", _BASE + ["bond0", "hash-policy"]), cmds)
+        self.assertIn(("delete", _BASE + ["bond0", "member"]), cmds)
+        self.assertIn(("delete", _BASE + ["bond0", "arp-monitor"]), cmds)
+
+    def test_merged_new_bond(self):
+        config = [{"name": "bond2", "mode": "802.3ad", "members": [{"member": "eth5"}]}]
+        cmds = build_commands(config, {}, "merged")
+        self.assertIn(("set", _BASE + ["bond2", "mode", "802.3ad"]), cmds)
+        self.assertIn(("set", _BASE + ["bond2", "member", "interface", "eth5"]), cmds)
 
 
 if __name__ == "__main__":