From 07ef0c5168089608099c0ce7f37a43dbcd41606b Mon Sep 17 00:00:00 2001 From: Kate Case Date: Wed, 26 Jul 2023 22:07:15 -0400 Subject: [PATCH] Add default cliconf plugin good enough to use cli_command most of the time (#569) * Add default cliconf plugin good enough to use cli_command most of the time * Delegate more functionality to CliconfBase * Add tests for default --- README.md | 5 + changelogs/fragments/default-cliconf.yaml | 5 + docs/ansible.netcommon.default_cliconf.rst | 43 +++++++ plugins/cliconf/default.py | 67 +++++++++++ plugins/connection/network_cli.py | 38 ++++--- plugins/plugin_utils/cliconf_base.py | 46 ++++++-- tests/unit/plugins/cliconf/test_default.py | 126 +++++++++++++++++++++ 7 files changed, 305 insertions(+), 25 deletions(-) create mode 100644 changelogs/fragments/default-cliconf.yaml create mode 100644 docs/ansible.netcommon.default_cliconf.rst create mode 100644 plugins/cliconf/default.py create mode 100644 tests/unit/plugins/cliconf/test_default.py diff --git a/README.md b/README.md index e3e13d5e2..f469ebe31 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,11 @@ Name | Description --- | --- [ansible.netcommon.enable](https://github.com/ansible-collections/ansible.netcommon/blob/main/docs/ansible.netcommon.enable_become.rst)|Switch to elevated permissions on a network device +### Cliconf plugins +Name | Description +--- | --- +[ansible.netcommon.default](https://github.com/ansible-collections/ansible.netcommon/blob/main/docs/ansible.netcommon.default_cliconf.rst)|General purpose cliconf plugin for new platforms + ### Connection plugins Name | Description --- | --- diff --git a/changelogs/fragments/default-cliconf.yaml b/changelogs/fragments/default-cliconf.yaml new file mode 100644 index 000000000..de1a87504 --- /dev/null +++ b/changelogs/fragments/default-cliconf.yaml @@ -0,0 +1,5 @@ +--- +minor_changes: + - Add a new cliconf plugin ``default`` that can be used when no cliconf + plugin is found for a given network_os. This plugin only supports ``get()``. + (https://github.com/ansible-collections/ansible.netcommon/pull/569) diff --git a/docs/ansible.netcommon.default_cliconf.rst b/docs/ansible.netcommon.default_cliconf.rst new file mode 100644 index 000000000..78e126aa1 --- /dev/null +++ b/docs/ansible.netcommon.default_cliconf.rst @@ -0,0 +1,43 @@ +.. _ansible.netcommon.default_cliconf: + + +************************* +ansible.netcommon.default +************************* + +**General purpose cliconf plugin for new platforms** + + +Version added: 5.2.0 + +.. contents:: + :local: + :depth: 1 + + +Synopsis +-------- +- This plugin attemts to provide low level abstraction apis for sending and receiving CLI commands from arbitrary network devices. + + + + + + + + + + + +Status +------ + + +Authors +~~~~~~~ + +- Ansible Networking Team (@ansible-network) + + +.. hint:: + Configuration entries for each entry type have a low to high priority order. For example, a variable that is lower in the list will override a variable that is higher up. diff --git a/plugins/cliconf/default.py b/plugins/cliconf/default.py new file mode 100644 index 000000000..5d418d54f --- /dev/null +++ b/plugins/cliconf/default.py @@ -0,0 +1,67 @@ +# (c) 2023 Red Hat Inc. +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import absolute_import, division, print_function + + +__metaclass__ = type + +DOCUMENTATION = """ +author: Ansible Networking Team (@ansible-network) +name: default +short_description: General purpose cliconf plugin for new platforms +description: +- This plugin attemts to provide low level abstraction apis for sending and receiving CLI + commands from arbitrary network devices. +version_added: 5.2.0 +""" + +import json + +from ansible.errors import AnsibleConnectionFailure + +from ansible_collections.ansible.netcommon.plugins.plugin_utils.cliconf_base import CliconfBase + + +class Cliconf(CliconfBase): + 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"] = "default" + self._device_info = device_info + + return self._device_info + + def get_config(self, flags=None, format=None): + network_os = self.get_device_info()["network_os"] + raise AnsibleConnectionFailure("get_config is not supported by network_os %s" % network_os) + + def edit_config(self, candidate=None, commit=True, replace=None, comment=None): + network_os = self.get_device_info()["network_os"] + raise AnsibleConnectionFailure("edit_config is not supported by network_os %s" % network_os) + + def get_capabilities(self): + result = super(Cliconf, self).get_capabilities() + result["device_operations"] = self.get_device_operations() + return json.dumps(result) + + def get_device_operations(self): + return { + "supports_diff_replace": False, + "supports_commit": False, + "supports_rollback": False, + "supports_defaults": False, + "supports_onbox_diff": False, + "supports_commit_comment": False, + "supports_multiline_delimiter": False, + "supports_diff_match": False, + "supports_diff_ignore_lines": False, + "supports_generate_diff": False, + "supports_replace": False, + } diff --git a/plugins/connection/network_cli.py b/plugins/connection/network_cli.py index b0da62dfd..5063b43e0 100644 --- a/plugins/connection/network_cli.py +++ b/plugins/connection/network_cli.py @@ -386,26 +386,30 @@ def __init__(self, play_context, new_stdin, *args, **kwargs): raise AnsibleConnectionFailure("network os %s is not supported" % self._network_os) self.cliconf = cliconf_loader.get(self._network_os, self) - if self.cliconf: - self._sub_plugin = { - "type": "cliconf", - "name": self.cliconf._load_name, - "obj": self.cliconf, - } + if not self.cliconf: self.queue_message( "vvvv", - "loaded cliconf plugin %s from path %s for network_os %s" - % ( - self.cliconf._load_name, - self.cliconf._original_path, - self._network_os, - ), - ) - else: - self.queue_message( - "vvvv", - "unable to load cliconf for network_os %s" % self._network_os, + "unable to load cliconf for network_os %s. Falling back to default" + % self._network_os, ) + self.cliconf = cliconf_loader.get("ansible.netcommon.default", self) + if not self.cliconf: + raise AnsibleConnectionFailure("Couldn't load fallback cliconf plugin") + + self._sub_plugin = { + "type": "cliconf", + "name": self.cliconf._load_name, + "obj": self.cliconf, + } + self.queue_message( + "vvvv", + "loaded cliconf plugin %s from path %s for network_os %s" + % ( + self.cliconf._load_name, + self.cliconf._original_path, + self._network_os, + ), + ) else: raise AnsibleConnectionFailure( "Unable to automatically determine host network os. Please " diff --git a/plugins/plugin_utils/cliconf_base.py b/plugins/plugin_utils/cliconf_base.py index 5b91d388b..30b4aa550 100644 --- a/plugins/plugin_utils/cliconf_base.py +++ b/plugins/plugin_utils/cliconf_base.py @@ -13,10 +13,13 @@ from ansible.errors import AnsibleConnectionFailure, AnsibleError from ansible.module_utils._text import to_bytes, to_text +from ansible.module_utils.common._collections_compat import Mapping # Needed to satisfy PluginLoader's required_base_class from ansible.plugins.cliconf import CliconfBase as CliconfBaseBase +from ansible_collections.ansible.netcommon.plugins.module_utils.network.common.utils import to_list + try: from scp import SCPClient @@ -70,12 +73,13 @@ class CliconfBase(CliconfBaseBase): """ __rpc__ = [ - "get_config", "edit_config", - "get_capabilities", - "get", "enable_response_logging", + "get", + "get_capabilities", + "get_config", "disable_response_logging", + "run_commands", ] def __init__(self, connection): @@ -241,7 +245,6 @@ def edit_config( """ pass - @abstractmethod def get( self, command=None, @@ -268,7 +271,17 @@ def get( given prompt. :return: The output from the device after executing the command """ - pass + if not command: + raise ValueError("must provide value of command to execute") + + return self.send_command( + command=command, + prompt=prompt, + answer=answer, + sendonly=sendonly, + newline=newline, + check_all=check_all, + ) @abstractmethod def get_capabilities(self): @@ -340,7 +353,7 @@ def commit(self, comment=None): :return: None """ - return self._connection.method_not_found( + raise AnsibleConnectionFailure( "commit is not supported by network_os %s" % self._play_context.network_os ) @@ -353,7 +366,7 @@ def discard_changes(self): :returns: None """ - return self._connection.method_not_found( + raise AnsibleConnectionFailure( "discard_changes is not supported by network_os %s" % self._play_context.network_os ) @@ -478,7 +491,24 @@ def run_commands(self, commands=None, check_rc=True): value is True an exception is raised. :return: List of returned response """ - pass + 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} + + 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 check_edit_config_capability( self, diff --git a/tests/unit/plugins/cliconf/test_default.py b/tests/unit/plugins/cliconf/test_default.py new file mode 100644 index 000000000..d78cf4b51 --- /dev/null +++ b/tests/unit/plugins/cliconf/test_default.py @@ -0,0 +1,126 @@ +# +# (c) 2016 Red Hat Inc. +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +# Make coding more python3-ish +from __future__ import absolute_import, division, print_function + + +__metaclass__ = type + +import json + +from unittest.mock import MagicMock + +import pytest + +from ansible.errors import AnsibleConnectionFailure +from ansible.plugins.loader import cliconf_loader + + +INFO = dict(network_os="default") +OPERATIONS = { + "supports_diff_replace": False, + "supports_commit": False, + "supports_rollback": False, + "supports_defaults": False, + "supports_onbox_diff": False, + "supports_commit_comment": False, + "supports_multiline_delimiter": False, + "supports_diff_match": False, + "supports_diff_ignore_lines": False, + "supports_generate_diff": False, + "supports_replace": False, +} +RPC = [ + "edit_config", + "enable_response_logging", + "get", + "get_capabilities", + "get_config", + "disable_response_logging", + "run_commands", +] + + +@pytest.fixture(name="cliconf") +def plugin_fixture(): + cliconf = cliconf_loader.get("ansible.netcommon.default", None) + return cliconf + + +def test_get_device_info(cliconf): + info = cliconf.get_device_info() + assert info == INFO + + # Test cached info works + assert info == cliconf.get_device_info() + + +def test_get_device_operations(cliconf): + ops = cliconf.get_device_operations() + assert ops == OPERATIONS + + +def test_get_capabilities(cliconf): + cap = json.loads(cliconf.get_capabilities()) + assert cap == dict( + rpc=RPC, + device_info=INFO, + network_api="cliconf", + device_operations=OPERATIONS, + ) + + +@pytest.mark.parametrize("method", ["get_config", "edit_config", "commit", "discard_changes"]) +def test_unsupported_method(cliconf, method): + cliconf._play_context = MagicMock() + cliconf._play_context.network_os = "default" + + with pytest.raises( + AnsibleConnectionFailure, match=f"{method} is not supported by network_os default" + ): + getattr(cliconf, method)() + + +def test_get(cliconf): + return_value = "ABC FakeOS v1.23.4" + conn = MagicMock() + conn.send.return_value = return_value + cliconf._connection = conn + resp = cliconf.get("show version") + assert resp == return_value + + +def test_get_no_command(cliconf): + with pytest.raises(ValueError, match="must provide value of command to execute"): + cliconf.get() + + +@pytest.mark.parametrize("commands", ["show version", {"command": "show version"}]) +def test_run_commands(cliconf, commands): + return_value = "ABC FakeOS v1.23.4" + conn = MagicMock() + conn.send.return_value = return_value + cliconf._connection = conn + resp = cliconf.run_commands([commands]) + assert resp == [return_value] + + +@pytest.mark.parametrize("check_rc", [True, False]) +def test_run_commands_check_rc(cliconf, check_rc): + error = AnsibleConnectionFailure("Invalid command: [sow]") + cliconf.send_command = MagicMock(side_effect=error) + + if check_rc: + with pytest.raises(AnsibleConnectionFailure): + resp = cliconf.run_commands(["sow version"], check_rc=check_rc) + else: + resp = cliconf.run_commands(["sow version"], check_rc=check_rc) + assert resp == [error] + + +def test_run_commands_no_commands(cliconf): + with pytest.raises(ValueError, match="'commands' value is required"): + cliconf.run_commands()