diff options
| -rw-r--r-- | changelogs/fragments/t8989_l3_interface_dict_op.yml | 3 | ||||
| -rw-r--r-- | changelogs/fragments/t8989_ospfv3_dict_op.yml | 3 | ||||
| -rw-r--r-- | changelogs/fragments/t8989_ospv2_dict_op.yml | 5 | ||||
| -rw-r--r-- | changelogs/fragments/t8989_vyos_get_value.yml | 3 | ||||
| -rw-r--r-- | docs/vyos.rest.vyos_l3_interfaces_module.rst | 51 | ||||
| -rw-r--r-- | docs/vyos.rest.vyos_ospfv2_module.rst | 659 | ||||
| -rw-r--r-- | docs/vyos.rest.vyos_ospfv3_module.rst | 16 | ||||
| -rw-r--r-- | plugins/module_utils/vyos.py | 6 | ||||
| -rw-r--r-- | plugins/modules/vyos_l3_interfaces.py | 372 | ||||
| -rw-r--r-- | plugins/modules/vyos_ospfv2.py | 1327 | ||||
| -rw-r--r-- | plugins/modules/vyos_ospfv3.py | 367 | ||||
| -rw-r--r-- | tests/unit/fixtures/l3_interfaces_running.json | 19 | ||||
| -rw-r--r-- | tests/unit/fixtures/ospfv2_running.json | 71 | ||||
| -rw-r--r-- | tests/unit/fixtures/ospfv3_running.json | 11 | ||||
| -rw-r--r-- | tests/unit/modules/test_vyos_l3_interfaces.py | 448 | ||||
| -rw-r--r-- | tests/unit/modules/test_vyos_ospfv2.py | 408 | ||||
| -rw-r--r-- | tests/unit/modules/test_vyos_ospfv3.py | 260 | ||||
| -rw-r--r-- | tests/unit/test_module_utils_vyos.py | 88 |
18 files changed, 2578 insertions, 1539 deletions
diff --git a/changelogs/fragments/t8989_l3_interface_dict_op.yml b/changelogs/fragments/t8989_l3_interface_dict_op.yml new file mode 100644 index 0000000..5cccc00 --- /dev/null +++ b/changelogs/fragments/t8989_l3_interface_dict_op.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - vyos_l3_interfaces - Refactor the module to comply with dict_op paradigm. diff --git a/changelogs/fragments/t8989_ospfv3_dict_op.yml b/changelogs/fragments/t8989_ospfv3_dict_op.yml new file mode 100644 index 0000000..b151ac3 --- /dev/null +++ b/changelogs/fragments/t8989_ospfv3_dict_op.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - vyos_ospfv3 - Refactor module to dict_op style. diff --git a/changelogs/fragments/t8989_ospv2_dict_op.yml b/changelogs/fragments/t8989_ospv2_dict_op.yml new file mode 100644 index 0000000..5a0f78d --- /dev/null +++ b/changelogs/fragments/t8989_ospv2_dict_op.yml @@ -0,0 +1,5 @@ +--- +minor_changes: + - vyos_ospfv2 - refactor command generation onto ``dict_op``. + - vyos_ospfv2 - add support for ``areas[].virtual_link``, ``max_metric``, + ``mpls_te``, ``timers``, and ``passive_interface_exclude``. diff --git a/changelogs/fragments/t8989_vyos_get_value.yml b/changelogs/fragments/t8989_vyos_get_value.yml new file mode 100644 index 0000000..f6b7043 --- /dev/null +++ b/changelogs/fragments/t8989_vyos_get_value.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - plugins/module_utils/vyos.py - Add VyOSModule.get_value() wrapper for the "returnValue" retrieve operation. diff --git a/docs/vyos.rest.vyos_l3_interfaces_module.rst b/docs/vyos.rest.vyos_l3_interfaces_module.rst index d251abb..784a565 100644 --- a/docs/vyos.rest.vyos_l3_interfaces_module.rst +++ b/docs/vyos.rest.vyos_l3_interfaces_module.rst @@ -18,7 +18,7 @@ Version added: 1.0.0 Synopsis -------- - Manages IPv4 and IPv6 address configuration on VyOS interfaces using the HTTPS REST API. -- Mirrors ``vyos.vyos.vyos_l3_interfaces`` but uses the HTTP API instead of CLI. +- L2 attributes (description, mtu, duplex, speed, vrf) and VIF presence/L2 attributes are owned by :ref:`vyos.rest.vyos_interfaces <vyos.rest.vyos_interfaces_module>`, not this module -- confirmed by design: a VIF's device path is shared between the two modules, and this module's own commands only ever touch the ``address`` leaf within it, never the VIF subtree as a whole or any of vyos_interfaces' own fields. @@ -251,21 +251,6 @@ Parameters <tr> <td colspan="4"> <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="4"> - <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"> @@ -279,8 +264,6 @@ Parameters <li>overridden</li> <li>deleted</li> <li>gathered</li> - <li>rendered</li> - <li>parsed</li> </ul> </td> <td> @@ -289,8 +272,6 @@ Parameters <div><code>overridden</code> - Replace addresses for all interfaces.</div> <div><code>deleted</code> - Remove listed or all interface addresses.</div> <div><code>gathered</code> - Read interface addresses 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> @@ -423,36 +404,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 the 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/docs/vyos.rest.vyos_ospfv2_module.rst b/docs/vyos.rest.vyos_ospfv2_module.rst index 1c5c248..9e0c41b 100644 --- a/docs/vyos.rest.vyos_ospfv2_module.rst +++ b/docs/vyos.rest.vyos_ospfv2_module.rst @@ -19,7 +19,8 @@ Synopsis -------- - Manages OSPFv2 configuration on VyOS devices via the REST API. - Uses REST API (``connection=httpapi``) instead of CLI. -- In VyOS 1.5+, passive interfaces use per-interface config rather than passive-interface. +- Scope matches the current vyos.vyos.vyos_ospfv2 (CLI collection) module: areas (including virtual_link), auto_cost, default_information, default_metric, distance, log_adjacency_changes, max_metric, mpls_te, neighbor, parameters, passive_interface, passive_interface_exclude, redistribute, timers. VyOS's OSPF schema is considerably larger than even this -- access-list, aggregation, capability, graceful-restart, ldp-sync, maximum-paths, per-interface tuning (bandwidth/hello-multiplier/network-type/authentication/intervals beyond passive), segment-routing, and summary-address are not modeled here, matching real gaps in the CLI module's own scope, not oversights. +- A standalone top-level ``route_map`` field exists in the CLI module's argspec but does not correspond to any real device path -- confirmed against a live VyOS 1.5.0 device's ``set protocols ospf`` completions, which list no ``route-map`` entry at that level (only nested under ``default-information originate`` and per-protocol under ``redistribute``, both of which are modeled here). Deliberately omitted rather than built against a confirmed non-functional field. @@ -31,12 +32,12 @@ Parameters <table border=0 cellpadding=0 class="documentation-table"> <tr> - <th colspan="5">Parameter</th> + <th colspan="6">Parameter</th> <th>Choices/<font color="blue">Defaults</font></th> <th width="100%">Comments</th> </tr> <tr> - <td colspan="5"> + <td colspan="6"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>config</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -52,7 +53,7 @@ Parameters </tr> <tr> <td class="elbow-placeholder"></td> - <td colspan="4"> + <td colspan="5"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>areas</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -70,7 +71,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>area_id</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -88,7 +89,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>area_type</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -106,7 +107,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>normal</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -128,7 +129,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>nssa</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -147,7 +148,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="1"> + <td colspan="2"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>default_cost</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -166,7 +167,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="1"> + <td colspan="2"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>no_summary</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -189,7 +190,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="1"> + <td colspan="2"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>set</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -212,7 +213,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="1"> + <td colspan="2"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>translate</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -236,7 +237,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>stub</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -255,7 +256,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="1"> + <td colspan="2"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>default_cost</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -274,7 +275,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="1"> + <td colspan="2"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>no_summary</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -297,7 +298,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="1"> + <td colspan="2"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>set</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -320,7 +321,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>authentication</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -341,7 +342,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>network</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -360,7 +361,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>address</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -379,7 +380,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>range</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -398,7 +399,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>address</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -417,7 +418,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>cost</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -435,7 +436,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>not_advertise</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -457,7 +458,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>substitute</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -475,7 +476,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>shortcut</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -494,11 +495,220 @@ Parameters <div>Shortcut mode.</div> </td> </tr> - <tr> <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>virtual_link</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">list</span> + / <span style="color: purple">elements=dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>Virtual link.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>address</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">string</span> + / <span style="color: red">required</span> + </div> + </td> + <td> + </td> + <td> + <div>Virtual link address (router ID of the remote ABR).</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>authentication</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>Virtual link authentication.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="2"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>md5</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">list</span> + / <span style="color: purple">elements=dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>MD5 key id based authentication.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="1"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>key_id</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> + </td> + <td> + <div>MD5 key id (1-255).</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="1"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>md5_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>MD5 key (16 characters or less).</div> + </td> + </tr> + + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="2"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>plaintext_password</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>Plain text password (8 characters or less).</div> + </td> + </tr> + + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>dead_interval</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> + </td> + <td> + <div>Interval after which a neighbor is declared dead.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>hello_interval</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> + </td> + <td> + <div>Interval between hello packets.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>retransmit_interval</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> + </td> + <td> + <div>Interval between retransmitting lost link state advertisements.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>transmit_delay</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> + </td> + <td> + <div>Link state transmit delay.</div> + </td> + </tr> + + + <tr> + <td class="elbow-placeholder"></td> + <td colspan="5"> + <div class="ansibleOptionAnchor" id="parameter-"></div> <b>auto_cost</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> <div style="font-size: small"> @@ -514,7 +724,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>reference_bandwidth</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -531,7 +741,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> - <td colspan="4"> + <td colspan="5"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>default_information</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -548,7 +758,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>originate</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -566,7 +776,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>always</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -588,7 +798,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>metric</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -606,7 +816,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>metric_type</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -624,7 +834,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>route_map</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -642,7 +852,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> - <td colspan="4"> + <td colspan="5"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>default_metric</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -658,7 +868,7 @@ Parameters </tr> <tr> <td class="elbow-placeholder"></td> - <td colspan="4"> + <td colspan="5"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>distance</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -675,7 +885,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>global</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -692,7 +902,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>ospf</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -710,7 +920,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>external</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -728,7 +938,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>inter_area</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -746,7 +956,7 @@ Parameters <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="2"> + <td colspan="3"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>intra_area</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -764,7 +974,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> - <td colspan="4"> + <td colspan="5"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>log_adjacency_changes</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -783,8 +993,156 @@ Parameters </tr> <tr> <td class="elbow-placeholder"></td> + <td colspan="5"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>max_metric</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>OSPFv2 maximum/infinite-distance metric.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="4"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>router_lsa</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>Advertise own Router-LSA with infinite distance (stub router).</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>administrative</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>no</li> + <li>yes</li> + </ul> + </td> + <td> + <div>Administratively apply, for an indefinite period.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>on_shutdown</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> + </td> + <td> + <div>Time (seconds) to advertise self as stub-router before shutdown.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>on_startup</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> + </td> + <td> + <div>Time (seconds) to advertise self as stub-router on startup.</div> + </td> + </tr> + + + <tr> + <td class="elbow-placeholder"></td> + <td colspan="5"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>mpls_te</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>MPLS Traffic Engineering parameters.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="4"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>enabled</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>no</li> + <li>yes</li> + </ul> + </td> + <td> + <div>Enable MPLS-TE functionality.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>router_address</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>Stable IP address of the advertising router.</div> + </td> + </tr> + + <tr> + <td class="elbow-placeholder"></td> + <td colspan="5"> + <div class="ansibleOptionAnchor" id="parameter-"></div> <b>neighbor</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> <div style="font-size: small"> @@ -801,7 +1159,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>neighbor_id</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -819,7 +1177,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>poll_interval</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -836,7 +1194,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>priority</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -853,7 +1211,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> - <td colspan="4"> + <td colspan="5"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>parameters</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -870,7 +1228,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>abr_type</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -893,7 +1251,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>opaque_lsa</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -914,7 +1272,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>rfc1583_compatibility</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -935,7 +1293,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>router_id</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -952,7 +1310,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> - <td colspan="4"> + <td colspan="5"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>passive_interface</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -964,12 +1322,29 @@ Parameters <td> </td> <td> - <div>Passive interfaces (VyOS 1.5+: configured via <code>protocols ospf interface <name> passive</code>).</div> + <div>Interfaces to suppress routing updates on, via per-interface configuration (<code>protocols ospf interface <name> passive</code>).</div> </td> </tr> <tr> <td class="elbow-placeholder"></td> - <td colspan="4"> + <td colspan="5"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>passive_interface_exclude</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">list</span> + / <span style="color: purple">elements=string</span> + </div> + </td> + <td> + </td> + <td> + <div>Interfaces to explicitly exclude from passive mode (via <code>protocols ospf interface <name> passive disable</code>) -- e.g. when passive is otherwise applied broadly.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td colspan="5"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>redistribute</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -987,7 +1362,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>metric</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -1004,7 +1379,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>metric_type</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -1021,7 +1396,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>route_map</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -1038,7 +1413,7 @@ Parameters <tr> <td class="elbow-placeholder"></td> <td class="elbow-placeholder"></td> - <td colspan="3"> + <td colspan="4"> <div class="ansibleOptionAnchor" id="parameter-"></div> <b>route_type</b> <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> @@ -1060,10 +1435,157 @@ Parameters </td> </tr> - <tr> + <td class="elbow-placeholder"></td> <td colspan="5"> <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>timers</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>Routing timers.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="4"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>refresh</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>Refresh parameters.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>timers</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> + </td> + <td> + <div>Refresh timer (seconds).</div> + </td> + </tr> + + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="4"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>throttle</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>Throttling adaptive timers.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="3"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>spf</b> + <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> + <div style="font-size: small"> + <span style="color: purple">dictionary</span> + </div> + </td> + <td> + </td> + <td> + <div>SPF timers.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="2"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>delay</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> + </td> + <td> + <div>Delay (ms) from first change received to SPF calculation.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="2"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>initial_holdtime</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> + </td> + <td> + <div>Initial hold time (ms) between consecutive SPF calculations.</div> + </td> + </tr> + <tr> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td class="elbow-placeholder"></td> + <td colspan="2"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>max_holdtime</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> + </td> + <td> + <div>Maximum hold time (ms).</div> + </td> + </tr> + + + + + <tr> + <td colspan="6"> + <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"> @@ -1096,9 +1618,16 @@ Notes .. note:: - Requires ``ansible_connection=httpapi`` with the VyOS httpapi plugin. - ``ansible_network_os`` must be set to ``vyos.rest.vyos``. - - VyOS 1.5+ uses per-interface passive configuration rather than the global ``passive-interface`` command used in VyOS 1.4. +See Also +-------- + +.. seealso:: + + :ref:`vyos.vyos.vyos_ospfv2_module` + The official documentation on the **vyos.vyos.vyos_ospfv2** module. + Examples -------- @@ -1119,10 +1648,6 @@ Examples normal: true network: - address: 192.0.2.0/24 - - area_id: "3" - area_type: - nssa: - set: true - area_id: "4" area_type: stub: @@ -1135,6 +1660,16 @@ Examples metric: 10 passive_interface: - eth1 + max_metric: + router_lsa: + administrative: true + mpls_te: + enabled: true + router_address: 192.0.11.11 + timers: + throttle: + spf: + delay: 200 state: merged - name: Delete all OSPFv2 configuration diff --git a/docs/vyos.rest.vyos_ospfv3_module.rst b/docs/vyos.rest.vyos_ospfv3_module.rst index 18bb48e..2b63cf5 100644 --- a/docs/vyos.rest.vyos_ospfv3_module.rst +++ b/docs/vyos.rest.vyos_ospfv3_module.rst @@ -19,6 +19,9 @@ Synopsis -------- - Manages OSPFv3 configuration on VyOS devices via the REST API. - Uses REST API (``connection=httpapi``) instead of CLI. +- Scope matches the current vyos.vyos.vyos_ospfv3 (CLI collection) module, confirmed against VyOS's official documentation (1.4+/1.5 LTS/rolling). +- ``areas.interface`` (an area-to-interface assignment list) exists in the CLI module's argspec but was deliberately NOT carried over here -- confirmed via VyOS's official docs across multiple versions that ``set protocols ospfv3 area <id> interface <name>`` is the superseded, 1.3-era syntax. The current mechanism, ``set protocols ospfv3 interface <name> area <id>``, is a per-interface setting and is modeled in :ref:`vyos.rest.vyos_ospf_interfaces <vyos.rest.vyos_ospf_interfaces_module>`'s ``area`` field instead (and is not affected by this module's ``state=replaced``). +- ``distance`` and ``graceful-restart`` are real, confirmed OSPFv3 features not modeled here, matching a genuine gap in the CLI module's own scope rather than an oversight. @@ -133,7 +136,7 @@ Parameters <td> </td> <td> - <div>Summarize routes matching prefix.</div> + <div>Summarize routes matching prefix (border routers only).</div> </td> </tr> <tr> @@ -278,6 +281,7 @@ Parameters <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a> <div style="font-size: small"> <span style="color: purple">string</span> + / <span style="color: red">required</span> </div> </td> <td> @@ -332,6 +336,16 @@ Notes - ``ansible_network_os`` must be set to ``vyos.rest.vyos``. +See Also +-------- + +.. seealso:: + + :ref:`vyos.vyos.vyos_ospfv3_module` + The official documentation on the **vyos.vyos.vyos_ospfv3** module. + :ref:`vyos.rest.vyos_ospf_interfaces_module` + The official documentation on the **vyos.rest.vyos_ospf_interfaces** module. + Examples -------- diff --git a/plugins/module_utils/vyos.py b/plugins/module_utils/vyos.py index f7c17f9..b7a519e 100644 --- a/plugins/module_utils/vyos.py +++ b/plugins/module_utils/vyos.py @@ -320,7 +320,8 @@ class VyOSModule: that's actually already correct. """ result = self._client.retrieve_return_value(path) - return result.get("data") or "" + data = result.get("data") + return "" if data is None else data def apply_commands(self, commands): if not commands: @@ -372,7 +373,8 @@ class VyOSModule: rather than being indistinguishable from a valid empty response. """ result = self._client.show(path) - return result.get("data") or "" + data = result.get("data") + return "" if data is None else data def save_config(self, file_path=None): """Save the running configuration to disk.""" diff --git a/plugins/modules/vyos_l3_interfaces.py b/plugins/modules/vyos_l3_interfaces.py index 9b33973..c9c6deb 100644 --- a/plugins/modules/vyos_l3_interfaces.py +++ b/plugins/modules/vyos_l3_interfaces.py @@ -14,7 +14,13 @@ short_description: Manage L3 interface configuration on VyOS devices via REST AP description: - Manages IPv4 and IPv6 address configuration on VyOS interfaces using the HTTPS REST API. - - Mirrors C(vyos.vyos.vyos_l3_interfaces) but uses the HTTP API instead of CLI. + - >- + L2 attributes (description, mtu, duplex, speed, vrf) and VIF + presence/L2 attributes are owned by M(vyos.rest.vyos_interfaces), not + this module -- confirmed by design: a VIF's device path is shared + between the two modules, and this module's own commands only ever + touch the C(address) leaf within it, never the VIF subtree as a whole + or any of vyos_interfaces' own fields. version_added: "1.0.0" author: - VyOS Community (@vyos) @@ -71,9 +77,6 @@ options: address: description: IPv6 address in CIDR notation, C(dhcpv6), or C(auto-config). type: str - running_config: - description: Used only with state C(parsed). - type: str state: description: - C(merged) - Add addresses without removing existing ones. @@ -81,10 +84,8 @@ options: - C(overridden) - Replace addresses for all interfaces. - C(deleted) - Remove listed or all interface addresses. - C(gathered) - Read interface addresses 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_l3_interfaces @@ -142,14 +143,6 @@ gathered: description: Current L3 interface configuration as structured data. returned: when state is gathered type: list -rendered: - description: Commands for the 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 @@ -161,10 +154,16 @@ 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, to_tag_dict + +_BASE = ["interfaces"] -_IFACE_TYPE = { +# Aligned with vyos_interfaces' own 11-type table -- confirmed +# inconsistent in the original (missing ppp/wlan here specifically), +# corrected for consistency between the two modules that share the +# same interface namespace. +_IFACE_TYPE_PREFIX = { "eth": "ethernet", "bond": "bonding", "lo": "loopback", @@ -173,23 +172,42 @@ _IFACE_TYPE = { "vti": "vti", "dum": "dummy", "vtun": "openvpn", + "ppp": "pppoe", + "wlan": "wireless", "br": "bridge", } -def _iface_type(name): - for prefix, itype in _IFACE_TYPE.items(): +def _guess_iface_type(name): + for prefix, itype in _IFACE_TYPE_PREFIX.items(): if name.startswith(prefix): return itype return "ethernet" -def _iface_base(name): - return ["interfaces", _iface_type(name), name] +def _resolve_iface_type(name, raw_have): + """Prefer the real type from the device's own raw response over a + name-prefix guess -- only fall back to guessing for a brand-new + interface that doesn't exist on the device yet. Same fix as + vyos_interfaces' own confirmed bug: the original guessed + unconditionally, even for interfaces the device already knows the + real type of. + """ + for itype, ifaces in (raw_have or {}).items(): + if name in to_tag_dict(ifaces): + return itype + return _guess_iface_type(name) + + +def _iface_base(name, raw_have): + return _BASE + [_resolve_iface_type(name, raw_have), name] def _addr_list(raw): - """Normalize address field — string or list → sorted list.""" + """VyOS's address leaf collapses to a bare string for a single + value, or a list for multiple -- confirmed device behavior, + handled explicitly here since it's a plain multi-value leaf, not + a tag-node (to_tag_dict doesn't apply).""" if not raw: return [] if isinstance(raw, str): @@ -200,92 +218,105 @@ def _addr_list(raw): def _split_addresses(addresses): - """Split address list into ipv4 and ipv6 lists.""" ipv4 = [] ipv6 = [] for addr in addresses: - if addr in ("dhcp", "dhcpv6"): - if addr == "dhcp": - ipv4.append(addr) - else: - ipv6.append(addr) - elif ":" in addr: + if addr == "dhcp": + ipv4.append(addr) + elif addr in ("dhcpv6", "auto-config") or ":" in addr: ipv6.append(addr) else: ipv4.append(addr) return sorted(ipv4), sorted(ipv6) -def _parse_iface(name, idata): - """Parse raw API interface data into argspec format.""" - idata = idata or {} - entry = {"name": name} - - addrs = _addr_list(idata.get("address")) - if addrs: - ipv4, ipv6 = _split_addresses(addrs) - if ipv4: - entry["ipv4"] = [{"address": a} for a in ipv4] - if ipv6: - entry["ipv6"] = [{"address": a} for a in ipv6] - - vif_data = idata.get("vif") or {} - if isinstance(vif_data, dict) and vif_data: - vifs = [] - for vlan_id, vdata in sorted(vif_data.items(), key=lambda x: int(x[0])): - vdata = vdata or {} - vif = {"vlan_id": int(vlan_id)} - vaddrs = _addr_list(vdata.get("address")) - if vaddrs: - vipv4, vipv6 = _split_addresses(vaddrs) - if vipv4: - vif["ipv4"] = [{"address": a} for a in vipv4] - if vipv6: - vif["ipv6"] = [{"address": a} for a in vipv6] - vifs.append(vif) - if vifs: - entry["vifs"] = vifs - - return entry +def _parse_addr_entry(idata): + """Returns (ipv4_argspec_list, ipv6_argspec_list) for one address + leaf's raw value -- shared by both interface-level and vif-level + parsing.""" + addrs = _addr_list((idata or {}).get("address")) + if not addrs: + return [], [] + ipv4, ipv6 = _split_addresses(addrs) + return ( + [{"address": a} for a in ipv4], + [{"address": a} for a in ipv6], + ) def get_running_config(vyos): - raw = vyos.get_config(["interfaces"]) - if not raw or not isinstance(raw, dict): - return [] + """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. + """ + return to_tag_dict(vyos.get_config(_BASE) or {}) + +def _device_to_argspec(raw): result = [] - for itype, ifaces in sorted(raw.items()): - if not isinstance(ifaces, dict): - continue - for iname, idata in sorted(ifaces.items()): - entry = _parse_iface(iname, idata) - # only include if there's at least one address or vif + for itype, ifaces in sorted((raw or {}).items()): + for name, idata in sorted(to_tag_dict(ifaces).items()): + idata = idata or {} + entry = {"name": name} + + ipv4, ipv6 = _parse_addr_entry(idata) + if ipv4: + entry["ipv4"] = ipv4 + if ipv6: + entry["ipv6"] = ipv6 + + vif_raw = idata.get("vif") + if vif_raw: + vifs = [] + for vlan_id, vdata in sorted(to_tag_dict(vif_raw).items(), key=lambda x: int(x[0])): + vipv4, vipv6 = _parse_addr_entry(vdata) + if vipv4 or vipv6: + vif = {"vlan_id": int(vlan_id)} + if vipv4: + vif["ipv4"] = vipv4 + if vipv6: + vif["ipv6"] = vipv6 + vifs.append(vif) + if vifs: + entry["vifs"] = vifs + if entry.get("ipv4") or entry.get("ipv6") or entry.get("vifs"): result.append(entry) - return result def _normalize(config): - """Convert argspec list to dict keyed by interface name.""" + """Argspec list -> dict keyed by interface name, with address + lists flattened to plain sorted string lists for set-diffing.""" result = {} for entry in config or []: - name = entry["name"] - ipv4 = sorted([a["address"] for a in (entry.get("ipv4") or [])]) - ipv6 = sorted([a["address"] for a in (entry.get("ipv6") or [])]) + name = entry.get("name") + if not name: + continue + ipv4 = sorted(a["address"] for a in (entry.get("ipv4") or []) if a.get("address")) + ipv6 = sorted(a["address"] for a in (entry.get("ipv6") or []) if a.get("address")) vifs = {} for vif in entry.get("vifs") or []: - vid = vif["vlan_id"] - vipv4 = sorted([a["address"] for a in (vif.get("ipv4") or [])]) - vipv6 = sorted([a["address"] for a in (vif.get("ipv6") or [])]) + vid = vif.get("vlan_id") + if vid is None: + continue + vipv4 = sorted(a["address"] for a in (vif.get("ipv4") or []) if a.get("address")) + vipv6 = sorted(a["address"] for a in (vif.get("ipv6") or []) if a.get("address")) vifs[vid] = {"ipv4": vipv4, "ipv6": vipv6} result[name] = {"ipv4": ipv4, "ipv6": ipv6, "vifs": vifs} return result def _addr_cmds(base, want_addrs, have_addrs, state): - """Generate set/delete commands for address lists.""" + """Address is a plain multi-value leaf, not a keyed/tag-node + section -- a set-diff is the correct, direct mechanism here + (dict_op's keyed-entry model doesn't fit a bare value list). + Confirmed scoped to exactly the "address" leaf under base, + never anything else -- base may be an interface or a vif, and + in neither case does this function ever touch a sibling leaf. + """ cmds = [] want_set = set(want_addrs) have_set = set(have_addrs) @@ -300,15 +331,25 @@ def _addr_cmds(base, want_addrs, have_addrs, state): return cmds -def _vif_cmds(iface_base, want_vifs, have_vifs, state): - """Generate commands for VIF subinterfaces.""" +def _vif_addr_cmds(iface_base, want_vifs, have_vifs, state): + """VIF address commands, explicitly scoped to only the address + leaf within each VIF -- never the VIF subtree as a whole. + + Confirmed severe bug in the original: an omitted VIF under + replaced/overridden/deleted generated a whole-VIF delete + (`delete ... vif <id>`), which would also destroy the VIF's own + description/mtu/disable settings -- fields owned by + vyos_interfaces, not this module. The same class of bug just + confirmed and fixed in vyos_interfaces for interface-level + "address" leaking the other way. Here, an omitted VIF under any + state only ever has its own known addresses individually deleted; + the VIF's own presence/other fields are never touched. + """ cmds = [] - if state in ("replaced", "overridden"): - for vid in set(have_vifs) - set(want_vifs): - cmds.append(("delete", iface_base + ["vif", str(vid)])) - - for vid, want_vif in want_vifs.items(): + all_vids = set(have_vifs) | set(want_vifs) + for vid in all_vids: + want_vif = want_vifs.get(vid, {"ipv4": [], "ipv6": []}) have_vif = have_vifs.get(vid, {"ipv4": [], "ipv6": []}) vif_base = iface_base + ["vif", str(vid)] cmds += _addr_cmds(vif_base, want_vif["ipv4"], have_vif["ipv4"], state) @@ -317,144 +358,119 @@ def _vif_cmds(iface_base, want_vifs, have_vifs, state): return cmds -def build_commands(config, have_raw, state): - cmds = [] - have_map = _normalize(have_raw) +def build_commands(config, raw_have, state): + raw_have = raw_have or {} + have_list = _device_to_argspec(raw_have) + have_map = _normalize(have_list) if state == "deleted": + cmds = [] if not config: for name, have in have_map.items(): - base = _iface_base(name) - for addr in have["ipv4"] + have["ipv6"]: - cmds.append(("delete", base + ["address", addr])) - for vid in have["vifs"]: - cmds.append(("delete", base + ["vif", str(vid)])) - else: - want_map = _normalize(config) - for name, want in want_map.items(): - have = have_map.get(name, {"ipv4": [], "ipv6": [], "vifs": {}}) - base = _iface_base(name) - if not want["ipv4"] and not want["ipv6"] and not want["vifs"]: - # delete all addresses for this interface - for addr in have["ipv4"] + have["ipv6"]: + base = _iface_base(name, raw_have) + cmds += _addr_cmds(base, [], have["ipv4"], "deleted") + cmds += _addr_cmds(base, [], have["ipv6"], "deleted") + cmds += _vif_addr_cmds(base, {}, have["vifs"], "deleted") + return cmds + for entry in config or []: + name = entry.get("name") + if not name: + continue + have = have_map.get(name, {"ipv4": [], "ipv6": [], "vifs": {}}) + base = _iface_base(name, raw_have) + named_addrs = entry.get("ipv4") or entry.get("ipv6") or entry.get("vifs") + if not named_addrs: + cmds += _addr_cmds(base, [], have["ipv4"], "deleted") + cmds += _addr_cmds(base, [], have["ipv6"], "deleted") + cmds += _vif_addr_cmds(base, {}, have["vifs"], "deleted") + else: + want = _normalize([entry])[name] + for addr in want["ipv4"] + want["ipv6"]: + if addr in have["ipv4"] + have["ipv6"]: cmds.append(("delete", base + ["address", addr])) - for vid in have["vifs"]: - cmds.append(("delete", base + ["vif", str(vid)])) - else: - for addr in want["ipv4"] + want["ipv6"]: - if addr in have["ipv4"] + have["ipv6"]: - cmds.append(("delete", base + ["address", addr])) - for vid in want["vifs"]: - if vid in have["vifs"]: - cmds.append(("delete", base + ["vif", str(vid)])) + for vid, want_vif in want["vifs"].items(): + have_vif = have["vifs"].get(vid, {"ipv4": [], "ipv6": []}) + vif_base = base + ["vif", str(vid)] + want_addrs = want_vif["ipv4"] + want_vif["ipv6"] + addrs = want_addrs or (have_vif["ipv4"] + have_vif["ipv6"]) + for addr in addrs: + if addr in have_vif["ipv4"] + have_vif["ipv6"]: + cmds.append(("delete", vif_base + ["address", addr])) return cmds want_map = _normalize(config) + commands = [] if state == "overridden": for name in set(have_map) - set(want_map): have = have_map[name] - base = _iface_base(name) - for addr in have["ipv4"] + have["ipv6"]: - cmds.append(("delete", base + ["address", addr])) - for vid in have["vifs"]: - cmds.append(("delete", base + ["vif", str(vid)])) + base = _iface_base(name, raw_have) + commands += _addr_cmds(base, [], have["ipv4"], "overridden") + commands += _addr_cmds(base, [], have["ipv6"], "overridden") + commands += _vif_addr_cmds(base, {}, have["vifs"], "overridden") for name, want in want_map.items(): have = have_map.get(name, {"ipv4": [], "ipv6": [], "vifs": {}}) - base = _iface_base(name) - cmds += _addr_cmds(base, want["ipv4"], have["ipv4"], state) - cmds += _addr_cmds(base, want["ipv6"], have["ipv6"], state) - cmds += _vif_cmds(base, want["vifs"], have["vifs"], state) + base = _iface_base(name, raw_have) + commands += _addr_cmds(base, want["ipv4"], have["ipv4"], state) + commands += _addr_cmds(base, want["ipv6"], have["ipv6"], state) + commands += _vif_addr_cmds(base, want["vifs"], have["vifs"], state) - return cmds + return commands +_ADDR_OPTIONS = dict(address=dict(type="str")) + +_VIF_OPTIONS = dict( + vlan_id=dict(type="int", required=True), + ipv4=dict(type="list", elements="dict", options=_ADDR_OPTIONS), + ipv6=dict(type="list", elements="dict", options=_ADDR_OPTIONS), +) + +_ENTRY_OPTIONS = dict( + name=dict(type="str", required=True), + ipv4=dict(type="list", elements="dict", options=_ADDR_OPTIONS), + ipv6=dict(type="list", elements="dict", options=_ADDR_OPTIONS), + vifs=dict(type="list", elements="dict", options=_VIF_OPTIONS), +) + ARGUMENT_SPEC = dict( - config=dict( - type="list", - elements="dict", - options=dict( - name=dict(type="str", required=True), - ipv4=dict( - type="list", - elements="dict", - options=dict(address=dict(type="str")), - ), - ipv6=dict( - type="list", - elements="dict", - options=dict(address=dict(type="str")), - ), - vifs=dict( - type="list", - elements="dict", - options=dict( - vlan_id=dict(type="int", required=True), - ipv4=dict( - type="list", - elements="dict", - options=dict(address=dict(type="str")), - ), - ipv6=dict( - type="list", - elements="dict", - options=dict(address=dict(type="str")), - ), - ), - ), - ), - ), - running_config=dict(type="str"), + 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": - # parsed is offline — just return empty for now - module.exit_json(parsed=[]) - - if state == "rendered": - # build commands without connecting - 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) 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) module.exit_json( changed=True, before=have, - after=get_running_config(vyos), + after=after, commands=commands, saved=saved, response=response, diff --git a/plugins/modules/vyos_ospfv2.py b/plugins/modules/vyos_ospfv2.py index 5ac333d..efdd2a5 100644 --- a/plugins/modules/vyos_ospfv2.py +++ b/plugins/modules/vyos_ospfv2.py @@ -1,6 +1,7 @@ #!/usr/bin/python # -*- coding: utf-8 -*- -# GNU General Public License v3.0+ +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + from __future__ import absolute_import, division, print_function @@ -13,7 +14,25 @@ short_description: Manage OSPFv2 configuration on VyOS devices using REST API description: - Manages OSPFv2 configuration on VyOS devices via the REST API. - Uses REST API (C(connection=httpapi)) instead of CLI. - - 'In VyOS 1.5+, passive interfaces use per-interface config rather than passive-interface.' + - >- + Scope matches the current vyos.vyos.vyos_ospfv2 (CLI collection) module: + areas (including virtual_link), auto_cost, default_information, + default_metric, distance, log_adjacency_changes, max_metric, mpls_te, + neighbor, parameters, passive_interface, passive_interface_exclude, + redistribute, timers. VyOS's OSPF schema is considerably larger than + even this -- access-list, aggregation, capability, graceful-restart, + ldp-sync, maximum-paths, per-interface + tuning (bandwidth/hello-multiplier/network-type/authentication/intervals beyond passive), + segment-routing, and summary-address are not modeled here, matching + real gaps in the CLI module's own scope, not oversights. + - >- + A standalone top-level C(route_map) field exists in the CLI module's + argspec but does not correspond to any real device path -- confirmed + against a live VyOS 1.5.0 device's C(set protocols ospf) completions, + which list no C(route-map) entry at that level (only nested under + C(default-information originate) and per-protocol under + C(redistribute), both of which are modeled here). Deliberately + omitted rather than built against a confirmed non-functional field. version_added: "1.0.0" author: - VyOS Community (@vyos) @@ -103,6 +122,45 @@ options: description: Shortcut mode. type: str choices: [default, disable, enable] + virtual_link: + description: Virtual link. + type: list + elements: dict + suboptions: + address: + description: Virtual link address (router ID of the remote ABR). + type: str + required: true + authentication: + description: Virtual link authentication. + type: dict + suboptions: + md5: + description: MD5 key id based authentication. + type: list + elements: dict + suboptions: + key_id: + description: MD5 key id (1-255). + type: int + md5_key: + description: MD5 key (16 characters or less). + type: str + plaintext_password: + description: Plain text password (8 characters or less). + type: str + dead_interval: + description: Interval after which a neighbor is declared dead. + type: int + hello_interval: + description: Interval between hello packets. + type: int + retransmit_interval: + description: Interval between retransmitting lost link state advertisements. + type: int + transmit_delay: + description: Link state transmit delay. + type: int auto_cost: description: Auto-cost reference bandwidth. type: dict @@ -157,6 +215,33 @@ options: description: Log adjacency changes. type: str choices: [detail] + max_metric: + description: OSPFv2 maximum/infinite-distance metric. + type: dict + suboptions: + router_lsa: + description: Advertise own Router-LSA with infinite distance (stub router). + type: dict + suboptions: + administrative: + description: Administratively apply, for an indefinite period. + type: bool + on_shutdown: + description: Time (seconds) to advertise self as stub-router before shutdown. + type: int + on_startup: + description: Time (seconds) to advertise self as stub-router on startup. + type: int + mpls_te: + description: MPLS Traffic Engineering parameters. + type: dict + suboptions: + enabled: + description: Enable MPLS-TE functionality. + type: bool + router_address: + description: Stable IP address of the advertising router. + type: str neighbor: description: OSPF neighbors. type: list @@ -190,9 +275,16 @@ options: description: Router ID. type: str passive_interface: - description: > - Passive interfaces (VyOS 1.5+: configured via - C(protocols ospf interface <name> passive)). + description: >- + Interfaces to suppress routing updates on, via per-interface + configuration (C(protocols ospf interface <name> passive)). + type: list + elements: str + passive_interface_exclude: + description: >- + Interfaces to explicitly exclude from passive mode (via + C(protocols ospf interface <name> passive disable)) -- e.g. + when passive is otherwise applied broadly. type: list elements: str redistribute: @@ -213,6 +305,34 @@ options: route_map: description: Route map. type: str + timers: + description: Routing timers. + type: dict + suboptions: + refresh: + description: Refresh parameters. + type: dict + suboptions: + timers: + description: Refresh timer (seconds). + type: int + throttle: + description: Throttling adaptive timers. + type: dict + suboptions: + spf: + description: SPF timers. + type: dict + suboptions: + delay: + description: Delay (ms) from first change received to SPF calculation. + type: int + initial_holdtime: + description: Initial hold time (ms) between consecutive SPF calculations. + type: int + max_holdtime: + description: Maximum hold time (ms). + type: int state: description: - Desired state. @@ -226,8 +346,8 @@ options: notes: - Requires C(ansible_connection=httpapi) with the VyOS httpapi plugin. - C(ansible_network_os) must be set to C(vyos.rest.vyos). - - VyOS 1.5+ uses per-interface passive configuration rather than - the global C(passive-interface) command used in VyOS 1.4. +seealso: + - module: vyos.vyos.vyos_ospfv2 """ EXAMPLES = r""" @@ -245,10 +365,6 @@ EXAMPLES = r""" normal: true network: - address: 192.0.2.0/24 - - area_id: "3" - area_type: - nssa: - set: true - area_id: "4" area_type: stub: @@ -261,6 +377,16 @@ EXAMPLES = r""" metric: 10 passive_interface: - eth1 + max_metric: + router_lsa: + administrative: true + mpls_te: + enabled: true + router_address: 192.0.11.11 + timers: + throttle: + spf: + delay: 200 state: merged - name: Delete all OSPFv2 configuration @@ -300,618 +426,706 @@ 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, + to_tag_dict, +) _BASE = ["protocols", "ospf"] -def _parse_areas(raw_areas): - if not raw_areas or not isinstance(raw_areas, dict): - return [] - areas = [] - for area_id, data in sorted(raw_areas.items()): - area = {"area_id": area_id} - data = data or {} +def _derive_key_field(options_spec): + """The field identifying each entry in a named-list section is + never inferable from a generic walk alone -- but it doesn't need + to be hand-declared either: every named-list section in this + argspec already marks exactly one suboption required=True. + """ + required = [k for k, spec in options_spec.items() if spec.get("required")] + if len(required) != 1: + raise ValueError( + "expected exactly one required suboption to serve as the key field, " + "found: {0}".format(required), + ) + return required[0] - # area-type - at = data.get("area-type", {}) - if at: - area_type = {} - if "normal" in at: - area_type["normal"] = True - if "nssa" in at: - nssa_data = at["nssa"] or {} - nssa = {"set": True} - if "default-cost" in nssa_data: - nssa["default_cost"] = int(nssa_data["default-cost"]) - if "no-summary" in nssa_data: - nssa["no_summary"] = True - if "translate" in nssa_data: - nssa["translate"] = nssa_data["translate"] - area_type["nssa"] = nssa - if "stub" in at: - stub_data = at["stub"] or {} - stub = {"set": True} - if "default-cost" in stub_data: - stub["default_cost"] = int(stub_data["default-cost"]) - if "no-summary" in stub_data: - stub["no_summary"] = True - area_type["stub"] = stub - if area_type: - area["area_type"] = area_type - - if "authentication" in data: - area["authentication"] = data["authentication"] - - if "shortcut" in data: - area["shortcut"] = data["shortcut"] - - # network - net = data.get("network") - if net: - if isinstance(net, str): - area["network"] = [{"address": net}] - elif isinstance(net, dict): - area["network"] = [{"address": a} for a in sorted(net.keys())] - elif isinstance(net, list): - area["network"] = [{"address": a} for a in sorted(net)] - - # range - rng = data.get("range", {}) - if rng and isinstance(rng, dict): - ranges = [] - for addr, rdata in sorted(rng.items()): - r = {"address": addr} - rdata = rdata or {} - if "cost" in rdata: - r["cost"] = int(rdata["cost"]) - if "not-advertise" in rdata: - r["not_advertise"] = True - if "substitute" in rdata: - r["substitute"] = rdata["substitute"] - ranges.append(r) - if ranges: - area["range"] = ranges - - areas.append(area) - return areas - - -def _parse_redistribute(raw): - if not raw or not isinstance(raw, dict): - return [] - result = [] - for rt, data in sorted(raw.items()): - entry = {"route_type": rt} - data = data or {} - if "metric" in data: - entry["metric"] = int(data["metric"]) - if "metric-type" in data: - entry["metric_type"] = int(data["metric-type"]) - if "route-map" in data: - entry["route_map"] = data["route-map"] - result.append(entry) + +def _keyed_list_to_device(items, key_field, entry_transform=None): + entry_transform = entry_transform or _kebab_fields + result = {} + for item in items or []: + if not item.get(key_field): + continue + rest = {k: v for k, v in item.items() if k != key_field} + result[str(item[key_field])] = entry_transform(rest) return result -def _parse_neighbor(raw): - if not raw or not isinstance(raw, dict): - return [] - result = [] - for nb_id, data in sorted(raw.items()): - entry = {"neighbor_id": nb_id} - data = data or {} - if "poll-interval" in data: - entry["poll_interval"] = int(data["poll-interval"]) - if "priority" in data: - entry["priority"] = int(data["priority"]) - result.append(entry) - return result +def _keyed_list_from_device(raw, key_field, entry_transform=None, key_cast=None): + entry_transform = entry_transform or from_device + key_cast = key_cast or (lambda k: k) + return [ + {key_field: key_cast(key), **entry_transform(data or {})} + for key, data in sorted(to_tag_dict(raw).items()) + ] -def _parse_parameters(raw): - if not raw or not isinstance(raw, dict): - return {} - result = {} - if "router-id" in raw: - result["router_id"] = raw["router-id"] - if "abr-type" in raw: - result["abr_type"] = raw["abr-type"] - if "opaque-lsa" in raw: - result["opaque_lsa"] = True - if "rfc1583-compatibility" in raw: - result["rfc1583_compatibility"] = True - return result +# --------------------------------------------------------------------------- +# area_type -- confirmed against vyos-1x: normal (presence), nssa (a node +# with default-cost/no-summary/translate -- presence of the node itself +# is the "set" flag, matching the argspec's own "set" boolean rather +# than a separate device leaf), stub (default-cost/no-summary, +# same presence pattern). Genuine structural exception: the argspec's +# nssa/stub "set" key doesn't exist as a device leaf at all -- the +# *node's presence* IS the set flag, so "set" must be stripped out +# before the walk and re-derived on the way back. +# --------------------------------------------------------------------------- -def _parse_default_information(raw): - if not raw or not isinstance(raw, dict): +def _area_type_to_device(area_type): + if not area_type: return {} - orig = raw.get("originate", {}) or {} - result = {} - if "always" in orig: - result["always"] = True - if "metric" in orig: - result["metric"] = int(orig["metric"]) - if "metric-type" in orig: - result["metric_type"] = int(orig["metric-type"]) - if "route-map" in orig: - result["route_map"] = orig["route-map"] - if result: - return {"originate": result} - return {} + device = {} + if area_type.get("normal"): + device["normal"] = {} + nssa = area_type.get("nssa") + if nssa: + nssa_device = _kebab_fields({k: v for k, v in nssa.items() if k != "set"}) + device["nssa"] = nssa_device + stub = area_type.get("stub") + if stub: + stub_device = _kebab_fields({k: v for k, v in stub.items() if k != "set"}) + device["stub"] = stub_device + return device + + +def _area_type_from_device(data): + if not data: + return None + entry = {} + if "normal" in data: + entry["normal"] = True + if "nssa" in data: + nssa = from_device(data["nssa"] or {}) + nssa["set"] = True + entry["nssa"] = nssa + if "stub" in data: + stub = from_device(data["stub"] or {}) + stub["set"] = True + entry["stub"] = stub + return entry or None + + +# --------------------------------------------------------------------------- +# virtual_link -- confirmed against vyos-1x: keyed by address, with an +# "authentication" node (md5 tag-node keyed by key-id, or a bare +# plaintext-password leaf), plus dead-interval/hello-interval/ +# retransmit-interval/transmit-delay as plain leaves. +# --------------------------------------------------------------------------- + + +def _vlink_auth_to_device(auth): + if not auth: + return {} + device = {} + md5_list = auth.get("md5") or [] + if md5_list: + md5_device = { + str(entry["key_id"]): {"md5-key": entry["md5_key"]} + for entry in md5_list + if entry.get("key_id") is not None and entry.get("md5_key") is not None + } + if md5_device: + device["md5"] = md5_device + if auth.get("plaintext_password"): + device["plaintext-password"] = auth["plaintext_password"] + return device + + +def _vlink_auth_from_device(data): + if not data: + return None + entry = {} + md5_raw = data.get("md5") + if md5_raw: + entry["md5"] = [ + {"key_id": int(key_id), "md5_key": (kdata or {}).get("md5-key")} + for key_id, kdata in sorted(to_tag_dict(md5_raw).items()) + ] + if data.get("plaintext-password"): + entry["plaintext_password"] = data["plaintext-password"] + return entry or None + + +def _vlink_entry_to_device(rest): + exclude = {"authentication"} + device = _kebab_fields({k: v for k, v in rest.items() if k not in exclude}) + if rest.get("authentication"): + auth_device = _vlink_auth_to_device(rest["authentication"]) + if auth_device: + device["authentication"] = auth_device + return device + + +def _vlink_entry_from_device(data): + exclude = {"authentication"} + entry = from_device({k: v for k, v in data.items() if k not in exclude}) + auth = _vlink_auth_from_device(data.get("authentication")) + if auth: + entry["authentication"] = auth + return entry + + +# --------------------------------------------------------------------------- +# area -- orchestrates area_type, network, range, virtual_link, and the +# plain leaves (authentication, shortcut). +# --------------------------------------------------------------------------- + +_NETWORK_KEY = "address" +_RANGE_KEY = "address" +_VLINK_KEY = "address" + + +def _area_entry_to_device(rest): + exclude = {"area_type", "network", "range", "virtual_link"} + device = autoclean({k: v for k, v in rest.items() if k not in exclude}) + + at = _area_type_to_device(rest.get("area_type")) + if at: + device["area-type"] = at + + networks = rest.get("network") or [] + if networks: + device["network"] = {n["address"]: {} for n in networks if n.get("address")} + + ranges = rest.get("range") or [] + if ranges: + device["range"] = _keyed_list_to_device(ranges, _RANGE_KEY) + + vlinks = rest.get("virtual_link") or [] + if vlinks: + device["virtual-link"] = _keyed_list_to_device(vlinks, _VLINK_KEY, _vlink_entry_to_device) + + return device + + +def _area_entry_from_device(data): + exclude = {"area-type", "network", "range", "virtual-link"} + entry = from_device({k: v for k, v in data.items() if k not in exclude}) + + at = _area_type_from_device(data.get("area-type")) + if at: + entry["area_type"] = at + + net_raw = data.get("network") + if net_raw: + entry["network"] = [{"address": addr} for addr in sorted(to_tag_dict(net_raw))] + + range_raw = data.get("range") + if range_raw: + entry["range"] = _keyed_list_from_device(range_raw, _RANGE_KEY) + + vlink_raw = data.get("virtual-link") + if vlink_raw: + entry["virtual_link"] = _keyed_list_from_device( + vlink_raw, + _VLINK_KEY, + _vlink_entry_from_device, + ) + + return entry + +# --------------------------------------------------------------------------- +# distance -- confirmed genuine structural exception: argspec's "global" +# is a reserved-adjacent-but-legal dict key (fine as a string key, no +# Python keyword issue since it's inside dict(**{...}), unlike "as" +# elsewhere in this collection), device path is distance.global (a +# leaf) and distance.ospf.{external,inter-area,intra-area}. +# --------------------------------------------------------------------------- -def _parse_distance(raw): - if not raw or not isinstance(raw, dict): + +def _distance_to_device(dist): + if not dist: return {} - result = {} - if "global" in raw: - result["global"] = int(raw["global"]) - ospf = raw.get("ospf", {}) or {} + device = {} + if dist.get("global") is not None: + device["global"] = dist["global"] + ospf = dist.get("ospf") if ospf: - od = {} - if "external" in ospf: - od["external"] = int(ospf["external"]) - if "inter-area" in ospf: - od["inter_area"] = int(ospf["inter-area"]) - if "intra-area" in ospf: - od["intra_area"] = int(ospf["intra-area"]) - if od: - result["ospf"] = od - return result + ospf_device = _kebab_fields(ospf) + if ospf_device: + device["ospf"] = ospf_device + return device + + +def _distance_from_device(data): + if not data: + return None + entry = {} + if "global" in data: + entry["global"] = int(data["global"]) + if data.get("ospf"): + entry["ospf"] = from_device(data["ospf"]) + return entry or None + + +# --------------------------------------------------------------------------- +# timers -- confirmed genuine structural exception: argspec groups +# "refresh" and "throttle" both under one "timers" parent, but the +# device has them as two SEPARATE top-level nodes ("refresh" and +# "timers.throttle") -- not a nested nesting-insertion like most +# exceptions in this collection, but a nesting *removal*/regrouping. +# Handled at the top level in build_commands/get_running_config rather +# than as a single self-contained entry-transform, since it spans two +# different top-level device keys. +# --------------------------------------------------------------------------- + + +def _timers_to_device_refresh(timers): + """Returns the device's top-level "refresh" node contents.""" + refresh = (timers or {}).get("refresh") or {} + if refresh.get("timers") is not None: + return {"timers": refresh["timers"]} + return {} -def get_running_config(vyos): - raw = vyos.get_config(_BASE) - if not raw or not isinstance(raw, dict): - return {} - result = {} +def _timers_to_device_throttle(timers): + """Returns the device's top-level "timers" node contents (just the + throttle.spf subtree, matching confirmed scope).""" + throttle = (timers or {}).get("throttle") or {} + spf = throttle.get("spf") or {} + spf_device = _kebab_fields(spf) + if spf_device: + return {"throttle": {"spf": spf_device}} + return {} - areas = _parse_areas(raw.get("area")) - if areas: - result["areas"] = areas - ac = raw.get("auto-cost", {}) - if ac and "reference-bandwidth" in ac: - result["auto_cost"] = {"reference_bandwidth": int(ac["reference-bandwidth"])} +def _timers_from_device(refresh_raw, timers_raw): + entry = {} + if refresh_raw and refresh_raw.get("timers") is not None: + entry["refresh"] = {"timers": int(refresh_raw["timers"])} + throttle_raw = (timers_raw or {}).get("throttle") or {} + spf_raw = throttle_raw.get("spf") + if spf_raw: + entry["throttle"] = {"spf": from_device(spf_raw)} + return entry or None + + +# --------------------------------------------------------------------------- +# passive_interface / passive_interface_exclude -- confirmed against +# vyos-1x: both map onto the SAME per-interface "interface <name> +# passive" node -- presence alone means passive-enabled, +# "passive.disable" (a generic-disable-node) means explicitly +# excluded. These share one device subtree, so both are handled +# together rather than as two independent list diffs. +# --------------------------------------------------------------------------- + + +def _passive_to_device(passive_list, exclude_list): + device = {} + for iface in passive_list or []: + device[iface] = {"passive": {}} + for iface in exclude_list or []: + device[iface] = {"passive": {"disable": {}}} + return device + + +def _passive_from_device(iface_raw): + passive = [] + excluded = [] + for name, data in sorted((iface_raw or {}).items()): + data = data or {} + passive_node = data.get("passive") + if passive_node is None: + continue + if isinstance(passive_node, dict) and "disable" in passive_node: + excluded.append(name) + else: + passive.append(name) + return passive, excluded - di = _parse_default_information(raw.get("default-information", {})) - if di: - result["default_information"] = di - if "default-metric" in raw: - result["default_metric"] = int(raw["default-metric"]) +# --------------------------------------------------------------------------- +# redistribute -- fully generic once keyed by route_type; metric/ +# metric-type/route-map are all plain leaves. +# --------------------------------------------------------------------------- - dist = _parse_distance(raw.get("distance", {})) - if dist: - result["distance"] = dist +_REDISTRIBUTE_KEY = "route_type" +_NEIGHBOR_KEY = "neighbor_id" - lac = raw.get("log-adjacency-changes", {}) - if lac: - if isinstance(lac, dict) and "detail" in lac: - result["log_adjacency_changes"] = "detail" - elif lac == "detail": - result["log_adjacency_changes"] = "detail" - neighbors = _parse_neighbor(raw.get("neighbor")) +def _want_to_device(config): + config = config or {} + device = {} + + areas = config.get("areas") or [] + if areas: + device["area"] = _keyed_list_to_device(areas, "area_id", _area_entry_to_device) + + ac = config.get("auto_cost") or {} + if ac.get("reference_bandwidth") is not None: + device["auto-cost"] = {"reference-bandwidth": ac["reference_bandwidth"]} + + di = (config.get("default_information") or {}).get("originate") or {} + di_device = _kebab_fields(di) + if di_device: + device["default-information"] = {"originate": di_device} + + if config.get("default_metric") is not None: + device["default-metric"] = config["default_metric"] + + dist_device = _distance_to_device(config.get("distance")) + if dist_device: + device["distance"] = dist_device + + if config.get("log_adjacency_changes"): + device["log-adjacency-changes"] = {config["log_adjacency_changes"]: {}} + + mm = (config.get("max_metric") or {}).get("router_lsa") or {} + mm_device = _kebab_fields(mm) + if mm_device: + device["max-metric"] = {"router-lsa": mm_device} + + mpls = config.get("mpls_te") or {} + mpls_device = {} + if mpls.get("enabled"): + mpls_device["enable"] = {} + if mpls.get("router_address"): + mpls_device["router-address"] = mpls["router_address"] + if mpls_device: + device["mpls-te"] = mpls_device + + neighbors = config.get("neighbor") or [] if neighbors: - result["neighbor"] = neighbors + device["neighbor"] = _keyed_list_to_device(neighbors, _NEIGHBOR_KEY) - params = _parse_parameters(raw.get("parameters")) - if params: - result["parameters"] = params + params_device = _kebab_fields(config.get("parameters") or {}) + if params_device: + device["parameters"] = params_device - # passive interfaces — VyOS 1.5 uses interface <name> passive - iface_raw = raw.get("interface", {}) or {} - passive = sorted( - [name for name, data in iface_raw.items() if isinstance(data, dict) and "passive" in data], + iface_device = _passive_to_device( + config.get("passive_interface"), + config.get("passive_interface_exclude"), ) - if passive: - result["passive_interface"] = passive + if iface_device: + device["interface"] = iface_device - redist = _parse_redistribute(raw.get("redistribute")) + redist = config.get("redistribute") or [] if redist: - result["redistribute"] = redist + device["redistribute"] = _keyed_list_to_device(redist, _REDISTRIBUTE_KEY) - return result + refresh_device = _timers_to_device_refresh(config.get("timers")) + if refresh_device: + device["refresh"] = refresh_device + throttle_device = _timers_to_device_throttle(config.get("timers")) + if throttle_device: + device.setdefault("timers", {}).update(throttle_device) + return device -def _area_type_cmds(abase, area_type, have_at): - cmds = [] - have_at = have_at or {} - if area_type.get("normal") and not have_at.get("normal"): - cmds.append(("set", abase + ["area-type", "normal"])) - nssa = area_type.get("nssa") or {} - if nssa: - have_nssa = have_at.get("nssa") or {} - if not have_nssa: - cmds.append(("set", abase + ["area-type", "nssa"])) - if nssa.get("default_cost") and nssa["default_cost"] != have_nssa.get("default_cost"): - cmds.append( - ( - "set", - abase - + [ - "area-type", - "nssa", - "default-cost", - str(nssa["default_cost"]), - ], - ), - ) - if nssa.get("no_summary") and not have_nssa.get("no_summary"): - cmds.append(("set", abase + ["area-type", "nssa", "no-summary"])) - if nssa.get("translate") and nssa["translate"] != have_nssa.get("translate"): - cmds.append(("set", abase + ["area-type", "nssa", "translate", nssa["translate"]])) - stub = area_type.get("stub") or {} - if stub: - have_stub = have_at.get("stub") or {} - if not have_stub: - if stub.get("default_cost"): - cmds.append( - ( - "set", - abase - + [ - "area-type", - "stub", - "default-cost", - str(stub["default_cost"]), - ], - ), - ) - else: - cmds.append(("set", abase + ["area-type", "stub"])) - elif stub.get("default_cost") and stub["default_cost"] != have_stub.get("default_cost"): - cmds.append( - ( - "set", - abase - + [ - "area-type", - "stub", - "default-cost", - str(stub["default_cost"]), - ], - ), - ) - return cmds - - -def _area_cmds(area, have_area): - cmds = [] - area_id = area["area_id"] - abase = _BASE + ["area", area_id] - have_area = have_area or {} - - if area.get("area_type"): - cmds += _area_type_cmds(abase, area["area_type"], have_area.get("area_type")) - - if area.get("authentication") and area["authentication"] != have_area.get("authentication"): - cmds.append(("set", abase + ["authentication", area["authentication"]])) - - if area.get("shortcut") and area["shortcut"] != have_area.get("shortcut"): - cmds.append(("set", abase + ["shortcut", area["shortcut"]])) - - want_nets = {n["address"] for n in (area.get("network") or [])} - have_nets = {n["address"] for n in (have_area.get("network") or [])} - for addr in want_nets - have_nets: - cmds.append(("set", abase + ["network", addr])) - - want_ranges = {r["address"]: r for r in (area.get("range") or [])} - have_ranges = {r["address"]: r for r in (have_area.get("range") or [])} - for addr, rng in want_ranges.items(): - have_rng = have_ranges.get(addr, {}) - if addr not in have_ranges: - cmds.append(("set", abase + ["range", addr])) - if rng.get("cost") and rng["cost"] != have_rng.get("cost"): - cmds.append(("set", abase + ["range", addr, "cost", str(rng["cost"])])) - if rng.get("not_advertise") and not have_rng.get("not_advertise"): - cmds.append(("set", abase + ["range", addr, "not-advertise"])) - if rng.get("substitute") and rng["substitute"] != have_rng.get("substitute"): - cmds.append(("set", abase + ["range", addr, "substitute", rng["substitute"]])) - - return cmds - - -def _parameters_cmds(params, have_params): - cmds = [] - have_params = have_params or {} - pbase = _BASE + ["parameters"] - if params.get("router_id") and params["router_id"] != have_params.get("router_id"): - cmds.append(("set", pbase + ["router-id", params["router_id"]])) - if params.get("abr_type") and params["abr_type"] != have_params.get("abr_type"): - cmds.append(("set", pbase + ["abr-type", params["abr_type"]])) - if params.get("opaque_lsa") and not have_params.get("opaque_lsa"): - cmds.append(("set", pbase + ["opaque-lsa"])) - if params.get("rfc1583_compatibility") and not have_params.get("rfc1583_compatibility"): - cmds.append(("set", pbase + ["rfc1583-compatibility"])) - return cmds - - -def _redistribute_cmds(redist_list, have_redist_list): - cmds = [] - want = {r["route_type"]: r for r in (redist_list or [])} - have = {r["route_type"]: r for r in (have_redist_list or [])} - for rt, entry in want.items(): - have_entry = have.get(rt, {}) - rbase = _BASE + ["redistribute", rt] - if rt not in have: - cmds.append(("set", rbase)) - if entry.get("metric") and entry["metric"] != have_entry.get("metric"): - cmds.append(("set", rbase + ["metric", str(entry["metric"])])) - if entry.get("metric_type") and entry["metric_type"] != have_entry.get("metric_type"): - cmds.append(("set", rbase + ["metric-type", str(entry["metric_type"])])) - if entry.get("route_map") and entry["route_map"] != have_entry.get("route_map"): - cmds.append(("set", rbase + ["route-map", entry["route_map"]])) - return cmds - - -def _neighbor_cmds(neighbors, have_neighbors): - cmds = [] - want = {n["neighbor_id"]: n for n in (neighbors or [])} - have = {n["neighbor_id"]: n for n in (have_neighbors or [])} - for nb_id, entry in want.items(): - have_entry = have.get(nb_id, {}) - nbase = _BASE + ["neighbor", nb_id] - if nb_id not in have: - cmds.append(("set", nbase)) - if entry.get("priority") and entry["priority"] != have_entry.get("priority"): - cmds.append(("set", nbase + ["priority", str(entry["priority"])])) - if entry.get("poll_interval") and entry["poll_interval"] != have_entry.get("poll_interval"): - cmds.append(("set", nbase + ["poll-interval", str(entry["poll_interval"])])) - return cmds - - -def _default_info_cmds(di, have_di): - cmds = [] - have_di = have_di or {} - orig = (di or {}).get("originate") or {} - have_orig = have_di.get("originate") or {} - if not orig: - return cmds - dbase = _BASE + ["default-information", "originate"] - if orig.get("always") and not have_orig.get("always"): - cmds.append(("set", dbase + ["always"])) - if orig.get("metric") and orig["metric"] != have_orig.get("metric"): - cmds.append(("set", dbase + ["metric", str(orig["metric"])])) - if orig.get("metric_type") and orig["metric_type"] != have_orig.get("metric_type"): - cmds.append(("set", dbase + ["metric-type", str(orig["metric_type"])])) - if orig.get("route_map") and orig["route_map"] != have_orig.get("route_map"): - cmds.append(("set", dbase + ["route-map", orig["route_map"]])) - return cmds - - -def build_commands(config, have, state): - cmds = [] - if state == "deleted": - if have: - cmds.append(("delete", _BASE)) - return cmds +def get_running_config(vyos): + return vyos.get_config(_BASE) or {} - if state == "replaced": - would_set = build_commands(config, {}, "merged") - have_set = build_commands(have, {}, "merged") - if would_set == have_set: - return [] - if have: - cmds.append(("delete", _BASE)) - have = {} - config = config or {} +def _device_to_argspec(raw): + if not raw: + return {} + entry = {} - # parameters - if config.get("parameters"): - cmds += _parameters_cmds(config["parameters"], have.get("parameters")) + area_raw = raw.get("area") + if area_raw: + entry["areas"] = _keyed_list_from_device(area_raw, "area_id", _area_entry_from_device) - # auto_cost - ac = config.get("auto_cost") or {} - have_ac = have.get("auto_cost") or {} - if ac.get("reference_bandwidth") and ac["reference_bandwidth"] != have_ac.get( - "reference_bandwidth", - ): - cmds.append( - ( - "set", - _BASE - + [ - "auto-cost", - "reference-bandwidth", - str(ac["reference_bandwidth"]), - ], - ), - ) + ac = raw.get("auto-cost") or {} + if ac.get("reference-bandwidth") is not None: + entry["auto_cost"] = {"reference_bandwidth": int(ac["reference-bandwidth"])} - # default_information - if config.get("default_information"): - cmds += _default_info_cmds( - config["default_information"], - have.get("default_information"), - ) + di_raw = (raw.get("default-information") or {}).get("originate") + if di_raw: + entry["default_information"] = {"originate": from_device(di_raw)} - # default_metric - if config.get("default_metric") and config["default_metric"] != have.get("default_metric"): - cmds.append(("set", _BASE + ["default-metric", str(config["default_metric"])])) - - # distance - dist = config.get("distance") or {} - have_dist = have.get("distance") or {} - if dist.get("global") and dist["global"] != have_dist.get("global"): - cmds.append(("set", _BASE + ["distance", "global", str(dist["global"])])) - ospf_dist = dist.get("ospf") or {} - have_ospf_dist = have_dist.get("ospf") or {} - for key, api_key in [ - ("external", "external"), - ("inter_area", "inter-area"), - ("intra_area", "intra-area"), - ]: - if ospf_dist.get(key) and ospf_dist[key] != have_ospf_dist.get(key): - cmds.append(("set", _BASE + ["distance", "ospf", api_key, str(ospf_dist[key])])) - - # log_adjacency_changes - if config.get("log_adjacency_changes") and config["log_adjacency_changes"] != have.get( - "log_adjacency_changes", - ): - cmds.append(("set", _BASE + ["log-adjacency-changes", config["log_adjacency_changes"]])) - - # neighbor - if config.get("neighbor"): - cmds += _neighbor_cmds(config["neighbor"], have.get("neighbor")) - - # redistribute - if config.get("redistribute"): - cmds += _redistribute_cmds(config["redistribute"], have.get("redistribute")) - - # passive_interface — VyOS 1.5 per-interface style - want_passive = set(config.get("passive_interface") or []) - have_passive = set(have.get("passive_interface") or []) - for iface in want_passive - have_passive: - cmds.append(("set", _BASE + ["interface", iface, "passive"])) - - # areas - have_areas = {a["area_id"]: a for a in (have.get("areas") or [])} - for area in config.get("areas") or []: - have_area = have_areas.get(area["area_id"], {}) - cmds += _area_cmds(area, have_area) - - return cmds + if "default-metric" in raw: + entry["default_metric"] = int(raw["default-metric"]) + dist = _distance_from_device(raw.get("distance")) + if dist: + entry["distance"] = dist -ARGUMENT_SPEC = dict( - config=dict( + lac = raw.get("log-adjacency-changes") + if lac: + lac_dict = to_tag_dict(lac) + if "detail" in lac_dict: + entry["log_adjacency_changes"] = "detail" + + mm_raw = (raw.get("max-metric") or {}).get("router-lsa") + if mm_raw: + entry["max_metric"] = {"router_lsa": from_device(mm_raw)} + + mpls_raw = raw.get("mpls-te") or {} + mpls_entry = {} + if "enable" in mpls_raw: + mpls_entry["enabled"] = True + if mpls_raw.get("router-address"): + mpls_entry["router_address"] = mpls_raw["router-address"] + if mpls_entry: + entry["mpls_te"] = mpls_entry + + neighbor_raw = raw.get("neighbor") + if neighbor_raw: + entry["neighbor"] = _keyed_list_from_device(neighbor_raw, _NEIGHBOR_KEY) + + params_raw = raw.get("parameters") + if params_raw: + entry["parameters"] = from_device(params_raw) + + passive, excluded = _passive_from_device(raw.get("interface")) + if passive: + entry["passive_interface"] = passive + if excluded: + entry["passive_interface_exclude"] = excluded + + redist_raw = raw.get("redistribute") + if redist_raw: + entry["redistribute"] = _keyed_list_from_device(redist_raw, _REDISTRIBUTE_KEY) + + timers = _timers_from_device(raw.get("refresh"), raw.get("timers")) + if timers: + entry["timers"] = timers + + return entry + + +def _kebab_fields(d): + """autoclean, then kebab-convert the resulting keys. + + Safe specifically because every call site below is a leaf-level + dict of schema field names (nssa/stub attributes, distance.ospf, + default_information.originate, max_metric.router_lsa, parameters, + timers.throttle.spf, virtual_link's plain fields, and range/ + neighbor/redistribute entry fields) with no further nested + tag-node-keyed structure underneath -- never an opaque value like + an area ID or interface name used as a dict key, which must stay + verbatim (confirmed real corruption risk: a blanket recursive + conversion turns "my_area" into "my-area"). + + Needed because dict_op requires have's keys to already be genuine + device kebab-case -- it only normalizes underscores to dashes for + its own lookup index, but uses have's key verbatim for the output + path. autoclean deliberately leaves keys exactly as given (dict_op + is meant to convert during its own want-vs-have comparison), which + only works when have comes straight from the device. Here, have is + instead reconstructed by round-tripping through this module's own + entry-transforms (needed for confirmed structural exceptions like + area-type's "set" flag or the shared interface/passive subtree), + so any field passed through unconverted stays snake_case and + dict_op has no way to recover the real device key. Confirmed as a + real bug: "default_cost" appeared in a generated delete command + instead of "default-cost". + """ + cleaned = autoclean(d) + return {k.replace("_", "-"): v for k, v in cleaned.items()} + + +def build_commands(config, raw_have, state): + raw_have = raw_have or {} + + if state == "deleted": + return [("delete", _BASE)] if raw_have else [] + + want = _want_to_device(config) + norm_have = _want_to_device(_device_to_argspec(raw_have)) + + commands = [] + if state == "replaced": + commands += dict_op(want, norm_have, _BASE, op="purge") + commands += dict_op(want, norm_have, _BASE, op="set") + return commands + + +_VLINK_AUTH_OPTIONS = dict( + md5=dict( + type="list", + elements="dict", + options=dict( + key_id=dict(type="int"), + md5_key=dict(type="str", no_log=True), + ), + ), + plaintext_password=dict(type="str", no_log=True), +) + +_VLINK_OPTIONS = dict( + address=dict(type="str", required=True), + authentication=dict(type="dict", options=_VLINK_AUTH_OPTIONS), + dead_interval=dict(type="int"), + hello_interval=dict(type="int"), + retransmit_interval=dict(type="int"), + transmit_delay=dict(type="int"), +) + +_AREA_OPTIONS = dict( + area_id=dict(type="str", required=True), + area_type=dict( type="dict", options=dict( - areas=dict( - type="list", - elements="dict", + normal=dict(type="bool"), + nssa=dict( + type="dict", options=dict( - area_id=dict(type="str", required=True), - area_type=dict( - type="dict", - options=dict( - normal=dict(type="bool"), - nssa=dict( - type="dict", - options=dict( - set=dict(type="bool"), - default_cost=dict(type="int"), - no_summary=dict(type="bool"), - translate=dict( - type="str", - choices=["always", "candidate", "never"], - ), - ), - ), - stub=dict( - type="dict", - options=dict( - set=dict(type="bool"), - default_cost=dict(type="int"), - no_summary=dict(type="bool"), - ), - ), - ), - ), - authentication=dict( - type="str", - choices=["plaintext-password", "md5"], - ), - network=dict( - type="list", - elements="dict", - options=dict( - address=dict(type="str", required=True), - ), - ), - range=dict( - type="list", - elements="dict", - options=dict( - address=dict(type="str", required=True), - cost=dict(type="int"), - not_advertise=dict(type="bool"), - substitute=dict(type="str"), - ), - ), - shortcut=dict(type="str", choices=["default", "disable", "enable"]), + set=dict(type="bool"), + default_cost=dict(type="int"), + no_summary=dict(type="bool"), + translate=dict(type="str", choices=["always", "candidate", "never"]), ), ), - auto_cost=dict( + stub=dict( type="dict", options=dict( - reference_bandwidth=dict(type="int"), + set=dict(type="bool"), + default_cost=dict(type="int"), + no_summary=dict(type="bool"), ), ), - default_information=dict( + ), + ), + authentication=dict(type="str", choices=["plaintext-password", "md5"]), + network=dict( + type="list", + elements="dict", + options=dict(address=dict(type="str", required=True)), + ), + range=dict( + type="list", + elements="dict", + options=dict( + address=dict(type="str", required=True), + cost=dict(type="int"), + not_advertise=dict(type="bool"), + substitute=dict(type="str"), + ), + ), + shortcut=dict(type="str", choices=["default", "disable", "enable"]), + virtual_link=dict(type="list", elements="dict", options=_VLINK_OPTIONS), +) + +_CONFIG_OPTIONS = dict( + areas=dict(type="list", elements="dict", options=_AREA_OPTIONS), + auto_cost=dict( + type="dict", + options=dict(reference_bandwidth=dict(type="int")), + ), + default_information=dict( + type="dict", + options=dict( + originate=dict( type="dict", options=dict( - originate=dict( - type="dict", - options=dict( - always=dict(type="bool"), - metric=dict(type="int"), - metric_type=dict(type="int"), - route_map=dict(type="str"), - ), - ), + always=dict(type="bool"), + metric=dict(type="int"), + metric_type=dict(type="int"), + route_map=dict(type="str"), ), ), - default_metric=dict(type="int"), - distance=dict( + ), + ), + default_metric=dict(type="int"), + distance=dict( + type="dict", + options=dict( + **{"global": dict(type="int")}, + ospf=dict( type="dict", options=dict( - **{"global": dict(type="int")}, - ospf=dict( - type="dict", - options=dict( - external=dict(type="int"), - inter_area=dict(type="int"), - intra_area=dict(type="int"), - ), - ), + external=dict(type="int"), + inter_area=dict(type="int"), + intra_area=dict(type="int"), ), ), - log_adjacency_changes=dict(type="str", choices=["detail"]), - neighbor=dict( - type="list", - elements="dict", + ), + ), + log_adjacency_changes=dict(type="str", choices=["detail"]), + max_metric=dict( + type="dict", + options=dict( + router_lsa=dict( + type="dict", options=dict( - neighbor_id=dict(type="str", required=True), - poll_interval=dict(type="int"), - priority=dict(type="int"), + administrative=dict(type="bool"), + on_shutdown=dict(type="int"), + on_startup=dict(type="int"), ), ), - parameters=dict( + ), + ), + mpls_te=dict( + type="dict", + options=dict( + enabled=dict(type="bool"), + router_address=dict(type="str"), + ), + ), + neighbor=dict( + type="list", + elements="dict", + options=dict( + neighbor_id=dict(type="str", required=True), + poll_interval=dict(type="int"), + priority=dict(type="int"), + ), + ), + parameters=dict( + type="dict", + options=dict( + abr_type=dict(type="str", choices=["cisco", "ibm", "shortcut", "standard"]), + opaque_lsa=dict(type="bool"), + rfc1583_compatibility=dict(type="bool"), + router_id=dict(type="str"), + ), + ), + passive_interface=dict(type="list", elements="str"), + passive_interface_exclude=dict(type="list", elements="str"), + redistribute=dict( + type="list", + elements="dict", + options=dict( + route_type=dict(type="str", choices=["bgp", "connected", "kernel", "rip", "static"]), + metric=dict(type="int"), + metric_type=dict(type="int"), + route_map=dict(type="str"), + ), + ), + timers=dict( + type="dict", + options=dict( + refresh=dict( type="dict", - options=dict( - abr_type=dict( - type="str", - choices=["cisco", "ibm", "shortcut", "standard"], - ), - opaque_lsa=dict(type="bool"), - rfc1583_compatibility=dict(type="bool"), - router_id=dict(type="str"), - ), + options=dict(timers=dict(type="int")), ), - passive_interface=dict(type="list", elements="str"), - redistribute=dict( - type="list", - elements="dict", + throttle=dict( + type="dict", options=dict( - route_type=dict( - type="str", - choices=["bgp", "connected", "kernel", "rip", "static"], + spf=dict( + type="dict", + options=dict( + delay=dict(type="int"), + initial_holdtime=dict(type="int"), + max_holdtime=dict(type="int"), + ), ), - metric=dict(type="int"), - metric_type=dict(type="int"), - route_map=dict(type="str"), ), ), ), ), +) + +ARGUMENT_SPEC = dict( + config=dict(type="dict", options=_CONFIG_OPTIONS), state=dict( default="merged", choices=["merged", "replaced", "deleted", "gathered"], @@ -926,23 +1140,28 @@ def main(): state = module.params["state"] config = module.params.get("config") or {} - have = get_running_config(vyos) + raw_have = get_running_config(vyos) + have = _device_to_argspec(raw_have) + cast_by_spec(have, _CONFIG_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) + cast_by_spec(after, _CONFIG_OPTIONS) module.exit_json( changed=True, before=have, - after=get_running_config(vyos), + after=after, commands=commands, saved=saved, response=response, diff --git a/plugins/modules/vyos_ospfv3.py b/plugins/modules/vyos_ospfv3.py index fcf404e..5923a61 100644 --- a/plugins/modules/vyos_ospfv3.py +++ b/plugins/modules/vyos_ospfv3.py @@ -1,6 +1,7 @@ #!/usr/bin/python # -*- coding: utf-8 -*- -# GNU General Public License v3.0+ +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + from __future__ import absolute_import, division, print_function @@ -13,6 +14,22 @@ short_description: Manage OSPFv3 configuration on VyOS devices using REST API description: - Manages OSPFv3 configuration on VyOS devices via the REST API. - Uses REST API (C(connection=httpapi)) instead of CLI. + - >- + Scope matches the current vyos.vyos.vyos_ospfv3 (CLI collection) module, + confirmed against VyOS's official documentation (1.4+/1.5 LTS/rolling). + - >- + C(areas.interface) (an area-to-interface assignment list) exists in the + CLI module's argspec but was deliberately NOT carried over here -- + confirmed via VyOS's official docs across multiple versions that + C(set protocols ospfv3 area <id> interface <name>) is the superseded, + 1.3-era syntax. The current mechanism, C(set protocols ospfv3 interface + <name> area <id>), is a per-interface setting and is modeled in + M(vyos.rest.vyos_ospf_interfaces)'s C(area) field instead (and is not + affected by this module's C(state=replaced)). + - >- + C(distance) and C(graceful-restart) are real, confirmed OSPFv3 features + not modeled here, matching a genuine gap in the CLI module's own scope + rather than an oversight. version_added: "1.0.0" author: - VyOS Community (@vyos) @@ -37,7 +54,7 @@ options: description: Name of import-list. type: str range: - description: Summarize routes matching prefix. + description: Summarize routes matching prefix (border routers only). type: list elements: dict suboptions: @@ -65,6 +82,7 @@ options: suboptions: route_type: description: Protocol to redistribute. + required: true type: str choices: [bgp, connected, kernel, ripng, static] route_map: @@ -83,6 +101,9 @@ options: notes: - Requires C(ansible_connection=httpapi) with the VyOS httpapi plugin. - C(ansible_network_os) must be set to C(vyos.rest.vyos). +seealso: + - module: vyos.vyos.vyos_ospfv3 + - module: vyos.rest.vyos_ospf_interfaces """ EXAMPLES = r""" @@ -139,184 +160,201 @@ 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, + to_tag_dict, +) _BASE = ["protocols", "ospfv3"] +def _kebab_fields(d): + """autoclean, then kebab-convert the resulting keys. + + Needed because dict_op requires have's keys to already be genuine + device kebab-case -- it only normalizes underscores to dashes for + its own lookup index, but uses have's key verbatim for the output + path. autoclean deliberately leaves keys exactly as given (dict_op + is meant to convert during its own want-vs-have comparison), which + only works when have comes straight from the device. Here, have is + reconstructed by round-tripping through this module's own entry- + transforms, so any field passed through unconverted would stay + snake_case and dict_op would have no way to recover the real + device key -- confirmed as a real bug during vyos_ospfv2's build. + Safe here since every call site is a leaf-level dict of schema + field names, never an opaque tag-node value like an area ID used + as a dict key. + """ + cleaned = autoclean(d) + return {k.replace("_", "-"): v for k, v in cleaned.items()} + + +def _derive_key_field(options_spec): + required = [k for k, spec in options_spec.items() if spec.get("required")] + if len(required) != 1: + raise ValueError( + "expected exactly one required suboption to serve as the key field, " + "found: {0}".format(required), + ) + return required[0] + + +def _keyed_list_to_device(items, key_field, entry_transform=None): + entry_transform = entry_transform or _kebab_fields + result = {} + for item in items or []: + if not item.get(key_field): + continue + rest = {k: v for k, v in item.items() if k != key_field} + result[str(item[key_field])] = entry_transform(rest) + return result + + +def _keyed_list_from_device(raw, key_field, entry_transform=None): + entry_transform = entry_transform or from_device + return [ + {key_field: key, **entry_transform(data or {})} + for key, data in sorted(to_tag_dict(raw).items()) + ] + + +# --------------------------------------------------------------------------- +# range -- confirmed against CLI: advertise/not_advertise are presence- +# only opposite flags (not both meaningful at once, though the argspec +# doesn't enforce mutual exclusivity -- matching the CLI's own scope). +# Fully generic once keyed by address. +# --------------------------------------------------------------------------- + +_RANGE_KEY = "address" + + +def _area_entry_to_device(rest): + exclude = {"range"} + device = _kebab_fields({k: v for k, v in rest.items() if k not in exclude}) + ranges = rest.get("range") or [] + if ranges: + device["range"] = _keyed_list_to_device(ranges, _RANGE_KEY) + return device + + +def _area_entry_from_device(data): + exclude = {"range"} + entry = from_device({k: v for k, v in data.items() if k not in exclude}) + range_raw = data.get("range") + if range_raw: + entry["range"] = _keyed_list_from_device(range_raw, _RANGE_KEY) + return entry + + +def _want_to_device(config): + config = config or {} + device = {} + + areas = config.get("areas") or [] + if areas: + device["area"] = _keyed_list_to_device(areas, _AREA_KEY, _area_entry_to_device) + + params_device = _kebab_fields(config.get("parameters") or {}) + if params_device: + device["parameters"] = params_device + + redist = config.get("redistribute") or [] + if redist: + device["redistribute"] = _keyed_list_to_device(redist, _REDISTRIBUTE_KEY) + + return device + + def get_running_config(vyos): - raw = vyos.get_config(_BASE) + """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 (an unguarded response + iterated character-by-character). Normalizing through to_tag_dict + unconditionally means callers always receive a genuine dict. + """ + return to_tag_dict(vyos.get_config(_BASE) or {}) + + +def _device_to_argspec(raw): if not raw or not isinstance(raw, dict): return {} - return _parse_ospfv3(raw) + entry = {} + area_raw = raw.get("area") + if area_raw: + areas = _keyed_list_from_device(area_raw, _AREA_KEY, _area_entry_from_device) + if areas: + entry["areas"] = areas -def _parse_ospfv3(raw): - result = {} + params_raw = raw.get("parameters") + if params_raw: + entry["parameters"] = from_device(params_raw) - # parameters - params = raw.get("parameters", {}) - if params: - result["parameters"] = {} - if "router-id" in params: - result["parameters"]["router_id"] = params["router-id"] - - # redistribute - redist_raw = raw.get("redistribute", {}) - if redist_raw and isinstance(redist_raw, dict): - redist = [] - for route_type, data in sorted(redist_raw.items()): - entry = {"route_type": route_type} - if isinstance(data, dict) and data.get("route-map"): - entry["route_map"] = data["route-map"] - redist.append(entry) - if redist: - result["redistribute"] = redist - - # areas - area_raw = raw.get("area", {}) - if area_raw and isinstance(area_raw, dict): - areas = [] - for area_id, area_data in sorted(area_raw.items()): - area = {"area_id": area_id} - area_data = area_data or {} - if area_data.get("export-list"): - area["export_list"] = area_data["export-list"] - if area_data.get("import-list"): - area["import_list"] = area_data["import-list"] - range_raw = area_data.get("range", {}) - if range_raw and isinstance(range_raw, dict): - ranges = [] - for prefix, rdata in sorted(range_raw.items()): - r = {"address": prefix} - rdata = rdata or {} - if "advertise" in rdata: - r["advertise"] = True - if "not-advertise" in rdata: - r["not_advertise"] = True - ranges.append(r) - if ranges: - area["range"] = ranges - areas.append(area) - if areas: - result["areas"] = areas + redist_raw = raw.get("redistribute") + if redist_raw: + entry["redistribute"] = _keyed_list_from_device(redist_raw, _REDISTRIBUTE_KEY) - return result + return entry -def build_commands(config, have, state): - cmds = [] +def build_commands(config, raw_have, state): + raw_have = raw_have or {} if state == "deleted": - if have: - cmds.append(("delete", _BASE)) - return cmds + return [("delete", _BASE)] if raw_have else [] + + want = _want_to_device(config) + norm_have = _want_to_device(_device_to_argspec(raw_have)) + commands = [] if state == "replaced": - # Build what we would set from scratch and compare to have - would_set = build_commands(config, {}, "merged") - have_set = build_commands(have, {}, "merged") - if would_set == have_set: - return [] - if have: - cmds.append(("delete", _BASE)) - have = {} - - # parameters - want_params = (config or {}).get("parameters") or {} - have_params = have.get("parameters") or {} - if want_params.get("router_id") and want_params["router_id"] != have_params.get("router_id"): - cmds.append(("set", _BASE + ["parameters", "router-id", want_params["router_id"]])) - - # redistribute - want_redist = {r["route_type"]: r for r in ((config or {}).get("redistribute") or [])} - have_redist = {r["route_type"]: r for r in (have.get("redistribute") or [])} - - for rt in set(have_redist) - set(want_redist): - if state == "merged": - pass # merged doesn't remove - for rt, entry in want_redist.items(): - if rt not in have_redist: - cmds.append(("set", _BASE + ["redistribute", rt])) - if entry.get("route_map"): - have_rm = have_redist.get(rt, {}).get("route_map") - if entry["route_map"] != have_rm: - cmds.append(("set", _BASE + ["redistribute", rt, "route-map", entry["route_map"]])) - - # areas - want_areas = {a["area_id"]: a for a in ((config or {}).get("areas") or [])} - have_areas = {a["area_id"]: a for a in (have.get("areas") or [])} - - for area_id, want_area in want_areas.items(): - have_area = have_areas.get(area_id, {}) - abase = _BASE + ["area", area_id] - - if want_area.get("export_list") and want_area["export_list"] != have_area.get( - "export_list", - ): - cmds.append(("set", abase + ["export-list", want_area["export_list"]])) - if want_area.get("import_list") and want_area["import_list"] != have_area.get( - "import_list", - ): - cmds.append(("set", abase + ["import-list", want_area["import_list"]])) - - want_ranges = {r["address"]: r for r in (want_area.get("range") or [])} - have_ranges = {r["address"]: r for r in (have_area.get("range") or [])} - - for addr in want_ranges: - if addr not in have_ranges: - cmds.append(("set", abase + ["range", addr])) - r = want_ranges[addr] - if r.get("not_advertise"): - cmds.append(("set", abase + ["range", addr, "not-advertise"])) - elif r.get("advertise"): - cmds.append(("set", abase + ["range", addr, "advertise"])) - - return cmds + commands += dict_op(want, norm_have, _BASE, op="purge") + commands += dict_op(want, norm_have, _BASE, op="set") + return commands -ARGUMENT_SPEC = dict( - config=dict( - type="dict", - options=dict( - areas=dict( - type="list", - elements="dict", - options=dict( - area_id=dict(type="str", required=True), - export_list=dict(type="str"), - import_list=dict(type="str"), - range=dict( - type="list", - elements="dict", - options=dict( - address=dict(type="str", required=True), - advertise=dict(type="bool"), - not_advertise=dict(type="bool"), - ), - ), - ), - ), - parameters=dict( - type="dict", - options=dict( - router_id=dict(type="str"), - ), - ), - redistribute=dict( - type="list", - elements="dict", - options=dict( - route_type=dict( - type="str", - choices=["bgp", "connected", "kernel", "ripng", "static"], - ), - route_map=dict(type="str"), - ), - ), - ), +_RANGE_OPTIONS = dict( + address=dict(type="str", required=True), + advertise=dict(type="bool"), + not_advertise=dict(type="bool"), +) + +_AREA_OPTIONS = dict( + area_id=dict(type="str", required=True), + export_list=dict(type="str"), + import_list=dict(type="str"), + range=dict(type="list", elements="dict", options=_RANGE_OPTIONS), +) + +_PARAMETERS_OPTIONS = dict( + router_id=dict(type="str"), +) + +_REDISTRIBUTE_OPTIONS = dict( + route_type=dict( + type="str", + required=True, + choices=["bgp", "connected", "kernel", "ripng", "static"], ), + route_map=dict(type="str"), +) + +_AREA_KEY = _derive_key_field(_AREA_OPTIONS) +_REDISTRIBUTE_KEY = _derive_key_field(_REDISTRIBUTE_OPTIONS) + +_CONFIG_OPTIONS = dict( + areas=dict(type="list", elements="dict", options=_AREA_OPTIONS), + parameters=dict(type="dict", options=_PARAMETERS_OPTIONS), + redistribute=dict(type="list", elements="dict", options=_REDISTRIBUTE_OPTIONS), +) + +ARGUMENT_SPEC = dict( + config=dict(type="dict", options=_CONFIG_OPTIONS), state=dict( default="merged", choices=["merged", "replaced", "deleted", "gathered"], @@ -331,12 +369,14 @@ def main(): state = module.params["state"] config = module.params.get("config") or {} - have = get_running_config(vyos) + raw_have = get_running_config(vyos) + have = _device_to_argspec(raw_have) + cast_by_spec(have, _CONFIG_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) @@ -344,10 +384,13 @@ def main(): if commands: response = vyos.apply_commands(commands) saved = vyos.save_config() + after_raw = get_running_config(vyos) + after = _device_to_argspec(after_raw) + cast_by_spec(after, _CONFIG_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/l3_interfaces_running.json b/tests/unit/fixtures/l3_interfaces_running.json index 676055b..3115861 100644 --- a/tests/unit/fixtures/l3_interfaces_running.json +++ b/tests/unit/fixtures/l3_interfaces_running.json @@ -1,21 +1,12 @@ { "ethernet": { "eth0": { - "address": "dhcp", - "hw-id": "52:54:00:65:5a:24", - "vif": { - "100": { - "address": "192.0.2.100/24" - } - } - } + "address": ["192.0.2.1/24", "2001:db8::1/64"], + "vif": { "100": { "address": ["192.0.2.100/24"] } } + }, + "eth1": {} }, "loopback": { - "lo": { - "address": ["10.0.0.1/32", "10.0.0.2/32"] - } - }, - "dummy": { - "dummy0": {} + "lo": { "address": ["10.0.0.1/32"] } } } diff --git a/tests/unit/fixtures/ospfv2_running.json b/tests/unit/fixtures/ospfv2_running.json index 9cc64a0..e5b5d72 100644 --- a/tests/unit/fixtures/ospfv2_running.json +++ b/tests/unit/fixtures/ospfv2_running.json @@ -2,41 +2,70 @@ "area": { "2": { "area-type": { "normal": {} }, - "network": "192.0.2.0/24" - }, - "3": { - "area-type": { "nssa": {} } + "authentication": "plaintext-password", + "shortcut": "enable" }, "4": { - "area-type": { "stub": { "default-cost": "20" } }, - "range": { - "192.0.3.0/24": { "cost": "10" }, - "192.0.4.0/24": {} + "area-type": { "stub": { "default-cost": "20", "no-summary": {} } }, + "network": { "192.0.2.0/24": {} }, + "range": { "192.0.3.0/24": { "cost": "10", "not-advertise": {} } }, + "virtual-link": { + "10.0.0.1": { + "authentication": { "plaintext-password": "secret" }, + "dead-interval": "40", + "hello-interval": "10" + } } + }, + "5": { + "area-type": { "nssa": { "default-cost": "5", "translate": "always" } } } }, "auto-cost": { "reference-bandwidth": "2" }, - "interface": { "eth1": { "passive": {} }, "eth2": { "passive": {} } }, + "default-information": { + "originate": { + "always": {}, + "metric": "10", + "metric-type": "2", + "route-map": "ingress" + } + }, + "default-metric": "5", + "distance": { + "global": "110", + "ospf": { "external": "150", "inter-area": "120", "intra-area": "100" } + }, "log-adjacency-changes": { "detail": {} }, - "neighbor": { - "192.0.11.12": { "priority": "2", "poll-interval": "10" } + "max-metric": { + "router-lsa": { + "administrative": {}, + "on-shutdown": "10", + "on-startup": "20" + } }, + "mpls-te": { "enable": {}, "router-address": "192.0.11.11" }, + "neighbor": { "192.0.11.12": { "priority": "2", "poll-interval": "30" } }, "parameters": { "router-id": "192.0.1.1", - "abr-type": "cisco", "opaque-lsa": {}, - "rfc1583-compatibility": {} + "rfc1583-compatibility": {}, + "abr-type": "cisco" + }, + "interface": { + "eth1": { "passive": {} }, + "eth2": { "passive": { "disable": {} } } }, "redistribute": { - "bgp": { "metric": "10", "metric-type": "2" }, - "connected": {} + "bgp": { "metric": "10", "metric-type": "2", "route-map": "redist-map" } }, - "default-information": { - "originate": { - "always": {}, - "metric": "10", - "metric-type": "2", - "route-map": "ingress" + "refresh": { "timers": "300" }, + "timers": { + "throttle": { + "spf": { + "delay": "200", + "initial-holdtime": "500", + "max-holdtime": "2000" + } } } } diff --git a/tests/unit/fixtures/ospfv3_running.json b/tests/unit/fixtures/ospfv3_running.json index 44a1bcd..6588a1d 100644 --- a/tests/unit/fixtures/ospfv3_running.json +++ b/tests/unit/fixtures/ospfv3_running.json @@ -1,6 +1,4 @@ { - "parameters": { "router-id": "192.0.2.10" }, - "redistribute": { "bgp": {}, "connected": { "route-map": "RM1" } }, "area": { "2": { "export-list": "export1", @@ -11,9 +9,12 @@ } }, "3": { - "range": { - "2001:db40::/32": {} - } + "range": { "2001:db40::/32": { "advertise": {} } } } + }, + "parameters": { "router-id": "192.0.2.10" }, + "redistribute": { + "bgp": { "route-map": "redist-map" }, + "static": {} } } diff --git a/tests/unit/modules/test_vyos_l3_interfaces.py b/tests/unit/modules/test_vyos_l3_interfaces.py index 66cd1fc..79fc8cf 100644 --- a/tests/unit/modules/test_vyos_l3_interfaces.py +++ b/tests/unit/modules/test_vyos_l3_interfaces.py @@ -4,300 +4,216 @@ 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_l3_interfaces import ( _addr_cmds, - _addr_list, - _normalize, - _parse_iface, + _device_to_argspec, + _guess_iface_type, + _iface_base, + _resolve_iface_type, _split_addresses, + _vif_addr_cmds, build_commands, 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"] 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 TestVyOSL3InterfacesAddrList(unittest.TestCase): - - def test_string_returns_list(self): - self.assertEqual(_addr_list("dhcp"), ["dhcp"]) - - def test_list_returned_sorted(self): - result = _addr_list(["10.0.0.2/32", "10.0.0.1/32"]) - self.assertEqual(result, ["10.0.0.1/32", "10.0.0.2/32"]) - - def test_none_returns_empty(self): - self.assertEqual(_addr_list(None), []) - - def test_empty_list_returns_empty(self): - self.assertEqual(_addr_list([]), []) - - -class TestVyOSL3InterfacesSplitAddresses(unittest.TestCase): - - def test_dhcp_goes_to_ipv4(self): - ipv4, ipv6 = _split_addresses(["dhcp"]) - self.assertIn("dhcp", ipv4) - self.assertEqual(ipv6, []) - - def test_dhcpv6_goes_to_ipv6(self): - ipv4, ipv6 = _split_addresses(["dhcpv6"]) - self.assertEqual(ipv4, []) - self.assertIn("dhcpv6", ipv6) - - def test_ipv4_cidr(self): - ipv4, ipv6 = _split_addresses(["192.0.2.1/24"]) - self.assertIn("192.0.2.1/24", ipv4) - self.assertEqual(ipv6, []) - - def test_ipv6_cidr(self): - ipv4, ipv6 = _split_addresses(["2001:db8::1/128"]) - self.assertEqual(ipv4, []) - self.assertIn("2001:db8::1/128", ipv6) - - def test_mixed(self): - ipv4, ipv6 = _split_addresses(["dhcp", "192.0.2.1/24", "2001:db8::1/128"]) - self.assertIn("dhcp", ipv4) - self.assertIn("192.0.2.1/24", ipv4) - self.assertIn("2001:db8::1/128", ipv6) - - -class TestVyOSL3InterfacesParseIface(unittest.TestCase): - - def test_parse_dhcp(self): - result = _parse_iface("eth0", {"address": "dhcp"}) - self.assertEqual(result["name"], "eth0") - self.assertEqual(result["ipv4"], [{"address": "dhcp"}]) - - def test_parse_multiple_addresses(self): - result = _parse_iface("lo", {"address": ["10.0.0.1/32", "10.0.0.2/32"]}) - addrs = [a["address"] for a in result["ipv4"]] - self.assertIn("10.0.0.1/32", addrs) - self.assertIn("10.0.0.2/32", addrs) - - def test_parse_vif(self): - result = _parse_iface( - "eth0", - { - "address": "dhcp", - "vif": {"100": {"address": "192.0.2.100/24"}}, - }, - ) - self.assertIn("vifs", result) - self.assertEqual(result["vifs"][0]["vlan_id"], 100) - self.assertEqual(result["vifs"][0]["ipv4"][0]["address"], "192.0.2.100/24") - - def test_parse_no_address(self): - result = _parse_iface("lo", {}) - self.assertNotIn("ipv4", result) - self.assertNotIn("ipv6", result) - - def test_hw_id_ignored(self): - result = _parse_iface("eth0", {"hw-id": "52:54:00:65:5a:24"}) - self.assertNotIn("hw_id", result) - self.assertNotIn("hw-id", result) - - -class TestVyOSL3InterfacesGetRunningFixture(VyOSModuleTestCase): - - def setUp(self): - super().setUp() self.fixture = load_fixture("l3_interfaces_running.json") + self.mock_vyos.get_config = MagicMock(return_value=self.fixture) - def test_fixture_parses_eth0_dhcp(self): - self.set_running_config(self.fixture) - result = get_running_config(self.mock_vyos) - eth0 = next((e for e in result if e["name"] == "eth0"), None) - self.assertIsNotNone(eth0) - addrs = [a["address"] for a in eth0.get("ipv4", [])] - self.assertIn("dhcp", addrs) + def gather(self): + return _device_to_argspec(self.fixture) - def test_fixture_parses_loopback_addresses(self): - self.set_running_config(self.fixture) - result = get_running_config(self.mock_vyos) - lo = next((e for e in result if e["name"] == "lo"), None) - self.assertIsNotNone(lo) - addrs = [a["address"] for a in lo.get("ipv4", [])] - self.assertIn("10.0.0.1/32", addrs) - self.assertIn("10.0.0.2/32", addrs) - - def test_fixture_parses_vif(self): - self.set_running_config(self.fixture) - result = get_running_config(self.mock_vyos) - eth0 = next(e for e in result if e["name"] == "eth0") - self.assertIn("vifs", eth0) - self.assertEqual(eth0["vifs"][0]["vlan_id"], 100) - - def test_empty_interface_not_included(self): - self.set_running_config(self.fixture) - result = get_running_config(self.mock_vyos) - names = [e["name"] for e in result] - # loopback with no addresses should not appear - self.assertNotIn("dummy0", names) - - -class TestVyOSL3InterfacesGetRunning(VyOSModuleTestCase): - def test_empty_returns_empty_list(self): - self.set_running_config({}) +class TestGetRunningConfig(VyOSModuleTestCase): + def test_returns_config_directly(self): result = get_running_config(self.mock_vyos) - self.assertEqual(result, []) - - def test_interface_without_address_excluded(self): - self.set_running_config( - { - "ethernet": {"eth0": {"hw-id": "52:54:00:65:5a:24"}}, - }, - ) - result = get_running_config(self.mock_vyos) - self.assertEqual(result, []) - - -class TestVyOSL3InterfacesNormalize(unittest.TestCase): - - def test_normalize_ipv4(self): - config = [ - { - "name": "lo", - "ipv4": [{"address": "10.0.0.1/32"}], - }, - ] - result = _normalize(config) - self.assertIn("lo", result) - self.assertIn("10.0.0.1/32", result["lo"]["ipv4"]) - - def test_normalize_vif(self): - config = [ - { - "name": "eth0", - "vifs": [{"vlan_id": 100, "ipv4": [{"address": "192.0.2.100/24"}]}], - }, - ] - result = _normalize(config) - self.assertIn(100, result["eth0"]["vifs"]) - self.assertIn("192.0.2.100/24", result["eth0"]["vifs"][100]["ipv4"]) - - def test_normalize_empty(self): - result = _normalize([]) - self.assertEqual(result, {}) - - -class TestVyOSL3InterfacesAddrCmds(unittest.TestCase): - - def test_adds_new_address(self): - base = ["interfaces", "loopback", "lo"] - cmds = _addr_cmds(base, ["10.0.0.1/32"], [], "merged") - self.assertIn(("set", base + ["address", "10.0.0.1/32"]), cmds) - - def test_idempotent(self): - base = ["interfaces", "loopback", "lo"] - cmds = _addr_cmds(base, ["10.0.0.1/32"], ["10.0.0.1/32"], "merged") - self.assertEqual(cmds, []) - - def test_merged_does_not_delete_extra(self): - base = ["interfaces", "loopback", "lo"] - cmds = _addr_cmds(base, ["10.0.0.1/32"], ["10.0.0.1/32", "10.0.0.2/32"], "merged") - self.assertEqual(cmds, []) - - def test_replaced_deletes_extra(self): - base = ["interfaces", "loopback", "lo"] - cmds = _addr_cmds(base, ["10.0.0.1/32"], ["10.0.0.1/32", "10.0.0.2/32"], "replaced") - self.assertIn(("delete", base + ["address", "10.0.0.2/32"]), cmds) - - -class TestVyOSL3InterfacesBuildCommands(unittest.TestCase): - - def _have_lo(self): - return [ - { - "name": "lo", - "ipv4": [ - {"address": "10.0.0.1/32"}, - {"address": "10.0.0.2/32"}, - ], - }, - ] - - def test_merged_adds_address(self): - config = [{"name": "lo", "ipv4": [{"address": "10.0.0.3/32"}]}] - cmds = build_commands(config, [], "merged") - self.assertIn( - ("set", ["interfaces", "loopback", "lo", "address", "10.0.0.3/32"]), - cmds, - ) - - def test_merged_idempotent(self): - cmds = build_commands(self._have_lo(), self._have_lo(), "merged") - self.assertEqual(cmds, []) - - def test_deleted_no_config_removes_all(self): - cmds = build_commands([], self._have_lo(), "deleted") - paths = [c[1] for c in cmds] - self.assertIn(["interfaces", "loopback", "lo", "address", "10.0.0.1/32"], paths) - self.assertIn(["interfaces", "loopback", "lo", "address", "10.0.0.2/32"], paths) - - def test_deleted_with_config_removes_interface_addresses(self): - config = [{"name": "lo"}] - cmds = build_commands(config, self._have_lo(), "deleted") - paths = [c[1] for c in cmds] - self.assertIn(["interfaces", "loopback", "lo", "address", "10.0.0.1/32"], paths) - - def test_deleted_idempotent_when_empty(self): - cmds = build_commands([], [], "deleted") + self.assertIn("ethernet", result) + + 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="eth0") + self.assertEqual(get_running_config(self.mock_vyos), {"eth0": {}}) + + +class TestTypeResolution(unittest.TestCase): + def test_resolves_from_have_when_present(self): + raw_have = {"loopback": {"lo": {}}} + self.assertEqual(_resolve_iface_type("lo", raw_have), "loopback") + + def test_falls_back_to_guess_for_new_interface(self): + self.assertEqual(_resolve_iface_type("eth5", {}), "ethernet") + + def test_guess_covers_all_11_types_aligned_with_vyos_interfaces(self): + """Confirmed original inconsistency: this module's own type + table was missing ppp/wlan compared to vyos_interfaces' table, + despite both modules operating on the same interface + namespace. Aligned here.""" + expected = { + "eth0": "ethernet", + "bond0": "bonding", + "lo": "loopback", + "tun0": "tunnel", + "wg0": "wireguard", + "vti0": "vti", + "dum0": "dummy", + "vtun0": "openvpn", + "ppp0": "pppoe", + "wlan0": "wireless", + "br0": "bridge", + } + for name, itype in expected.items(): + self.assertEqual(_guess_iface_type(name), itype) + + def test_iface_base_uses_resolved_type(self): + raw_have = {"bonding": {"bond0": {}}} + self.assertEqual(_iface_base("bond0", raw_have), ["interfaces", "bonding", "bond0"]) + + +class TestSplitAddresses(unittest.TestCase): + def test_ipv4_ipv6_dhcp_dhcpv6(self): + ipv4, ipv6 = _split_addresses(["192.0.2.1/24", "2001:db8::1/64", "dhcp", "dhcpv6"]) + self.assertEqual(ipv4, ["192.0.2.1/24", "dhcp"]) + self.assertEqual(ipv6, ["2001:db8::1/64", "dhcpv6"]) + + def test_auto_config(self): + ipv4, ipv6 = _split_addresses(["auto-config"]) + self.assertEqual(ipv6, ["auto-config"]) + + +class TestAddrCmds(unittest.TestCase): + """Confirmed scoped correctly: address commands only ever touch + the "address" leaf under the given base, regardless of whether + that base is an interface or a vif.""" + + def test_set_new_address(self): + cmds = _addr_cmds(["interfaces", "ethernet", "eth0"], ["192.0.2.1/24"], [], "merged") + expected = ("set", ["interfaces", "ethernet", "eth0", "address", "192.0.2.1/24"]) + self.assertEqual(cmds, [expected]) + + def test_merged_never_deletes(self): + cmds = _addr_cmds(["interfaces", "ethernet", "eth0"], [], ["192.0.2.1/24"], "merged") self.assertEqual(cmds, []) - def test_replaced_removes_extra_address(self): - config = [{"name": "lo", "ipv4": [{"address": "10.0.0.1/32"}]}] - cmds = build_commands(config, self._have_lo(), "replaced") - self.assertIn( - ("delete", ["interfaces", "loopback", "lo", "address", "10.0.0.2/32"]), - cmds, + def test_replaced_deletes_omitted(self): + cmds = _addr_cmds(["interfaces", "ethernet", "eth0"], [], ["192.0.2.1/24"], "replaced") + expected = ("delete", ["interfaces", "ethernet", "eth0", "address", "192.0.2.1/24"]) + self.assertEqual(cmds, [expected]) + + +class TestVifAddrCmdsNeverWholeSubtree(unittest.TestCase): + """Primary confirmed severe bug: the original generated a whole- + VIF delete (`delete ... vif <id>`) for an omitted VIF under + replaced/overridden/deleted. Since VIFs are shared with + vyos_interfaces (which owns description/mtu/disable on them), + this would destroy that module's own fields, not just this + module's addresses. Confirmed 4 separate occurrences in the + original build_commands. Fixed: every VIF operation here is + scoped to exactly the address leaf, never the VIF subtree.""" + + def test_omitted_vif_only_deletes_its_addresses(self): + have_vifs = {100: {"ipv4": ["192.0.2.100/24"], "ipv6": []}} + cmds = _vif_addr_cmds(["interfaces", "ethernet", "eth0"], {}, have_vifs, "replaced") + whole_vif_delete = ("delete", ["interfaces", "ethernet", "eth0", "vif", "100"]) + self.assertNotIn(whole_vif_delete, cmds) + vif_addr_delete = ( + "delete", + ["interfaces", "ethernet", "eth0", "vif", "100", "address", "192.0.2.100/24"], ) - - def test_overridden_removes_unlisted_interface(self): - config = [{"name": "lo", "ipv4": [{"address": "10.0.0.1/32"}]}] - have = self._have_lo() + [ - { - "name": "eth0", - "ipv4": [{"address": "192.0.2.1/24"}], - }, - ] - cmds = build_commands(config, have, "overridden") - self.assertIn( - ("delete", ["interfaces", "ethernet", "eth0", "address", "192.0.2.1/24"]), - cmds, - ) - - def test_vif_added(self): + self.assertIn(vif_addr_delete, cmds) + + def test_deleted_state_also_never_whole_subtree(self): + have_vifs = {100: {"ipv4": ["192.0.2.100/24"], "ipv6": []}} + cmds = _vif_addr_cmds(["interfaces", "ethernet", "eth0"], {}, have_vifs, "deleted") + whole_vif_delete = ("delete", ["interfaces", "ethernet", "eth0", "vif", "100"]) + self.assertNotIn(whole_vif_delete, cmds) + + def test_overridden_also_never_whole_subtree(self): + have_vifs = {100: {"ipv4": ["192.0.2.100/24"], "ipv6": []}} + cmds = _vif_addr_cmds(["interfaces", "ethernet", "eth0"], {}, have_vifs, "overridden") + whole_vif_delete = ("delete", ["interfaces", "ethernet", "eth0", "vif", "100"]) + self.assertNotIn(whole_vif_delete, cmds) + + +class TestDeviceToArgspecFixture(VyOSModuleTestCase): + def test_all_interfaces_with_addresses_present(self): + have = self.gather() + names = {e["name"] for e in have} + # eth1 has no addresses at all -- must not appear + self.assertEqual(names, {"eth0", "lo"}) + + def test_eth0_addresses_and_vif_parsed(self): + have = self.gather() + eth0 = next(e for e in have if e["name"] == "eth0") + self.assertEqual(eth0["ipv4"][0]["address"], "192.0.2.1/24") + self.assertEqual(eth0["ipv6"][0]["address"], "2001:db8::1/64") + self.assertEqual(eth0["vifs"][0]["vlan_id"], 100) + self.assertEqual(eth0["vifs"][0]["ipv4"][0]["address"], "192.0.2.100/24") + + def test_collapsed_single_address_string(self): + entry = _device_to_argspec({"ethernet": {"eth0": {"address": "192.0.2.1/24"}}}) + self.assertEqual(entry[0]["ipv4"][0]["address"], "192.0.2.1/24") + + +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_deleted_all_never_whole_interface_subtree(self): + raw_have = {"ethernet": {"eth0": {"address": ["192.0.2.1/24"]}}} + cmds = build_commands([], raw_have, "deleted") + self.assertNotIn(("delete", _BASE + ["ethernet", "eth0"]), cmds) + self.assertIn(("delete", _BASE + ["ethernet", "eth0", "address", "192.0.2.1/24"]), cmds) + + def test_deleted_named_never_whole_interface_subtree(self): + raw_have = {"ethernet": {"eth0": {"address": ["192.0.2.1/24"]}}} + cmds = build_commands([{"name": "eth0"}], raw_have, "deleted") + self.assertNotIn(("delete", _BASE + ["ethernet", "eth0"]), cmds) + + def test_overridden_omitted_interface_never_whole_subtree(self): + raw_have = {"ethernet": {"eth0": {"address": ["192.0.2.1/24"]}}} + cmds = build_commands([], raw_have, "overridden") + self.assertNotIn(("delete", _BASE + ["ethernet", "eth0"]), cmds) + + def test_merged_adds_without_removing(self): + raw_have = {"ethernet": {"eth0": {"address": ["192.0.2.1/24"]}}} + config = [{"name": "eth0", "ipv4": [{"address": "192.0.2.2/24"}]}] + cmds = build_commands(config, raw_have, "merged") + self.assertIn(("set", _BASE + ["ethernet", "eth0", "address", "192.0.2.2/24"]), cmds) + self.assertFalse(any(c[0] == "delete" for c in cmds)) + + def test_replaced_removes_unlisted_address(self): + raw_have = {"ethernet": {"eth0": {"address": ["192.0.2.1/24", "192.0.2.2/24"]}}} + config = [{"name": "eth0", "ipv4": [{"address": "192.0.2.1/24"}]}] + cmds = build_commands(config, raw_have, "replaced") + self.assertIn(("delete", _BASE + ["ethernet", "eth0", "address", "192.0.2.2/24"]), cmds) + + def test_merged_new_vif_address(self): config = [ - { - "name": "eth0", - "vifs": [{"vlan_id": 100, "ipv4": [{"address": "192.0.2.100/24"}]}], - }, + {"name": "eth0", "vifs": [{"vlan_id": 200, "ipv4": [{"address": "192.0.2.200/24"}]}]}, ] - cmds = build_commands(config, [], "merged") + cmds = build_commands(config, {}, "merged") self.assertIn( - ("set", ["interfaces", "ethernet", "eth0", "vif", "100", "address", "192.0.2.100/24"]), + ("set", _BASE + ["ethernet", "eth0", "vif", "200", "address", "192.0.2.200/24"]), cmds, ) diff --git a/tests/unit/modules/test_vyos_ospfv2.py b/tests/unit/modules/test_vyos_ospfv2.py index c05dd77..01738a3 100644 --- a/tests/unit/modules/test_vyos_ospfv2.py +++ b/tests/unit/modules/test_vyos_ospfv2.py @@ -4,30 +4,36 @@ 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_ospfv2 import ( - _parse_areas, - _parse_default_information, - _parse_neighbor, - _parse_parameters, - _parse_redistribute, + _CONFIG_OPTIONS, + ARGUMENT_SPEC, + _area_type_from_device, + _area_type_to_device, + _derive_key_field, + _device_to_argspec, + _distance_from_device, + _distance_to_device, + _kebab_fields, + _passive_from_device, + _passive_to_device, + _timers_from_device, + _timers_to_device_refresh, + _timers_to_device_throttle, + _vlink_auth_from_device, + _vlink_auth_to_device, build_commands, + cast_by_spec, get_running_config, ) - -_BASE = ["protocols", "ospf"] +from .base import load_fixture -def load_fixture(filename): - fixtures_dir = os.path.join(os.path.dirname(__file__), "..", "fixtures") - with open(os.path.join(fixtures_dir, filename)) as f: - return json.load(f) +_BASE = ["protocols", "ospf"] class VyOSModuleTestCase(unittest.TestCase): @@ -36,144 +42,278 @@ class VyOSModuleTestCase(unittest.TestCase): self.fixture = load_fixture("ospfv2_running.json") self.mock_vyos.get_config = MagicMock(return_value=self.fixture) + def gather(self): + have = _device_to_argspec(self.fixture) + cast_by_spec(have, _CONFIG_OPTIONS) + return have -class TestVyOSOspfv2Parse(VyOSModuleTestCase): - - def test_parse_parameters(self): - result = _parse_parameters(self.fixture["parameters"]) - self.assertEqual(result["router_id"], "192.0.1.1") - self.assertEqual(result["abr_type"], "cisco") - self.assertTrue(result["opaque_lsa"]) - self.assertTrue(result["rfc1583_compatibility"]) - - def test_parse_redistribute(self): - result = _parse_redistribute(self.fixture["redistribute"]) - route_types = [r["route_type"] for r in result] - self.assertIn("bgp", route_types) - self.assertIn("connected", route_types) - bgp = next(r for r in result if r["route_type"] == "bgp") - self.assertEqual(bgp["metric"], 10) - self.assertEqual(bgp["metric_type"], 2) - - def test_parse_neighbor(self): - result = _parse_neighbor(self.fixture["neighbor"]) - self.assertEqual(len(result), 1) - nb = result[0] - self.assertEqual(nb["neighbor_id"], "192.0.11.12") - self.assertEqual(nb["priority"], 2) - self.assertEqual(nb["poll_interval"], 10) - - def test_parse_default_information(self): - result = _parse_default_information(self.fixture["default-information"]) - orig = result["originate"] - self.assertTrue(orig["always"]) - self.assertEqual(orig["metric"], 10) - self.assertEqual(orig["metric_type"], 2) - self.assertEqual(orig["route_map"], "ingress") - - def test_parse_areas(self): - result = _parse_areas(self.fixture["area"]) - self.assertEqual(len(result), 3) - area2 = next(a for a in result if a["area_id"] == "2") - self.assertTrue(area2["area_type"]["normal"]) - self.assertEqual(area2["network"][0]["address"], "192.0.2.0/24") - - area3 = next(a for a in result if a["area_id"] == "3") - self.assertTrue(area3["area_type"]["nssa"]["set"]) - - area4 = next(a for a in result if a["area_id"] == "4") - self.assertEqual(area4["area_type"]["stub"]["default_cost"], 20) - self.assertEqual(len(area4["range"]), 2) - r = next(r for r in area4["range"] if r["address"] == "192.0.3.0/24") - self.assertEqual(r["cost"], 10) - - def test_get_running_config(self): - result = get_running_config(self.mock_vyos) - self.assertEqual(result["parameters"]["router_id"], "192.0.1.1") - self.assertIn("eth1", result["passive_interface"]) - self.assertIn("eth2", result["passive_interface"]) - self.assertEqual(result["auto_cost"]["reference_bandwidth"], 2) - self.assertEqual(result["log_adjacency_changes"], "detail") - - def test_get_running_config_empty(self): - self.mock_vyos.get_config = MagicMock(return_value={}) + +class TestGetRunningConfig(VyOSModuleTestCase): + def test_returns_config_directly(self): result = get_running_config(self.mock_vyos) + self.assertIn("area", result) + + def test_empty_config(self): + self.mock_vyos.get_config = MagicMock(return_value=None) + self.assertEqual(get_running_config(self.mock_vyos), {}) + + +class TestDeriveKeyField(unittest.TestCase): + def test_derives_area_id(self): + area_opts = ARGUMENT_SPEC["config"]["options"]["areas"]["options"] + self.assertEqual(_derive_key_field(area_opts), "area_id") + + def test_derives_neighbor_id(self): + nb_opts = ARGUMENT_SPEC["config"]["options"]["neighbor"]["options"] + self.assertEqual(_derive_key_field(nb_opts), "neighbor_id") + + def test_raises_if_none_required(self): + with self.assertRaises(ValueError): + _derive_key_field({"a": {"type": "str"}}) + + +class TestKebabFields(unittest.TestCase): + """Regression tests for the primary confirmed bug found during + development: dict_op requires have's keys to already be genuine + device kebab-case (it only normalizes underscores for its own + lookup index, not for the output path). autoclean deliberately + leaves keys as given, so any multi-word field reconstructed via + this module's own entry-transforms (rather than coming straight + from the device) needs explicit conversion, or a generated delete + command uses the wrong (snake_case) path segment.""" + + def test_converts_multiword_keys(self): + result = _kebab_fields({"default_cost": 20, "no_summary": True}) + self.assertEqual(result, {"default-cost": 20, "no-summary": {}}) + + def test_drops_none_and_false(self): + result = _kebab_fields({"default_cost": None, "no_summary": False}) self.assertEqual(result, {}) + def test_single_word_keys_unaffected(self): + result = _kebab_fields({"cost": 10}) + self.assertEqual(result, {"cost": 10}) -class TestVyOSOspfv2BuildCommands(unittest.TestCase): - def test_deleted_with_have(self): - have = {"parameters": {"router_id": "192.0.1.1"}} - cmds = build_commands({}, have, "deleted") - self.assertEqual(cmds, [("delete", _BASE)]) +class TestAreaType(unittest.TestCase): + def test_nssa_set_flag_is_node_presence_not_a_device_leaf(self): + """Confirmed genuine structural exception: the argspec's + nssa.set/stub.set boolean doesn't exist as a device leaf -- + the node's own presence IS the set flag.""" + result = _area_type_to_device({"nssa": {"set": True, "default_cost": 5}}) + self.assertEqual(result, {"nssa": {"default-cost": 5}}) + self.assertNotIn("set", result["nssa"]) - def test_deleted_without_have(self): - cmds = build_commands({}, {}, "deleted") - self.assertEqual(cmds, []) + def test_stub_no_summary(self): + """Regression test for the confirmed original bug: stub's + no_summary was declared in ARGUMENT_SPEC but never checked + anywhere in the original hand-rolled command-building logic.""" + result = _area_type_to_device({"stub": {"no_summary": True}}) + self.assertEqual(result, {"stub": {"no-summary": {}}}) - def test_merged_parameters(self): - config = {"parameters": {"router_id": "192.0.1.1"}} - cmds = build_commands(config, {}, "merged") - self.assertIn(("set", _BASE + ["parameters", "router-id", "192.0.1.1"]), cmds) + def test_from_device_restores_set_flag(self): + entry = _area_type_from_device({"nssa": {"default-cost": "5"}}) + self.assertTrue(entry["nssa"]["set"]) - def test_merged_redistribute(self): - config = {"redistribute": [{"route_type": "bgp", "metric": 10}]} - cmds = build_commands(config, {}, "merged") - self.assertIn(("set", _BASE + ["redistribute", "bgp"]), cmds) - self.assertIn(("set", _BASE + ["redistribute", "bgp", "metric", "10"]), cmds) + def test_normal_presence(self): + result = _area_type_to_device({"normal": True}) + self.assertEqual(result, {"normal": {}}) - def test_merged_passive_interface(self): - config = {"passive_interface": ["eth1"]} - cmds = build_commands(config, {}, "merged") - self.assertIn(("set", _BASE + ["interface", "eth1", "passive"]), cmds) + def test_empty(self): + self.assertEqual(_area_type_to_device({}), {}) + self.assertIsNone(_area_type_from_device({})) - def test_merged_area_normal(self): - config = {"areas": [{"area_id": "2", "area_type": {"normal": True}}]} - cmds = build_commands(config, {}, "merged") - self.assertIn(("set", _BASE + ["area", "2", "area-type", "normal"]), cmds) - def test_merged_area_stub_with_cost(self): - config = {"areas": [{"area_id": "4", "area_type": {"stub": {"default_cost": 20}}}]} - cmds = build_commands(config, {}, "merged") - self.assertIn( - ("set", _BASE + ["area", "4", "area-type", "stub", "default-cost", "20"]), - cmds, - ) +class TestDistance(unittest.TestCase): + def test_global_and_ospf(self): + result = _distance_to_device({"global": 110, "ospf": {"inter_area": 120}}) + self.assertEqual(result, {"global": 110, "ospf": {"inter-area": 120}}) - def test_merged_idempotent(self): - have = {"parameters": {"router_id": "192.0.1.1"}} - config = {"parameters": {"router_id": "192.0.1.1"}} - cmds = build_commands(config, have, "merged") - self.assertEqual(cmds, []) - - def test_replaced_idempotent(self): - have = {"parameters": {"router_id": "192.0.1.1"}} - config = {"parameters": {"router_id": "192.0.1.1"}} - cmds = build_commands(config, have, "replaced") - self.assertEqual(cmds, []) - - def test_replaced_rebuilds_on_change(self): - have = {"parameters": {"router_id": "192.0.1.1"}} - config = {"parameters": {"router_id": "192.0.1.2"}} - cmds = build_commands(config, have, "replaced") - self.assertEqual(cmds[0], ("delete", _BASE)) - self.assertIn( - ("set", _BASE + ["parameters", "router-id", "192.0.1.2"]), - cmds, - ) + def test_from_device(self): + entry = _distance_from_device({"global": "110", "ospf": {"external": "150"}}) + self.assertEqual(entry["global"], 110) + self.assertEqual(entry["ospf"]["external"], "150") # cast_by_spec's job downstream + + +class TestTimers(unittest.TestCase): + """Confirmed genuine structural exception: the argspec groups + "refresh" and "throttle" both under one "timers" parent, but the + device has them as two separate top-level nodes.""" + + def test_refresh_maps_to_separate_top_level_node(self): + result = _timers_to_device_refresh({"refresh": {"timers": 300}}) + self.assertEqual(result, {"timers": 300}) + + def test_throttle_maps_to_device_timers_node(self): + result = _timers_to_device_throttle({"throttle": {"spf": {"delay": 200}}}) + self.assertEqual(result, {"throttle": {"spf": {"delay": 200}}}) + + def test_from_device_recombines_both(self): + entry = _timers_from_device({"timers": "300"}, {"throttle": {"spf": {"delay": "200"}}}) + self.assertEqual(entry["refresh"]["timers"], 300) + self.assertEqual(entry["throttle"]["spf"]["delay"], "200") - def test_merged_neighbor(self): - config = {"neighbor": [{"neighbor_id": "192.0.11.12", "priority": 2}]} + +class TestPassiveInterface(unittest.TestCase): + """Confirmed genuine structural exception: passive_interface and + passive_interface_exclude both map onto the same per-interface + "interface <name> passive" device subtree -- presence alone means + enabled, "passive.disable" means explicitly excluded.""" + + def test_to_device_both(self): + result = _passive_to_device(["eth1"], ["eth2"]) + self.assertEqual(result, {"eth1": {"passive": {}}, "eth2": {"passive": {"disable": {}}}}) + + def test_from_device_both(self): + passive, excluded = _passive_from_device( + {"eth1": {"passive": {}}, "eth2": {"passive": {"disable": {}}}}, + ) + self.assertEqual(passive, ["eth1"]) + self.assertEqual(excluded, ["eth2"]) + + def test_from_device_ignores_non_passive_interfaces(self): + passive, excluded = _passive_from_device({"eth3": {"some-other-key": {}}}) + self.assertEqual(passive, []) + self.assertEqual(excluded, []) + + +class TestVirtualLinkAuth(unittest.TestCase): + def test_md5_to_device(self): + result = _vlink_auth_to_device({"md5": [{"key_id": 10, "md5_key": "secret"}]}) + self.assertEqual(result, {"md5": {"10": {"md5-key": "secret"}}}) + + def test_plaintext_to_device(self): + result = _vlink_auth_to_device({"plaintext_password": "pw"}) + self.assertEqual(result, {"plaintext-password": "pw"}) + + def test_md5_from_device(self): + entry = _vlink_auth_from_device({"md5": {"10": {"md5-key": "secret"}}}) + self.assertEqual(entry["md5"], [{"key_id": 10, "md5_key": "secret"}]) + + +class TestDeviceToArgspecFixture(VyOSModuleTestCase): + def test_areas_parsed(self): + have = self.gather() + area_ids = [a["area_id"] for a in have["areas"]] + self.assertEqual(set(area_ids), {"2", "4", "5"}) + + def test_stub_area_no_summary_parsed(self): + have = self.gather() + area4 = next(a for a in have["areas"] if a["area_id"] == "4") + self.assertTrue(area4["area_type"]["stub"]["no_summary"]) + + def test_virtual_link_parsed(self): + have = self.gather() + area4 = next(a for a in have["areas"] if a["area_id"] == "4") + vlink = area4["virtual_link"][0] + self.assertEqual(vlink["address"], "10.0.0.1") + self.assertEqual(vlink["authentication"]["plaintext_password"], "secret") + self.assertEqual(vlink["dead_interval"], 40) + + def test_passive_interface_and_exclude_parsed(self): + have = self.gather() + self.assertEqual(have["passive_interface"], ["eth1"]) + self.assertEqual(have["passive_interface_exclude"], ["eth2"]) + + def test_timers_parsed(self): + have = self.gather() + self.assertEqual(have["timers"]["refresh"]["timers"], 300) + self.assertEqual(have["timers"]["throttle"]["spf"]["delay"], 200) + + def test_max_metric_parsed(self): + have = self.gather() + self.assertTrue(have["max_metric"]["router_lsa"]["administrative"]) + self.assertEqual(have["max_metric"]["router_lsa"]["on_shutdown"], 10) + + def test_mpls_te_parsed(self): + have = self.gather() + self.assertTrue(have["mpls_te"]["enabled"]) + self.assertEqual(have["mpls_te"]["router_address"], "192.0.11.11") + + def test_empty_config(self): + self.assertEqual(_device_to_argspec({}), {}) + self.assertEqual(_device_to_argspec(None), {}) + + +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_omitted_attribute_on_replaced(self): + """Primary confirmed bug from the original hand-rolled + implementation: clearing an omitted attribute never generated + a delete command. Also the exact scenario that caught the + kebab-key regression during development.""" + raw_have = {"area": {"4": {"area-type": {"stub": {"default-cost": "20"}}}}} + config = {"areas": [{"area_id": "4", "area_type": {"stub": {"set": True}}}]} + cmds = build_commands(config, raw_have, "replaced") + expected = ("delete", _BASE + ["area", "4", "area-type", "stub", "default-cost"]) + self.assertIn(expected, cmds) + + def test_stub_no_summary_now_works(self): + config = {"areas": [{"area_id": "4", "area_type": {"stub": {"no_summary": True}}}]} + cmds = build_commands(config, {}, "merged") + expected = ("set", _BASE + ["area", "4", "area-type", "stub", "no-summary"]) + self.assertIn(expected, cmds) + + def test_replaced_only_touches_what_changed(self): + """Confirmed fix for the original's disruptive "delete + everything and recreate" replaced heuristic -- a targeted + dict_op purge only touches the sections that actually + differ.""" + raw_have = { + "area": {"2": {"area-type": {"normal": {}}}}, + "parameters": {"router-id": "1.1.1.1"}, + } + config = { + "areas": [{"area_id": "2", "area_type": {"normal": True}}], + "parameters": {"router_id": "2.2.2.2"}, + } + cmds = build_commands(config, raw_have, "replaced") + self.assertFalse(any("area" in str(c) for c in cmds)) + expected = ("set", _BASE + ["parameters", "router-id", "2.2.2.2"]) + self.assertIn(expected, cmds) + + def test_passive_interface_and_exclude_together(self): + config = {"passive_interface": ["eth1"], "passive_interface_exclude": ["eth2"]} cmds = build_commands(config, {}, "merged") - self.assertIn(("set", _BASE + ["neighbor", "192.0.11.12"]), cmds) - self.assertIn(("set", _BASE + ["neighbor", "192.0.11.12", "priority", "2"]), cmds) + self.assertIn(("set", _BASE + ["interface", "eth1", "passive"]), cmds) + self.assertIn(("set", _BASE + ["interface", "eth2", "passive", "disable"]), cmds) + + def test_deleted_with_have(self): + cmds = build_commands({}, {"parameters": {"router-id": "1.1.1.1"}}, "deleted") + self.assertEqual(cmds, [("delete", _BASE)]) - def test_merged_default_information(self): - config = {"default_information": {"originate": {"always": True, "metric": 10}}} + def test_deleted_no_have_is_noop(self): + self.assertEqual(build_commands({}, {}, "deleted"), []) + + def test_merged_new_virtual_link(self): + config = { + "areas": [ + { + "area_id": "4", + "virtual_link": [ + {"address": "10.0.0.1", "authentication": {"plaintext_password": "pw"}}, + ], + }, + ], + } cmds = build_commands(config, {}, "merged") - self.assertIn(("set", _BASE + ["default-information", "originate", "always"]), cmds) - self.assertIn(("set", _BASE + ["default-information", "originate", "metric", "10"]), cmds) + vlink_path = _BASE + [ + "area", + "4", + "virtual-link", + "10.0.0.1", + "authentication", + "plaintext-password", + "pw", + ] + self.assertIn(("set", vlink_path), cmds) if __name__ == "__main__": diff --git a/tests/unit/modules/test_vyos_ospfv3.py b/tests/unit/modules/test_vyos_ospfv3.py index a84041b..a19b5fa 100644 --- a/tests/unit/modules/test_vyos_ospfv3.py +++ b/tests/unit/modules/test_vyos_ospfv3.py @@ -4,136 +4,218 @@ 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_ospfv3 import ( + _CONFIG_OPTIONS, + ARGUMENT_SPEC, + _area_entry_from_device, + _area_entry_to_device, + _derive_key_field, + _device_to_argspec, + _kebab_fields, 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") - with open(os.path.join(fixtures_dir, filename)) as f: - return json.load(f) + +_BASE = ["protocols", "ospfv3"] 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 TestVyOSOspfv3Parse(VyOSModuleTestCase): - - def setUp(self): - super().setUp() self.fixture = load_fixture("ospfv3_running.json") + self.mock_vyos.get_config = MagicMock(return_value=self.fixture) - def test_parses_parameters(self): - self.set_running_config(self.fixture) - result = get_running_config(self.mock_vyos) - self.assertEqual(result["parameters"]["router_id"], "192.0.2.10") + def gather(self): + have = _device_to_argspec(self.fixture) + cast_by_spec(have, _CONFIG_OPTIONS) + return have - def test_parses_redistribute(self): - self.set_running_config(self.fixture) + +class TestGetRunningConfig(VyOSModuleTestCase): + def test_returns_config_directly(self): result = get_running_config(self.mock_vyos) - route_types = [r["route_type"] for r in result["redistribute"]] - self.assertIn("bgp", route_types) - self.assertIn("connected", route_types) - connected = next(r for r in result["redistribute"] if r["route_type"] == "connected") - self.assertEqual(connected["route_map"], "RM1") - - def test_parses_areas(self): - self.set_running_config(self.fixture) + self.assertIn("area", result) + + 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): + """Regression coverage for the confirmed failure mode from + vyos_ospf_interfaces's build: an unguarded collapsed response + (VyOS's single-value tag-node quirk) would otherwise be + iterated character-by-character downstream.""" + self.mock_vyos.get_config = MagicMock(return_value="eth1") result = get_running_config(self.mock_vyos) - self.assertEqual(len(result["areas"]), 2) - area2 = next(a for a in result["areas"] if a["area_id"] == "2") - self.assertEqual(area2["export_list"], "export1") - self.assertEqual(area2["import_list"], "import1") - self.assertEqual(len(area2["range"]), 2) - not_adv = next(r for r in area2["range"] if r["address"] == "2001:db20::/32") - self.assertTrue(not_adv["not_advertise"]) + self.assertEqual(result, {"eth1": {}}) - def test_empty_config_returns_empty(self): - self.set_running_config({}) - result = get_running_config(self.mock_vyos) - self.assertEqual(result, {}) +class TestDeriveKeyField(unittest.TestCase): + def test_derives_area_id(self): + area_opts = ARGUMENT_SPEC["config"]["options"]["areas"]["options"] + self.assertEqual(_derive_key_field(area_opts), "area_id") -class TestVyOSOspfv3BuildCommands(unittest.TestCase): + def test_raises_if_none_required(self): + with self.assertRaises(ValueError): + _derive_key_field({"a": {"type": "str"}}) - def test_deleted_with_have(self): - have = {"parameters": {"router_id": "192.0.2.10"}} - cmds = build_commands({}, have, "deleted") - self.assertEqual(cmds, [("delete", ["protocols", "ospfv3"])]) - def test_deleted_without_have(self): - cmds = build_commands({}, {}, "deleted") - self.assertEqual(cmds, []) +class TestKebabFields(unittest.TestCase): + def test_converts_multiword_keys(self): + result = _kebab_fields({"router_id": "1.1.1.1"}) + self.assertEqual(result, {"router-id": "1.1.1.1"}) - def test_merged_parameters(self): - config = {"parameters": {"router_id": "192.0.2.10"}} - cmds = build_commands(config, {}, "merged") - self.assertIn( - ("set", ["protocols", "ospfv3", "parameters", "router-id", "192.0.2.10"]), - cmds, + +class TestAreaEntry(unittest.TestCase): + def test_export_import_list_to_device(self): + result = _area_entry_to_device({"export_list": "el1", "import_list": "il1"}) + self.assertEqual(result, {"export-list": "el1", "import-list": "il1"}) + + def test_range_to_device(self): + result = _area_entry_to_device( + {"range": [{"address": "2001:db1::/32", "not_advertise": True}]}, ) + self.assertEqual(result, {"range": {"2001:db1::/32": {"not-advertise": {}}}}) - def test_merged_redistribute(self): - config = {"redistribute": [{"route_type": "bgp"}]} - cmds = build_commands(config, {}, "merged") - self.assertIn( - ("set", ["protocols", "ospfv3", "redistribute", "bgp"]), - cmds, + def test_from_device(self): + entry = _area_entry_from_device( + {"export-list": "el1", "range": {"2001:db1::/32": {"advertise": {}}}}, ) + self.assertEqual(entry["export_list"], "el1") + self.assertEqual(entry["range"][0]["address"], "2001:db1::/32") + self.assertTrue(entry["range"][0]["advertise"]) + + def test_empty(self): + self.assertEqual(_area_entry_to_device({}), {}) + self.assertEqual(_area_entry_from_device({}), {}) - def test_merged_idempotent(self): - config = {"parameters": {"router_id": "192.0.2.10"}} - have = {"parameters": {"router_id": "192.0.2.10"}} - cmds = build_commands(config, have, "merged") - self.assertEqual(cmds, []) - def test_merged_area_range(self): +class TestDeviceToArgspecFixture(VyOSModuleTestCase): + def test_areas_parsed(self): + have = self.gather() + area_ids = {a["area_id"] for a in have["areas"]} + self.assertEqual(area_ids, {"2", "3"}) + + def test_area_export_import_list_parsed(self): + have = self.gather() + area2 = next(a for a in have["areas"] if a["area_id"] == "2") + self.assertEqual(area2["export_list"], "export1") + self.assertEqual(area2["import_list"], "import1") + + def test_range_advertise_not_advertise_parsed(self): + have = self.gather() + area2 = next(a for a in have["areas"] if a["area_id"] == "2") + r1 = next(r for r in area2["range"] if r["address"] == "2001:db10::/32") + r2 = next(r for r in area2["range"] if r["address"] == "2001:db20::/32") + self.assertNotIn("advertise", r1) + self.assertNotIn("not_advertise", r1) + self.assertTrue(r2["not_advertise"]) + + area3 = next(a for a in have["areas"] if a["area_id"] == "3") + self.assertTrue(area3["range"][0]["advertise"]) + + def test_parameters_parsed(self): + have = self.gather() + self.assertEqual(have["parameters"]["router_id"], "192.0.2.10") + + def test_redistribute_parsed(self): + have = self.gather() + route_types = {r["route_type"] for r in have["redistribute"]} + self.assertEqual(route_types, {"bgp", "static"}) + bgp = next(r for r in have["redistribute"] if r["route_type"] == "bgp") + self.assertEqual(bgp["route_map"], "redist-map") + + def test_empty(self): + self.assertEqual(_device_to_argspec({}), {}) + self.assertEqual(_device_to_argspec(None), {}) + + +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_changing_existing_range_flag_via_replaced(self): + """Primary confirmed bug from the original hand-rolled + implementation: range advertise/not_advertise were only ever + set on initial range creation (an "if addr not in have_ranges" + guard) -- changing an existing range's flag generated no + command under either state. dict_op's purge (via "replaced") + correctly handles this generically.""" + raw_have = {"area": {"2": {"range": {"2001:db20::/32": {"not-advertise": {}}}}}} config = { - "areas": [{"area_id": "2", "range": [{"address": "2001:db10::/32"}]}], + "areas": [ + { + "area_id": "2", + "range": [ + {"address": "2001:db20::/32", "advertise": True}, + ], + }, + ], } - cmds = build_commands(config, {}, "merged") + cmds = build_commands(config, raw_have, "replaced") self.assertIn( - ("set", ["protocols", "ospfv3", "area", "2", "range", "2001:db10::/32"]), + ("delete", _BASE + ["area", "2", "range", "2001:db20::/32", "not-advertise"]), cmds, ) - - def test_replaced_idempotent(self): - config = {"parameters": {"router_id": "192.0.2.10"}} - have = {"parameters": {"router_id": "192.0.2.10"}} - cmds = build_commands(config, have, "replaced") - self.assertEqual(cmds, []) - - def test_replaced_rebuilds_on_change(self): - config = {"parameters": {"router_id": "192.0.2.11"}} - have = {"parameters": {"router_id": "192.0.2.10"}} - cmds = build_commands(config, have, "replaced") - self.assertEqual(cmds[0], ("delete", ["protocols", "ospfv3"])) self.assertIn( - ("set", ["protocols", "ospfv3", "parameters", "router-id", "192.0.2.11"]), + ("set", _BASE + ["area", "2", "range", "2001:db20::/32", "advertise"]), cmds, ) - def test_merged_area_export_list(self): - config = {"areas": [{"area_id": "2", "export_list": "export1"}]} + def test_merged_does_not_clear_conflicting_flag(self): + """merged never runs a purge pass, by design (established, + consistent semantic throughout this collection) -- confirms + it correctly does NOT clear not-advertise even when advertise + is set instead, unlike replaced above.""" + raw_have = {"area": {"2": {"range": {"2001:db20::/32": {"not-advertise": {}}}}}} + config = { + "areas": [ + { + "area_id": "2", + "range": [ + {"address": "2001:db20::/32", "advertise": True}, + ], + }, + ], + } + cmds = build_commands(config, raw_have, "merged") + self.assertFalse(any(c[0] == "delete" and "not-advertise" in str(c) for c in cmds)) + + def test_replaced_is_whole_resource_not_scoped(self): + """Confirmed matching this module's own documented contract + ("replaced replaces the module-managed OSPFv3 configuration") and + vyos_ospfv2's own established precedent -- unlike + vyos_static_routes/vyos_route_maps, which scope replaced per + named item.""" + raw_have = {"parameters": {"router-id": "1.1.1.1"}, "redistribute": {"bgp": {}}} + config = {"parameters": {"router_id": "1.1.1.1"}} + cmds = build_commands(config, raw_have, "replaced") + self.assertIn(("delete", _BASE + ["redistribute"]), cmds) + + def test_deleted_with_have(self): + cmds = build_commands({}, {"parameters": {"router-id": "1.1.1.1"}}, "deleted") + self.assertEqual(cmds, [("delete", _BASE)]) + + def test_deleted_no_have_is_noop(self): + self.assertEqual(build_commands({}, {}, "deleted"), []) + + def test_merged_new_area(self): + config = {"areas": [{"area_id": "5", "export_list": "el5"}]} cmds = build_commands(config, {}, "merged") - self.assertIn( - ("set", ["protocols", "ospfv3", "area", "2", "export-list", "export1"]), - cmds, - ) + self.assertIn(("set", _BASE + ["area", "5", "export-list", "el5"]), cmds) if __name__ == "__main__": diff --git a/tests/unit/test_module_utils_vyos.py b/tests/unit/test_module_utils_vyos.py index 920e9a5..213a2fb 100644 --- a/tests/unit/test_module_utils_vyos.py +++ b/tests/unit/test_module_utils_vyos.py @@ -14,7 +14,12 @@ __metaclass__ = type import unittest -from ansible_collections.vyos.rest.plugins.module_utils.vyos import cast_by_spec +from unittest.mock import MagicMock + +from ansible_collections.vyos.rest.plugins.module_utils.vyos import ( + VyOSModule, + cast_by_spec, +) class TestCastBySpecIntCollapse(unittest.TestCase): @@ -51,5 +56,86 @@ class TestCastBySpecIntCollapse(unittest.TestCase): self.assertIsNone(entry["distance"]) +class TestGetValue(unittest.TestCase): + """Regression tests for a confirmed bug (Copilot): get_value() + previously did `result.get("data") or ""`, which incorrectly + converts any falsy-but-valid scalar (0, False, an already-empty + string) into an empty string -- indistinguishable from the value + being genuinely unset. Fixed to only treat a missing "data" key + (None) as unset, leaving every other value -- including falsy + ones -- exactly as returned.""" + + def _vyos_with_data(self, data): + vyos = VyOSModule.__new__(VyOSModule) + vyos._client = MagicMock() + vyos._client.retrieve_return_value.return_value = {"data": data} + return vyos + + def test_zero_preserved_not_emptied(self): + vyos = self._vyos_with_data(0) + result = vyos.get_value(["some", "path"]) + self.assertEqual(result, 0) + self.assertIs(type(result), int) + + def test_false_preserved_not_emptied(self): + vyos = self._vyos_with_data(False) + self.assertIs(vyos.get_value(["some", "path"]), False) + + def test_genuine_string_value_passes_through(self): + vyos = self._vyos_with_data("vyos-core-01") + self.assertEqual(vyos.get_value(["some", "path"]), "vyos-core-01") + + def test_already_empty_string_stays_empty(self): + vyos = self._vyos_with_data("") + self.assertEqual(vyos.get_value(["some", "path"]), "") + + def test_missing_data_key_becomes_empty_string(self): + vyos = VyOSModule.__new__(VyOSModule) + vyos._client = MagicMock() + vyos._client.retrieve_return_value.return_value = {} + self.assertEqual(vyos.get_value(["some", "path"]), "") + + +class TestShow(unittest.TestCase): + """Regression tests for the same confirmed bug class as + TestGetValue, found independently in show(): `result.get("data") + or ""` incorrectly converts any falsy-but-valid scalar (0, False, + an already-empty string) from an operational show command into an + empty string -- indistinguishable from the command genuinely + returning nothing. Fixed to only treat a missing "data" key + (None) as empty, leaving every other value -- including falsy + ones -- exactly as returned.""" + + def _vyos_with_data(self, data): + vyos = VyOSModule.__new__(VyOSModule) + vyos._client = MagicMock() + vyos._client.show.return_value = {"data": data} + return vyos + + def test_zero_preserved_not_emptied(self): + vyos = self._vyos_with_data(0) + result = vyos.show(["some", "op", "path"]) + self.assertEqual(result, 0) + self.assertIs(type(result), int) + + def test_false_preserved_not_emptied(self): + vyos = self._vyos_with_data(False) + self.assertIs(vyos.show(["some", "op", "path"]), False) + + def test_genuine_output_passes_through(self): + vyos = self._vyos_with_data("interface eth0 up") + self.assertEqual(vyos.show(["some", "op", "path"]), "interface eth0 up") + + def test_already_empty_string_stays_empty(self): + vyos = self._vyos_with_data("") + self.assertEqual(vyos.show(["some", "op", "path"]), "") + + def test_missing_data_key_becomes_empty_string(self): + vyos = VyOSModule.__new__(VyOSModule) + vyos._client = MagicMock() + vyos._client.show.return_value = {} + self.assertEqual(vyos.show(["some", "op", "path"]), "") + + if __name__ == "__main__": unittest.main() |