From a38e727380964b66fd94c5b760ef1c90a21f5dd7 Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Fri, 16 Jun 2017 09:15:08 -0400 Subject: [PATCH] nxos_nxapi fix (#25306) * Add nxos_nxapi tests * Simple changes to nxos_nxapi * Move validation to check_args * Don't mark protocol change unless change is requested * Add different regex to handle HTTP{,S} ports on a different version of nxos --- .../modules/network/nxos/nxos_nxapi.py | 75 +++++++++---------- .../nxos/fixtures/nxos_nxapi/show_nxapi | 4 + .../modules/network/nxos/test_nxos_nxapi.py | 69 +++++++++++++++++ 3 files changed, 109 insertions(+), 39 deletions(-) create mode 100644 test/units/modules/network/nxos/fixtures/nxos_nxapi/show_nxapi create mode 100644 test/units/modules/network/nxos/test_nxos_nxapi.py diff --git a/lib/ansible/modules/network/nxos/nxos_nxapi.py b/lib/ansible/modules/network/nxos/nxos_nxapi.py index ddeae6fb806..580dca08dc7 100644 --- a/lib/ansible/modules/network/nxos/nxos_nxapi.py +++ b/lib/ansible/modules/network/nxos/nxos_nxapi.py @@ -124,13 +124,10 @@ updates: """ import re -from functools import partial - from ansible.module_utils.nxos import run_commands, load_config from ansible.module_utils.nxos import nxos_argument_spec from ansible.module_utils.nxos import check_args as nxos_check_args from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.netcfg import NetworkConfig from ansible.module_utils.six import iteritems def check_args(module, warnings): @@ -159,11 +156,15 @@ def check_args(module, warnings): if module.params[key]: warnings.append('argument %s is deprecated and will be ignored' % key) + for key in ['http_port', 'https_port']: + if module.params[key] is not None: + if not 1 <= module.params[key] <= 65535: + module.fail_json(msg='%s must be between 1 and 65535' % key) + return warnings -def map_obj_to_commands(updates, module): +def map_obj_to_commands(want, have, module): commands = list() - want, have = updates needs_update = lambda x: want.get(x) is not None and (want.get(x) != have.get(x)) @@ -172,14 +173,14 @@ def map_obj_to_commands(updates, module): return ['no feature nxapi'] commands.append('feature nxapi') - if any((needs_update('http'), needs_update('http_port'))): + if needs_update('http') or (have.get('http') and needs_update('http_port')): if want['http'] is True or (want['http'] is None and have['http'] is True): port = want['http_port'] or 80 commands.append('nxapi http port %s' % port) elif want['http'] is False: commands.append('no nxapi http') - if any((needs_update('https'), needs_update('https_port'))): + if needs_update('https') or (have.get('https') and needs_update('https_port')): if want['https'] is True or (want['https'] is None and have['https'] is True): port = want['https_port'] or 443 commands.append('nxapi https port %s' % port) @@ -195,32 +196,42 @@ def map_obj_to_commands(updates, module): return commands def parse_http(data): - match = re.search('HTTP Port:\s+(\d+)', data, re.M) - if match: - return {'http': True, 'http_port': int(match.group(1))} - else: - return {'http': False, 'http_port': None} + http_res = [r'HTTP Port:\s+(\d+)', r'HTTP Listen on port (\d+)'] + http_port = None + + for regex in http_res: + match = re.search(regex, data, re.M) + if match: + http_port = int(match.group(1)) + break + + return {'http': http_port is not None, 'http_port': http_port} def parse_https(data): - match = re.search('HTTPS Port:\s+(\d+)', data, re.M) - if match: - return {'https': True, 'https_port': int(match.group(1))} - else: - return {'https': False, 'https_port': None} + https_res = [r'HTTPS Port:\s+(\d+)', r'HTTPS Listen on port (\d+)'] + https_port = None + + for regex in https_res: + match = re.search(regex, data, re.M) + if match: + https_port = int(match.group(1)) + break + + return {'https': https_port is not None, 'https_port': https_port} def parse_sandbox(data): - match = re.search('Sandbox:\s+(.+)$', data, re.M) - value = None + match = re.search(r'Sandbox:\s+(.+)$', data, re.M) + value = False if match: value = match.group(1) == 'Enabled' return {'sandbox': value} def map_config_to_obj(module): - out = run_commands(module, ['show nxapi'], check_rc=False) - if out[0] == '': + out = run_commands(module, ['show nxapi'], check_rc=False)[0] + if out == '': return {'state': 'absent'} - out = str(out[0]).strip() + out = str(out).strip() obj = {'state': 'present'} obj.update(parse_http(out)) @@ -229,14 +240,6 @@ def map_config_to_obj(module): return obj -def validate_http_port(value, module): - if not 1 <= module.params['http_port'] <= 65535: - module.fail_json(msg='http_port must be between 1 and 65535') - -def validate_https_port(value, module): - if not 1 <= module.params['https_port'] <= 65535: - module.fail_json(msg='https_port must be between 1 and 65535') - def map_params_to_obj(module): obj = { 'http': module.params['http'], @@ -247,12 +250,6 @@ def map_params_to_obj(module): 'state': module.params['state'] } - for key, value in iteritems(obj): - if value: - validator = globals().get('validate_%s' % key) - if validator: - validator(value, module) - return obj def main(): @@ -279,16 +276,16 @@ def main(): supports_check_mode=True) - result = {'changed': False} warnings = list() check_args(module, warnings) - result['warnings'] = warnings + + result = {'changed': False, 'warnings': warnings} want = map_params_to_obj(module) have = map_config_to_obj(module) - commands = map_obj_to_commands((want, have), module) + commands = map_obj_to_commands(want, have, module) result['commands'] = commands if commands: diff --git a/test/units/modules/network/nxos/fixtures/nxos_nxapi/show_nxapi b/test/units/modules/network/nxos/fixtures/nxos_nxapi/show_nxapi new file mode 100644 index 00000000000..834a46b0a40 --- /dev/null +++ b/test/units/modules/network/nxos/fixtures/nxos_nxapi/show_nxapi @@ -0,0 +1,4 @@ + +NX-API: Enabled Sandbox: Disabled +HTTP Port: 80 HTTPS Port: Disabled + diff --git a/test/units/modules/network/nxos/test_nxos_nxapi.py b/test/units/modules/network/nxos/test_nxos_nxapi.py new file mode 100644 index 00000000000..78c49467ced --- /dev/null +++ b/test/units/modules/network/nxos/test_nxos_nxapi.py @@ -0,0 +1,69 @@ +# (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 + +import json +import os + +from ansible.compat.tests.mock import patch +from ansible.modules.network.nxos import nxos_nxapi +from .nxos_module import TestNxosModule, load_fixture, set_module_args + + +class TestNxosNxapiModule(TestNxosModule): + + module = nxos_nxapi + + def setUp(self): + self.mock_run_commands = patch('ansible.modules.network.nxos.nxos_nxapi.run_commands') + self.run_commands = self.mock_run_commands.start() + + self.mock_load_config = patch('ansible.modules.network.nxos.nxos_nxapi.load_config') + self.load_config = self.mock_load_config.start() + + def tearDown(self): + self.mock_run_commands.stop() + self.mock_load_config.stop() + + def load_fixtures(self, commands=None): + def load_from_file(*args, **kwargs): + module, commands = args + output = list() + + for command in commands: + filename = str(command).replace(' ', '_') + filename = os.path.join('nxos_nxapi', filename) + output.append(load_fixture(filename)) + return output + + self.run_commands.side_effect = load_from_file + self.load_config.return_value = None + + def test_nxos_nxapi_no_change(self): + set_module_args(dict(http=True, https=False, http_port=80, https_port=443, sandbox=False)) + self.execute_module(changed=False, commands=[]) + + def test_nxos_nxapi_disable(self): + set_module_args(dict(state='absent')) + self.execute_module(changed=True, commands=['no feature nxapi']) + + def test_nxos_nxapi_no_http(self): + set_module_args(dict(https=True, http=False, https_port=8443)) + self.execute_module(changed=True, commands=['no nxapi http', 'nxapi https port 8443'])