T8989: vyos_hostname rework

2 files changed, 128 insertions, 268 deletions

diff --git a/plugins/module_utils/vyos.py b/plugins/module_utils/vyos.py
index a37a0c2..bdbc2c4 100644
--- a/plugins/module_utils/vyos.py
+++ b/plugins/module_utils/vyos.py
@@ -19,85 +19,6 @@ from ansible_collections.vyos.rest.plugins.module_utils.vyos_rest import (
 
 
 # ---------------------------------------------------------------------------
-# Legacy dynamic config utilities (used by Wave 1-3 modules)
-# ---------------------------------------------------------------------------
-
-
-def _kebab_to_snake(s):
-    """Convert kebab-case string to snake_case."""
-    return s.replace("-", "_")
-
-
-def _snake_to_kebab(s):
-    """Convert snake_case string to kebab-case."""
-    return s.replace("_", "-")
-
-
-def normalize(raw):
-    """Recursively normalize an API response dict to snake_case keys."""
-    if isinstance(raw, dict):
-        return {_kebab_to_snake(k): normalize(v) for k, v in raw.items()}
-    if isinstance(raw, list):
-        return [normalize(v) for v in raw]
-    return raw
-
-
-def denormalize_path(path):
-    """Convert a snake_case path list to kebab-case for the API."""
-    return [_snake_to_kebab(p) for p in path]
-
-
-def _diff_value(want_val, have_val, path, cmds, delete_missing):
-    if isinstance(want_val, dict):
-        if not want_val:
-            if have_val is None:
-                cmds.append(("set", denormalize_path(path)))
-        else:
-            have_dict = have_val if isinstance(have_val, dict) else {}
-            _diff_dict(want_val, have_dict, path, cmds, delete_missing)
-    elif isinstance(want_val, list):
-        have_set = set(have_val) if isinstance(have_val, list) else set()
-        for item in want_val:
-            if item not in have_set:
-                cmds.append(("set", denormalize_path(path + [str(item)])))
-        if delete_missing:
-            want_set = set(str(i) for i in want_val)
-            for item in have_val or []:
-                if str(item) not in want_set:
-                    cmds.append(("delete", denormalize_path(path + [str(item)])))
-    else:
-        if want_val != have_val:
-            cmds.append(("set", denormalize_path(path + [str(want_val)])))
-
-
-def _diff_dict(want, have, path, cmds, delete_missing):
-    for key, want_val in want.items():
-        _diff_value(want_val, have.get(key), path + [key], cmds, delete_missing)
-    if delete_missing:
-        for key in have:
-            if key not in want:
-                cmds.append(("delete", denormalize_path(path + [key])))
-
-
-def diff_configs(want, have, base_path, delete_missing=False):
-    """Diff two normalized config dicts and return API command tuples.
-
-    Args:
-        want (dict): Desired configuration (snake_case keys).
-        have (dict): Current configuration (snake_case keys).
-        base_path (list): Base API path for commands.
-        delete_missing (bool): Generate delete commands for keys in
-            ``have`` absent from ``want``.
-
-    Returns:
-        list: Tuples of ``("set", path)`` or ``("delete", path)``.
-    """
-    cmds = []
-    _diff_dict(want, have, base_path, cmds, delete_missing)
-    return cmds
-
-
-# ---------------------------------------------------------------------------
 # Generic dict diff engine (used by Wave 4+ modules)
 #
 # Design principles:
@@ -229,7 +150,10 @@ def cast_by_spec(entry, options):
             continue
         spec_type = spec.get("type")
         if spec_type == "int":
-            entry[key] = int(entry[key])
+            val = entry[key]
+            if isinstance(val, list):
+                val = val[0] if val else None
+            entry[key] = int(val) if val is not None else None
         elif spec_type == "dict":
             cast_by_spec(entry[key], spec.get("options"))
         elif spec_type == "list":
@@ -380,6 +304,24 @@ class VyOSModule:
         except VyOSRestError:
             return {}
 
+    def get_value(self, path):
+        """Retrieve a single scalar leaf value at *path*.
+
+        Uses VyOS's dedicated "returnValue" retrieve operation --
+        genuinely distinct from get_config's "showConfig" operation,
+        which returns a config subtree rather than a single value.
+        Appropriate for a plain leafNode (e.g. "system host-name"),
+        not a container.
+
+        Errors genuinely propagate rather than being swallowed into a
+        misleading "absent" result: a transient failure here must not
+        be indistinguishable from the value legitimately being unset,
+        since a module could otherwise decide to overwrite a value
+        that's actually already correct.
+        """
+        result = self._client.retrieve_return_value(path)
+        return result.get("data") or ""
+
     def apply_commands(self, commands):
         if not commands:
             return []
diff --git a/plugins/modules/vyos_hostname.py b/plugins/modules/vyos_hostname.py
index 86444ad..3218333 100644
--- a/plugins/modules/vyos_hostname.py
+++ b/plugins/modules/vyos_hostname.py
@@ -12,83 +12,51 @@ DOCUMENTATION = r"""
 module: vyos_hostname
 short_description: Manage the system hostname on a VyOS device via the REST API.
 description:
-  - Manages the C(set system host-name) configuration on a VyOS device
-    using the HTTPS REST API.
-  - Mirrors the behaviour of C(vyos.vyos.vyos_hostname) but uses the HTTP
-    API instead of SSH/network_cli.
-  - The states C(replaced), C(overridden) behave identically to C(merged)
-    for this single-value resource.
+  - Manages the C(system host-name) configuration on a VyOS device using the HTTPS REST API.
 version_added: "1.0.0"
 author:
   - VyOS Community (@vyos)
 options:
   config:
-    description:
-      - Hostname configuration.
+    description: Hostname configuration.
     type: dict
     suboptions:
       hostname:
-        description:
-          - System hostname (max 63 characters, no underscores).
+        description: System hostname (max 63 characters, no underscores).
         type: str
         required: true
-  running_config:
-    description:
-      - Used only with state C(parsed).
-      - The value should be the output of
-        B(show configuration commands | grep host-name) from the device.
-    type: str
   state:
     description:
       - C(merged) - Ensure the hostname is set to the value in I(config).
-      - C(replaced) - Identical to C(merged) for this single-value resource.
-      - C(overridden) - Identical to C(merged) for this single-value resource.
-      - C(deleted) - Remove the configured hostname (resets to default).
-      - C(gathered) - Read the current hostname from the device and return it
-        in I(gathered) without making changes.
-      - C(rendered) - Return the CLI commands for the given config without
-        connecting to the device.
-      - C(parsed) - Parse the C(running_config) string and return structured
-        data without connecting to the device.
+      - >-
+        C(replaced) and C(overridden) behave identically to C(merged) for
+        this single-value resource -- there is nothing else to distinctly
+        replace or override when there is only one field.
+      - C(deleted) - Remove the configured hostname.
+      - C(gathered) - Read the current hostname from the device without making changes.
     type: str
-    choices:
-      - merged
-      - replaced
-      - overridden
-      - deleted
-      - gathered
-      - rendered
-      - parsed
+    choices: [merged, replaced, overridden, deleted, gathered]
     default: merged
-  hostname:
-    description:
-      - IP address or FQDN of the VyOS device (not needed with httpapi inventory).
-    type: str
-  port:
-    description:
-      - HTTPS port for the REST API.
-    type: int
-    default: 443
-  api_key:
-    description:
-      - API key configured on the device.
-    type: str
-  timeout:
-    description:
-      - Request timeout in seconds.
-    type: int
-    default: 30
-  verify_ssl:
-    description:
-      - Validate the device's TLS certificate.
-    type: bool
-    default: false
-requirements:
-  - VyOS 1.3+
 seealso:
   - module: vyos.vyos.vyos_hostname
 """
 
+EXAMPLES = r"""
+- name: Set hostname
+  vyos.rest.vyos_hostname:
+    config:
+      hostname: vyos-core-01
+    state: merged
+
+- name: Gather current hostname
+  vyos.rest.vyos_hostname:
+    state: gathered
+
+- name: Delete hostname configuration
+  vyos.rest.vyos_hostname:
+    state: deleted
+"""
+
 RETURN = r"""
 before:
   description: Configuration on the device before the module ran.
@@ -102,173 +70,123 @@ gathered:
   description: Hostname read from the device (state=gathered only).
   returned: when state is gathered
   type: dict
-rendered:
-  description: CLI commands for the provided config (state=rendered only).
-  returned: when state is rendered
-  type: list
-parsed:
-  description: Structured data parsed from running_config (state=parsed only).
-  returned: when state is parsed
-  type: dict
 commands:
-  description: REST API commands dispatched.
+  description: List of API command tuples sent to the device.
   returned: always
   type: list
+saved:
+  description: Whether the config was saved after changes.
+  returned: when changes are applied
+  type: bool
+response:
+  description: Raw API response.
+  returned: when changes are applied
+  type: dict
 """
 
-EXAMPLES = r"""
-- name: Set hostname
-  vyos.rest.vyos_hostname:
-    config:
-      hostname: vyos-core-01
-    state: merged
-
-- name: Replace hostname
-  vyos.rest.vyos_hostname:
-    config:
-      hostname: vyos-core-02
-    state: replaced
-
-- name: Gather current hostname
-  vyos.rest.vyos_hostname:
-    state: gathered
-  register: result
-
-- name: Delete hostname configuration
-  vyos.rest.vyos_hostname:
-    state: deleted
-
-- name: Render commands without connecting
-  vyos.rest.vyos_hostname:
-    config:
-      hostname: vyos-core-01
-    state: rendered
-
-- name: Parse running config
-  vyos.rest.vyos_hostname:
-    running_config: "set system host-name 'vyos'"
-    state: parsed
-"""
-
-import re
-
 from ansible.module_utils.basic import AnsibleModule
-from ansible_collections.vyos.rest.plugins.module_utils.vyos_rest import (
-    VYOS_REST_CONNECTION_ARGSPEC,
-    VyOSRestClient,
+from ansible_collections.vyos.rest.plugins.module_utils.vyos import (
+    VyOSModule,
     VyOSRestError,
 )
 
 
-_PATH = ["system", "host-name"]
+_BASE = ["system", "host-name"]
 
 
-def _get_hostname(client):
-    try:
-        result = client.retrieve_return_value(_PATH)
-        return result.get("data", "")
-    except VyOSRestError:
-        return ""
+def get_running_config(vyos):
+    """ "system host-name" is a plain leafNode, not a container -- a
+    single scalar value, not a config subtree. get_value (VyOS's
+    "returnValue" retrieve operation) is the correct fetch for this,
+    distinct from get_config's "showConfig" operation used everywhere
+    else in this collection for genuine nested config sections.
+    """
+    return vyos.get_value(_BASE)
 
 
-def _parse_hostname(running_config):
-    """Parse hostname from 'show configuration commands | grep host-name' output."""
-    match = re.search(r"host-name\s+['\"]?(\S+?)['\"]?\s*$", running_config, re.M)
-    return match.group(1) if match else ""
+def build_commands(config, current, state):
+    if state == "deleted":
+        return [("delete", _BASE)] if current else []
 
+    desired = (config or {}).get("hostname")
+    if not desired or desired == current:
+        return []
+    return [("set", _BASE + [desired])]
 
-def main():
-    argument_spec = dict(
-        config=dict(
-            type="dict",
-            options=dict(
-                hostname=dict(type="str", required=True),
-            ),
-        ),
-        running_config=dict(type="str"),
-        state=dict(
-            type="str",
-            default="merged",
-            choices=[
-                "merged",
-                "replaced",
-                "overridden",
-                "deleted",
-                "gathered",
-                "rendered",
-                "parsed",
-            ],
+
+ARGUMENT_SPEC = dict(
+    config=dict(
+        type="dict",
+        options=dict(
+            hostname=dict(type="str", required=True),
         ),
-    )
-    argument_spec.update(VYOS_REST_CONNECTION_ARGSPEC)
+    ),
+    state=dict(
+        type="str",
+        default="merged",
+        choices=["merged", "replaced", "overridden", "deleted", "gathered"],
+    ),
+)
 
+
+def main():
     module = AnsibleModule(
-        argument_spec=argument_spec,
-        mutually_exclusive=[["config", "running_config"]],
+        ARGUMENT_SPEC,
         required_if=[
             ("state", "merged", ["config"]),
             ("state", "replaced", ["config"]),
             ("state", "overridden", ["config"]),
-            ("state", "rendered", ["config"]),
-            ("state", "parsed", ["running_config"]),
         ],
         supports_check_mode=True,
     )
+    vyos = VyOSModule(module)
 
+    # Collapsed states: replaced/overridden are identical to merged for
+    # this single-value resource -- there is nothing else to distinctly
+    # replace or override when there's only one field.
     state = module.params["state"]
-
-    # rendered — offline, no device connection needed
-    if state == "rendered":
-        hostname = module.params["config"]["hostname"]
-        module.exit_json(
-            rendered=["set system host-name '{h}'".format(h=hostname)],
-            commands=[],
-        )
-
-    # parsed — offline, no device connection needed
-    if state == "parsed":
-        hostname = _parse_hostname(module.params["running_config"] or "")
-        module.exit_json(
-            parsed={"hostname": hostname},
-            commands=[],
-        )
-
-    # collapsed states — replaced and overridden are identical to merged
     if state in ("replaced", "overridden"):
         state = "merged"
 
-    client = VyOSRestClient(module)
-    commands = []
-    changed = False
-
-    current = _get_hostname(client)
-    before = {"hostname": current}
+    config = module.params.get("config") or {}
 
-    if state == "gathered":
-        module.exit_json(changed=False, gathered=before, before=before, commands=[])
+    try:
+        current = get_running_config(vyos)
+    except VyOSRestError as exc:
+        module.fail_json(msg="failed to read current hostname: {e}".format(e=str(exc)))
 
-    if module.check_mode:
-        module.exit_json(changed=True, before=before, commands=["(check mode)"])
+    have = {"hostname": current}
 
-    try:
-        if state == "merged":
-            desired = module.params["config"]["hostname"]
-            if current != desired:
-                client.configure_set(_PATH, desired)
-                commands.append("set system host-name '{h}'".format(h=desired))
-                changed = True
+    if state == "gathered":
+        module.exit_json(changed=False, gathered=have, commands=[])
 
-        elif state == "deleted":
-            if current:
-                client.configure_delete(_PATH)
-                commands.append("delete system host-name")
-                changed = True
+    commands = build_commands(config, current, state)
 
-    except VyOSRestError as exc:
-        module.fail_json(msg=str(exc))
+    if module.check_mode:
+        module.exit_json(changed=bool(commands), commands=commands, before=have, after=have)
+
+    if commands:
+        response = vyos.apply_commands(commands)
+        saved = vyos.save_config()
+        try:
+            after_current = get_running_config(vyos)
+        except VyOSRestError as exc:
+            module.fail_json(
+                msg="hostname change applied but failed to read back result: {e}".format(
+                    e=str(exc),
+                ),
+            )
+        after = {"hostname": after_current}
+        module.exit_json(
+            changed=True,
+            before=have,
+            after=after,
+            commands=commands,
+            saved=saved,
+            response=response,
+        )
 
-    after = {"hostname": _get_hostname(client)} if changed else before
-    module.exit_json(changed=changed, before=before, after=after, commands=commands)
+    module.exit_json(changed=False, before=have, after=have, commands=[])
 
 
 if __name__ == "__main__":