T8323: vyos_system module

2 files changed, 139 insertions, 201 deletions

diff --git a/plugins/module_utils/vyos.py b/plugins/module_utils/vyos.py
index fedcc13..a0a36c6 100644
--- a/plugins/module_utils/vyos.py
+++ b/plugins/module_utils/vyos.py
@@ -5,15 +5,6 @@ Provides ``get_config()``, ``apply_commands()``, and ``save_config()``
 on top of ``VyOSRestClient``, so resource modules can work with simple
 ``("set", path)`` / ``("delete", path)`` command tuples rather than
 calling the REST client directly.
-
-Version-adaptive paths
-----------------------
-Some VyOS config paths changed between minor versions (e.g. the NTP
-``allow-client address`` node was removed in 1.5+).  ``apply_commands``
-handles this transparently: if a ``set`` command is rejected by the device
-it retries with the last path segment removed (one level up), covering
-the most common schema simplifications.  A ``delete`` that fails is
-treated as a no-op (already absent).
 """
 
 from __future__ import absolute_import, division, print_function
@@ -28,7 +19,7 @@ from ansible_collections.vyos.rest.plugins.module_utils.vyos_rest import (
 
 
 # ---------------------------------------------------------------------------
-# Dynamic config utilities
+# Legacy dynamic config utilities (used by Wave 1-3 modules)
 # ---------------------------------------------------------------------------
 
 
@@ -106,30 +97,127 @@ def diff_configs(want, have, base_path, delete_missing=False):
     return cmds
 
 
-class VyOSModule:
-    """Thin wrapper around VyOSRestClient for resource modules.
+# ---------------------------------------------------------------------------
+# Generic dict diff engine (used by Wave 4+ modules)
+#
+# Design principles:
+#   - want uses snake_case (from YAML/argspec)
+#   - have uses kebab-case (from device API)
+#   - Conversion between - and _ happens here, once, in the core
+#   - Modules only need _BASE path — no key mapping anywhere
+#   - want is the reference dataset — drives all operations
+# ---------------------------------------------------------------------------
+
+
+def owned_config(have, argspec):
+    """Filter raw device config to only keys owned by this module.
+
+    Ownership is declared by the module's argspec — the single source
+    of truth for what this module manages. Keys in have not present in
+    argspec (after normalization) are excluded from before/after output.
 
     Args:
-        module: AnsibleModule instance.
+        have (dict): Raw device config (kebab-case keys).
+        argspec (dict): Module argument_spec dict.
+
+    Returns:
+        dict: Filtered have with only module-owned keys.
     """
