Merge pull request #28 from vyos/T8989_vyos_hostname_SIT_fix

T8989: vyos_hostname rework

8 files changed, 507 insertions, 353 deletions

diff --git a/changelogs/fragments/t8989_hostname_rework.yml b/changelogs/fragments/t8989_hostname_rework.yml
new file mode 100644
index 0000000..6e1db7c
--- /dev/null
+++ b/changelogs/fragments/t8989_hostname_rework.yml
@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - vyos_hostname - Added method to VyOS helper; fixed bugs in the module and in UAT/SIT tests.
diff --git a/docs/vyos.rest.vyos_hostname_module.rst b/docs/vyos.rest.vyos_hostname_module.rst
index c8ea1c5..7e4e37e 100644
--- a/docs/vyos.rest.vyos_hostname_module.rst
+++ b/docs/vyos.rest.vyos_hostname_module.rst
@@ -17,18 +17,10 @@ Version added: 1.0.0
 
 Synopsis
 --------
-- Manages the ``set system host-name`` configuration on a VyOS device using the HTTPS REST API.
-- Mirrors the behaviour of ``vyos.vyos.vyos_hostname`` but uses the HTTP API instead of SSH/network_cli.
-- The states ``replaced``, ``overridden`` behave identically to ``merged`` for this single-value resource.
+- Manages the ``system host-name`` configuration on a VyOS device using the HTTPS REST API.
 
 
 
-Requirements
-------------
-The below requirements are needed on the host that executes this module.
-
-- VyOS 1.3+
-
 
 Parameters
 ----------
@@ -44,21 +36,6 @@ Parameters
             <tr>
                 <td colspan="2">
                     <div class="ansibleOptionAnchor" id="parameter-"></div>
-                    <b>api_key</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>API key configured on the device.</div>
-                </td>
-            </tr>
-            <tr>
-                <td colspan="2">
-                    <div class="ansibleOptionAnchor" id="parameter-"></div>
                     <b>config</b>
                     <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
                     <div style="font-size: small">
@@ -92,53 +69,6 @@ Parameters
             <tr>
                 <td colspan="2">
                     <div class="ansibleOptionAnchor" id="parameter-"></div>
-                    <b>hostname</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>IP address or FQDN of the VyOS device (not needed with httpapi inventory).</div>
-                </td>
-            </tr>
-            <tr>
-                <td colspan="2">
-                    <div class="ansibleOptionAnchor" id="parameter-"></div>
-                    <b>port</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>
-                        <b>Default:</b><br/><div style="color: blue">443</div>
-                </td>
-                <td>
-                        <div>HTTPS port for the REST API.</div>
-                </td>
-            </tr>
-            <tr>
-                <td colspan="2">
-                    <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>
-                        <div>The value should be the output of <b>show configuration commands | grep host-name</b> from the device.</div>
-                </td>
-            </tr>
-            <tr>
-                <td colspan="2">
-                    <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">
@@ -152,53 +82,13 @@ Parameters
                                     <li>overridden</li>
                                     <li>deleted</li>
                                     <li>gathered</li>
-                                    <li>rendered</li>
-                                    <li>parsed</li>
                         </ul>
                 </td>
                 <td>
                         <div><code>merged</code> - Ensure the hostname is set to the value in <em>config</em>.</div>
