diff --git a/changelogs/fragments/T6837_vyos_config-replace.yml b/changelogs/fragments/T6837_vyos_config-replace.yml new file mode 100644 index 00000000..c8cf68ac --- /dev/null +++ b/changelogs/fragments/T6837_vyos_config-replace.yml @@ -0,0 +1,18 @@ +--- +bugfixes: + - cliconf - fixed replace-mode diff in get_diff() leaving orphaned/invalid + config nodes on the device when an entire config node (not just a leaf + value) was absent from the candidate configuration, by making the diff + structure-aware instead of comparing flat config lines + (https://vyos.dev/T6837). + - cliconf - fixed get_diff() emitting a redundant delete alongside the + correct set command in replace mode when only a scalar attribute's + value changed. + - cliconf - fixed get_diff() applying quote-insensitive line matching to + non-replace calls, which could mask genuine configuration differences + during normal (non-replace) operation for any caller, including + ansible.netcommon.cli_config. +minor_changes: + - vyos_config - replace mode now requests the hierarchical config format + from the device so the structure-aware diff in cliconf can correctly + distinguish node removal from leaf value changes. diff --git a/docs/vyos.vyos.vyos_config_module.rst b/docs/vyos.vyos.vyos_config_module.rst index bf91bb0f..09b726a3 100644 --- a/docs/vyos.vyos.vyos_config_module.rst +++ b/docs/vyos.vyos.vyos_config_module.rst @@ -1,458 +1,496 @@ .. _vyos.vyos.vyos_config_module: ********************* vyos.vyos.vyos_config ********************* **Manage VyOS configuration on remote device** Version added: 1.0.0 .. contents:: :local: :depth: 1 Synopsis -------- - This module provides configuration file management of VyOS devices. It provides arguments for managing both the configuration file and state of the active configuration. All configuration statements are based on `set` and `delete` commands in the device configuration. Parameters ---------- .. raw:: html + + + + +
Parameter Choices/Defaults Comments
allow_password_change
string
    Choices:
  • all
  • plaintext ←
  • encrypted
  • none
The allow_password_change 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 all.
backup
boolean
    Choices:
  • no ←
  • yes
The backup argument will backup the current devices active configuration to the Ansible control host prior to making any changes. If the backup_options value is not given, the backup file will be located in the backup folder in 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.
backup_options
dictionary
This is a dict object containing configurable options related to backup file path. The value of this option is read only when backup is set to true, if backup is set to false this option will be silently ignored.
dir_path
path
This option provides the path ending with directory name in which the backup configuration file will be stored. If the directory does not exist it will be first created and the filename is either the value of filename or default filename as described in filename options description. If the path value is not given in that case a backup directory will be created in the current working directory and backup configuration will be copied in filename within backup directory.
filename
string
The filename to be used to store the backup configuration. If the filename is not given it will be generated based on the hostname, current time and date in format defined by <hostname>_config.<current-date>@<current-time>
comment
string
Default:
"configured by vyos_config"
Allows a commit description to be specified to be included when the configuration is committed. If the configuration is not changed or committed, this argument is ignored.
config
string
The config 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.
confirm
string
    Choices:
  • automatic
  • manual
  • none ←
The confirm argument will tell vyos to revert to the previous configuration if not explicitly confirmed after applying the new config. When set to automatic this module will automatically confirm the configuration, if the current session remains working with the new config. When set to manual, this module does not issue the confirmation itself.
confirm_timeout
integer
Default:
10
Minutes to wait for confirmation before reverting the configuration. Does not apply when confirm is set to none .
lines
list / elements=string
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.
match
string
    Choices:
  • line ←
  • none
The 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 match argument is set to none the active configuration is ignored and the configuration is always loaded.
+
+ replace + +
+ boolean +
+ 		+
    Choices: +
  • no ←
    • +
  • yes
    • +
+ 		+
The replace argument replaces the device's entire configuration with the supplied candidate, rather than merging the candidate into the existing configuration.
+
replace requires the candidate (lines/src) to represent the complete desired configuration. Any configuration present on the device but not included in the candidate will be deleted, including management interfaces, SSH, and login users if they are omitted. Always provide a full configuration when using replace, never a partial one.
+
replace is only supported when match is set to line (the default). Combining replace with match set to none results in an error.
+
For backwards compatibility, the default is false.
+
save
boolean
    Choices:
  • no ←
  • yes
The 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 true, the active configuration is saved.
src
path
The src 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.

Notes ----- .. note:: - 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`` 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 ``replace`` to a subtree. - For more information on using Ansible to manage network devices see the :ref:`Ansible Network Guide ` Examples -------- .. code-block:: yaml - name: configure the remote device vyos.vyos.vyos_config: lines: - set system host-name {{ inventory_hostname }} - set service lldp - delete service dhcp-server - name: backup and load from file vyos.vyos.vyos_config: src: vyos.cfg backup: true - name: render a Jinja2 template onto the VyOS router vyos.vyos.vyos_config: src: vyos_template.j2 - name: revert after ten minutes, if connection is lost vyos.vyos.vyos_config: src: vyos_template.j2 confirm: automatic - name: for idempotency, use full-form commands vyos.vyos.vyos_config: lines: # - set int eth eth2 description 'OUTSIDE' - set interface ethernet eth2 description 'OUTSIDE' - name: configurable backup path vyos.vyos.vyos_config: backup: true backup_options: filename: backup.cfg dir_path: /home/user + - name: replace the entire running config with a fully edited candidate + # replace requires the complete desired configuration -- never a partial + # one. A safe pattern is to back up the current config, edit it, then + # replace with the edited whole, as shown here. + vyos.vyos.vyos_config: + backup: true + backup_options: + filename: pre_replace_backup.cfg + register: backup_result + + - name: (edit backup_result's backup file as needed, then) + vyos.vyos.vyos_config: + src: /home/user/pre_replace_backup_edited.cfg + replace: true + Return Values ------------- Common return values are documented `here `_, the following are the fields unique to this module: .. raw:: html
Key Returned Description
backup_path
string
when backup is yes
The full path to the backup file

Sample:
/playbooks/ansible/backup/vyos_config.2016-07-16@22:28:34
commands
list
always
The list of configuration commands sent to the device

Sample:
['...', '...']
date
string
when backup is yes
The date extracted from the backup file name

Sample:
2016-07-16
filename
string
when backup is yes and filename is not specified in backup options
The name of the backup file

Sample:
vyos_config.2016-07-16@22:28:34
filtered
list
always
The list of configuration commands removed to avoid a load failure

Sample:
['...', '...']
shortname
string
when backup is yes and filename is not specified in backup options
The full path to the backup file excluding the timestamp

Sample:
/playbooks/ansible/backup/vyos_config
time
string
when backup is yes
The time extracted from the backup file name

Sample:
22:28:34


Status ------ Authors ~~~~~~~ - Nathaniel Case (@Qalthos) diff --git a/plugins/cliconf/vyos.py b/plugins/cliconf/vyos.py index 96c24c15..2a47c7e3 100644 --- a/plugins/cliconf/vyos.py +++ b/plugins/cliconf/vyos.py @@ -1,356 +1,518 @@ # (c) 2017 Red Hat Inc. # # 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 . # from __future__ import absolute_import, division, print_function __metaclass__ = type DOCUMENTATION = """ author: Ansible Networking Team (@ansible-network) name: vyos short_description: Use vyos cliconf to run command on VyOS platform description: - This vyos plugin provides low level abstraction apis for sending and receiving CLI commands from VyOS network devices. version_added: 1.0.0 options: config_commands: description: - Specifies a list of commands that can make configuration changes to the target device. - When `ansible_network_single_user_mode` is enabled, if a command sent to the device is present in this list, the existing cache is invalidated. version_added: 2.0.0 type: list elements: str default: [] vars: - name: ansible_vyos_config_commands """ import json 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_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 class Cliconf(CliconfBase): __rpc__ = CliconfBase.__rpc__ + [ "commit", "discard_changes", "get_diff", "run_commands", ] def __init__(self, *args, **kwargs): super(Cliconf, self).__init__(*args, **kwargs) self._device_info = {} def get_device_info(self): if not self._device_info: device_info = {} device_info["network_os"] = "vyos" reply = self.get("show version") data = to_text(reply, errors="surrogate_or_strict").strip() match = re.search(r"Version:\s*(.*)", data) if match: device_info["network_os_version"] = match.group(1) if device_info["network_os_version"]: match = re.search(r"VyOS\s*(\d+\.\d+)", device_info["network_os_version"]) if match: device_info["network_os_major_version"] = match.group(1) match = re.search(r"(?:HW|Hardware) model:\s*(\S+)", data) if match: device_info["network_os_model"] = match.group(1) reply = self.get("show host name") device_info["network_os_hostname"] = to_text( reply, errors="surrogate_or_strict", ).strip() self._device_info = device_info return self._device_info def get_config(self, flags=None, format=None): if format: option_values = self.get_option_values() if format not in option_values["format"]: raise ValueError( "'format' value %s is invalid. Valid values of format are %s" % (format, ", ".join(option_values["format"])), ) if not flags: flags = [] if format == "text": command = "show configuration" else: command = "show configuration commands" command += " ".join(to_list(flags)) command = command.strip() out = self.send_command(command) 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() self.check_edit_config_capability(operations, candidate, commit, replace, comment) results = [] requests = [] self.send_command("configure") for cmd in to_list(candidate): if not isinstance(cmd, Mapping): cmd = {"command": cmd} results.append(self.send_command(**cmd)) requests.append(cmd["command"]) out = self.get("compare") out = to_text(out, errors="surrogate_or_strict") diff_config = out if not out.startswith("No changes") else None if diff_config: if commit: try: self.commit(comment, confirm) except AnsibleConnectionFailure as e: msg = "commit failed: %s" % e.message self.discard_changes() raise AnsibleConnectionFailure(msg) else: self.send_command("exit") else: self.discard_changes() else: self.send_command("exit") if ( to_text(self._connection.get_prompt(), errors="surrogate_or_strict") .strip() .endswith("#") ): self.discard_changes() if diff_config: resp["diff"] = diff_config resp["response"] = results resp["request"] = requests return resp def get( self, command=None, prompt=None, answer=None, sendonly=False, newline=True, output=None, check_all=False, ): if not command: raise ValueError("must provide value of command to execute") if output: raise ValueError("'output' value %s is not supported for get" % output) return self.send_command( command=command, prompt=prompt, answer=answer, sendonly=sendonly, newline=newline, check_all=check_all, ) def commit(self, comment=None, confirm=None): if confirm: if comment: command = 'commit-confirm {0} comment "{1}"'.format(confirm, comment) else: command = "commit-confirm {0}".format(confirm) self.send_command(command, "Proceed?", "\n") else: if comment: command = 'commit comment "{0}"'.format(comment) else: command = "commit" self.send_command(command) def discard_changes(self): self.send_command("exit discard") def get_diff( self, candidate=None, running=None, diff_match="line", diff_ignore_lines=None, path=None, - diff_replace=None, + diff_replace=False, ): diff = {} device_operations = self.get_device_operations() option_values = self.get_option_values() if candidate is None and device_operations["supports_generate_diff"]: raise ValueError("candidate configuration is required to generate diff") if diff_match not in option_values["diff_match"]: raise ValueError( "'match' value %s in invalid, valid values are %s" % (diff_match, ", ".join(option_values["diff_match"])), ) - if diff_replace: - raise ValueError("'replace' in diff is not supported") - if diff_ignore_lines: raise ValueError("'diff_ignore_lines' in diff is not supported") if path: raise ValueError("'path' in diff is not supported") + if diff_replace and diff_match == "none": + # Module documentation states replace only works when match is + # 'line'. Without this check, diff_replace silently has no + # effect under match='none' (that branch returns before the + # diff_replace logic below ever runs) -- failing loudly here + # is safer than letting a user believe replace ran. + raise ValueError("'replace' is not supported when 'match' is set to 'none'") + set_format = candidate.startswith("set") or candidate.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): del commands[index] break commands.append(item) candidate_commands = ["set %s" % cmd.replace(" {", "") for cmd in commands] else: candidate_commands = str(candidate).strip().split("\n") if diff_match == "none": diff["config_diff"] = list(candidate_commands) return diff - running_commands = [str(c).replace("'", "") for c in running.splitlines()] + if diff_replace: + # `running` is hierarchical/brace text in replace mode (see the + # tree-aware block below). It must be flattened to full-path + # "set" commands the same way candidate is above -- naively + # splitting on newlines here would compare raw brace-syntax + # fragments (e.g. " host-name router") against candidate's + # flat commands and never match, making every candidate line + # look incorrectly "missing". + if running.lstrip().startswith(("set ", "delete ")): + raise ValueError( + "diff_replace requires 'running' in hierarchical config " + "format, not flat set/delete commands", + ) + running_obj = NetworkConfig(indent=4, contents=running) + running_lines = [c.line for c in running_obj.items] + running_flat = list() + for item in running_lines: + for index, entry in enumerate(running_flat): + if item.startswith(entry): + del running_flat[index] + break + running_flat.append(item) + running_commands = ["set %s" % cmd.replace(" {", "") for cmd in running_flat] + else: + running_commands = [str(c).replace("'", "") for c in running.splitlines()] updates = list() visited = set() + if diff_replace: + # Precompute once instead of scanning + regex-substituting + # running_commands for every candidate line below. This turns + # the set-line match from O(N*M) with two regex subs per + # comparison into O(N+M), while preserving quote-insensitive + # equality by stripping both single and double quotes. + running_commands_normalized = {re.sub("['\"]", "", rline) for rline in running_commands} + for line in candidate_commands: item = str(line).replace("'", "") if not item.startswith("set") and not item.startswith("delete"): raise ValueError("line must start with either `set` or `delete`") - elif item.startswith("set") and item not in running_commands: - updates.append(line) + elif item.startswith("set"): + if diff_replace: + # Quote-insensitive comparison is only needed for replace + # mode, where `running` values may be re-quoted before + # being compared here. Gating this behind diff_replace + # preserves the original exact-match idempotency check + # for all existing (non-replace) callers. + match = re.sub("['\"]", "", item) in running_commands_normalized + else: + match = item in running_commands + if not match: + updates.append(line) elif item.startswith("delete"): if not running_commands: updates.append(line) else: item = re.sub(r"delete", "set", item) for entry in running_commands: if re.match(rf"^{re.escape(item)}\b", entry) and line not in visited: updates.append(line) visited.add(line) - diff["config_diff"] = list(updates) + if diff_replace: + # T6837: replace mode must operate on the config's actual tree + # structure, not flat line text. `running` is required to be in + # hierarchical/brace form here (get_config(..., format="text")), + # so that intermediate nodes (e.g. a firewall rule) are visible + # as distinct entities from their leaf values. Diffing on flat + # "set" lines alone cannot tell "a whole node was removed" apart + # from "a leaf's value changed", which is what caused both the + # orphaned-node bug and the redundant-delete-on-value-change bug. + + candidate_bodies = [ + _strip_cmd_prefix(c) for c in candidate_commands if c.startswith("set ") + ] + candidate_bodies_normalized = {re.sub("['\"]", "", b) for b in candidate_bodies} + running_tree = NetworkConfig(indent=4, contents=running) + + replace_deletes = list() + visited_nodes = set() + + for item in running_tree.items: + prefix = _node_prefix(item) + + if item.children: + # intermediate node: does an equivalent structural path + # exist anywhere in candidate? If not, the whole subtree + # is missing -- emit a single delete for the node itself + # rather than descending into per-leaf deletes. + if not _candidate_has_prefix(prefix, candidate_bodies): + if not any( + prefix == v or prefix.startswith(v + " ") for v in visited_nodes + ): + replace_deletes.append("delete %s" % prefix) + visited_nodes.add(prefix) + continue + + # leaf node + if re.sub("['\"]", "", prefix) in candidate_bodies_normalized: + continue # exact match, nothing to do + + parent_prefix = " ".join(p.replace(" {", "") for p in item.parents) + if any( + parent_prefix == v or parent_prefix.startswith(v + " ") for v in visited_nodes + ): + continue # already covered by an ancestor delete above + + # Leaf is either genuinely absent from candidate, or its + # value changed. Delete it unconditionally -- there is no + # reliable way to tell a coincidentally-single-valued + # list-style attribute (e.g. a single 'name-server' entry) + # apart from a genuinely scalar one (e.g. 'host-name') from + # config text alone; treating them differently by observed + # cardinality can leave stale values behind for list-style + # attributes (see T6837 review discussion). This is made + # safe by ordering: replace_deletes are placed before the + # candidate-driven `set` commands below, so each delete + # always targets the value that is still genuinely active, + # never one a `set` has already superseded. + replace_deletes.append("delete %s" % prefix) + + if diff_replace: + # A candidate may include an explicit "delete ..." line for a + # node that the structure-aware pass above also independently + # determined is absent. Without deduplication both paths emit + # the same delete, and a repeated delete for an already-deleted + # path can fail commit idempotency on strict devices. + existing_deletes = { + str(c).strip() for c in updates if str(c).strip().startswith("delete ") + } + replace_deletes = [c for c in replace_deletes if c not in existing_deletes] + diff["config_diff"] = replace_deletes + list(updates) + else: + diff["config_diff"] = list(updates) return diff def run_commands(self, commands=None, check_rc=True): if commands is None: raise ValueError("'commands' value is required") responses = list() for cmd in to_list(commands): if not isinstance(cmd, Mapping): cmd = {"command": cmd} output = cmd.pop("output", None) if output: raise ValueError("'output' value %s is not supported for run_commands" % output) try: out = self.send_command(**cmd) except AnsibleConnectionFailure as e: if check_rc: raise out = getattr(e, "err", e) responses.append(out) return responses def get_device_operations(self): return { - "supports_diff_replace": False, + "supports_diff_replace": True, "supports_commit": True, "supports_rollback": False, "supports_defaults": False, "supports_onbox_diff": True, "supports_commit_comment": True, "supports_multiline_delimiter": False, "supports_diff_match": True, "supports_diff_ignore_lines": False, "supports_generate_diff": False, "supports_replace": False, } def get_option_values(self): return { "format": ["text", "set"], "diff_match": ["line", "none"], - "diff_replace": [], + "diff_replace": [True, False], "output": [], } def get_capabilities(self): result = super(Cliconf, self).get_capabilities() result["device_operations"] = self.get_device_operations() result.update(self.get_option_values()) return json.dumps(result) def set_cli_prompt_context(self): """ Make sure we are in the operational cli mode :return: None """ if self._connection.connected: self._update_cli_prompt_context(config_context="#", exit_command="exit discard") + + +def match_cmd(cmd1, cmd2): + cmd1 = re.sub("['\"]", "", cmd1) + cmd2 = re.sub("['\"]", "", cmd2) + if cmd1 == cmd2: + return True + else: + return False + + +def _strip_cmd_prefix(cmd): + """Remove a leading 'set ' / 'delete ' keyword, leaving the bare config path.""" + if cmd.startswith("set "): + return cmd[4:] + if cmd.startswith("delete "): + return cmd[7:] + return cmd + + +def _node_prefix(item): + """Return the full structural path for a config tree node (parents + own + text). Brace markers are stripped and parts are space-joined. Note that + intermediate nodes may include key/value-like tokens (for example + ``rule 200`` or ``ethernet eth1``) -- do not assume item.text is a bare + identifier. What makes intermediate-node identity unambiguous for this + diff isn't that the text is a pure keyword, but that the full parents + + text path is a complete, structural node identifier with no separate + "value" component to guess at, unlike a leaf's own text which mixes an + attribute keyword with a value. + """ + parts = [p.replace(" {", "").strip() for p in item.parents] + parts.append(item.text.replace(" {", "").strip()) + return " ".join(p for p in parts if p) + + +def _candidate_has_prefix(prefix, candidate_bodies): + return any(body == prefix or body.startswith(prefix + " ") for body in candidate_bodies) diff --git a/plugins/modules/vyos_config.py b/plugins/modules/vyos_config.py index 46e51f3b..ab7db2c9 100644 --- a/plugins/modules/vyos_config.py +++ b/plugins/modules/vyos_config.py @@ -1,449 +1,494 @@ #!/usr/bin/python # # 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 . # from __future__ import absolute_import, division, print_function __metaclass__ = type DOCUMENTATION = """ module: vyos_config author: Nathaniel Case (@Qalthos) short_description: Manage VyOS configuration on remote device description: - This module provides configuration file management of VyOS devices. It provides arguments for managing both the configuration file and state of the active configuration. All configuration statements are based on `set` and `delete` commands in the device configuration. version_added: 1.0.0 extends_documentation_fragment: - vyos.vyos.vyos 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 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) 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 C(replace) to a subtree. options: lines: description: - 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. type: list elements: str src: description: - The C(src) 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. 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. type: str default: line choices: - line - none backup: description: - The C(backup) argument will backup the current devices active configuration to the Ansible control host prior to making any changes. If the C(backup_options) value is not given, the backup file will be located in the backup folder in 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 comment: description: - Allows a commit description to be specified to be included when the configuration is committed. If the configuration is not changed or committed, this argument is ignored. default: configured by vyos_config type: str confirm: description: - The C(confirm) argument will tell vyos to revert to the previous configuration if not explicitly confirmed after applying the new config. When set to C(automatic) 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. type: str default: none choices: - automatic - manual - none confirm_timeout: description: - Minutes to wait for confirmation before reverting the configuration. Does not apply when C(confirm) is set to C(none) . type: int default: 10 config: description: - The C(config) 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. 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. type: bool default: false 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. suboptions: filename: description: - The filename to be used to store the backup configuration. If the filename is not given it will be generated based on the hostname, current time and date in format defined by _config.@ type: str dir_path: description: - This option provides the path ending with directory name in which the backup configuration file will be stored. If the directory does not exist it will be first created and the filename is either the value of C(filename) or default filename as described in C(filename) options description. If the path value is not given in that case a I(backup) directory will be created in the current working directory and backup configuration will be copied in C(filename) within I(backup) directory. type: path type: dict + replace: + description: + - The C(replace) argument replaces the device's entire configuration with + the supplied candidate, rather than merging the candidate into the + existing configuration. + - C(replace) requires the candidate (C(lines)/C(src)) to represent the + complete desired configuration. Any configuration present on the + device but not included in the candidate will be deleted, including + management interfaces, SSH, and login users if they are omitted. + Always provide a full configuration when using C(replace), never a + partial one. + - C(replace) is only supported when C(match) is set to C(line) (the + default). Combining C(replace) with C(match) set to C(none) results + in an error. + - For backwards compatibility, the default is C(false). + type: bool + default: no allow_password_change: description: - The C(allow_password_change) 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 C(all). type: str default: plaintext choices: - all - plaintext - encrypted - none """ EXAMPLES = """ - name: configure the remote device vyos.vyos.vyos_config: lines: - set system host-name {{ inventory_hostname }} - set service lldp - delete service dhcp-server - name: backup and load from file vyos.vyos.vyos_config: src: vyos.cfg backup: true - name: render a Jinja2 template onto the VyOS router vyos.vyos.vyos_config: src: vyos_template.j2 - name: revert after ten minutes, if connection is lost vyos.vyos.vyos_config: src: vyos_template.j2 confirm: automatic - name: for idempotency, use full-form commands vyos.vyos.vyos_config: lines: # - set int eth eth2 description 'OUTSIDE' - set interface ethernet eth2 description 'OUTSIDE' - name: configurable backup path vyos.vyos.vyos_config: backup: true backup_options: filename: backup.cfg dir_path: /home/user + +- name: replace the entire running config with a fully edited candidate + # replace requires the complete desired configuration -- never a partial + # one. A safe pattern is to back up the current config, edit it, then + # replace with the edited whole, as shown here. + vyos.vyos.vyos_config: + backup: true + backup_options: + filename: pre_replace_backup.cfg + register: backup_result + +- name: (edit backup_result's backup file as needed, then) + vyos.vyos.vyos_config: + src: /home/user/pre_replace_backup_edited.cfg + replace: true """ RETURN = """ commands: description: The list of configuration commands sent to the device returned: always type: list sample: ['...', '...'] filtered: description: The list of configuration commands removed to avoid a load failure returned: always type: list sample: ['...', '...'] backup_path: description: The full path to the backup file returned: when backup is yes type: str sample: /playbooks/ansible/backup/vyos_config.2016-07-16@22:28:34 filename: description: The name of the backup file returned: when backup is yes and filename is not specified in backup options type: str sample: vyos_config.2016-07-16@22:28:34 shortname: description: The full path to the backup file excluding the timestamp returned: when backup is yes and filename is not specified in backup options type: str sample: /playbooks/ansible/backup/vyos_config date: description: The date extracted from the backup file name returned: when backup is yes type: str sample: "2016-07-16" time: description: The time extracted from the backup file name returned: when backup is yes type: str sample: "22:28:34" """ import re from ansible.module_utils._text import 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 ( 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", ) def get_candidate(module): contents = module.params["src"] or module.params["lines"] if module.params["src"]: contents = contents.splitlines() if len(contents) > 0: line = contents[0].split() if len(line) > 0 and line[0] in ("set", "delete"): contents = format_commands(contents) contents = "\n".join(contents) return contents def format_commands(commands): """ This function format the input commands and removes the prepend white spaces for command lines having 'set' or 'delete' and it skips empty lines. :param commands: :return: list of commands """ return [ line.strip() if line.split()[0] in ("set", "delete") else line for line in commands if len(line.strip()) > 0 ] def diff_config(commands, config): config = [str(c).replace("'", "") for c in config.splitlines()] updates = list() visited = set() for line in commands: item = str(line).replace("'", "") if not item.startswith("set") and not item.startswith("delete"): raise ValueError("line must start with either `set` or `delete`") elif item.startswith("set") and item not in config: updates.append(line) elif item.startswith("delete"): if not config: updates.append(line) else: item = re.sub(r"delete", "set", item) for entry in config: if entry.startswith(item) and line not in visited: updates.append(line) visited.add(line) 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 - - config = module.params["config"] or get_config(module) + # the config param. + # replace mode requires the hierarchical/brace config form so the + # tree-aware diff in get_diff() can distinguish whole nodes from leaf + # values (see T6837) -- gated behind replace so every other caller + # keeps the existing flat "set" command format unchanged. + if module.params["config"]: + config = module.params["config"] + elif module.params["replace"]: + config = get_config(module, format="text") + else: + config = get_config(module) # create the candidate config object from the arguments candidate = get_candidate(module) # create loadable config that includes only the configuration updates connection = get_connection(module) try: response = connection.get_diff( candidate=candidate, running=config, diff_match=module.params["match"], + diff_replace=module.params["replace"], ) - except ConnectionError as exc: + except (ConnectionError, ValueError) as exc: module.fail_json(msg=to_text(exc, errors="surrogate_then_replace")) commands = response.get("config_diff") allow_password_change = module.params["allow_password_change"] sanitize_config(commands, result, allow=allow_password_change) result["commands"] = commands commit = not module.check_mode comment = module.params["comment"] confirm = None if module.params["confirm"] == "automatic" or module.params["confirm"] == "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": run_commands(module, ["configure", "confirm", "exit"]) if result.get("filtered"): result["warnings"].append( "Some configuration commands were removed, please see the filtered key", ) result["changed"] = True 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"]), comment=dict(default=DEFAULT_COMMENT), 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="bool", default=False), allow_password_change=dict( default="plaintext", choices=["all", "encrypted", "plaintext", "none"], ), ) mutually_exclusive = [("lines", "src")] module = AnsibleModule( argument_spec=argument_spec, mutually_exclusive=mutually_exclusive, supports_check_mode=True, ) warnings = list() result = dict(changed=False, warnings=warnings) if module.params["backup"]: result["__backup__"] = get_config(module=module) if any((module.params["src"], module.params["lines"])): run(module, result) if module.params["save"]: diff = run_commands(module, commands=["configure", "compare saved"])[1] if diff not in { "[edit]", "No changes between working and saved configurations.\n\n[edit]", }: if not module.check_mode: run_commands(module, commands=["save"]) result["changed"] = True run_commands(module, commands=["exit"]) if result.get("changed") 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" " the running configuration on device" ) if module.params["src"]: msg += " including the indentation" if "warnings" in result: result["warnings"].append(msg) else: result["warnings"] = msg module.exit_json(**result) if __name__ == "__main__": main() diff --git a/tests/integration/targets/vyos_config/tests/cli/replaced.yaml b/tests/integration/targets/vyos_config/tests/cli/replaced.yaml new file mode 100644 index 00000000..63529bb4 --- /dev/null +++ b/tests/integration/targets/vyos_config/tests/cli/replaced.yaml @@ -0,0 +1,74 @@ +--- +- debug: msg="START cli/replaced.yaml on connection={{ ansible_connection }}" + +# SAFETY NOTE: `replace: true` currently has no path/scope parameter (see +# T6837). Any candidate that omits a line present on the device will queue +# a delete for it -- including management interfaces, SSH, and login users. +# Every step below therefore operates on a FULL backup of the running +# config, edited in place, never a partial/minimal candidate. Do not +# shortcut this pattern until replace supports scoping. + +- name: setup baseline firewall rules + vyos.vyos.vyos_config: + lines: + - set firewall ipv4 name example rule 100 action drop + - set firewall ipv4 name example rule 200 action accept + match: none + +- name: backup full running config before mutating it + register: backup_result + vyos.vyos.vyos_config: + backup: true + backup_options: + dir_path: "{{ role_path }}/tests/output" + filename: "replace_baseline_{{ inventory_hostname_short }}.cfg" + +- name: build edited candidate lines (drop only rule 200, keep everything else verbatim) + # lookup('file', ...) runs inside Jinja on the controller process itself -- + # no connection plugin involved at all, so this sidesteps the delegate_to + + # inherited network_cli conflict entirely rather than fighting it. + ansible.builtin.set_fact: + edited_candidate_lines: >- + {{ lookup('file', backup_result.backup_path).splitlines() + | reject('search', 'firewall ipv4 name example rule 200') + | list }} + +- name: sanity check candidate still contains SSH/management essentials + ansible.builtin.assert: + that: + # adjust these to whatever this lab image's real baseline contains -- + # the point is: FAIL LOUDLY here rather than push a candidate missing + # management config, instead of discovering it via a dropped SSH session. + - "edited_candidate_lines | select('search', 'service ssh') | list | length > 0" + - "edited_candidate_lines | select('search', 'interfaces ethernet eth0') | list | length > 0" + +- name: replace with edited full config (rule 200 removed, everything else intact) + register: result + vyos.vyos.vyos_config: + lines: "{{ edited_candidate_lines }}" + replace: true + +- assert: + that: + - result.changed == true + - '''delete firewall ipv4 name example rule 200'' in result.commands or + ''delete firewall ipv4 name example rule 200 action "accept"'' in result.commands' + # the real regression check for T6837: no leftover empty rule 200 stub + - result.commands | select('match', '^set firewall ipv4 name example rule 200') | list | length == 0 + +- name: verify connectivity survived (would hang/fail above already if not, but confirm explicitly) + vyos.vyos.vyos_facts: + gather_subset: min + register: facts_check + +- assert: + that: + - facts_check is succeeded + +- name: teardown firewall rule + vyos.vyos.vyos_config: + lines: + - delete firewall ipv4 name example + match: none + +- debug: msg="END cli/replaced.yaml on connection={{ ansible_connection }}" diff --git a/tests/unit/modules/network/vyos/test_vyos_config.py b/tests/unit/modules/network/vyos/test_vyos_config.py index e732ca60..8e53c971 100644 --- a/tests/unit/modules/network/vyos/test_vyos_config.py +++ b/tests/unit/modules/network/vyos/test_vyos_config.py @@ -1,179 +1,397 @@ # # (c) 2016 Red Hat Inc. # # 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 . # Make coding more python3-ish from __future__ import absolute_import, division, print_function __metaclass__ = type from unittest.mock import MagicMock, patch from ansible_collections.vyos.vyos.plugins.cliconf.vyos import Cliconf from ansible_collections.vyos.vyos.plugins.modules import vyos_config from ansible_collections.vyos.vyos.tests.unit.modules.utils import set_module_args from .vyos_module import TestVyosModule, load_fixture class TestVyosConfigModule(TestVyosModule): module = vyos_config def setUp(self): super(TestVyosConfigModule, self).setUp() self.mock_get_config = patch( "ansible_collections.vyos.vyos.plugins.modules.vyos_config.get_config", ) self.get_config = self.mock_get_config.start() self.mock_load_config = patch( "ansible_collections.vyos.vyos.plugins.modules.vyos_config.load_config", ) self.load_config = self.mock_load_config.start() self.mock_run_commands = patch( "ansible_collections.vyos.vyos.plugins.modules.vyos_config.run_commands", ) self.run_commands = self.mock_run_commands.start() self.mock_get_connection = patch( "ansible_collections.vyos.vyos.plugins.modules.vyos_config.get_connection", ) self.get_connection = self.mock_get_connection.start() self.cliconf_obj = Cliconf(MagicMock()) self.running_config = load_fixture("vyos_config_config.cfg") self.conn = self.get_connection() self.conn.edit_config = MagicMock() self.running_config = load_fixture("vyos_config_config.cfg") def tearDown(self): super(TestVyosConfigModule, self).tearDown() self.mock_get_config.stop() self.mock_load_config.stop() self.mock_run_commands.stop() self.mock_get_connection.stop() def load_fixtures(self, commands=None, filename=None): config_file = "vyos_config_config.cfg" self.get_config.return_value = load_fixture(config_file) self.load_config.return_value = None def test_vyos_config_unchanged(self): src = load_fixture("vyos_config_config.cfg") self.conn.get_diff = MagicMock(return_value=self.cliconf_obj.get_diff(src, src)) set_module_args(dict(src=src)) self.execute_module() def test_vyos_config_src(self): src = load_fixture("vyos_config_src.cfg") set_module_args(dict(src=src)) candidate = "\n".join(self.module.format_commands(src.splitlines())) commands = [ "set system host-name foo", "delete interfaces ethernet eth0 address", ] self.conn.get_diff = MagicMock( return_value=self.cliconf_obj.get_diff(candidate, self.running_config), ) self.execute_module(changed=True, commands=commands) def test_vyos_config_src_brackets(self): src = load_fixture("vyos_config_src_brackets.cfg") set_module_args(dict(src=src)) commands = [ "set interfaces ethernet eth0 address 10.10.10.10/24", "set policy route testroute rule 1 set table 10", "set system host-name foo", ] self.conn.get_diff = MagicMock(side_effect=self.cliconf_obj.get_diff) self.execute_module(changed=True, commands=commands) def test_vyos_config_backup(self): set_module_args(dict(backup=True)) result = self.execute_module() self.assertIn("__backup__", result) def test_vyos_config_lines(self): 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) def test_vyos_config_config(self): config = "set system host-name localhost" new_config = ["set system host-name router"] set_module_args(dict(lines=new_config, config=config)) candidate = "\n".join(new_config) self.conn.get_diff = MagicMock(return_value=self.cliconf_obj.get_diff(candidate, config)) self.execute_module(changed=True, commands=new_config) def test_vyos_config_match_none(self): lines = [ "set system interfaces ethernet eth0 address 1.2.3.4/24", "set system interfaces ethernet eth0 description test string", ] set_module_args(dict(lines=lines, match="none")) candidate = "\n".join(lines) self.conn.get_diff = MagicMock( return_value=self.cliconf_obj.get_diff(candidate, None, diff_match="none"), ) self.execute_module(changed=True, commands=lines, sort=False) def test_vyos_config_confirm_automatic(self): src = load_fixture("vyos_config_src.cfg") confirm_timeout = 7 set_module_args(dict(src=src, confirm="automatic", confirm_timeout=confirm_timeout)) candidate = "\n".join(self.module.format_commands(src.splitlines())) commands = [ "set system host-name foo", "delete interfaces ethernet eth0 address", ] self.conn.get_diff = MagicMock( return_value=self.cliconf_obj.get_diff(candidate, self.running_config), ) self.execute_module(changed=True, commands=commands) 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_confirm_manual(self): lines = [ "set system host-name foo", ] confirm_timeout = 12 set_module_args(dict(lines=lines, confirm="manual", confirm_timeout=confirm_timeout)) 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.assertEqual(self.load_config.call_args[1]["confirm"], confirm_timeout) self.run_commands.assert_not_called() + + def test_vyos_config_replace_leaf_change(self): + """replace=True with a full candidate: a changed scalar value gets an + explicit delete-then-set pair. + + Earlier design suppressed the delete here based on observed + cardinality (1 running value, 1 candidate value -> assumed unique + scalar). That heuristic was unsound: it can't distinguish a + genuinely scalar attribute from a list-style attribute that merely + has one value right now (see test_vyos_config_replace_list_value_change + below for the failure case this caused). Deleting unconditionally, + ordered before the corresponding set, is correct for both cases and + carries no risk of the delete clobbering the just-applied set, since + the delete always runs first, while its target value is still active. + """ + running_hierarchical = "\n".join( + [ + "system {", + " host-name router", + " domain-name example.com", + "}", + ], + ) + candidate = "\n".join( + [ + "set system host-name 'foo'", + "set system domain-name 'example.com'", + ], + ) + diff = self.cliconf_obj.get_diff(candidate, running_hierarchical, diff_replace=True) + assert diff["config_diff"] == [ + "delete system host-name router", + "set system host-name 'foo'", + ] + + def test_vyos_config_replace_list_value_change(self): + """replace=True: a list-style attribute with one value changing to a + different single value must not retain the old value alongside the + new one. Regression guard for a real bug: a cardinality-based + heuristic previously mistook this for a scalar attribute update and + suppressed the delete, leaving both values configured. + """ + running_hierarchical = "system {\n name-server 8.8.8.8\n}\n" + candidate = "set system name-server 8.8.4.4" + diff = self.cliconf_obj.get_diff(candidate, running_hierarchical, diff_replace=True) + assert diff["config_diff"] == [ + "delete system name-server 8.8.8.8", + "set system name-server 8.8.4.4", + ] + + def test_vyos_config_replace_removes_missing_leaf(self): + """replace=True: a leaf present on router but absent from candidate gets deleted.""" + running_hierarchical = "\n".join( + [ + "system {", + " host-name router", + "}", + "interfaces {", + " ethernet eth1 {", + " address 6.7.8.9/24", + ' description "test string"', + " }", + "}", + ], + ) + candidate = "\n".join( + [ + "set system host-name router", + "set interfaces ethernet eth1 address '6.7.8.9/24'", + ], + ) + diff = self.cliconf_obj.get_diff(candidate, running_hierarchical, diff_replace=True) + assert "delete interfaces ethernet eth1 description" in " ".join(diff["config_diff"]) + + def test_vyos_config_replace_does_not_affect_default_match(self): + """Regression guard: replace=False must produce byte-identical diff to pre-PR behavior.""" + src = load_fixture("vyos_config_src.cfg") + candidate = "\n".join(self.module.format_commands(src.splitlines())) + diff_default = self.cliconf_obj.get_diff(candidate, self.running_config) + diff_explicit_false = self.cliconf_obj.get_diff( + candidate, + self.running_config, + diff_replace=False, + ) + assert diff_default == diff_explicit_false + + def test_vyos_config_replace_quoted_value_not_falsely_flagged(self): + """Double-quote-insensitive matching must stay scoped to replace mode. + + Single quotes are already stripped unconditionally on both sides before + this comparison, so they can't distinguish the two code paths. Double + quotes are the actual difference: match_cmd() (used only when + diff_replace=True) strips them too, while the default exact-match path + does not. This locks in that the default path still treats a + double-quoted running value as distinct from an unquoted candidate value, + while replace mode (which requires hierarchical running) correctly + treats them as the same value. + """ + candidate = "set system host-name foo" + + running_flat = 'set system host-name "foo"' + diff_default = self.cliconf_obj.get_diff(candidate, running_flat) + assert diff_default["config_diff"] == ["set system host-name foo"] + + running_hierarchical = 'system {\n host-name "foo"\n}\n' + diff_replace = self.cliconf_obj.get_diff( + candidate, + running_hierarchical, + diff_replace=True, + ) + assert diff_replace["config_diff"] == [] + + def test_vyos_config_replace_removes_orphaned_node(self): + """A rule entirely removed from candidate must not leave an empty stub node.""" + running_hierarchical = "\n".join( + [ + "firewall {", + " ipv4 {", + " name example {", + " rule 100 {", + " action drop", + " }", + " rule 200 {", + " action accept", + " }", + " }", + " }", + "}", + ], + ) + candidate = "set firewall ipv4 name example rule 100 action 'drop'" + diff = self.cliconf_obj.get_diff(candidate, running_hierarchical, diff_replace=True) + commands = diff["config_diff"] + # must delete the whole rule node, not just its leaf + assert "delete firewall ipv4 name example rule 200" in commands + assert commands.count("delete firewall ipv4 name example rule 200") == 1 + assert not any( + "action" in c and "rule 200" in c for c in commands if c.startswith("delete") + ) + + def test_vyos_config_replace_explicit_delete_line_not_treated_as_present(self): + """replace=True: an explicit delete line for a node must not fool the + tree-diff into thinking that node is still 'present' in the desired + state. + + Regression guard for a real bug: candidate_bodies previously included + stripped bodies from delete lines too, so a candidate containing + `delete firewall ipv4 name example rule 200` (while otherwise valid, + e.g. with other unrelated set lines making up the rest of a full + candidate) made _candidate_has_prefix() think that path "existed", + suppressing the clean whole-node delete and instead deleting rule + 200's children one at a time -- the same empty-stub orphan pattern + this whole diff_replace rewrite exists to fix, just reached via an + explicit delete line instead of omission. + """ + running_hierarchical = "\n".join( + [ + "firewall {", + " ipv4 {", + " name example {", + " rule 100 {", + " action drop", + " }", + " rule 200 {", + " action accept", + " description example", + " }", + " }", + " }", + "}", + "system {", + " host-name router", + "}", + ], + ) + candidate = "\n".join( + [ + "set firewall ipv4 name example rule 100 action drop", + "delete firewall ipv4 name example rule 200", + "set system host-name router", + ], + ) + diff = self.cliconf_obj.get_diff(candidate, running_hierarchical, diff_replace=True) + commands = diff["config_diff"] + + # the whole node must be deleted as a single unit, not per-leaf + assert "delete firewall ipv4 name example rule 200" in commands + assert not any( + "action" in c and "rule 200" in c for c in commands if c.startswith("delete") + ) + assert not any( + "description" in c and "rule 200" in c for c in commands if c.startswith("delete") + ) + + # unrelated paths present in the full candidate must be untouched + assert not any("rule 100" in c and c.startswith("delete") for c in commands) + assert not any("host-name" in c and c.startswith("delete") for c in commands) + + # must not have wrongly deleted the entire firewall subtree + assert "delete firewall" not in commands + + def test_vyos_config_replace_requests_hierarchical_config(self): + """replace=True must request get_config(module, format='text') so + get_diff() receives hierarchical running config, not flat set-lines. + + Regression guard for the module-side wiring: all the other + replace-mode tests call Cliconf.get_diff() directly and never + exercise run()'s own get_config() call, so a future change to that + call site (e.g. dropping the format="text" argument) would go + completely undetected by the rest of the suite. + """ + lines = ["set system host-name foo"] + set_module_args(dict(lines=lines, replace=True)) + self.conn.get_diff = MagicMock(return_value={"config_diff": lines}) + + self.execute_module(changed=True, commands=lines) + + assert self.get_config.call_args.kwargs.get("format") == "text"