+    owned = set(argspec.keys()) - {"state"}
+    return {k: v for k, v in have.items() if k.replace("-", "_") in owned}
+
+
+def dict_op(want, have, base_path, op="set"):
+    """Generic dict diff engine for VyOS REST API.
+
+    Compares want (snake_case, from YAML) against have (kebab-case, from
+    device) and generates API command tuples. All key normalization between
+    snake_case and kebab-case happens here — modules never need to convert.
+
+    Set operations on the two datasets:
+        op="set"    want - have       present: apply what is missing
+        op="delete" want ∩ have       absent:  remove what exists
+        op="purge"  have - want       replaced: remove what is extra
+
+    Args:
+        want (dict): Desired config, snake_case keys (from YAML/argspec).
+        have (dict): Current config, kebab-case keys (raw from device API).
+        base_path (list): Base API path — the only module-specific knowledge.
+        op (str): "set", "delete", or "purge".
+
+    Returns:
+        list: Tuples of ("set", path) or ("delete", path).
+    """
+    cmds = []
+
+    # Index have by normalized key for O(1) lookup.
+    # Preserves original kebab-case key for use in API paths.
+    have_idx = {k.replace("-", "_"): (k, v) for k, v in (have or {}).items()}
+
+    if op == "purge":
+        # have - want: delete have keys not present in want
+        # Scoped naturally by _BASE — only this subtree is in have
+        want_keys = {k.replace("-", "_") for k in (want or {})}
+        for norm_k, (orig_k, have_v) in have_idx.items():
+            if norm_k not in want_keys:
+                cmds.append(("delete", base_path + [orig_k]))
+        return cmds
+
+    for key, want_val in (want or {}).items():
+        if want_val is None:
+            continue
+
+        # Normalize want key for lookup, get original device key for path
+        norm_key = key.replace("-", "_")
+        orig_key, have_val = have_idx.get(norm_key, (key.replace("_", "-"), None))
+        path = base_path + [orig_key]
+
+        if isinstance(want_val, dict):
+            if not want_val:
+                # Presence node
+                if op == "set" and not have_val:
+                    cmds.append(("set", path))
+                elif op == "delete" and have_val is not None:
+                    cmds.append(("delete", path))
+            else:
+                # Recurse — have_val passed raw, conversion happens recursively
+                cmds += dict_op(want_val, have_val or {}, path, op)
+
+        elif isinstance(want_val, list):
+            have_set = set(str(i) for i in (have_val or []))
+            if op == "set":
+                # want - have: add missing items
+                for item in want_val:
+                    if str(item) not in have_set:
+                        cmds.append(("set", path + [str(item)]))
+            elif op == "delete":
+                # want ∩ have: remove items that exist
+                for item in want_val:
+                    if str(item) in have_set:
+                        cmds.append(("delete", path + [str(item)]))
+
+        else:
+            # Scalar leaf
+            have_str = str(have_val) if have_val is not None else ""
+            if op == "set" and str(want_val) != have_str:
+                cmds.append(("set", path + [str(want_val)]))
+            elif op == "delete" and have_val is not None:
+                cmds.append(("delete", path))
+
+    return cmds
+
+
+class VyOSModule:
+    """Thin wrapper around VyOSRestClient for resource modules."""
 
     def __init__(self, module):
         self._module = module
         self._client = VyOSRestClient(module)
 
-    # ------------------------------------------------------------------
-    # Read
-    # ------------------------------------------------------------------
-
     def get_config(self, path=None):
         """Retrieve the configuration subtree at *path*.
 
-        Args:
-            path (list, optional): Config path tokens. ``[]`` or ``None``
-                returns the entire running configuration.
-
-        Returns:
-            dict: Configuration subtree, or ``{}`` if the path doesn't exist.
+        Returns raw device dict with kebab-case keys.
         """
         try:
             result = self._client.retrieve_show_config(path or [])
@@ -137,63 +225,6 @@ class VyOSModule:
         except VyOSRestError:
             return {}
 
-    # ------------------------------------------------------------------
-    # Write
-    # ------------------------------------------------------------------
-
-    # def apply_commands(self, commands):
-    #     """Execute a list of ``(op, path)`` command tuples against the device.
-
-    #     Args:
-    #         commands (list): Each item is a 2-tuple ``(op, path)`` where:
-    #             - *op* is ``"set"`` or ``"delete"``
-    #             - *path* is a list of config path tokens
-
-    #             Optionally a 3-tuple ``(op, path, value)`` for leaf nodes
-    #             that carry a value.
-
-    #     Returns:
-    #         list: Results from each command (dicts with ``op`` and ``path``).
-
-    #     Raises:
-    #         VyOSRestError: If a command fails and cannot be recovered.
-
-    #     Notes:
-    #         **Version-adaptive set:** if a ``set`` is rejected by the device,
-    #         the method retries once with the last path segment removed.
-    #         This handles schema simplifications between VyOS releases
-    #         (e.g. ``allow-client address <prefix>`` -> ``allow-client <prefix>``
-    #         in VyOS 1.5+).
-
-    #         **Idempotent delete:** ``delete`` failures are silently ignored
-    #         (the path is already absent).
-    #     """
-    #     results = []
-
-    #     for cmd in commands:
-    #         # Support two command formats:
-    #         # Tuple: ("set", ["path", "tokens"])  or  ("set", ["path"], "value")
-    #         # Dict:  {"op": "set", "path": ["path", "tokens"]}
-    #         if isinstance(cmd, dict):
-    #             op = cmd["op"]
-    #             path = list(cmd["path"])
-    #             value = cmd.get("value")
-    #         else:
-    #             op = cmd[0]
-    #             path = list(cmd[1])
-    #             value = cmd[2] if len(cmd) > 2 else None
-
-    #         if op == "set":
-    #             results.append(self._apply_set(path, value))
-    #         elif op == "delete":
-    #             results.append(self._apply_delete(path))
-    #         else:
-    #             self._module.fail_json(
-    #                 msg="Unknown command op '{op}' in apply_commands".format(op=op),
-    #             )
-
-    #     return results
-
     def apply_commands(self, commands):
         if not commands:
             return []
