diff options
| -rw-r--r-- | changelogs/fragments/t6828-vyos_config-enforce.yml | 3 | ||||
| -rw-r--r-- | changelogs/fragments/t6837-vyos_config-replace.yml | 19 | ||||
| -rw-r--r-- | docs/vyos.vyos.vyos_config_module.rst | 69 | ||||
| -rw-r--r-- | docs/vyos.vyos.vyos_user_module.rst | 4 | ||||
| -rw-r--r-- | plugins/cliconf/vyos.py | 82 | ||||
| -rw-r--r-- | plugins/cliconf_utils/__init__.py | 0 | ||||
| -rw-r--r-- | plugins/cliconf_utils/vyosconf.py | 257 | ||||
| -rw-r--r-- | plugins/module_utils/network/vyos/vyos.py | 31 | ||||
| -rw-r--r-- | plugins/modules/vyos_config.py | 302 | ||||
| -rw-r--r-- | tests/integration/targets/vyos_config/templates/replace_config_candidate.cfg | 1 | ||||
| -rw-r--r-- | tests/integration/targets/vyos_config/tests/cli/enforce.yaml | 124 | ||||
| -rw-r--r-- | tests/integration/targets/vyos_config/tests/cli/replace_config.yaml | 289 | ||||
| -rw-r--r-- | tests/unit/cliconf/__init__.py | 0 | ||||
| -rw-r--r-- | tests/unit/cliconf/test_utils_vyosconf.py | 217 | ||||
| -rw-r--r-- | tests/unit/modules/network/vyos/test_vyos_config.py | 535 |
15 files changed, 1876 insertions, 57 deletions
diff --git a/changelogs/fragments/t6828-vyos_config-enforce.yml b/changelogs/fragments/t6828-vyos_config-enforce.yml new file mode 100644 index 00000000..f1f63603 --- /dev/null +++ b/changelogs/fragments/t6828-vyos_config-enforce.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - vyos_config - Add `enforce` match mode, which enforces the supplied configuration as the desired end-state. diff --git a/changelogs/fragments/t6837-vyos_config-replace.yml b/changelogs/fragments/t6837-vyos_config-replace.yml new file mode 100644 index 00000000..9a0ff2b8 --- /dev/null +++ b/changelogs/fragments/t6837-vyos_config-replace.yml @@ -0,0 +1,19 @@ +--- +minor_changes: + - vyos_config - added a new ``replace`` option value, ``config`` (in addition + to the existing default, ``line``). When set to ``replace=config``, the + module uploads the complete candidate configuration supplied via ``src`` + to the device and issues VyOS's native ``load`` command in configuration + mode, letting VyOS's own configuration engine perform the replacement, + rather than the module computing a set/delete command diff. This mirrors + the mechanism offered by ``cisco.iosxr.iosxr_config``'s ``replace=config`` + (https://vyos.dev/T6837). +bugfixes: + - vyos_config - the ``allow_password_change`` filter used a regular + expression that only matched ``set`` lines + (``set system login user ... authentication (encrypted|plaintext)-password``), + so a ``delete`` line for the same path was never filtered regardless of + the ``allow_password_change`` value. This meant an account omitted from a + full-config candidate could have its password deleted without the + existing safety filter ever inspecting the line. The regular expression + now matches both ``set`` and ``delete`` lines. diff --git a/docs/vyos.vyos.vyos_config_module.rst b/docs/vyos.vyos.vyos_config_module.rst index bf91bb0f..bdc0df07 100644 --- a/docs/vyos.vyos.vyos_config_module.rst +++ b/docs/vyos.vyos.vyos_config_module.rst @@ -52,6 +52,7 @@ Parameters </td> <td> <div>The <code>allow_password_change</code> argument specifies whether any configuration lines which would change a user's password should be filtered out. By default only plaintext password changes are allowed and any encrypted-password keys are filtered out. In order to allow all password updates, both plaintext and encrypted, set this argument to <code>all</code>.</div> + <div>Not applied when <code>replace</code> is set to <code>config</code>; the candidate is loaded as-is via VyOS's native <code>load</code>, which has no equivalent filtering mechanism.</div> </td> </tr> <tr> @@ -85,7 +86,7 @@ Parameters <td> </td> <td> - <div>This is a dict object containing configurable options related to backup file path. The value of this option is read only when <code>backup</code> is set to <code>true</code>, if <code>backup</code> is set to <code>false</code> this option will be silently ignored.</div> + <div>This is a dict object containing configurable options related to backup file path. The value of this option is read only when <code>backup</code> is set to <em>yes</em>, if <code>backup</code> is set to <em>no</em> this option will be silently ignored.</div> </td> </tr> <tr> @@ -150,6 +151,7 @@ Parameters </td> <td> <div>The <code>config</code> argument specifies the base configuration to use to compare against the desired configuration. If this value is not specified, the module will automatically retrieve the current active configuration from the remote device. The configuration lines in the option value should be similar to how it will appear if present in the running-configuration of the device including indentation to ensure idempotency and correct diff.</div> + <div>Ignored when <code>replace</code> is set to <code>config</code>.</div> </td> </tr> <tr> @@ -165,11 +167,12 @@ Parameters <ul style="margin: 0; padding: 0"><b>Choices:</b> <li>automatic</li> <li>manual</li> - <li><div style="color: blue"><b>none</b> ←</div></li> + <li>none</li> </ul> </td> <td> <div>The <code>confirm</code> argument will tell vyos to revert to the previous configuration if not explicitly confirmed after applying the new config. When set to <code>automatic</code> this module will automatically confirm the configuration, if the current session remains working with the new config. When set to <code>manual</code>, this module does not issue the confirmation itself.</div> + <div>Defaults to <code>automatic</code> when <code>match</code> is set to <code>enforce</code>, since <code>enforce</code> can generate <code>delete</code> commands for configuration not mentioned in the candidate and a bad commit should self-revert rather than leave the device unreachable. Defaults to <code>none</code> for all other <code>match</code> values.</div> </td> </tr> <tr> @@ -202,6 +205,7 @@ Parameters </td> <td> <div>The ordered set of commands that should be configured in the section. The commands must be the exact same commands as found in the device running-config as found in the device running-config to ensure idempotency and correct diff. Be sure to note the configuration command syntax as some commands are automatically modified by the device config parser.</div> + <div>Not supported when <code>replace</code> is set to <code>config</code> -- see <code>replace</code> below.</div> </td> </tr> <tr> @@ -216,11 +220,42 @@ Parameters <td> <ul style="margin: 0; padding: 0"><b>Choices:</b> <li><div style="color: blue"><b>line</b> ←</div></li> + <li>enforce</li> <li>none</li> </ul> </td> <td> - <div>The <code>match</code> argument controls the method used to match against the current active configuration. By default, the desired config is matched against the active config and the deltas are loaded. If the <code>match</code> argument is set to <code>none</code> the active configuration is ignored and the configuration is always loaded.</div> + <div>The <code>match</code> argument controls the method used to match against the current active configuration. By default, the desired config is matched against the active config and the deltas are loaded. If the <code>match</code> argument is set to <code>none</code>, the active configuration is ignored and the configuration is always loaded. If the <code>match</code> argument is set to <code>enforce</code>, the supplied <code>lines</code> or <code>src</code> are treated as the complete desired end-state of the configuration, rather than a set of deltas to apply. <code>enforce</code> enforces only the top-level configuration sections present in the supplied candidate as complete end-states; existing configuration within those sections but not mentioned in the candidate is removed, so <code>enforce</code> can generate <code>delete</code> commands for configuration the candidate does not mention. Top-level sections the candidate does not reference at all are left completely untouched. <code>enforce</code> is intended for candidates made up of <code>set</code> commands only; supplying <code>delete</code> lines alongside <code>match=enforce</code> is not supported and will raise an error.</div> + <div>Ignored when <code>replace</code> is set to <code>config</code>, since no line-level diff is computed in that mode.</div> + </td> + </tr> + <tr> + <td colspan="2"> + <div class="ansibleOptionAnchor" id="parameter-"></div> + <b>replace</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> + <ul style="margin: 0; padding: 0"><b>Choices:</b> + <li><div style="color: blue"><b>line</b> ←</div></li> + <li>config</li> + </ul> + </td> + <td> + <div>Controls how the module applies configuration to the device.</div> + <div>When set to <code>line</code> (default), the module computes a set/delete command diff and pushes only the changed lines -- this is the existing behavior, unchanged.</div> + <div>When set to <code>config</code>, the module uploads the full candidate configuration (<code>src</code>) to the device and issues VyOS's native <code>load</code> command in configuration mode, which replaces the running configuration wholesale with the candidate's exact contents. VyOS's own configuration engine performs the reconciliation, rather than the module computing per-line deltas. This mirrors the mechanism offered by <code>cisco.iosxr.iosxr_config</code>'s <code>replace=config</code>.</div> + <div><code>replace=config</code> requires <code>src</code> and does not accept <code>lines</code> -- there is no way to convert flat set/delete commands into the hierarchical form <code>load</code> requires without re-implementing VyOS's own config-tree builder.</div> + <div>As with <code>src</code> in the default <code>line</code> mode, the module does not validate the candidate's contents or format under <code>replace=config</code> -- supplying a well-formed, complete configuration is the caller's responsibility.</div> + <div><code>replace=config</code> requires the device to accept file transfer (SCP) over the same <code>network_cli</code> SSH session used for configuration commands.</div> + <div><code>replace=config</code> writes the candidate to a fixed path on the device (overwritten on each run, matching <code>cisco.iosxr.iosxr_config</code>'s own <code>replace=config</code> precedent). Running <code>replace=config</code> concurrently against the same host is not supported.</div> + <div>Any configuration present on the device but omitted from the candidate will be removed, including management interfaces, SSH access, and login users if they are omitted. Always supply a complete configuration, never a partial one.</div> + <div>When capturing a candidate from the device's own output (for example via <code>show configuration</code>) rather than from a trusted, separately maintained source, be aware that VyOS may return masked placeholder values (for example a run of literal asterisks) in place of local users' <code>encrypted-password</code>/<code>plaintext-password</code> values when queried through automation, even though the identical command returns the real value when typed interactively at a terminal. Pushing a masked capture back through <code>replace=config</code> sends the literal placeholder as the new password value; VyOS's own commit-time validation is expected to reject an obviously malformed hash, but a masked value that happens to pass basic format validation could apply silently. Prefer sourcing <code>replace=config</code> candidates from a trusted, version-controlled artifact rather than a live automated capture whenever the configuration contains local password-based users.</div> + <div>Even under <code>check_mode</code>, the candidate is written to a temporary file on the device so that VyOS's own <code>compare</code> can produce an accurate preview diff. No <code>commit</code> occurs in check mode.</div> + <div>When combined with <code>backup=yes</code>, the value of <code>changed</code> reflects whether the backup file's content changed on the Ansible control node, not whether the device configuration changed -- this is existing behavior in the shared netcommon action plugin backing config-family modules across collections, not specific to <code>replace=config</code>.</div> </td> </tr> <tr> @@ -239,7 +274,7 @@ Parameters </ul> </td> <td> - <div>The <code>save</code> argument controls whether or not changes made to the active configuration are saved to disk. This is independent of committing the config. When set to <code>true</code>, the active configuration is saved.</div> + <div>The <code>save</code> argument controls whether or not changes made to the active configuration are saved to disk. This is independent of committing the config. When set to <code>True</code>, the active configuration is saved.</div> </td> </tr> <tr> @@ -255,6 +290,7 @@ Parameters </td> <td> <div>The <code>src</code> argument specifies the path to the source config file to load. The source config file can either be in bracket format or set format. The source file can include Jinja2 template variables. The configuration lines in the source file should be similar to how it will appear if present in the running-configuration of the device including indentation to ensure idempotency and correct diff.</div> + <div>When <code>replace</code> is set to <code>config</code>, <code>src</code> is required and must contain a complete configuration in hierarchical/bracket format -- the same format produced by <code>show configuration</code> or found in <code>/config/config.boot</code>. Flat <code>set</code>/<code>delete</code> command format (as produced by <code>show configuration commands</code>) is not accepted in that mode; VyOS's native <code>load</code> command rejects it with a parse error.</div> </td> </tr> </table> @@ -268,6 +304,7 @@ Notes - Tested against VyOS 1.3.8, 1.4.2, the upcoming 1.5, and the rolling release of spring 2025. - This module works with connection ``ansible.netcommon.network_cli``. See `the VyOS OS Platform Options <../network/user_guide/platform_vyos.html>`_. - To ensure idempotency and correct diff the configuration lines in the relevant module options should be similar to how they appear if present in the running configuration on device including the indentation. + - ``replace=config`` currently has no way to scope its effect to part of the configuration; it always operates against the entire device configuration. There is no ``path`` parameter to constrain it to a subtree. - For more information on using Ansible to manage network devices see the :ref:`Ansible Network Guide <network_guide>` @@ -291,6 +328,7 @@ Examples - name: render a Jinja2 template onto the VyOS router vyos.vyos.vyos_config: + match: enforce src: vyos_template.j2 - name: revert after ten minutes, if connection is lost @@ -311,6 +349,23 @@ Examples filename: backup.cfg dir_path: /home/user + - name: capture the complete hierarchical configuration for editing + # replace=config requires the complete desired configuration in + # hierarchical/bracket format -- never a partial one, and never flat + # set-command format. `backup: true` alone won't work here: it captures + # flat set-command output (via `show configuration commands`), which + # replace=config's underlying `load` command rejects. Capture the + # hierarchical form directly instead, edit it, then replace with the + # edited whole, as shown here. + vyos.vyos.vyos_command: + commands: "show configuration" + register: current_config + + - name: (edit current_config.stdout[0] as needed, save it locally, then) + vyos.vyos.vyos_config: + src: /home/user/edited_config.cfg + replace: config + Return Values @@ -353,7 +408,8 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late </td> <td>always</td> <td> - <div>The list of configuration commands sent to the device</div> + <div>In <code>replace=line</code> mode (default), the list of set/delete commands sent to the device.</div> + <div>In <code>replace=config</code> mode, contains only the single <code>load <path></code> command actually issued to the device -- not an itemized diff. See <code>diff</code> for the actual change content, sourced from VyOS's own <code>compare</code> output.</div> <br/> <div style="font-size: smaller"><b>Sample:</b></div> <div style="font-size: smaller; color: blue; word-wrap: break-word; word-break: break-all;">['...', '...']</div> @@ -404,7 +460,8 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late </td> <td>always</td> <td> - <div>The list of configuration commands removed to avoid a load failure</div> + <div>The list of configuration commands removed to avoid a load failure.</div> + <div>Not populated when <code>replace</code> is set to <code>config</code>.</div> <br/> <div style="font-size: smaller"><b>Sample:</b></div> <div style="font-size: smaller; color: blue; word-wrap: break-word; word-break: break-all;">['...', '...']</div> diff --git a/docs/vyos.vyos.vyos_user_module.rst b/docs/vyos.vyos.vyos_user_module.rst index 42b8ccce..0f1ab9a2 100644 --- a/docs/vyos.vyos.vyos_user_module.rst +++ b/docs/vyos.vyos.vyos_user_module.rst @@ -188,6 +188,8 @@ Parameters <li>ecdsa-sha2-nistp384</li> <li>ssh-ed25519</li> <li>ecdsa-sha2-nistp521</li> + <li>sk-ecdsa-sha2-nistp256@openssh.com</li> + <li>sk-ssh-ed25519@openssh.com</li> </ul> </td> <td> @@ -365,6 +367,8 @@ Parameters <li>ecdsa-sha2-nistp384</li> <li>ssh-ed25519</li> <li>ecdsa-sha2-nistp521</li> + <li>sk-ecdsa-sha2-nistp256@openssh.com</li> + <li>sk-ssh-ed25519@openssh.com</li> </ul> </td> <td> diff --git a/plugins/cliconf/vyos.py b/plugins/cliconf/vyos.py index 96c24c15..e693f820 100644 --- a/plugins/cliconf/vyos.py +++ b/plugins/cliconf/vyos.py @@ -17,6 +17,7 @@ # from __future__ import absolute_import, division, print_function + __metaclass__ = type DOCUMENTATION = """ @@ -48,11 +49,13 @@ import re from ansible.errors import AnsibleConnectionFailure from ansible.module_utils._text import to_text from ansible.module_utils.common._collections_compat import Mapping +from ansible.plugins.cliconf import CliconfBase from ansible_collections.ansible.netcommon.plugins.module_utils.network.common.config import ( NetworkConfig, ) from ansible_collections.ansible.netcommon.plugins.module_utils.network.common.utils import to_list -from ansible_collections.ansible.netcommon.plugins.plugin_utils.cliconf_base import CliconfBase + +from ansible_collections.vyos.vyos.plugins.cliconf_utils.vyosconf import VyosConf class Cliconf(CliconfBase): @@ -122,7 +125,13 @@ class Cliconf(CliconfBase): return out def edit_config( - self, candidate=None, commit=True, replace=None, diff=False, comment=None, confirm=None + self, + candidate=None, + commit=True, + replace=None, + diff=False, + comment=None, + confirm=None, ): resp = {} operations = self.get_device_operations() @@ -240,14 +249,21 @@ class Cliconf(CliconfBase): if path: raise ValueError("'path' in diff is not supported") - set_format = candidate.startswith("set") or candidate.startswith("delete") + first_line = next( + ( + stripped + for stripped in (line.strip() for line in candidate.splitlines()) + if stripped and not stripped.startswith("#") + ), + "", + ) + set_format = first_line.startswith("set") or first_line.startswith("delete") candidate_obj = NetworkConfig(indent=4, contents=candidate) if not set_format: config = [c.line for c in candidate_obj.items] commands = list() - # this filters out less specific lines for item in config: for index, entry in enumerate(commands): if item.startswith(entry): @@ -259,11 +275,65 @@ class Cliconf(CliconfBase): else: - candidate_commands = str(candidate).strip().split("\n") + candidate_commands = [ + line.strip() + for line in str(candidate).splitlines() + if line.strip() and not line.lstrip().startswith("#") + ] if diff_match == "none": diff["config_diff"] = list(candidate_commands) return diff + if diff_match == "enforce": + if running is None: + raise ValueError( + "diff_match=enforce requires a running configuration to diff against", + ) + + enforce_candidate_lines = list(candidate_commands) + + if not enforce_candidate_lines: + raise ValueError( + "diff_match=enforce received an empty candidate (after stripping blank/" + "comment lines); refusing to treat that as a desired end-state of " + "'delete everything'. Provide 'set' commands describing the desired " + "configuration.", + ) + + for line in enforce_candidate_lines: + tokens = line.strip().split() + if tokens[0] != "set": + raise ValueError( + "diff_match=enforce treats the candidate as the complete desired " + "configuration end-state and only supports 'set' commands; " + "line does not start with 'set' (found: {0!r})".format( + line.strip(), + ), + ) + if len(VyosConf().parse_line(line)[1]) < 1: + raise ValueError( + "diff_match=enforce only supports complete 'set' commands with at least " + "a path and a leaf; got: {0!r}".format(line.strip()), + ) + running_conf = VyosConf( + [ + line + for line in running.splitlines() + if line.strip() and not line.lstrip().startswith("#") + ], + ) + + candidate_conf = VyosConf(enforce_candidate_lines) + diff["config_diff"] = running_conf.diff_commands_to(candidate_conf) + for cmd in diff["config_diff"]: + if re.match(r"^delete\s+service\s+ssh\b", cmd): + raise ValueError( + "diff_match=enforce refuses to generate 'delete service ssh ...' " + "commands, since this could sever the management connection. " + "Remove SSH configuration explicitly with a separate match=line " + "or match=none task instead.", + ) + return diff running_commands = [str(c).replace("'", "") for c in running.splitlines()] @@ -336,7 +406,7 @@ class Cliconf(CliconfBase): def get_option_values(self): return { "format": ["text", "set"], - "diff_match": ["line", "none"], + "diff_match": ["line", "enforce", "none"], "diff_replace": [], "output": [], } diff --git a/plugins/cliconf_utils/__init__.py b/plugins/cliconf_utils/__init__.py new file mode 100644 index 00000000..e69de29b --- /dev/null +++ b/plugins/cliconf_utils/__init__.py diff --git a/plugins/cliconf_utils/vyosconf.py b/plugins/cliconf_utils/vyosconf.py new file mode 100644 index 00000000..a3ed6887 --- /dev/null +++ b/plugins/cliconf_utils/vyosconf.py @@ -0,0 +1,257 @@ +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see <http://www.gnu.org/licenses/>. +# +from __future__ import absolute_import, division, print_function + + +__metaclass__ = type + +import re + + +class _KeepExistingSentinel: + """Unique marker for 'preserve whatever's already here' in a diff. + Deliberately not a plain string/value: a real config leaf could + legitimately be "..." (e.g. a description), and a string sentinel + would collide with it. An object identity never can. + """ + + def __repr__(self): + return "<keep-existing>" + + +KEEP_EXISTING_VALUES = _KeepExistingSentinel() + + +class VyosConf: + def __init__(self, commands=None): + self.config = {} + if isinstance(commands, list): + self.run_commands(commands) + + def set_entry(self, path, leaf): + """ + This function sets a value in the configuration given a path. + :param path: list of strings to traverse in the config + :param leaf: value to set at the destination + :return: dict + """ + target = self.config + path = path + [leaf] + for key in path: + if key not in target or not isinstance(target[key], dict): + target[key] = {} + target = target[key] + return self.config + + def del_entry(self, path, leaf): + """ + This function deletes a value from the configuration given a path + and also removes all the parents that are now empty. If the leaf + does not exist at the given path, the configuration is left + unchanged (delete is treated as a no-op, matching VyOS's own + behaviour when deleting a path that isn't set). + :param path: list of strings to traverse in the config + :param leaf: value to delete at the destination + :return: dict + """ + target = self.config + first_no_sibling_key = None + for key in path: + if key not in target: + return self.config + if len(target[key]) <= 1: + if first_no_sibling_key is None: + first_no_sibling_key = [target, key] + else: + first_no_sibling_key = None + target = target[key] + + if leaf not in target: + return self.config + + if first_no_sibling_key is None: + first_no_sibling_key = [target, leaf] + + target = first_no_sibling_key[0] + target_key = first_no_sibling_key[1] + del target[target_key] + return self.config + + def check_entry(self, path, leaf): + """ + This function checks if a value exists in the config. + :param path: list of strings to traverse in the config + :param leaf: value to check for existence + :return: bool + """ + target = self.config + path = path + [leaf] + for key in path: + if key not in target or not isinstance(target[key], dict): + return False + target = target[key] + return True + + def parse_line(self, line): + """ + This function parses a given command from string. + :param line: line to parse + :return: [command, path, leaf] + """ + line = re.match(r"^('(.*)'|\"(.*)\"|([^#\"']*))*", line).group(0).strip() + if not line: + return ["", [], ""] + path = re.findall(r"('.*?'|\".*?\"|\S+)", line) + if not path: + return ["", [], ""] + leaf = path[-1] + if leaf.startswith('"') and leaf.endswith('"'): + leaf = leaf[1:-1] + if leaf.startswith("'") and leaf.endswith("'"): + leaf = leaf[1:-1] + return [path[0], path[1:-1], leaf] + + def run_command(self, command): + """ + This function runs a given command string. + :param command: command to run + :return: dict + """ + [cmd, path, leaf] = self.parse_line(command) + if cmd.startswith("set"): + self.set_entry(path, leaf) + if cmd.startswith("del"): + self.del_entry(path, leaf) + return self.config + + def run_commands(self, commands): + """ + This function runs a list of command strings. + :param commands: commands to run + :return: dict + """ + for c in commands: + self.run_command(c) + return self.config + + def check_command(self, command): + """ + This function checks a command for existence in the config. + :param command: command to check + :return: bool + """ + [cmd, path, leaf] = self.parse_line(command) + if cmd.startswith("set"): + return self.check_entry(path, leaf) + if cmd.startswith("del"): + return not self.check_entry(path, leaf) + return True + + def check_commands(self, commands): + """ + This function checks a list of commands for existence in the config. + :param commands: list of commands to check + :return: [bool] + """ + return [self.check_command(c) for c in commands] + + def quote_key(self, key): + """ + This function adds quotes to key if quotes are needed for correct parsing. + :param key: str to wrap in quotes if needed + :return: str + """ + if len(key) == 0: + return "" + if '"' in key: + return "'" + key + "'" + if "'" in key: + return '"' + key + '"' + if not re.match(r"^[a-zA-Z0-9./-]*$", key): + return "'" + key + "'" + return key + + def build_commands(self, structure=None, nested=False): + """ + This function builds a list of commands to recreate the current configuration. + :return: [str] + """ + if not isinstance(structure, dict): + structure = self.config + if len(structure) == 0: + return [""] if nested else [] + commands = [] + for key, value in structure.items(): + quoted_key = self.quote_key(key) + for c in self.build_commands(value, True): + commands.append((quoted_key + " " + c).strip()) + if nested: + return commands + return ["set " + c for c in commands] + + def diff_to(self, other, structure): + if not isinstance(other, dict): + other = {} + if len(structure) == 0: + return ([], [""]) + if not isinstance(structure, dict): + structure = {} + if len(other) == 0: + return ([""], []) + if len(other) == 0 and len(structure) == 0: + return ([], []) + + toset = [] + todel = [] + for key in structure.keys(): + quoted_key = self.quote_key(key) + if key in other: + # keys in both configs, pls compare subkeys + (subset, subdel) = self.diff_to(other[key], structure[key]) + for s in subset: + toset.append(quoted_key + " " + s) + for d in subdel: + todel.append(quoted_key + " " + d) + else: + # keys only in this, delete if KEEP_EXISTING_VALUES not set + if KEEP_EXISTING_VALUES not in other: + todel.append(quoted_key) + continue # del + for key, value in other.items(): + if key == KEEP_EXISTING_VALUES: + continue + quoted_key = self.quote_key(key) + if key not in structure: + # keys only in other, pls set all subkeys + (subset, subdel) = self.diff_to(other[key], None) + for s in subset: + toset.append(quoted_key + " " + s) + + return (toset, todel) + + def diff_commands_to(self, other): + """ + This function calculates the required commands to change the current into + the given configuration. Only top-level sections present in the desired + configuration are enforced; top-level sections the candidate does not + mention at all are left completely untouched. + :param other: VyosConf + :return: [str] + """ + scoped_structure = {k: v for k, v in self.config.items() if k in other.config} + (toset, todel) = self.diff_to(other.config, scoped_structure) + return ["delete " + c.strip() for c in todel] + ["set " + c.strip() for c in toset] diff --git a/plugins/module_utils/network/vyos/vyos.py b/plugins/module_utils/network/vyos/vyos.py index 92c0f1b8..4983221e 100644 --- a/plugins/module_utils/network/vyos/vyos.py +++ b/plugins/module_utils/network/vyos/vyos.py @@ -27,12 +27,14 @@ # from __future__ import absolute_import, division, print_function + __metaclass__ = type import json from ansible.module_utils._text import to_text from ansible.module_utils.connection import Connection, ConnectionError + _DEVICE_CONFIGS = {} @@ -85,6 +87,30 @@ def get_config(module, flags=None, format=None): return cfg +def copy_file(module, source, destination, proto="scp"): + """Copy a local file to the remote device over the existing network_cli + SSH session, using netcommon's generic connection-level file transfer + RPC (the same mechanism ansible.netcommon.net_put uses). + + Requires the device to have SCP/SFTP reachable over the same SSH + session used for network_cli. Mirrors the calling convention of + cisco.iosxr's module_utils copy_file(module, source, destination, proto), + confirmed against cisco.iosxr's iosxr_config.py call site: + copy_file(module, src, dst, "sftp"). + """ + connection = get_connection(module) + try: + timeout = connection.get_option("persistent_command_timeout") + connection.copy_file( + source=source, + destination=destination, + proto=proto, + timeout=timeout, + ) + except ConnectionError as exc: + module.fail_json(msg=to_text(exc, errors="surrogate_then_replace")) + + def run_commands(module, commands, check_rc=True): connection = get_connection(module) try: @@ -99,7 +125,10 @@ def load_config(module, commands, commit=False, comment=None, confirm=None): try: response = connection.edit_config( - candidate=commands, commit=commit, comment=comment, confirm=confirm + candidate=commands, + commit=commit, + comment=comment, + confirm=confirm, ) except ConnectionError as exc: module.fail_json(msg=to_text(exc, errors="surrogate_then_replace")) diff --git a/plugins/modules/vyos_config.py b/plugins/modules/vyos_config.py index 46e51f3b..53f8e043 100644 --- a/plugins/modules/vyos_config.py +++ b/plugins/modules/vyos_config.py @@ -17,6 +17,7 @@ # from __future__ import absolute_import, division, print_function + __metaclass__ = type @@ -37,6 +38,9 @@ notes: - This module works with connection C(ansible.netcommon.network_cli). See L(the VyOS OS Platform Options,../network/user_guide/platform_vyos.html). - To ensure idempotency and correct diff the configuration lines in the relevant module options should be similar to how they appear if present in the running configuration on device including the indentation. +- C(replace=config) currently has no way to scope its effect to part of the + configuration; it always operates against the entire device configuration. + There is no C(path) parameter to constrain it to a subtree. options: lines: description: @@ -45,6 +49,7 @@ options: device running-config to ensure idempotency and correct diff. Be sure to note the configuration command syntax as some commands are automatically modified by the device config parser. + - Not supported when C(replace) is set to C(config) -- see C(replace) below. type: list elements: str src: @@ -54,18 +59,38 @@ options: file can include Jinja2 template variables. The configuration lines in the source file should be similar to how it will appear if present in the running-configuration of the device including indentation to ensure idempotency and correct diff. + - When C(replace) is set to C(config), C(src) is required and must contain a + complete configuration in hierarchical/bracket format -- the same format + produced by C(show configuration) or found in C(/config/config.boot). Flat + C(set)/C(delete) command format (as produced by C(show configuration + commands)) is not accepted in that mode; VyOS's native C(load) command + rejects it with a parse error. type: path match: description: - The C(match) argument controls the method used to match against the current active configuration. By default, the desired config is matched against the active config and the deltas are loaded. If the C(match) argument is set to - C(none) the active configuration is ignored and the configuration is always - loaded. + C(none), the active configuration is ignored and the configuration is always + loaded. If the C(match) argument is set to C(enforce), the supplied C(lines) + or C(src) are treated as the complete desired end-state of the configuration, + rather than a set of deltas to apply. + C(enforce) enforces only the top-level configuration + sections present in the supplied candidate as complete end-states; + existing configuration within those sections but not mentioned in the + candidate is removed, so C(enforce) can generate C(delete) commands for + configuration the candidate does not mention. Top-level sections the + candidate does not reference at all are left completely untouched. + C(enforce) is intended for candidates made up of C(set) commands only; + supplying C(delete) lines alongside C(match=enforce) is not supported + and will raise an error. + - Ignored when C(replace) is set to C(config), since no line-level diff is + computed in that mode. type: str default: line choices: - line + - enforce - none backup: description: @@ -75,7 +100,7 @@ options: the playbook root directory or role root directory, if playbook is part of an ansible role. If the directory does not exist, it is created. type: bool - default: false + default: no comment: description: - Allows a commit description to be specified to be included when the configuration @@ -90,8 +115,11 @@ options: this module will automatically confirm the configuration, if the current session remains working with the new config. When set to C(manual), this module does not issue the confirmation itself. + - Defaults to C(automatic) when C(match) is set to C(enforce), since C(enforce) + can generate C(delete) commands for configuration not mentioned in the + candidate and a bad commit should self-revert rather than leave the device + unreachable. Defaults to C(none) for all other C(match) values. type: str - default: none choices: - automatic - manual @@ -110,19 +138,20 @@ options: The configuration lines in the option value should be similar to how it will appear if present in the running-configuration of the device including indentation to ensure idempotency and correct diff. + - Ignored when C(replace) is set to C(config). type: str save: description: - The C(save) argument controls whether or not changes made to the active configuration are saved to disk. This is independent of committing the config. When set - to C(true), the active configuration is saved. + to C(True), the active configuration is saved. type: bool - default: false + default: no backup_options: description: - This is a dict object containing configurable options related to backup file - path. The value of this option is read only when C(backup) is set to C(true), - if C(backup) is set to C(false) this option will be silently ignored. + path. The value of this option is read only when C(backup) is set to I(yes), + if C(backup) is set to I(no) this option will be silently ignored. suboptions: filename: description: @@ -141,6 +170,62 @@ options: in C(filename) within I(backup) directory. type: path type: dict + replace: + description: + - Controls how the module applies configuration to the device. + - When set to C(line) (default), the module computes a set/delete command + diff and pushes only the changed lines -- this is the existing behavior, + unchanged. + - When set to C(config), the module uploads the full candidate configuration + (C(src)) to the device and issues VyOS's native C(load) command in + configuration mode, which replaces the running configuration wholesale + with the candidate's exact contents. VyOS's own configuration engine + performs the reconciliation, rather than the module computing per-line + deltas. This mirrors the mechanism offered by C(cisco.iosxr.iosxr_config)'s + C(replace=config). + - C(replace=config) requires C(src) and does not accept C(lines) -- there is + no way to convert flat set/delete commands into the hierarchical form + C(load) requires without re-implementing VyOS's own config-tree builder. + - As with C(src) in the default C(line) mode, the module does not validate + the candidate's contents or format under C(replace=config) -- supplying a + well-formed, complete configuration is the caller's responsibility. + - C(replace=config) requires the device to accept file transfer (SCP) over + the same C(network_cli) SSH session used for configuration commands. + - C(replace=config) writes the candidate to a fixed path on the device + (overwritten on each run, matching C(cisco.iosxr.iosxr_config)'s own + C(replace=config) precedent). Running C(replace=config) concurrently + against the same host is not supported. + - Any configuration present on the device but omitted from the candidate + will be removed, including management interfaces, SSH access, and login + users if they are omitted. Always supply a complete configuration, never + a partial one. + - When capturing a candidate from the device's own output (for example + via C(show configuration)) rather than from a trusted, separately + maintained source, be aware that VyOS may return masked placeholder + values (for example a run of literal asterisks) in place of local + users' C(encrypted-password)/C(plaintext-password) values when queried + through automation, even though the identical command returns the real + value when typed interactively at a terminal. Pushing a masked capture + back through C(replace=config) sends the literal placeholder as the new + password value; VyOS's own commit-time validation is expected to reject + an obviously malformed hash, but a masked value that happens to pass + basic format validation could apply silently. Prefer sourcing + C(replace=config) candidates from a trusted, version-controlled + artifact rather than a live automated capture whenever the + configuration contains local password-based users. + - Even under C(check_mode), the candidate is written to a temporary file on + the device so that VyOS's own C(compare) can produce an accurate preview + diff. No C(commit) occurs in check mode. + - When combined with C(backup=yes), the value of C(changed) reflects + whether the backup file's content changed on the Ansible control node, + not whether the device configuration changed -- this is existing + behavior in the shared netcommon action plugin backing config-family + modules across collections, not specific to C(replace=config). + type: str + default: line + choices: + - line + - config allow_password_change: description: - The C(allow_password_change) argument specifies whether any configuration lines which @@ -148,6 +233,9 @@ options: password changes are allowed and any encrypted-password keys are filtered out. In order to allow all password updates, both plaintext and encrypted, set this argument to C(all). + - Not applied when C(replace) is set to C(config); the candidate is loaded + as-is via VyOS's native C(load), which has no equivalent filtering + mechanism. type: str default: plaintext choices: @@ -172,6 +260,7 @@ EXAMPLES = """ - name: render a Jinja2 template onto the VyOS router vyos.vyos.vyos_config: + match: enforce src: vyos_template.j2 - name: revert after ten minutes, if connection is lost @@ -191,16 +280,40 @@ EXAMPLES = """ backup_options: filename: backup.cfg dir_path: /home/user + +- name: capture the complete hierarchical configuration for editing + # replace=config requires the complete desired configuration in + # hierarchical/bracket format -- never a partial one, and never flat + # set-command format. `backup: true` alone won't work here: it captures + # flat set-command output (via `show configuration commands`), which + # replace=config's underlying `load` command rejects. Capture the + # hierarchical form directly instead, edit it, then replace with the + # edited whole, as shown here. + vyos.vyos.vyos_command: + commands: "show configuration" + register: current_config + +- name: (edit current_config.stdout[0] as needed, save it locally, then) + vyos.vyos.vyos_config: + src: /home/user/edited_config.cfg + replace: config """ RETURN = """ commands: - description: The list of configuration commands sent to the device + description: + - In C(replace=line) mode (default), the list of set/delete commands sent to + the device. + - In C(replace=config) mode, contains only the single C(load <path>) command + actually issued to the device -- not an itemized diff. See C(diff) for the + actual change content, sourced from VyOS's own C(compare) output. returned: always type: list sample: ['...', '...'] filtered: - description: The list of configuration commands removed to avoid a load failure + description: + - The list of configuration commands removed to avoid a load failure. + - Not populated when C(replace) is set to C(config). returned: always type: list sample: ['...', '...'] @@ -230,25 +343,68 @@ time: type: str sample: "22:28:34" """ +import os import re +import tempfile -from ansible.module_utils._text import to_text +from ansible.module_utils._text import to_bytes, to_text from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.connection import ConnectionError from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.vyos import ( + copy_file, get_config, get_connection, load_config, run_commands, ) + DEFAULT_COMMENT = "configured by vyos_config" PASSWORD_NEEDLE = re.compile( - r"set system login user \S+ authentication (encrypted|plaintext)-password", + r"(?:set|delete) system login user \S+ authentication (encrypted|plaintext)-password", ) +# diff_match=enforce's scoping can collapse an entire untouched subtree into +# a single parent delete (e.g. "delete system login" when a candidate +# touches system without restating login, or "delete system login user +# admin" without a specific authentication line). PASSWORD_NEEDLE can't see +# into a collapsed delete to know whether it removes a password -- since +# real users almost always have one configured, treat any subtree-level +# login deletion as password-bearing by default, same conservative stance +# as PASSWORD_NEEDLE itself. +LOGIN_SUBTREE_DELETE_NEEDLE = re.compile( + r"^delete system login(?:\s+user\s+\S+(?:\s+authentication)?)?\s*$", +) + + +def sanitize_config(config, result, allow): + result["filtered"] = list() + + if allow == "all": + return + + index_to_filter = list() + + for index, line in enumerate(list(config)): + found = PASSWORD_NEEDLE.search(line) + + if found is not None: + if allow == found[1]: + continue + result["filtered"].append(line) + index_to_filter.append(index) + continue + + if LOGIN_SUBTREE_DELETE_NEEDLE.match(line.strip()): + result["filtered"].append(line) + index_to_filter.append(index) + + # Delete all filtered configs + for filter_index in sorted(index_to_filter, reverse=True): + del config[filter_index] + def get_candidate(module): contents = module.params["src"] or module.params["lines"] @@ -306,31 +462,6 @@ def diff_config(commands, config): return list(updates) -def sanitize_config(config, result, allow): - result["filtered"] = list() - - if allow == "all": - return - - index_to_filter = list() - - for index, line in enumerate(list(config)): - found = PASSWORD_NEEDLE.search(line) - - if found is None: - continue - - if allow == found[1]: - continue - - result["filtered"].append(line) - index_to_filter.append(index) - - # Delete all filtered configs - for filter_index in sorted(index_to_filter, reverse=True): - del config[filter_index] - - def run(module, result): # get the current active config from the node or passed in via # the config param @@ -358,16 +489,20 @@ def run(module, result): result["commands"] = commands + confirm_param = module.params["confirm"] + if confirm_param is None: + confirm_param = "automatic" if module.params["match"] == "enforce" else "none" + commit = not module.check_mode comment = module.params["comment"] confirm = None - if module.params["confirm"] == "automatic" or module.params["confirm"] == "manual": + if confirm_param in ("automatic", "manual"): confirm = module.params["confirm_timeout"] diff = None if commands: diff = load_config(module, commands, commit=commit, comment=comment, confirm=confirm) - if module.params["confirm"] == "automatic": + if confirm_param == "automatic" and not module.check_mode: run_commands(module, ["configure", "confirm", "exit"]) if result.get("filtered"): @@ -381,19 +516,92 @@ def run(module, result): result["diff"] = {"prepared": diff} +def run_replace_config(module, result): + # replace=config: push the full candidate to the device and let VyOS's + # own `load` command perform the replacement natively, rather than + # computing a set/delete diff in Python. + # + # Deliberately smaller than cisco.iosxr's equivalent implementation: + # - No bidirectional pre-diff to decide whether anything changed -- + # confirmed on real VyOS 1.5 hardware that `load` of an + # already-applied file, followed by `compare`, natively reports + # "No changes between working and active configurations" with no + # Python-side pre-check needed. + # - No special `replace=<path>` argument threaded through load_config()/ + # edit_config() -- confirmed that `load <path>` behaves as an ordinary + # configuration command through the existing configure/compare/commit + # flow already implemented in Cliconf.edit_config(), unmodified. + # + # Candidate format requirement (hierarchical/bracket, not flat + # set/delete) is enforced by VyOS's own `load` parser, not by this + # module -- confirmed empirically: flat set-command input produces + # "ValueError: Failed to parse config: Syntax error...". + # module.params["src"] is already the rendered file *content* by this + # point, not a path -- netcommon's generic action plugin for src-based + # network config modules reads the local file and substitutes its + # (Jinja2-rendered) content into this param before the module runs. Same + # assumption get_candidate() already relies on elsewhere in this file. + candidate = to_bytes(module.params["src"], errors="surrogate_or_strict") + + tmp = tempfile.NamedTemporaryFile(delete=False) + local_path = tmp.name + try: + tmp.write(candidate) + tmp.close() + + # Fixed remote filename, always overwritten -- same precedent as + # cisco.iosxr.iosxr_config's copy_file_to_node(), which always + # writes to the same "/harddisk:/ansible_config.txt". Avoids + # per-run temp-file accumulation on the device, at the accepted + # cost (shared with iosxr_config) that two concurrent replace=config + # runs against the same host could race on this path. + remote_path = "/tmp/ansible_vyos_replace.cfg" + copy_file(module, local_path, remote_path, "scp") + finally: + os.unlink(local_path) + + confirm_param = module.params["confirm"] + if confirm_param is None: + confirm_param = "none" + + commit = not module.check_mode + comment = module.params["comment"] + confirm = None + if confirm_param in ("automatic", "manual"): + confirm = module.params["confirm_timeout"] + + diff = load_config( + module, + ["load %s" % remote_path], + commit=commit, + comment=comment, + confirm=confirm, + ) + if confirm_param == "automatic" and diff and not module.check_mode: + run_commands(module, ["configure", "confirm", "exit"]) + + result["commands"] = ["load %s" % remote_path] + result["filtered"] = [] + result["changed"] = bool(diff) + + if module._diff: + result["diff"] = {"prepared": diff} + + def main(): backup_spec = dict(filename=dict(), dir_path=dict(type="path")) argument_spec = dict( src=dict(type="path"), lines=dict(type="list", elements="str"), - match=dict(default="line", choices=["line", "none"]), + match=dict(default="line", choices=["line", "enforce", "none"]), comment=dict(default=DEFAULT_COMMENT), - confirm=dict(choices=["automatic", "manual", "none"], default="none"), + confirm=dict(choices=["automatic", "manual", "none"], default=None), confirm_timeout=dict(type="int", default=10), config=dict(), backup=dict(type="bool", default=False), backup_options=dict(type="dict", options=backup_spec), save=dict(type="bool", default=False), + replace=dict(type="str", default="line", choices=["line", "config"]), allow_password_change=dict( default="plaintext", choices=["all", "encrypted", "plaintext", "none"], @@ -401,10 +609,12 @@ def main(): ) mutually_exclusive = [("lines", "src")] + required_if = [("replace", "config", ["src"])] module = AnsibleModule( argument_spec=argument_spec, mutually_exclusive=mutually_exclusive, + required_if=required_if, supports_check_mode=True, ) @@ -415,7 +625,9 @@ def main(): if module.params["backup"]: result["__backup__"] = get_config(module=module) - if any((module.params["src"], module.params["lines"])): + if module.params["replace"] == "config": + run_replace_config(module, result) + elif any((module.params["src"], module.params["lines"])): run(module, result) if module.params["save"]: @@ -429,7 +641,11 @@ def main(): result["changed"] = True run_commands(module, commands=["exit"]) - if result.get("changed") and any((module.params["src"], module.params["lines"])): + if ( + result.get("changed") + and module.params["replace"] != "config" + and any((module.params["src"], module.params["lines"])) + ): msg = ( "To ensure idempotency and correct diff the input configuration lines should be" " similar to how they appear if present in" diff --git a/tests/integration/targets/vyos_config/templates/replace_config_candidate.cfg b/tests/integration/targets/vyos_config/templates/replace_config_candidate.cfg new file mode 100644 index 00000000..4a29cdea --- /dev/null +++ b/tests/integration/targets/vyos_config/templates/replace_config_candidate.cfg @@ -0,0 +1 @@ +{{ replace_config_candidate_content }} diff --git a/tests/integration/targets/vyos_config/tests/cli/enforce.yaml b/tests/integration/targets/vyos_config/tests/cli/enforce.yaml new file mode 100644 index 00000000..8fa0e61b --- /dev/null +++ b/tests/integration/targets/vyos_config/tests/cli/enforce.yaml @@ -0,0 +1,124 @@ +--- +- debug: msg="START cli/enforce.yaml on connection={{ ansible_connection }}" + +# SAFETY: this file exercises match=enforce's full end-state enforcement, +# which fully enforces every top-level section the candidate touches. +# NEVER include any "system ..." line in an enforce candidate below -- +# system login (including the currently-authenticated user) lives under +# that top-level section, and enforce would attempt to delete it if not +# restated. service ssh must also be restated in every candidate that +# touches "service", or the module's built-in guard refuses the run +# (see the dedicated guard test near the end of this file). + +- name: setup baseline config + vyos.vyos.vyos_config: + lines: + - set system host-name {{ inventory_hostname_short }} + - set service lldp + - set protocols static + - set service ssh port 22 + match: none + +- block: + - name: enforce end-state with match=enforce (should remove lldp, keep static+ssh, add ntp) + register: result + vyos.vyos.vyos_config: + lines: + - set protocols static + - set service ssh port 22 + - set service ntp server 192.0.2.1 + match: enforce + + - assert: + that: + - result.changed == true + - "'delete service lldp' in result.commands" + - "'set service ntp server 192.0.2.1' in result.commands" + - "'delete protocols static' not in result.commands" + - "'delete service ssh port 22' not in result.commands" + + - name: check match=enforce is idempotent against the same end-state + register: result + vyos.vyos.vyos_config: + lines: + - set protocols static + - set service ssh port 22 + - set service ntp server 192.0.2.1 + match: enforce + + - assert: + that: + - result.changed == false + + - name: match=enforce tolerates blank lines and comments in the candidate + register: result + vyos.vyos.vyos_config: + lines: + - "# this is a comment" + - "" + - set protocols static + - set service ssh port 22 + - set service ntp server 192.0.2.1 + match: enforce + + - assert: + that: + - result.changed == false + + - name: match=enforce rejects an incomplete set command + register: result + ignore_errors: true + vyos.vyos.vyos_config: + lines: + - set service ssh port 22 + - set + match: enforce + + - assert: + that: + - result.failed == true + + - name: match=enforce rejects delete lines in the candidate + register: result + ignore_errors: true + vyos.vyos.vyos_config: + lines: + - set service ssh port 22 + - delete protocols static + match: enforce + + - assert: + that: + - result.failed == true + + - name: match=enforce refuses a candidate that would delete service ssh + register: result + ignore_errors: true + vyos.vyos.vyos_config: + lines: + - set service ntp server 192.0.2.1 + match: enforce + + - assert: + that: + - result.failed == true + - "'delete service ssh' in result.msg" + + always: + - name: teardown + vyos.vyos.vyos_config: + lines: + - set system host-name {{ inventory_hostname_short }} + - set service ssh port 22 + match: none + + - name: remove leftover test config + vyos.vyos.vyos_config: + lines: + - delete service ntp + - delete protocols static + - delete service lldp + match: none + ignore_errors: true + +- debug: msg="END cli/enforce.yaml on connection={{ ansible_connection }}" diff --git a/tests/integration/targets/vyos_config/tests/cli/replace_config.yaml b/tests/integration/targets/vyos_config/tests/cli/replace_config.yaml new file mode 100644 index 00000000..1e208275 --- /dev/null +++ b/tests/integration/targets/vyos_config/tests/cli/replace_config.yaml @@ -0,0 +1,289 @@ +--- +- debug: msg="START cli/replace_config.yaml on connection={{ ansible_connection }}" + +# SAFETY NOTE: replace=config requires a COMPLETE configuration in +# hierarchical/bracket format (the same format `show configuration` / +# /config/config.boot use) -- never flat set/delete commands (VyOS's `load` +# rejects those with a parse error) and never a partial candidate (anything +# omitted is deleted, including management interfaces, SSH, and login +# users). Every step below operates on a full baseline captured live from +# the device itself, edited in place -- never a minimal hand-written or +# statically-shipped candidate. This also sidesteps a real safety concern: +# a checked-in static fixture would either need fake device-specific data +# (interface hw-ids, password hashes) that's wrong for every real target, +# or genuine credentials baked into git -- capturing live avoids both. +# +# NOTE on `src` / templates/replace_config_candidate.cfg: this file's +# entire content is the single Jinja2 expression +# "{{ replace_config_candidate_content }}". netcommon's action plugin +# (_handle_src_option) reads whatever `src` points to and renders it as a +# Jinja2 template using the play's own templar *before* the module runs -- +# this lets us point `src` at one static, checked-in, device-agnostic file +# for every task below, and change only the in-memory fact +# (replace_config_candidate_content) each time, with zero local file +# writes and therefore no delegate_to/connection wrangling at all. +# This mechanism is deprecated upstream (removal after 2028-01-01) in favor +# of a `content:` module parameter -- cisco.iosxr.iosxr_config already has +# one; vyos_config does not yet. Adding `content:` to vyos_config, mirroring +# iosxr_config, would let this test drop the deprecated path entirely, but +# that's real module-feature scope beyond this PR, not test plumbing. +# +# NOTE on `backup`: intentionally not combined with the idempotency +# assertions in this file. `backup=yes`'s `changed` reflects whether the +# backup file on the control node changed, not whether the device changed +# (see DOCUMENTATION) -- this is pre-existing, shared netcommon action +# plugin behavior, not specific to replace=config, and asserting on it here +# would conflate two unrelated things. +# +# NOTE on teardown: this file captures the device's true pre-test SAVED +# configuration *before* making any changes (original_config_capture below) +# and restores exactly that via the same replace=config mechanism the rest +# of the file exercises, inside an `always` block -- so a failure partway +# through (not just a clean run) still leaves the device as it was found, +# rather than at a hard-coded value that may differ from whatever was +# actually there originally. +# +# NOTE on unsaved changes: the restore above is only correct if the device +# had NO unsaved running-config changes before this test started -- this +# file cannot safely capture or restore the live running config (see the +# masked-secrets note below), only the saved state in config.boot. The +# pre-test check immediately below fails fast rather than silently +# discarding any pre-existing unsaved work. + +- name: check for pre-existing unsaved changes before this test begins + vyos.vyos.vyos_command: + commands: + - configure + - compare saved + - exit + register: pre_test_save_diff + +- name: fail fast if the device has unsaved changes before this test starts + ansible.builtin.assert: + that: + - >- + pre_test_save_diff.stdout[1] in + ['[edit]', 'No changes between working and saved configurations.\n\n[edit]'] + fail_msg: >- + This device has unsaved configuration changes from before this test + started. Teardown restores from a capture of /config/config.boot (the + saved state) taken before setup, and cannot safely preserve unsaved + running-config changes that predate this test -- doing so would + silently discard them. Save or discard any pending changes on this + device before running this test. + +- name: capture original full config before any changes, for teardown restoration + vyos.vyos.vyos_command: + commands: "cat /config/config.boot" + register: original_config_capture + +- name: safety check -- fail fast if the device masked secrets in the original capture + ansible.builtin.assert: + that: + - "'****************' not in original_config_capture.stdout[0]" + fail_msg: >- + show configuration returned masked secret placeholders in this + capture. This is a known VyOS behavior when querying via automation + (see DOCUMENTATION note on replace=config); this test cannot safely + continue with a masked candidate. Re-run against a lab image/session + where show configuration returns real values, or adjust the baseline + capture method. + +- block: + - name: setup baseline marker value + vyos.vyos.vyos_config: + lines: + - set system option reboot-on-upgrade-failure '7' + match: none + # save=true is required here: commit alone updates the running config + # but not /config/config.boot, which our baseline capture below reads + # directly. Without an explicit save, config.boot can reflect a stale + # value from any earlier save in the device's history rather than what + # was just committed -- confirmed as the root cause of a real failure + # during development (a stale, unrelated value silently made it into + # the candidate instead of this task's freshly-committed one). + save: true + + - name: capture full hierarchical baseline config from the device + # cat /config/config.boot directly, rather than `show configuration` -- + # confirmed during development that `show configuration` returns masked + # placeholder values for local users' password hashes when queried via + # automation (vyos_command/network_cli), even though the same rendering + # shows real values when typed interactively. /config/config.boot is the + # same underlying file `show configuration` renders (confirmed identical + # structure during development), read directly rather than through + # VyOS's `show` masking layer. + vyos.vyos.vyos_command: + commands: "cat /config/config.boot" + register: baseline_show + + # VyOS appears to mask local users' encrypted-password/plaintext-password + # values (a run of literal asterisks) specifically when show configuration + # is queried through automation (vyos_command/network_cli), even though the + # identical command returns the real hash when typed interactively at a + # terminal -- observed directly during development of this test. Pushing a + # masked capture back through replace=config sends the literal placeholder + # as the new password value and fails at commit (see DOCUMENTATION for the + # broader implication of this for replace=config generally). Fail fast here + # with a clear, actionable message rather than letting that surface as a + # confusing device-side "Invalid encrypted password" commit failure deep + # inside the actual test. + - name: safety check -- fail fast if the device masked secrets in this capture + ansible.builtin.assert: + that: + - "'****************' not in baseline_show.stdout[0]" + fail_msg: >- + show configuration returned masked secret placeholders in this + capture. This is a known VyOS behavior when querying via automation + (see DOCUMENTATION note on replace=config); this test cannot safely + continue with a masked candidate. Re-run against a lab image/session + where show configuration returns real values, or adjust the baseline + capture method. + + - name: build edited candidate content (v1 -- change the marker value only) + ansible.builtin.set_fact: + edited_candidate_v1: >- + {{ baseline_show.stdout[0] + | replace('reboot-on-upgrade-failure "7"', 'reboot-on-upgrade-failure "12"') }} + + # Fail fast here if the substitution above silently didn't match -- + # config.boot quotes leaf values (reboot-on-upgrade-failure "7"), and a + # search string that misses that quoting produces a silent no-op edit + # rather than an error, which is a real bug caught during development: + # the candidate ends up byte-identical to the (possibly stale) capture, + # and any resulting diff reflects leftover device state rather than this + # test's intended edit. + - name: sanity check the marker edit actually took effect + ansible.builtin.assert: + that: + - ('reboot-on-upgrade-failure "12"' in edited_candidate_v1) + - ('reboot-on-upgrade-failure "7"' not in edited_candidate_v1) + fail_msg: >- + The marker substitution did not match anything in the captured + baseline -- edited_candidate_v1 is identical to the raw capture. + Check that the baseline actually contains + reboot-on-upgrade-failure "7" (quoted) and that the setup task's + save=true actually persisted before this capture ran. + + - name: sanity check candidate still contains SSH/management essentials + ansible.builtin.assert: + that: + - "'service' in edited_candidate_v1" + - "'ssh' in edited_candidate_v1" + - "'login' in edited_candidate_v1" + + - name: replace with edited full config (native load) + register: result + diff: true + vyos.vyos.vyos_config: + src: "{{ role_path }}/templates/replace_config_candidate.cfg" + replace: config + vars: + replace_config_candidate_content: "{{ edited_candidate_v1 }}" + + - assert: + that: + - result.changed == true + - result.commands == ["load /tmp/ansible_vyos_replace.cfg"] + # replace=config surfaces VyOS's own compare() output verbatim in + # diff, not an itemized set/delete list -- commands is deliberately + # not the change content in this mode (see RETURN docs). Checking + # the specific new value (not just the field name) is deliberate -- + # a weaker check here previously let a wrong-value regression through + # undetected for several tasks. + - ('reboot-on-upgrade-failure "12"' in (result.diff.prepared | default(''))) + + - name: verify connectivity survived + vyos.vyos.vyos_facts: + gather_subset: min + register: facts_check + + - assert: + that: + - facts_check is succeeded + + - name: re-run the identical replace (idempotency check, no backup involved) + register: result_repeat + vyos.vyos.vyos_config: + src: "{{ role_path }}/templates/replace_config_candidate.cfg" + replace: config + vars: + replace_config_candidate_content: "{{ edited_candidate_v1 }}" + + - assert: + that: + - result_repeat.changed == false + + - name: confirm replace=config rejects a candidate without src + register: no_src_result + ignore_errors: true + vyos.vyos.vyos_config: + replace: config + + - assert: + that: + - no_src_result is failed + - "'src' in no_src_result.msg" + + - name: confirm replace=config rejects lines+src together + register: lines_and_src_result + ignore_errors: true + vyos.vyos.vyos_config: + replace: config + src: "{{ role_path }}/templates/replace_config_candidate.cfg" + lines: + - "set system host-name foo" + vars: + replace_config_candidate_content: "{{ edited_candidate_v1 }}" + + - assert: + that: + - lines_and_src_result is failed + + - name: build edited candidate content (v2 -- further marker change, for check_mode) + ansible.builtin.set_fact: + edited_candidate_v2: >- + {{ edited_candidate_v1 + | replace('reboot-on-upgrade-failure "12"', 'reboot-on-upgrade-failure "20"') }} + + - name: sanity check the v2 marker edit actually took effect + ansible.builtin.assert: + that: + - ('reboot-on-upgrade-failure "20"' in edited_candidate_v2) + - ('reboot-on-upgrade-failure "12"' not in edited_candidate_v2) + + - name: check_mode preview does not commit + check_mode: true + diff: true + register: check_result + vyos.vyos.vyos_config: + src: "{{ role_path }}/templates/replace_config_candidate.cfg" + replace: config + vars: + replace_config_candidate_content: "{{ edited_candidate_v2 }}" + + - assert: + that: + - check_result.changed == true + - ('reboot-on-upgrade-failure "20"' in (check_result.diff.prepared | default(''))) + + - name: confirm check_mode preview above was not actually applied + vyos.vyos.vyos_command: + commands: "show configuration commands | match reboot-on-upgrade-failure" + register: post_check_value + + - assert: + that: + - "'20' not in post_check_value.stdout[0]" + - "'12' in post_check_value.stdout[0]" + + always: + - name: teardown -- restore the true original saved configuration captured before any changes + vyos.vyos.vyos_config: + src: "{{ role_path }}/templates/replace_config_candidate.cfg" + replace: config + save: true + vars: + replace_config_candidate_content: "{{ original_config_capture.stdout[0] }}" + +- debug: msg="END cli/replace_config.yaml on connection={{ ansible_connection }}" diff --git a/tests/unit/cliconf/__init__.py b/tests/unit/cliconf/__init__.py new file mode 100644 index 00000000..e69de29b --- /dev/null +++ b/tests/unit/cliconf/__init__.py diff --git a/tests/unit/cliconf/test_utils_vyosconf.py b/tests/unit/cliconf/test_utils_vyosconf.py new file mode 100644 index 00000000..dbc296e6 --- /dev/null +++ b/tests/unit/cliconf/test_utils_vyosconf.py @@ -0,0 +1,217 @@ +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see <http://www.gnu.org/licenses/>. +# +from __future__ import absolute_import, division, print_function + + +__metaclass__ = type + +import unittest + +from ansible_collections.vyos.vyos.plugins.cliconf_utils.vyosconf import ( + KEEP_EXISTING_VALUES, + VyosConf, +) + + +class TestListElements(unittest.TestCase): + def test_add(self): + conf = VyosConf() + conf.set_entry(["a", "b"], "c") + self.assertEqual(conf.config, {"a": {"b": {"c": {}}}}) + conf.set_entry(["a", "b"], "d") + self.assertEqual(conf.config, {"a": {"b": {"c": {}, "d": {}}}}) + conf.set_entry(["a", "c"], "b") + self.assertEqual( + conf.config, + {"a": {"b": {"c": {}, "d": {}}, "c": {"b": {}}}}, + ) + conf.set_entry(["a", "c", "b"], "d") + self.assertEqual( + conf.config, + {"a": {"b": {"c": {}, "d": {}}, "c": {"b": {"d": {}}}}}, + ) + + def test_del(self): + conf = VyosConf() + conf.set_entry(["a", "b"], "c") + conf.set_entry(["a", "c", "b"], "d") + conf.set_entry(["a", "b"], "d") + self.assertEqual( + conf.config, + {"a": {"b": {"c": {}, "d": {}}, "c": {"b": {"d": {}}}}}, + ) + conf.del_entry(["a", "c", "b"], "d") + self.assertEqual(conf.config, {"a": {"b": {"c": {}, "d": {}}}}) + conf.set_entry(["a", "b", "c"], "d") + conf.del_entry(["a", "b", "c"], "d") + self.assertEqual(conf.config, {"a": {"b": {"d": {}}}}) + + def test_del_missing_leaf_is_noop(self): + """ + Deleting a leaf that was never set must leave the config unchanged. + Regression test: del_entry() used to raise KeyError when the leaf's + parent had siblings, and could delete an unrelated ancestor subtree + (or the entire config) when the parent path had no siblings. + """ + # parent has siblings: previously raised KeyError + conf = VyosConf() + conf.set_entry(["a", "b"], "c") + conf.set_entry(["a", "b"], "d") + conf.del_entry(["a", "b"], "nonexistent") + self.assertEqual(conf.config, {"a": {"b": {"c": {}, "d": {}}}}) + + # parent path is an unbranched chain: previously deleted the + # entire config instead of no-op'ing + conf = VyosConf() + conf.set_entry(["a", "b"], "d") + conf.del_entry(["a", "b"], "c") + self.assertEqual(conf.config, {"a": {"b": {"d": {}}}}) + + # missing intermediate path element already behaved correctly; + # confirm it still does + conf = VyosConf() + conf.set_entry(["a", "b"], "c") + conf.del_entry(["a", "x"], "c") + self.assertEqual(conf.config, {"a": {"b": {"c": {}}}}) + + def test_parse(self): + conf = VyosConf() + self.assertListEqual( + conf.parse_line("set a b c"), + ["set", ["a", "b"], "c"], + ) + self.assertListEqual( + conf.parse_line('set a b "c"'), + ["set", ["a", "b"], "c"], + ) + self.assertListEqual( + conf.parse_line("set a b 'c d'"), + ["set", ["a", "b"], "c d"], + ) + self.assertListEqual( + conf.parse_line("set a b 'c'"), + ["set", ["a", "b"], "c"], + ) + self.assertListEqual( + conf.parse_line("delete a b 'c'"), + ["delete", ["a", "b"], "c"], + ) + self.assertListEqual( + conf.parse_line("del a b 'c'"), + ["del", ["a", "b"], "c"], + ) + self.assertListEqual( + conf.parse_line("set a b '\"c'"), + ["set", ["a", "b"], '"c'], + ) + self.assertListEqual( + conf.parse_line("set a b 'c' #this is a comment"), + ["set", ["a", "b"], "c"], + ) + self.assertListEqual( + conf.parse_line("set a b '#c'"), + ["set", ["a", "b"], "#c"], + ) + + def test_run_commands(self): + self.assertEqual( + VyosConf(["set a b 'c'", "set a c 'b'"]).config, + {"a": {"b": {"c": {}}, "c": {"b": {}}}}, + ) + self.assertEqual( + VyosConf(["set a b c 'd'", "set a c 'b'", "del a b c d"]).config, + {"a": {"c": {"b": {}}}}, + ) + + def test_build_commands(self): + self.assertEqual( + sorted( + VyosConf( + [ + "set a b 'c a'", + "set a c a", + "set a c b", + "delete a c a", + ], + ).build_commands(), + ), + sorted(["set a b 'c a'", "set a c b"]), + ) + self.assertEqual( + sorted( + VyosConf( + [ + "set a b 10.0.0.1/24", + "set a c ABCabc123+/=", + "set a d $6$ABC.abc.123.+./=..", + ], + ).build_commands(), + ), + sorted( + [ + "set a b 10.0.0.1/24", + "set a c 'ABCabc123+/='", + "set a d '$6$ABC.abc.123.+./=..'", + ], + ), + ) + + def test_check_commands(self): + conf = VyosConf(["set a b 'c a'", "set a c b"]) + self.assertListEqual( + conf.check_commands( + ["set a b 'c a'", "del a c b", "set a b 'c'", "del a a a"], + ), + [True, False, False, True], + ) + + def test_diff_commands_to(self): + conf = VyosConf(["set a b 'c a'", "set a c b"]) + + self.assertListEqual( + conf.diff_commands_to(VyosConf(["set a c b"])), + ["delete a b"], + ) + self.assertListEqual( + conf.diff_commands_to(VyosConf(["set a b 'c a'", "set a c b"])), + [], + ) + + # KEEP_EXISTING_VALUES is no longer reachable via 'set'/'delete' + # command text (see #6): a literal "..." leaf is now an ordinary + # value, not a sentinel, so nothing is suppressed here. + self.assertListEqual( + conf.diff_commands_to(VyosConf(["set a b ..."])), + ["delete a b 'c a'", "delete a c", "set a b ..."], + ) + + def test_diff_commands_to_keep_existing_values_sentinel(self): + # KEEP_EXISTING_VALUES is only reachable via the Python API now. + # Build the candidate tree directly to prove diff_to() still + # honours it when used that way. + conf = VyosConf(["set a b 'c a'", "set a c b"]) + candidate = VyosConf() + candidate.config = {"a": {"b": {KEEP_EXISTING_VALUES: {}}}} + + self.assertListEqual( + conf.diff_commands_to(candidate), + ["delete a c"], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/modules/network/vyos/test_vyos_config.py b/tests/unit/modules/network/vyos/test_vyos_config.py index e732ca60..ffafc85d 100644 --- a/tests/unit/modules/network/vyos/test_vyos_config.py +++ b/tests/unit/modules/network/vyos/test_vyos_config.py @@ -1,4 +1,3 @@ -# # (c) 2016 Red Hat Inc. # # This file is part of Ansible @@ -19,6 +18,7 @@ # Make coding more python3-ish from __future__ import absolute_import, division, print_function + __metaclass__ = type from unittest.mock import MagicMock, patch @@ -56,6 +56,11 @@ class TestVyosConfigModule(TestVyosModule): ) self.get_connection = self.mock_get_connection.start() + self.mock_copy_file = patch( + "ansible_collections.vyos.vyos.plugins.modules.vyos_config.copy_file", + ) + self.copy_file = self.mock_copy_file.start() + self.cliconf_obj = Cliconf(MagicMock()) self.running_config = load_fixture("vyos_config_config.cfg") @@ -70,6 +75,7 @@ class TestVyosConfigModule(TestVyosModule): self.mock_load_config.stop() self.mock_run_commands.stop() self.mock_get_connection.stop() + self.mock_copy_file.stop() def load_fixtures(self, commands=None, filename=None): config_file = "vyos_config_config.cfg" @@ -140,6 +146,37 @@ class TestVyosConfigModule(TestVyosModule): ) self.execute_module(changed=True, commands=lines, sort=False) + def test_vyos_config_match_enforce(self): + lines = [ + "set interfaces ethernet eth0 address '1.2.3.4/24'", + "set interfaces ethernet eth0 description 'test string'", + ] + set_module_args(dict(lines=lines, match="enforce")) + candidate = "\n".join(lines) + + response = self.cliconf_obj.get_diff( + candidate, + self.running_config, + diff_match="enforce", + ) + + self.conn.get_diff = MagicMock(return_value=response) + result = self.execute_module(changed=True, sort=False) + + self.conn.get_diff.assert_called_once_with( + candidate=candidate, + running=self.running_config, + diff_match="enforce", + ) + + expected_config_diff = [ + "delete interfaces ethernet eth1", + ] + self.assertEqual(response["config_diff"], expected_config_diff) + + expected_commands = expected_config_diff + self.assertEqual(result["commands"], expected_commands) + def test_vyos_config_confirm_automatic(self): src = load_fixture("vyos_config_src.cfg") confirm_timeout = 7 @@ -177,3 +214,499 @@ class TestVyosConfigModule(TestVyosModule): self.assertEqual(self.load_config.call_args[1]["confirm"], confirm_timeout) self.run_commands.assert_not_called() + + def test_vyos_config_match_enforce_blank_lines(self): + """enforce diff must not raise IndexError on blank lines in running config.""" + running_with_blanks = self.running_config + "\n\n" + candidate = "set interfaces ethernet eth0 address 1.2.3.4/24" + response = self.cliconf_obj.get_diff(candidate, running_with_blanks, diff_match="enforce") + self.assertIn("config_diff", response) + + def test_vyos_config_match_enforce_additions(self): + lines = [ + "set interfaces ethernet eth0 address '1.2.3.4/24'", + "set interfaces ethernet eth0 description 'test string'", + "set interfaces ethernet eth2 address '192.0.2.1/24'", + ] + set_module_args(dict(lines=lines, match="enforce")) + candidate = "\n".join(lines) + response = self.cliconf_obj.get_diff( + candidate, + self.running_config, + diff_match="enforce", + ) + self.conn.get_diff = MagicMock(return_value=response) + result = self.execute_module(changed=True, sort=False) + self.conn.get_diff.assert_called_once_with( + candidate=candidate, + running=self.running_config, + diff_match="enforce", + ) + self.assertIn( + "set interfaces ethernet eth2 address 192.0.2.1/24", + response["config_diff"], + ) + self.assertEqual(result["commands"], response["config_diff"]) + + def test_vyos_config_match_enforce_rejects_delete_lines(self): + """ + match=enforce treats the candidate as the complete desired end-state. + A candidate containing 'delete' lines must be rejected rather than + silently producing a diff that removes most/all of the running + config (regression test for a candidate that is a no-op/delete-only + input generating deletes for everything the candidate omits). + """ + lines = ["delete interfaces ethernet eth0 address"] + candidate = "\n".join(lines) + + with self.assertRaises(ValueError): + self.cliconf_obj.get_diff( + candidate, + self.running_config, + diff_match="enforce", + ) + + def test_vyos_config_match_enforce_rejects_empty_candidate(self): + """ + A candidate that is empty, whitespace-only, or comment-only must be + rejected rather than silently treated as an empty desired end-state + (which would generate deletes for the entire running config). + Comment-only candidates are also stripped away entirely by upstream + NetworkConfig parsing before reaching VyosConf, so this is a second, + distinct route to the same mass-deletion failure mode as the + 'delete' lines case above. + """ + for candidate in ("", " ", "# just a comment"): + with self.assertRaises(ValueError): + self.cliconf_obj.get_diff( + candidate, + self.running_config, + diff_match="enforce", + ) + + def test_vyos_config_match_enforce_requires_running(self): + """ + diff_match=enforce with running=None must raise a clear ValueError + instead of falling through to an AttributeError on + running.splitlines(). + """ + with self.assertRaises(ValueError): + self.cliconf_obj.get_diff( + "set system host-name foo", + None, + diff_match="enforce", + ) + + def test_vyos_config_match_enforce_ignores_comment_lines(self): + """ + Comment lines mixed in with 'set' lines must be stripped out rather + than causing the whole candidate to be rejected as not starting + with 'set'. + """ + candidate = "\n".join( + [ + "set interfaces ethernet eth0 address '1.2.3.4/24'", + "# a note about this interface", + "set interfaces ethernet eth0 description 'test string'", + ], + ) + running = "set interfaces ethernet eth0 address '1.2.3.4/24'" + response = self.cliconf_obj.get_diff( + candidate, + running, + diff_match="enforce", + ) + self.assertIn( + "set interfaces ethernet eth0 description 'test string'", + response["config_diff"], + ) + + def test_sanitize_config_filters_password_delete_lines(self): + """ + sanitize_config()/PASSWORD_NEEDLE must filter 'delete ... password' + lines the same way it filters 'set ... password' lines, since + match=enforce can generate deletes for password config the candidate + omits. Without this, allow_password_change=none/plaintext/encrypted + would fail to catch a password-affecting delete. + """ + result = {} + commands = [ + "set system host-name foo", + "delete system login user admin authentication encrypted-password", + "set system login user admin authentication plaintext-password 'secret'", + ] + vyos_config.sanitize_config(commands, result, allow="none") + self.assertIn( + "delete system login user admin authentication encrypted-password", + result["filtered"], + ) + self.assertIn( + "set system login user admin authentication plaintext-password 'secret'", + result["filtered"], + ) + self.assertNotIn("set system host-name foo", result["filtered"]) + + def test_vyos_config_match_enforce_refuses_ssh_deletion(self): + """ + match=enforce must refuse to generate 'delete service ssh ...' + commands, since this could sever the management connection. + Regression test for the incident where an enforce candidate that + didn't restate 'service ssh' generated a delete for it. + """ + running = "\n".join( + [ + "set service ssh port '22'", + "set service lldp", + ], + ) + candidate = "set service lldp" + + with self.assertRaises(ValueError): + self.cliconf_obj.get_diff( + candidate, + running, + diff_match="enforce", + ) + + def test_vyos_config_confirm_defaults_to_automatic_for_match_enforce(self): + """ + confirm defaults to 'automatic' when match=enforce and confirm is + not explicitly set, since enforce can generate broad deletes and a + bad commit should self-revert rather than leave the device + unreachable. + """ + lines = [ + "set interfaces ethernet eth0 address '1.2.3.4/24'", + "set interfaces ethernet eth0 description 'test string'", + ] + set_module_args(dict(lines=lines, match="enforce")) + candidate = "\n".join(lines) + response = self.cliconf_obj.get_diff( + candidate, + self.running_config, + diff_match="enforce", + ) + self.conn.get_diff = MagicMock(return_value=response) + + self.execute_module(changed=True, sort=False) + + self.assertEqual(self.load_config.call_args[1]["confirm"], 10) + self.run_commands.assert_called_once() + self.assertEqual( + ["configure", "confirm", "exit"], + self.run_commands.call_args[0][1], + ) + + def test_vyos_config_confirm_stays_none_for_other_match_values(self): + """ + confirm stays 'none' (no confirm kwarg passed, no auto-confirm + run_commands call) when match is not 'enforce' and confirm is not + explicitly set -- the new conditional default must not change + existing behaviour for match=line/none. + """ + lines = ["set system host-name foo"] + set_module_args(dict(lines=lines)) + candidate = "\n".join(lines) + self.conn.get_diff = MagicMock( + return_value=self.cliconf_obj.get_diff(candidate, self.running_config), + ) + + self.execute_module(changed=True, commands=lines) + + self.assertIsNone(self.load_config.call_args[1]["confirm"]) + self.run_commands.assert_not_called() + + def test_vyos_config_match_enforce_rejects_comment_disguised_as_command(self): + """ + A line like 'set # comment' has 3 raw tokens (passing a naive + token-count check) but parse_line() strips the trailing comment, + leaving no actual path/leaf. This must still be rejected rather + than silently contributing an empty/degenerate entry to the diff. + """ + for bad_line in ("set # comment", "set foo # comment"): + candidate = "\n".join(["set system host-name foo", bad_line]) + with self.assertRaises(ValueError): + self.cliconf_obj.get_diff( + candidate, + self.running_config, + diff_match="enforce", + ) + + def test_sanitize_config_filters_collapsed_login_subtree_deletes(self): + """ + match=enforce's scoping can collapse an untouched subtree into a + single parent delete (e.g. 'delete system login' when a candidate + touches system without restating login, rather than an itemized + per-field delete). PASSWORD_NEEDLE alone can't see into a collapsed + delete to know it removes a password -- it must be treated as + password-bearing by default under any restrictive + allow_password_change value. + """ + result = {} + commands = [ + "set system host-name foo", + "delete system login", + ] + vyos_config.sanitize_config(commands, result, allow="none") + self.assertIn("delete system login", result["filtered"]) + self.assertNotIn("set system host-name foo", result["filtered"]) + + def test_sanitize_config_filters_collapsed_login_user_subtree_delete(self): + """ + Same collapse risk at the per-user level: 'delete system login + user admin' (no specific authentication line) must also be + treated as password-bearing. + """ + result = {} + commands = [ + "set system host-name foo", + "delete system login user admin", + ] + vyos_config.sanitize_config(commands, result, allow="none") + self.assertIn("delete system login user admin", result["filtered"]) + + def test_sanitize_config_allows_collapsed_login_subtree_delete_when_all(self): + """ + allow_password_change=all must still let a collapsed login-subtree + delete through, same as it already does for explicit password + lines. + """ + result = {} + commands = [ + "set system host-name foo", + "delete system login", + ] + vyos_config.sanitize_config(commands, result, allow="all") + self.assertEqual(result["filtered"], []) + + def test_vyos_config_match_enforce_accepts_bracket_format_src(self): + """ + match=enforce must accept bracket-format candidates the same way + match=line/none already do -- enforce_candidate_lines is now built + from the same shared, correctly-normalized candidate_commands + rather than parsing raw candidate text independently (which had no + concept of bracket format at all). + """ + candidate = "system {\n host-name foo\n}\n" + response = self.cliconf_obj.get_diff( + candidate, + self.running_config, + diff_match="enforce", + ) + self.assertIn("set system host-name foo", response["config_diff"]) + + def test_vyos_config_match_line_ignores_comment_and_blank_lines(self): + """ + A src/lines candidate containing comment or blank lines must not + raise under match=line -- these are stripped during candidate + normalization the same way match=enforce already does, rather than + hitting the 'line must start with set or delete' check. + """ + lines = [ + "# a note", + "", + "set system host-name foo", + ] + set_module_args(dict(lines=lines)) + candidate = "\n".join(lines) + self.conn.get_diff = MagicMock( + return_value=self.cliconf_obj.get_diff(candidate, self.running_config), + ) + self.execute_module(changed=True, commands=["set system host-name foo"]) + + def test_sanitize_config_filters_collapsed_login_user_authentication_subtree_delete(self): + """ + A candidate that keeps other settings for a user but omits that + user's entire authentication subtree collapses to 'delete system + login user <name> authentication' -- one level deeper than the + per-user collapse already covered. This must also be treated as + password-bearing under the default allow_password_change=plaintext, + not just under allow_password_change=none. + """ + result = {} + commands = [ + "set system host-name foo", + "delete system login user admin authentication", + ] + vyos_config.sanitize_config(commands, result, allow="plaintext") + self.assertIn( + "delete system login user admin authentication", + result["filtered"], + ) + self.assertNotIn("set system host-name foo", result["filtered"]) + + # -- replace=config (T6837, cisco.iosxr.iosxr_config replace=config analogue) -- + + def test_vyos_config_replace_config_requires_src(self): + """replace=config without src must fail argument validation, not run.""" + set_module_args(dict(replace="config")) + result = self.execute_module(failed=True) + self.assertIn("src", result["msg"]) + + def test_vyos_config_replace_config_rejects_lines_only(self): + """replace=config with only lines (no src) must fail -- + required_if demands src regardless of what else is set. + """ + set_module_args(dict(replace="config", lines=["set system host-name foo"])) + self.execute_module(failed=True) + + def test_vyos_config_replace_config_rejects_lines_and_src_together(self): + """lines/src remain mutually exclusive regardless of replace -- + this is the pre-existing constraint, unaffected by replace=config.""" + set_module_args( + dict( + replace="config", + src="system {\n host-name router\n}\n", + lines=["set system host-name foo"], + ), + ) + self.execute_module(failed=True) + + def test_vyos_config_replace_config_pushes_and_loads(self): + """replace=config with a real change: copies the candidate to a fixed + remote path, issues a single `load <path>` command, and reports the + device's own diff verbatim -- not an itemized set/delete list. + """ + src = "interfaces {\n ethernet eth0 {\n address dhcp\n }\n}\n" + set_module_args(dict(replace="config", src=src)) + self.load_config.side_effect = lambda *a, **kw: ( + "[edit interfaces]\n+ethernet eth0 {\n+ address dhcp\n+}" + ) + + result = self.execute_module(changed=True) + + self.assertEqual(result["commands"], ["load /tmp/ansible_vyos_replace.cfg"]) + self.copy_file.assert_called_once() + # positional call: copy_file(module, local_path, remote_path, proto) + self.assertEqual(self.copy_file.call_args[0][2], "/tmp/ansible_vyos_replace.cfg") + self.assertEqual(self.copy_file.call_args[0][3], "scp") + self.assertEqual( + self.load_config.call_args[0][1], + ["load /tmp/ansible_vyos_replace.cfg"], + ) + + def test_vyos_config_replace_config_noop(self): + """replace=config with load_config() returning falsy (VyOS's own + `compare` reported no changes) must report changed=False, not + unconditionally True. + """ + src = "system {\n host-name router\n}\n" + set_module_args(dict(replace="config", src=src)) + self.load_config.return_value = None + + result = self.execute_module(changed=False) + self.assertEqual(result["commands"], ["load /tmp/ansible_vyos_replace.cfg"]) + + def test_vyos_config_replace_config_check_mode(self): + """Under check_mode, commit=False must be passed through to + load_config() -- the candidate is still copied/loaded for an accurate + compare-based preview diff, but nothing is committed. + """ + src = "system {\n host-name router\n}\n" + set_module_args(dict(replace="config", src=src, _ansible_check_mode=True)) + self.load_config.side_effect = lambda *a, **kw: ( + "[edit system]\n-host-name foo\n+host-name router" + ) + + self.execute_module(changed=True) + + self.assertEqual(self.load_config.call_args[1]["commit"], False) + + def test_vyos_config_replace_config_confirm_automatic(self): + src = "system {\n host-name router\n}\n" + confirm_timeout = 9 + set_module_args( + dict( + replace="config", + src=src, + confirm="automatic", + confirm_timeout=confirm_timeout, + ), + ) + self.load_config.side_effect = lambda *a, **kw: ( + "[edit system]\n-host-name foo\n+host-name router" + ) + + self.execute_module(changed=True) + + self.assertEqual(self.load_config.call_args[1]["confirm"], confirm_timeout) + self.run_commands.assert_called_once() + self.assertEqual(["configure", "confirm", "exit"], self.run_commands.call_args[0][1]) + + def test_vyos_config_replace_config_diff(self): + """With --diff, result['diff']['prepared'] must carry VyOS's own + compare() output verbatim -- not an itemized command list, since none + is computed in this mode. + """ + src = "system {\n host-name router\n}\n" + set_module_args(dict(replace="config", src=src, _ansible_diff=True)) + raw_compare = "[edit system]\n-host-name foo\n+host-name router" + self.load_config.side_effect = lambda *a, **kw: raw_compare + + result = self.execute_module(changed=True) + + self.assertEqual(result["diff"]["prepared"], raw_compare) + + def test_vyos_config_replace_config_does_not_use_line_diff_path(self): + """replace=config must never call connection.get_diff() -- that path + (and match/allow_password_change) is specific to replace=line and is + documented as ignored under replace=config. + """ + src = "system {\n host-name router\n}\n" + set_module_args(dict(replace="config", src=src, match="none")) + self.load_config.side_effect = lambda *a, **kw: ( + "[edit system]\n-host-name foo\n+host-name router" + ) + + self.execute_module(changed=True) + + self.conn.get_diff.assert_not_called() + + def test_vyos_config_replace_config_confirm_automatic_check_mode_no_confirm_sent(self): + """Regression guard: confirm=automatic must not send the + configure/confirm/exit sequence under check_mode, even when a real + diff is present -- nothing was actually committed to confirm. + """ + src = "system {\n host-name router\n}\n" + set_module_args( + dict( + replace="config", + src=src, + confirm="automatic", + _ansible_check_mode=True, + ), + ) + self.load_config.side_effect = lambda *a, **kw: ( + "[edit system]\n-host-name foo\n+host-name router" + ) + + self.execute_module(changed=True) + + self.run_commands.assert_not_called() + + def test_vyos_config_replace_config_confirm_automatic_noop_no_confirm_sent(self): + """Regression guard: confirm=automatic must not send the + configure/confirm/exit sequence when load_config() reports no diff + (VyOS's own compare() found nothing to commit) -- there is nothing + pending to confirm. + """ + src = "system {\n host-name router\n}\n" + set_module_args(dict(replace="config", src=src, confirm="automatic")) + self.load_config.return_value = None + + self.execute_module(changed=False) + + self.run_commands.assert_not_called() + + def test_vyos_config_replace_line_default_unaffected(self): + """Regression guard: default replace='line' must behave identically + to the pre-patch module -- copy_file() must never be invoked. + """ + commands = ["set system host-name foo"] + set_module_args(dict(lines=commands)) + candidate = "\n".join(commands) + self.conn.get_diff = MagicMock( + return_value=self.cliconf_obj.get_diff(candidate, self.running_config), + ) + self.execute_module(changed=True, commands=commands) + self.copy_file.assert_not_called() |