T8989: vyos_static_routes dict_op refactor (#25)

* T8989: vyos_static_routes dict_op refactor

* T8989: Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

1 files changed, 59 insertions, 5 deletions

diff --git a/plugins/modules/vyos_static_routes.py b/plugins/modules/vyos_static_routes.py
index 3276588..11a636d 100644
--- a/plugins/modules/vyos_static_routes.py
+++ b/plugins/modules/vyos_static_routes.py
@@ -65,9 +65,17 @@ options:
                 description: Administrative distance for this next-hop (1-255).
                 type: int
               enabled:
-                description: Whether this next-hop is enabled.
+                description:
+                  - Whether this next-hop is enabled.
+                  - >-
+                    Deliberately has no default: leaving it unset means
+                    "no opinion" and an existing device-side disabled
+                    state is left alone under C(merged). Explicitly
+                    setting C(true) actively clears a previously
+                    disabled next-hop, which C(merged) could otherwise
+                    never do (only C(replaced)/C(overridden) run a
+                    purge pass capable of noticing an omitted value).
                 type: bool
-                default: true
               interface:
                 description: Outgoing interface name.
                 type: str
@@ -75,7 +83,9 @@ options:
     description:
       - C(merged) - Add routes without removing existing ones.
       - C(replaced) - Replace each named route (by afi + dest) exactly as specified.
-      - C(overridden) - Replace the entire static route table.
+      - C(overridden) - Replace the module-known parts of the static route
+        table (see module scope note above; unmodeled attributes on
+        unmodeled routes are not touched).
       - C(deleted) - Remove listed or all static routes.
       - C(gathered) - Read static routes from device without changes.
     type: str
@@ -316,6 +326,41 @@ def _device_to_argspec(raw):
     return result
 
 
+def _clear_stale_disable_commands(config, norm_have):
+    """dict_op(op="set") only ever adds/updates keys present in want --
+    it structurally cannot reach a "disable" leaf that's simply absent
+    from want, so under "merged" (which never runs a purge pass at
+    all) there was no way for the generic engine to clear a previously
+    disabled next-hop, confirmed as a real gap. This checks the
+    original argspec-shape config directly (not the device-shape
+    want, where explicit True and unset/None are indistinguishable by
+    construction) for an explicit enabled: true against a next-hop
+    that's currently disabled on the device, and emits an explicit
+    delete for its "disable" leaf when the two disagree.
+
+    Not needed for replaced/overridden: their purge pass already
+    clears an omitted "disable" correctly regardless of why it's
+    omitted (purge compares the full want/have shape, not just this
+    one leaf), so calling this there would only be harmless, redundant
+    work -- it's scoped to merged specifically to avoid that.
+    """
+    cmds = []
+    for entry in config or []:
+        route_key = _ROUTE_KEY.get(entry.get("afi"))
+        if not route_key:
+            continue
+        for route in entry.get("routes") or []:
+            dest = route.get("dest")
+            if not dest:
+                continue
+            have_nhs = ((norm_have.get(route_key) or {}).get(dest) or {}).get("next-hop") or {}
+            for nh in route.get("next_hops") or []:
+                addr = nh.get("forward_router_address")
+                if addr and nh.get("enabled") is True and "disable" in (have_nhs.get(addr) or {}):
+                    cmds.append(("delete", _BASE + [route_key, dest, "next-hop", addr, "disable"]))
+    return cmds
+
+
 def build_commands(config, raw_have, state):
     raw_have = raw_have or {}
     config = config or []
@@ -375,6 +420,8 @@ def build_commands(config, raw_have, state):
                     _BASE + [route_key, dest],
                     op="purge",
                 )
+    elif state == "merged":
+        commands += _clear_stale_disable_commands(config, norm_have)
     commands += dict_op(want, norm_have, _BASE, op="set")
     return commands
 
@@ -382,7 +429,7 @@ def build_commands(config, raw_have, state):
 _NEXT_HOP_OPTIONS = dict(
     forward_router_address=dict(type="str", required=True),
     admin_distance=dict(type="int"),
-    enabled=dict(type="bool", default=True),
+    enabled=dict(type="bool"),
     interface=dict(type="str"),
 )
 
@@ -441,7 +488,14 @@ def main():
     commands = build_commands(config, raw_have, state)
 
     if module.check_mode:
-        module.exit_json(changed=bool(commands), commands=commands, before=have)
+        # Predicting the exact post-apply shape without applying would
+        # mean re-implementing dict_op's effects in reverse. Rather
+        # than that, or silently omitting "after" (which RETURN
+        # documents as present "when changed" -- true here whenever
+        # commands is non-empty), return have as the best honestly
+        # available value: not a prediction, but at least a defined,
+        # non-crashing one for playbooks that read result.after.
+        module.exit_json(changed=bool(commands), commands=commands, before=have, after=have)
 
     if commands:
         response = vyos.apply_commands(commands)