-                        <div><code>replaced</code> - Identical to <code>merged</code> for this single-value resource.</div>
-                        <div><code>overridden</code> - Identical to <code>merged</code> for this single-value resource.</div>
-                        <div><code>deleted</code> - Remove the configured hostname (resets to default).</div>
-                        <div><code>gathered</code> - Read the current hostname from the device and return it in <em>gathered</em> without making changes.</div>
-                        <div><code>rendered</code> - Return the CLI commands for the given config without connecting to the device.</div>
-                        <div><code>parsed</code> - Parse the <code>running_config</code> string and return structured data without connecting to the device.</div>
-                </td>
-            </tr>
-            <tr>
-                <td colspan="2">
-                    <div class="ansibleOptionAnchor" id="parameter-"></div>
-                    <b>timeout</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>
-                        <b>Default:</b><br/><div style="color: blue">30</div>
-                </td>
-                <td>
-                        <div>Request timeout in seconds.</div>
-                </td>
-            </tr>
-            <tr>
-                <td colspan="2">
-                    <div class="ansibleOptionAnchor" id="parameter-"></div>
-                    <b>verify_ssl</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><div style="color: blue"><b>no</b>&nbsp;&larr;</div></li>
-                                    <li>yes</li>
-                        </ul>
-                </td>
-                <td>
-                        <div>Validate the device&#x27;s TLS certificate.</div>
+                        <div><code>replaced</code> and <code>overridden</code> behave identically to <code>merged</code> for this single-value resource -- there is nothing else to distinctly replace or override when there is only one field.</div>
+                        <div><code>deleted</code> - Remove the configured hostname.</div>
+                        <div><code>gathered</code> - Read the current hostname from the device without making changes.</div>
                 </td>
             </tr>
     </table>
@@ -226,32 +116,14 @@ Examples
           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
-
 
 
 Return Values
@@ -307,7 +179,7 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                 </td>
                 <td>always</td>
                 <td>
-                            <div>REST API commands dispatched.</div>
+                            <div>List of API command tuples sent to the device.</div>
                     <br/>
                 </td>
             </tr>
@@ -329,30 +201,30 @@ 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>
+                    <b>response</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
                       <span style="color: purple">dictionary</span>
                     </div>
                 </td>
-                <td>when state is parsed</td>
+                <td>when changes are applied</td>
                 <td>
-                            <div>Structured data parsed from running_config (state=parsed only).</div>
+                            <div>Raw API response.</div>
                     <br/>
                 </td>
             </tr>
             <tr>
                 <td colspan="1">
                     <div class="ansibleOptionAnchor" id="return-"></div>
-                    <b>rendered</b>
+                    <b>saved</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">list</span>
+                      <span style="color: purple">boolean</span>
                     </div>
                 </td>
-                <td>when state is rendered</td>
+                <td>when changes are applied</td>
                 <td>
-                            <div>CLI commands for the provided config (state=rendered only).</div>
+                            <div>Whether the config was saved after changes.</div>
                     <br/>
                 </td>
             </tr>
diff --git a/plugins/module_utils/vyos.py b/plugins/module_utils/vyos.py
index 92549d6..f7c17f9 100644
--- a/plugins/module_utils/vyos.py
+++ b/plugins/module_utils/vyos.py
@@ -304,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..b7bdd50 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,128 @@ 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 == "merged" and not config.get("hostname"):
+        module.fail_json(msg="config.hostname must be a non-empty string")
 