@@ -217,10 +248,6 @@ class VyOSModule:
             self._client.configure_set(path, value)
             return {"op": "set", "path": path, "status": "ok"}
         except VyOSRestError as exc:
-            # Retry without the second-to-last segment (version-adaptive).
-            # Example: ["service","ntp","allow-client","address","10.0.0.0/24"]
-            #       -> ["service","ntp","allow-client","10.0.0.0/24"]
-            # Only attempt if path is long enough to have an intermediate node.
             if len(path) >= 3:
                 short_path = path[:-2] + [path[-1]]
                 try:
@@ -241,35 +268,15 @@ class VyOSModule:
             return {"op": "delete", "path": path, "status": "noop"}
 
     def show(self, path):
-        """Run an operational show command via the /show endpoint.
-
-        Args:
-            path (list): Show command path tokens, e.g. ``["version"]``
-                or ``["ip", "route"]``.
-
-        Returns:
-            str: Text output from the show command, or empty string on failure.
-        """
+        """Run an operational show command via the /show endpoint."""
         try:
             result = self._client.show(path)
             return result.get("data") or ""
         except VyOSRestError:
             return ""
 
-    # ------------------------------------------------------------------
-    # Persist
-    # ------------------------------------------------------------------
-
     def save_config(self, file_path=None):
-        """Save the running configuration to disk.
-
-        Args:
-            file_path (str, optional): Destination path on the device.
-                Defaults to ``/config/config.boot``.
-
-        Returns:
-            bool: ``True`` if saved successfully, ``False`` otherwise.
-        """
+        """Save the running configuration to disk."""
         try:
             self._client.config_file_save(file_path)
             return True
diff --git a/plugins/modules/vyos_system.py b/plugins/modules/vyos_system.py
index a7f7d09..474a804 100644
--- a/plugins/modules/vyos_system.py
+++ b/plugins/modules/vyos_system.py
@@ -12,29 +12,24 @@ module: vyos_system
 short_description: Manage system settings on VyOS devices using REST API
 description:
   - Manages basic system settings on VyOS devices via the REST API.
-  - Covers hostname, domain name, name servers, and domain search.
   - Uses REST API (C(connection=httpapi)) instead of CLI.
 version_added: "1.0.0"
 author:
   - VyOS Community (@vyos)
 options:
   host_name:
-    description:
-      - Device hostname.
+    description: Device hostname.
     type: str
   domain_name:
-    description:
-      - Device domain name.
+    description: Device domain name.
     type: str
   name_server:
-    description:
-      - List of DNS name servers.
+    description: List of DNS name servers.
     type: list
     elements: str
     aliases: [name_servers]
   domain_search:
-    description:
-      - List of domain search suffixes.
+    description: List of domain search suffixes.
     type: list
     elements: str
   state:
