From 825d9df5ea27260de2fad42911bd849023d8d701 Mon Sep 17 00:00:00 2001 From: Ganesh Nalawade Date: Wed, 24 May 2017 19:40:38 +0530 Subject: [PATCH] Add eos changes for Python3 (#24600) * eos python3 changes * changes to convert response from byte to text * Add dellos6 python3 changes Make `execute_command` arguments and its return value complaint to PY3 changes made in PR #24431 * Fix py3 prompt issue for invalid show command * Fix review comments * Add generic fix for error prompt in py3 * Fix CI issue * Fix network_cli unit test failure --- lib/ansible/module_utils/eos.py | 52 +++++++------ lib/ansible/plugins/connection/network_cli.py | 76 +++++++++++++------ lib/ansible/plugins/terminal/dellos6.py | 4 +- lib/ansible/plugins/terminal/eos.py | 49 ++++++------ test/sanity/pep8/legacy-files.txt | 2 - .../plugins/connection/test_network_cli.py | 6 +- 6 files changed, 110 insertions(+), 79 deletions(-) diff --git a/lib/ansible/module_utils/eos.py b/lib/ansible/module_utils/eos.py index dd960c17992..e3fbf415fa6 100644 --- a/lib/ansible/module_utils/eos.py +++ b/lib/ansible/module_utils/eos.py @@ -30,6 +30,7 @@ import os import time +from ansible.module_utils._text import to_text, to_native from ansible.module_utils.basic import env_fallback, return_values from ansible.module_utils.connection import exec_command from ansible.module_utils.network_common import to_list, ComplexList @@ -64,18 +65,17 @@ ARGS_DEFAULT_VALUE = { 'validate_certs': True } + def check_args(module, warnings): provider = module.params['provider'] or {} for key in eos_argument_spec: if module._name == 'eos_user': if (key not in ['username', 'password', 'provider', 'transport', 'authorize'] and module.params[key]): - warnings.append('argument %s has been deprecated and will be ' - 'removed in a future version' % key) + warnings.append('argument %s has been deprecated and will be removed in a future version' % key) else: if key not in ['provider', 'authorize'] and module.params[key]: - warnings.append('argument %s has been deprecated and will be ' - 'removed in a future version' % key) + warnings.append('argument %s has been deprecated and will be removed in a future version' % key) # set argument's default value if not provided in input # This is done to avoid unwanted argument deprecation warning @@ -89,6 +89,7 @@ def check_args(module, warnings): if provider.get(param): module.no_log_values.update(return_values(provider[param])) + def load_params(module): provider = module.params.get('provider') or dict() for key, value in iteritems(provider): @@ -96,6 +97,7 @@ def load_params(module): if module.params.get(key) is None and value is not None: module.params[key] = value + def get_connection(module): global _DEVICE_CONNECTION if not _DEVICE_CONNECTION: @@ -131,6 +133,7 @@ class Cli: def check_authorization(self): for cmd in ['show clock', 'prompt()']: rc, out, err = self.exec_command(cmd) + out = to_text(out, errors='surrogate_then_replace') return out.endswith('#') def get_config(self, flags=[]): @@ -145,8 +148,9 @@ class Cli: except KeyError: conn = get_connection(self) rc, out, err = self.exec_command(cmd) + out = to_text(out, errors='surrogate_then_replace') if rc != 0: - self._module.fail_json(msg=err) + self._module.fail_json(msg=to_text(err, errors='surrogate_then_replace')) cfg = str(out).strip() self._device_configs[cmd] = cfg return cfg @@ -158,9 +162,9 @@ class Cli: for cmd in to_list(commands): rc, out, err = self.exec_command(cmd) - + out = to_text(out, errors='surrogate_then_replace') if check_rc and rc != 0: - self._module.fail_json(msg=err) + self._module.fail_json(msg=to_text(err, errors='surrogate_then_replace')) try: out = self._module.from_json(out) @@ -185,9 +189,9 @@ class Cli: rc, out, err = self.exec_command(command) if rc != 0: - return (rc, out, err) + return (rc, out, to_text(err, errors='surrogate_then_replace')) - return (rc, 'ok','') + return (rc, 'ok', '') def configure(self, commands): """Sends configuration commands to the remote device @@ -199,11 +203,11 @@ class Cli: rc, out, err = self.exec_command('configure') if rc != 0: - self._module.fail_json(msg='unable to enter configuration mode', output=err) + self._module.fail_json(msg='unable to enter configuration mode', output=to_text(err, errors='surrogate_then_replace')) rc, out, err = self.send_config(commands) if rc != 0: - self._module.fail_json(msg=err) + self._module.fail_json(msg=to_text(err, errors='surrogate_then_replace')) self.exec_command('end') return {} @@ -221,7 +225,7 @@ class Cli: pass if not all((bool(use_session), self.supports_sessions)): - return configure(self, commands) + return self.configure(self, commands) conn = get_connection(self) session = 'ansible_%s' % int(time.time()) @@ -229,7 +233,7 @@ class Cli: rc, out, err = self.exec_command('configure session %s' % session) if rc != 0: - self._module.fail_json(msg='unable to enter configuration mode', output=err) + self._module.fail_json(msg='unable to enter configuration mode', output=to_text(err, errors='surrogate_then_replace')) if replace: self.exec_command('rollback clean-config', check_rc=True) @@ -237,11 +241,11 @@ class Cli: rc, out, err = self.send_config(commands) if rc != 0: self.exec_command('abort') - self._module.fail_json(msg=err, commands=commands) + self._module.fail_json(msg=to_text(err, errors='surrogate_then_replace'), commands=commands) rc, out, err = self.exec_command('show session-config diffs') if rc == 0 and out: - result['diff'] = out.strip() + result['diff'] = to_text(out, errors='surrogate_then_replace').strip() if commit: self.exec_command('commit') @@ -250,13 +254,14 @@ class Cli: return result + class Eapi: def __init__(self, module): self._module = module self._enable = None self._session_support = None - self._device_configs = {} + self._device_configs = {} host = module.params['provider']['host'] port = module.params['provider']['port'] @@ -312,7 +317,7 @@ class Eapi: try: data = response.read() - response = self._module.from_json(data) + response = self._module.from_json(to_text(data, errors='surrogate_then_replace')) except ValueError: self._module.fail_json(msg='unable to load response from device', data=data) @@ -394,7 +399,7 @@ class Eapi: there will be no returned diff or session values """ if not self.supports_sessions: - return configure(self, commands) + return self.configure(self, config) session = 'ansible_%s' % int(time.time()) result = {'session': session} @@ -425,16 +430,17 @@ class Eapi: return result -is_json = lambda x: str(x).endswith('| json') -is_text = lambda x: not str(x).endswith('| json') -supports_sessions = lambda x: get_connection(module).supports_sessions +def is_json(cmd): + return to_native(cmd, errors='surrogate_then_replace').endswith('| json') + def is_eapi(module): transport = module.params['transport'] provider_transport = (module.params['provider'] or {}).get('transport') return 'eapi' in (transport, provider_transport) + def to_command(module, commands): if is_eapi(module): default_output = 'json' @@ -450,15 +456,17 @@ def to_command(module, commands): return transform(to_list(commands)) + def get_config(module, flags=[]): conn = get_connection(module) return conn.get_config(flags) + def run_commands(module, commands): conn = get_connection(module) return conn.run_commands(to_command(module, commands)) + def load_config(module, config, commit=False, replace=False): conn = get_connection(module) return conn.load_config(config, commit, replace) - diff --git a/lib/ansible/plugins/connection/network_cli.py b/lib/ansible/plugins/connection/network_cli.py index 6f004c00fc8..dd2b1800135 100644 --- a/lib/ansible/plugins/connection/network_cli.py +++ b/lib/ansible/plugins/connection/network_cli.py @@ -28,7 +28,7 @@ from collections import Sequence from ansible import constants as C from ansible.errors import AnsibleConnectionFailure -from ansible.module_utils.six import BytesIO, binary_type, text_type +from ansible.module_utils.six import BytesIO, binary_type from ansible.module_utils._text import to_bytes, to_text from ansible.plugins import terminal_loader from ansible.plugins.connection import ensure_connect @@ -151,7 +151,7 @@ class Connection(_Connection): window = self._strip(recv.read()) if obj and (obj.get('prompt') and not handled): - handled = self._handle_prompt(window, obj) + handled = self._handle_prompt(window, obj['prompt'], obj['answer']) if self._find_prompt(window): self._last_response = recv.getvalue() @@ -177,17 +177,23 @@ class Connection(_Connection): data = regex.sub(b'', data) return data - def _handle_prompt(self, resp, obj): - """Matches the command prompt and responds""" - if isinstance(obj, (binary_type, text_type)) or not isinstance(obj['prompt'], Sequence): - obj['prompt'] = [obj['prompt']] - prompts = [re.compile(r, re.I) for r in obj['prompt']] - answer = obj['answer'] + def _handle_prompt(self, resp, prompts, answer): + """ + Matches the command prompt and responds + + :arg resp: Byte string containing the raw response from the remote + :arg prompts: Sequence of byte strings that we consider prompts for input + :arg answer: Byte string to send back to the remote if we find a prompt. + A carriage return is automatically appended to this string. + :returns: True if a prompt was found in ``resp``. False otherwise + """ + prompts = [re.compile(r, re.I) for r in prompts] for regex in prompts: match = regex.search(resp) if match: self._shell.sendall(b'%s\r' % answer) return True + return False def _sanitize(self, resp, obj=None): """Removes elements from the response before returning to the caller""" @@ -197,27 +203,38 @@ class Connection(_Connection): if (command and line.startswith(command.strip())) or self._matched_prompt.strip() in line: continue cleaned.append(line) - return b"\n".join(cleaned).strip() + return b'\n'.join(cleaned).strip() def _find_prompt(self, response): """Searches the buffered response for a matching command prompt""" errored_response = None + is_error_message = False for regex in self._terminal.terminal_stderr_re: if regex.search(response): - errored_response = response - break + is_error_message = True - for regex in self._terminal.terminal_stdout_re: - match = regex.search(response) - if match: - self._matched_pattern = regex.pattern - self._matched_prompt = match.group() - if not errored_response: - return True + # Check if error response ends with command prompt if not + # receive it buffered prompt + for regex in self._terminal.terminal_stdout_re: + match = regex.search(response) + if match: + errored_response = response + break + + if not is_error_message: + for regex in self._terminal.terminal_stdout_re: + match = regex.search(response) + if match: + self._matched_pattern = regex.pattern + self._matched_prompt = match.group() + if not errored_response: + return True if errored_response: raise AnsibleConnectionFailure(errored_response) + return False + def alarm_handler(self, signum, frame): """Alarm handler raised in case of command timeout """ display.display('closing shell due to sigalarm', log_only=True) @@ -231,11 +248,11 @@ class Connection(_Connection): second form is as a utf8 JSON byte string with additional keywords. Keywords supported for cmd: - * command - the command string to execute - * prompt - the expected prompt generated by executing command - * answer - the string to respond to the prompt with - * sendonly - bool to disable waiting for response - + :command: the command string to execute + :prompt: the expected prompt generated by executing command. + This can be a string or a list of strings + :answer: the string to respond to the prompt with + :sendonly: bool to disable waiting for response :arg cmd: the byte string that represents the command to be executed which can be a single command or a json encoded string. :returns: a tuple of (return code, stdout, stderr). The return @@ -243,10 +260,21 @@ class Connection(_Connection): """ try: obj = json.loads(to_text(cmd, errors='surrogate_or_strict')) - obj = dict((k, to_bytes(v, errors='surrogate_or_strict', nonstring='passthru')) for k, v in obj.items()) except (ValueError, TypeError): obj = {'command': to_bytes(cmd.strip(), errors='surrogate_or_strict')} + obj = dict((k, to_bytes(v, errors='surrogate_or_strict', nonstring='passthru')) for k, v in obj.items()) + if 'prompt' in obj: + if isinstance(obj['prompt'], binary_type): + # Prompt was a string + obj['prompt'] = [obj['prompt']] + elif not isinstance(obj['prompt'], Sequence): + # Convert nonstrings into byte strings (to_bytes(5) => b'5') + if obj['prompt'] is not None: + obj['prompt'] = [to_bytes(obj['prompt'], errors='surrogate_or_strict')] + else: + # Prompt was a Sequence of strings. Make sure they're byte strings + obj['prompt'] = [to_bytes(p, errors='surrogate_or_strict') for p in obj['prompt'] if p is not None] if obj['command'] == b'close_shell()': return self.close_shell() elif obj['command'] == b'open_shell()': diff --git a/lib/ansible/plugins/terminal/dellos6.py b/lib/ansible/plugins/terminal/dellos6.py index b939318e5e3..4a9272130c8 100644 --- a/lib/ansible/plugins/terminal/dellos6.py +++ b/lib/ansible/plugins/terminal/dellos6.py @@ -48,8 +48,8 @@ class TerminalModule(TerminalBase): cmd = {u'command': u'enable'} if passwd: - cmd['prompt'] = to_text(r"[\r\n]?password: $", errors='surrogate_or_strict') - cmd['answer'] = passwd + cmd[u'prompt'] = to_text(r"[\r\n]?password: $", errors='surrogate_or_strict') + cmd[u'answer'] = passwd try: self._exec_cli_command(to_bytes(json.dumps(cmd), errors='surrogate_or_strict')) except AnsibleConnectionFailure: diff --git a/lib/ansible/plugins/terminal/eos.py b/lib/ansible/plugins/terminal/eos.py index 98d96b2807a..951293e584a 100644 --- a/lib/ansible/plugins/terminal/eos.py +++ b/lib/ansible/plugins/terminal/eos.py @@ -24,46 +24,47 @@ import json from ansible.plugins.terminal import TerminalBase from ansible.errors import AnsibleConnectionFailure +from ansible.module_utils._text import to_bytes, to_text class TerminalModule(TerminalBase): terminal_stdout_re = [ - re.compile(r"[\r\n]?[\w+\-\.:\/\[\]]+(?:\([^\)]+\)){,3}(?:>|#) ?$"), - re.compile(r"\[\w+\@[\w\-\.]+(?: [^\]])\] ?[>#\$] ?$") + re.compile(br"[\r\n]?[\w+\-\.:\/\[\]]+(?:\([^\)]+\)){,3}(?:>|#) ?$"), + re.compile(br"\[\w+\@[\w\-\.]+(?: [^\]])\] ?[>#\$] ?$") ] terminal_stderr_re = [ - re.compile(r"% ?Error"), - re.compile(r"^% \w+", re.M), - re.compile(r"% User not present"), - re.compile(r"% ?Bad secret"), - re.compile(r"invalid input", re.I), - re.compile(r"(?:incomplete|ambiguous) command", re.I), - re.compile(r"connection timed out", re.I), - re.compile(r"[^\r\n]+ not found", re.I), - re.compile(r"'[^']' +returned error code: ?\d+"), - re.compile(r"[^\r\n]\/bin\/(?:ba)?sh") + re.compile(br"% ?Error"), + # re.compile(br"^% \w+", re.M), + re.compile(br"% User not present"), + re.compile(br"% ?Bad secret"), + re.compile(br"invalid input", re.I), + re.compile(br"(?:incomplete|ambiguous) command", re.I), + re.compile(br"connection timed out", re.I), + re.compile(br"[^\r\n]+ not found", re.I), + re.compile(br"'[^']' +returned error code: ?\d+"), + re.compile(br"[^\r\n]\/bin\/(?:ba)?sh") ] def on_open_shell(self): try: - for cmd in ['terminal length 0', 'terminal width 512']: + for cmd in (b'terminal length 0', b'terminal width 512'): self._exec_cli_command(cmd) except AnsibleConnectionFailure: raise AnsibleConnectionFailure('unable to set terminal parameters') def on_authorize(self, passwd=None): - if self._get_prompt().endswith('#'): + if self._get_prompt().endswith(b'#'): return - cmd = {'command': 'enable'} + cmd = {u'command': u'enable'} if passwd: - cmd['prompt'] = r"[\r\n]?password: $" - cmd['answer'] = passwd + cmd[u'prompt'] = to_text(r"[\r\n]?password: $", errors='surrogate_or_strict') + cmd[u'answer'] = passwd try: - self._exec_cli_command(json.dumps(cmd)) + self._exec_cli_command(to_bytes(json.dumps(cmd), errors='surrogate_or_strict')) except AnsibleConnectionFailure: raise AnsibleConnectionFailure('unable to elevate privilege to enable mode') @@ -73,11 +74,9 @@ class TerminalModule(TerminalBase): # if prompt is None most likely the terminal is hung up at a prompt return - if '(config' in prompt: - self._exec_cli_command('end') - self._exec_cli_command('disable') - - elif prompt.endswith('#'): - self._exec_cli_command('disable') - + if b'(config' in prompt: + self._exec_cli_command(b'end') + self._exec_cli_command(b'disable') + elif prompt.endswith(b'#'): + self._exec_cli_command(b'disable') diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index c474455267f..0877ceb22fa 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/test/sanity/pep8/legacy-files.txt @@ -43,7 +43,6 @@ lib/ansible/module_utils/connection.py lib/ansible/module_utils/database.py lib/ansible/module_utils/docker_common.py lib/ansible/module_utils/ec2.py -lib/ansible/module_utils/eos.py lib/ansible/module_utils/f5_utils.py lib/ansible/module_utils/facts.py lib/ansible/module_utils/fortios.py @@ -808,7 +807,6 @@ lib/ansible/plugins/strategy/debug.py lib/ansible/plugins/strategy/free.py lib/ansible/plugins/strategy/linear.py lib/ansible/plugins/terminal/asa.py -lib/ansible/plugins/terminal/eos.py lib/ansible/plugins/terminal/junos.py lib/ansible/plugins/test/core.py lib/ansible/plugins/test/files.py diff --git a/test/units/plugins/connection/test_network_cli.py b/test/units/plugins/connection/test_network_cli.py index b9e4c8e40eb..133e806a3d7 100644 --- a/test/units/plugins/connection/test_network_cli.py +++ b/test/units/plugins/connection/test_network_cli.py @@ -148,7 +148,6 @@ class TestConnectionClass(unittest.TestCase): pc = PlayContext() new_stdin = StringIO() conn = network_cli.Connection(pc, new_stdin) - mock__terminal = MagicMock() mock__terminal.terminal_stdout_re = [re.compile(b'device#')] mock__terminal.terminal_stderr_re = [re.compile(b'^ERROR')] @@ -171,9 +170,8 @@ class TestConnectionClass(unittest.TestCase): self.assertEqual(output, b'command response') mock__shell.reset_mock() - mock__shell.recv.return_value = b"ERROR: error message" + mock__shell.recv.return_value = b"ERROR: error message device#" with self.assertRaises(AnsibleConnectionFailure) as exc: conn.send({'command': b'command'}) - self.assertEqual(str(exc.exception), 'ERROR: error message') - + self.assertEqual(str(exc.exception), 'ERROR: error message device#')