-    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:
+        # Matches the established convention across the rest of the
+        # collection: omit "after" entirely in check mode, rather than
+        # reporting have as if it were the post-change state.
+        module.exit_json(changed=bool(commands), commands=commands, before=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__":
diff --git a/tests/integration/targets/vyos_hostname/tests/httpapi/deleted.yaml b/tests/integration/targets/vyos_hostname/tests/httpapi/deleted.yaml
new file mode 100644
index 0000000..8ab278b
--- /dev/null
+++ b/tests/integration/targets/vyos_hostname/tests/httpapi/deleted.yaml
@@ -0,0 +1,39 @@
+---
+- debug:
+    msg: START vyos_hostname deleted integration tests on connection={{ ansible_connection }}
+
+- include_tasks: _remove_config.yaml
+- include_tasks: _populate_config.yaml
+
+- block:
+    - name: Delete hostname configuration
+      register: result
+      vyos.rest.vyos_hostname: &id001
+        state: deleted
+
+    - assert:
+        that:
+          - result.changed == true
+
+    - name: Gather after delete
+      register: gathered
+      vyos.rest.vyos_hostname:
+        state: gathered
+
+    - name: Assert hostname no longer the populated test value
+      assert:
+        that:
+          - gathered.gathered.hostname != "ansible-test-host"
+
+    - name: Delete hostname configuration (IDEMPOTENT)
+      register: result
+      vyos.rest.vyos_hostname: *id001
+
+    - name: Assert idempotent
+      assert:
+        that:
+          - result.changed == false
+          - result.commands == []
+
+  always:
+    - include_tasks: _remove_config.yaml
diff --git a/tests/integration/targets/vyos_hostname/tests/httpapi/overridden.yaml b/tests/integration/targets/vyos_hostname/tests/httpapi/overridden.yaml
index 671ac41..68c245c 100644
--- a/tests/integration/targets/vyos_hostname/tests/httpapi/overridden.yaml
+++ b/tests/integration/targets/vyos_hostname/tests/httpapi/overridden.yaml
@@ -1,27 +1,24 @@
 ---
 - debug:
-    msg: START vyos_interfaces overridden integration tests on connection={{ ansible_connection }}
+    msg: START vyos_hostname overridden integration tests on connection={{ ansible_connection }}
 
 - include_tasks: _remove_config.yaml
-- include_tasks: _populate_config.yaml
 
 - block:
-    - name: Override interface configuration
+    - name: Override hostname configuration
       register: result
-      vyos.rest.vyos_interfaces: &id001
+      vyos.rest.vyos_hostname: &id001
         config:
-          - name: eth0
-            description: Ansible overridden interface
-            mtu: 1400
+          hostname: ansible-overridden-host
         state: overridden
 
     - assert:
         that:
           - result.changed == true
 
-    - name: Override interface configuration (IDEMPOTENT)
+    - name: Override hostname configuration (IDEMPOTENT)
       register: result
-      vyos.rest.vyos_interfaces: *id001
+      vyos.rest.vyos_hostname: *id001
 
     - assert:
         that:
diff --git a/tests/integration/targets/vyos_hostname/tests/httpapi/rtt.yaml b/tests/integration/targets/vyos_hostname/tests/httpapi/rtt.yaml
index 74261f8..233f1ef 100644
--- a/tests/integration/targets/vyos_hostname/tests/httpapi/rtt.yaml
+++ b/tests/integration/targets/vyos_hostname/tests/httpapi/rtt.yaml
@@ -1,48 +1,41 @@
 ---
 - debug:
-    msg: START vyos_interfaces round trip integration tests on connection={{ ansible_connection }}
+    msg: START vyos_hostname round trip integration tests on connection={{ ansible_connection }}
 
 - include_tasks: _remove_config.yaml
 
 - block:
     - name: RTT - Apply base configuration
-      vyos.rest.vyos_interfaces:
+      vyos.rest.vyos_hostname:
         config:
-          - name: eth0
-            description: Ansible RTT interface
-            mtu: 1450
+          hostname: ansible-rtt-host
         state: merged
 
     - name: RTT - Gather configuration
       register: gathered
-      vyos.rest.vyos_interfaces:
+      vyos.rest.vyos_hostname:
         state: gathered
 
     - name: RTT - Assert gathered matches applied
       assert:
         that:
-          - gathered.gathered | selectattr('name', 'eq', 'eth0') | list | length == 1
-          - gathered.gathered | selectattr('name', 'eq', 'eth0') | map(attribute='description') | first == 'Ansible RTT interface'
-          - gathered.gathered | selectattr('name', 'eq', 'eth0') | map(attribute='mtu') | first == 1450
+          - gathered.gathered.hostname == "ansible-rtt-host"
 
     - name: RTT - Modify configuration
-      vyos.rest.vyos_interfaces:
+      vyos.rest.vyos_hostname:
         config:
-          - name: eth0
-            description: Ansible RTT modified
-            mtu: 1400
+          hostname: ansible-rtt-modified
         state: replaced
 
     - name: RTT - Gather modified configuration
       register: gathered2
-      vyos.rest.vyos_interfaces:
+      vyos.rest.vyos_hostname:
         state: gathered
 
     - name: RTT - Assert modification applied correctly
       assert:
         that:
-          - gathered2.gathered | selectattr('name', 'eq', 'eth0') | map(attribute='description') | first == 'Ansible RTT modified'
-          - gathered2.gathered | selectattr('name', 'eq', 'eth0') | map(attribute='mtu') | first == 1400
+          - gathered2.gathered.hostname == "ansible-rtt-modified"
 
   always:
     - include_tasks: _remove_config.yaml
diff --git a/tests/unit/modules/test_vyos_hostname.py b/tests/unit/modules/test_vyos_hostname.py
new file mode 100644
index 0000000..3a7d99d
--- /dev/null
+++ b/tests/unit/modules/test_vyos_hostname.py
@@ -0,0 +1,309 @@
+# -*- coding: utf-8 -*-
+from __future__ import absolute_import, division, print_function
+
+
+__metaclass__ = type
+
+import unittest
+
+from unittest.mock import MagicMock
+
+from ansible_collections.vyos.rest.plugins.modules.vyos_hostname import (
+    _BASE,
+    ARGUMENT_SPEC,
+    build_commands,
+    get_running_config,
+)
+
+
+class TestGetRunningConfig(unittest.TestCase):
+    def test_returns_current_hostname(self):
+        mock_vyos = MagicMock()
+        mock_vyos.get_value = MagicMock(return_value="vyos-core-01")
+        self.assertEqual(get_running_config(mock_vyos), "vyos-core-01")
+
+    def test_uses_get_value_not_get_config(self):
+        """Regression test for the confirmed architectural bug: this
+        module was previously calling a "showConfig"-equivalent
+        operation directly via a bare VyOSRestClient, appropriate for
+        config subtrees, not the "returnValue" operation VyOS provides
+        specifically for a single scalar leaf like this one. Confirms
+        get_running_config goes through VyOSModule.get_value, not
+        get_config."""
+        mock_vyos = MagicMock()
+        mock_vyos.get_value = MagicMock(return_value="vyos")
+        mock_vyos.get_config = MagicMock(return_value={})
+        get_running_config(mock_vyos)
+        mock_vyos.get_value.assert_called_once_with(_BASE)
+        mock_vyos.get_config.assert_not_called()
+
+
+class TestBuildCommands(unittest.TestCase):
+    def test_merged_sets_new_hostname(self):
+        cmds = build_commands({"hostname": "newhost"}, "vyos", "merged")
+        self.assertEqual(cmds, [("set", _BASE + ["newhost"])])
+
+    def test_merged_idempotent_when_already_correct(self):
+        cmds = build_commands({"hostname": "vyos"}, "vyos", "merged")
+        self.assertEqual(cmds, [])
+
+    def test_merged_noop_when_hostname_not_specified(self):
+        cmds = build_commands({}, "vyos", "merged")
+        self.assertEqual(cmds, [])
+
+    def test_deleted_with_existing_value(self):
+        cmds = build_commands({}, "somehost", "deleted")
+        self.assertEqual(cmds, [("delete", _BASE)])
+
+    def test_deleted_idempotent_when_already_empty(self):
+        cmds = build_commands({}, "", "deleted")
+        self.assertEqual(cmds, [])
+
+    def test_config_none_does_not_crash(self):
+        cmds = build_commands(None, "vyos", "merged")
+        self.assertEqual(cmds, [])
+
+
+class TestHostnameKeyAlwaysPresent(unittest.TestCase):
+    """Regression test for a real bug caught via integration testing:
+    "hostname" was conditionally omitted from the gathered/before/after
+    dict entirely when its value was empty (e.g. after deletion),
+    rather than being present with an empty value. This broke any
+    downstream access like gathered.gathered.hostname with an
+    AttributeError-equivalent ("object of type 'dict' has no attribute
+    'hostname'"), rather than a clean value comparison."""
+
+    def test_gathered_includes_hostname_key_when_empty(self):
+        import json
+        import sys
+
+        from unittest.mock import patch
+
+        from ansible.module_utils import basic
+
+        argv = ["x", json.dumps({"ANSIBLE_MODULE_ARGS": {"state": "gathered"}})]
+        captured = {}
+
+        def fake_exit_json(self_mod, **kwargs):
+            captured.update(kwargs)
+            raise SystemExit(0)
+
+        basic._ANSIBLE_ARGS = None
+        basic._PARSED_MODULE_ARGS = None
+        with patch.object(sys, "argv", argv), patch(
+            "ansible_collections.vyos.rest.plugins.module_utils.vyos.VyOSRestClient",
+        ) as mock_client, patch(
+            "ansible.module_utils.basic.AnsibleModule.exit_json",
+            fake_exit_json,
+        ):
+            mock_client.return_value.retrieve_return_value.return_value = {"data": ""}
+            from ansible_collections.vyos.rest.plugins.modules import vyos_hostname
+
+            with self.assertRaises(SystemExit):
+                vyos_hostname.main()
+
+        self.assertIn("hostname", captured["gathered"])
+        self.assertEqual(captured["gathered"]["hostname"], "")
+
+
+class TestGatheredReturnsCommands(unittest.TestCase):
+    """Regression test for a bug caught during rework: the gathered
+    branch initially dropped commands=[] entirely, contradicting the
+    RETURN doc's own "returned: always" claim and regressing from the
+    original module's behavior."""
+
+    def test_gathered_includes_empty_commands(self):
+        import json
+        import sys
+
+        from unittest.mock import patch
+
+        from ansible.module_utils import basic
+
+        argv = [
+            "x",
+            json.dumps({"ANSIBLE_MODULE_ARGS": {"state": "gathered"}}),
+        ]
+        captured = {}
+
+        def fake_exit_json(self_mod, **kwargs):
+            captured.update(kwargs)
+            raise SystemExit(0)
+
+        basic._ANSIBLE_ARGS = None
+        basic._PARSED_MODULE_ARGS = None
+        with patch.object(sys, "argv", argv), patch(
+            "ansible_collections.vyos.rest.plugins.module_utils.vyos.VyOSRestClient",
+        ) as mock_client, patch(
+            "ansible.module_utils.basic.AnsibleModule.exit_json",
+            fake_exit_json,
+        ):
+            mock_client.return_value.retrieve_return_value.return_value = {"data": "vyos"}
+            from ansible_collections.vyos.rest.plugins.modules import vyos_hostname
+
+            with self.assertRaises(SystemExit):
+                vyos_hostname.main()
+
+        self.assertIn("commands", captured)
+        self.assertEqual(captured["commands"], [])
+        self.assertEqual(captured["gathered"], {"hostname": "vyos"})
+
+
+class TestCollapsedStates(unittest.TestCase):
+    """replaced/overridden collapse onto merged for this single-value
+    resource -- there is nothing else to distinctly replace/override
+    when there is only one field. Confirmed via the argspec: no
+    separate branch exists for them in build_commands, matching the
+    module's own documented design decision."""
+
+    def test_argspec_declares_all_four_states(self):
+        self.assertEqual(
+            set(ARGUMENT_SPEC["state"]["choices"]),
+            {"merged", "replaced", "overridden", "deleted", "gathered"},
+        )
+
+    def test_no_rendered_or_parsed_states(self):
+        """Confirmed architectural fix: rendered/parsed are CLI-
+        collection concepts (offline command/config-text rendering)
+        that don't correspond to anything meaningful for a REST
+        transport, where the actual payload is a structured API call,
+        not a CLI line."""
+        self.assertNotIn("rendered", ARGUMENT_SPEC["state"]["choices"])
+        self.assertNotIn("parsed", ARGUMENT_SPEC["state"]["choices"])
+
+
+class TestArgumentSpecNoConnectionParams(unittest.TestCase):
+    """Confirmed architectural fix: module-level hostname/port/api_key/
+    timeout/verify_ssl params were a genuine outlier in this collection
+    -- every other module relies exclusively on the httpapi connection
+    plugin for transport/auth, never on module params."""
+
+    def test_no_connection_params_in_argspec(self):
+        for key in ("hostname", "port", "api_key", "timeout", "verify_ssl"):
+            self.assertNotIn(key, ARGUMENT_SPEC)
+
+
+class TestEmptyHostnameFailsExplicitly(unittest.TestCase):
+    """Confirmed real bug (Copilot): with state=merged, an empty
+    string for config.hostname previously produced a silent no-op --
+    build_commands' falsy check treated "" the same as "not
+    specified" -- even though hostname is declared required=True.
+    Ansible's own argspec validation only checks presence, not
+    non-emptiness, so this reached build_commands unnoticed. Confirmed
+    by direct reproduction before fixing; now fails explicitly instead.
+
+    In-process pattern (patch.object for sys.argv, matching the
+    established tests elsewhere in this file) rather than subprocess
+    isolation: confirmed the subprocess approach breaks under
+    ansible-test --docker's custom collection-loading machinery, which
+    a freshly spawned subprocess does not inherit.
+    """
+
+    def _run_main(self, config, state="merged"):
+        import json
+        import sys
+
+        from unittest.mock import patch
+
+        from ansible.module_utils import basic
+
+        args = {"state": state}
+        if config is not None:
+            args["config"] = config
+        argv = ["x", json.dumps({"ANSIBLE_MODULE_ARGS": args})]
+        captured = {}
+
+        def fake_exit_json(self_mod, **kwargs):
+            captured.update(kwargs)
+            raise SystemExit(0)
+
+        def fake_fail_json(self_mod, **kwargs):
+            captured.update(kwargs)
+            captured["failed"] = True
+            raise SystemExit(1)
+
+        basic._ANSIBLE_ARGS = None
+        basic._PARSED_MODULE_ARGS = None
+        with patch.object(sys, "argv", argv), patch(
+            "ansible_collections.vyos.rest.plugins.module_utils.vyos.VyOSRestClient",
+        ) as mock_client, patch(
+            "ansible.module_utils.basic.AnsibleModule.exit_json",
+            fake_exit_json,
+        ), patch(
+            "ansible.module_utils.basic.AnsibleModule.fail_json",
+            fake_fail_json,
+        ):
+            mock_client.return_value.retrieve_return_value.return_value = {
+                "data": "existing-host",
+            }
+            from ansible_collections.vyos.rest.plugins.modules import vyos_hostname
+
+            with self.assertRaises(SystemExit):
+                vyos_hostname.main()
+        return captured
+
+    def test_empty_hostname_fails_explicitly(self):
+        result = self._run_main({"hostname": ""})
+        self.assertTrue(result.get("failed"))
+        self.assertIn("non-empty", result.get("msg", ""))
+
+    def test_non_empty_hostname_still_works(self):
+        result = self._run_main({"hostname": "newhost"})
+        self.assertNotIn("failed", result)
+
+
+class TestCheckModeOmitsAfter(unittest.TestCase):
+    """Confirmed real bug (Copilot): check_mode returned after=have
+    even when commands was non-empty, misrepresenting the pre-change
+    state as if it were the post-change result. Now matches the
+    established convention across the rest of the collection
+    (confirmed against vyos_nat, vyos_ha, vyos_snmp_server,
+    vyos_ntp_global): omit "after" entirely in check mode.
+    """
+
+    def test_check_mode_omits_after(self):
+        import json
+        import sys
+
+        from unittest.mock import patch
+
+        from ansible.module_utils import basic
+
+        argv = [
+            "x",
+            json.dumps(
+                {
+                    "ANSIBLE_MODULE_ARGS": {
+                        "config": {"hostname": "newhost"},
+                        "state": "merged",
+                        "_ansible_check_mode": True,
+                    },
+                },
+            ),
+        ]
+        captured = {}
+
+        def fake_exit_json(self_mod, **kwargs):
+            captured.update(kwargs)
+            raise SystemExit(0)
+
+        basic._ANSIBLE_ARGS = None
+        basic._PARSED_MODULE_ARGS = None
+        with patch.object(sys, "argv", argv), patch(
+            "ansible_collections.vyos.rest.plugins.module_utils.vyos.VyOSRestClient",
+        ) as mock_client, patch(
+            "ansible.module_utils.basic.AnsibleModule.exit_json",
+            fake_exit_json,
+        ):
+            mock_client.return_value.retrieve_return_value.return_value = {"data": "oldhost"}
+            from ansible_collections.vyos.rest.plugins.modules import vyos_hostname
+
+            with self.assertRaises(SystemExit):
+                vyos_hostname.main()
+
+        self.assertTrue(captured.get("changed"))
+        self.assertNotIn("after", captured)
+
+
+if __name__ == "__main__":
+    unittest.main()