@@ -54,22 +49,11 @@ EXAMPLES = r"""
   vyos.rest.vyos_system:
     host_name: router1
     domain_name: example.com
-    state: present
-
-- name: Configure name servers
-  vyos.rest.vyos_system:
     name_server:
       - 8.8.8.8
       - 8.8.4.4
     state: present
 
-- name: Configure domain search
-  vyos.rest.vyos_system:
-    domain_search:
-      - sub1.example.com
-      - sub2.example.com
-    state: present
-
 - name: Remove domain name and name servers
   vyos.rest.vyos_system:
     domain_name: example.com
@@ -80,11 +64,11 @@ EXAMPLES = r"""
 
 RETURN = r"""
 before:
-  description: System configuration before this module ran.
+  description: Module-owned system configuration before this module ran.
   returned: always
   type: dict
 after:
-  description: System configuration after this module ran.
+  description: Module-owned system configuration after this module ran.
   returned: when changed
   type: dict
 commands:
@@ -102,71 +86,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,
+    dict_op,
+    owned_config,
+)
 
 
 _BASE = ["system"]
 
-
-def _to_list(val):
-    if val is None:
-        return []
-    if isinstance(val, list):
-        return val
-    return [val]
-
-
-def get_running_config(vyos):
-    raw = vyos.get_config(_BASE)
-    if not raw or not isinstance(raw, dict):
-        return {}
-    result = {}
-    if "host-name" in raw:
-        result["host_name"] = raw["host-name"]
-    if "domain-name" in raw:
-        result["domain_name"] = raw["domain-name"]
-    if "name-server" in raw:
-        result["name_server"] = _to_list(raw["name-server"])
-    if "domain-search" in raw:
-        result["domain_search"] = _to_list(raw["domain-search"])
-    return result
-
-
-def build_commands(config, have, state):
-    cmds = []
-
-    if state == "absent":
-        if config.get("host_name") and have.get("host_name"):
-            cmds.append(("delete", _BASE + ["host-name"]))
-        if config.get("domain_name") and have.get("domain_name"):
-            cmds.append(("delete", _BASE + ["domain-name"]))
-        for ns in config.get("name_server") or []:
-            if ns in (have.get("name_server") or []):
-                cmds.append(("delete", _BASE + ["name-server", ns]))
-        for ds in config.get("domain_search") or []:
-            if ds in (have.get("domain_search") or []):
-                cmds.append(("delete", _BASE + ["domain-search", ds]))
-        return cmds
-
-    # state == "present"
-    if config.get("host_name") and config["host_name"] != have.get("host_name"):
-        cmds.append(("set", _BASE + ["host-name", config["host_name"]]))
-    if config.get("domain_name") and config["domain_name"] != have.get("domain_name"):
-        cmds.append(("set", _BASE + ["domain-name", config["domain_name"]]))
-
-    have_ns = set(have.get("name_server") or [])
-    for ns in config.get("name_server") or []:
-        if ns not in have_ns:
-            cmds.append(("set", _BASE + ["name-server", ns]))
-
-    have_ds = set(have.get("domain_search") or [])
-    for ds in config.get("domain_search") or []:
-        if ds not in have_ds:
-            cmds.append(("set", _BASE + ["domain-search", ds]))
-
-    return cmds
-
-
 ARGUMENT_SPEC = dict(
     host_name=dict(type="str"),
     domain_name=dict(type="str"),
@@ -181,33 +109,36 @@ def main():
     vyos = VyOSModule(module)
 
     state = module.params["state"]
-    config = {
-        "host_name": module.params.get("host_name"),
-        "domain_name": module.params.get("domain_name"),
-        "name_server": module.params.get("name_server"),
-        "domain_search": module.params.get("domain_search"),
-    }
 
-    have = get_running_config(vyos)
+    # want: snake_case keys from YAML, nulls removed
+    want = {k: v for k, v in module.params.items() if k != "state" and v is not None}
+
+    # have: raw kebab-case keys from device, scoped by _BASE
+    have = vyos.get_config(_BASE)
+
+    # before/after: only keys owned by this module (declared in argspec)
+    before = owned_config(have, ARGUMENT_SPEC)
 
-    commands = build_commands(config, have, state)
+    op = "set" if state == "present" else "delete"
+    commands = dict_op(want, have, _BASE, op=op)
 
     if module.check_mode:
-        module.exit_json(changed=bool(commands), commands=commands, before=have)
+        module.exit_json(changed=bool(commands), commands=commands, before=before)
 
     if commands:
         response = vyos.apply_commands(commands)
         saved = vyos.save_config()
+        after = owned_config(vyos.get_config(_BASE), ARGUMENT_SPEC)
         module.exit_json(
             changed=True,
-            before=have,
-            after=get_running_config(vyos),
+            before=before,
+            after=after,
             commands=commands,
             saved=saved,
             response=response,
         )
 
-    module.exit_json(changed=False, before=have, after=have, commands=[])
+    module.exit_json(changed=False, before=before, after=before, commands=[])
 
 
 if __name__ == "__main__":