diff --git a/changelogs/fragments/nxos_bugfixes.yaml b/changelogs/fragments/nxos_bugfixes.yaml index 367d7515f20..1495c9178ee 100644 --- a/changelogs/fragments/nxos_bugfixes.yaml +++ b/changelogs/fragments/nxos_bugfixes.yaml @@ -15,3 +15,5 @@ bugfixes: - nxos_acl - Fix nxos_acl (https://github.com/ansible/ansible/pull/38283) - nxos_static_route - Fix nxos_static_route (https://github.com/ansible/ansible/pull/37614) - nxos_acl_interface test - Fix nxos_acl_interface test (https://github.com/ansible/ansible/pull/38230) +- nxos_igmp - Fix nxos_igmp (https://github.com/ansible/ansible/pull/38496) +- nxos_hsrp - Fix nxos_hsrp (https://github.com/ansible/ansible/pull/38410) diff --git a/lib/ansible/modules/network/nxos/nxos_hsrp.py b/lib/ansible/modules/network/nxos/nxos_hsrp.py index 0faba42ffba..84f6b4658aa 100644 --- a/lib/ansible/modules/network/nxos/nxos_hsrp.py +++ b/lib/ansible/modules/network/nxos/nxos_hsrp.py @@ -53,38 +53,32 @@ options: version: description: - HSRP version. - required: false - default: 2 + default: 1 choices: ['1','2'] priority: description: - - HSRP priority. - required: false - default: null + - HSRP priority or keyword 'default'. preempt: description: - Enable/Disable preempt. choices: ['enabled', 'disabled'] vip: description: - - HSRP virtual IP address. - required: false - default: null + - HSRP virtual IP address or keyword 'default' auth_string: description: - - Authentication string. - required: false - default: null + - Authentication string. If this needs to be hidden(for md5 type), the string + should be 7 followed by the key string. Otherwise, it can be 0 followed by + key string or just key string (for backward compatibility). For text type, + this should be just be a key string. if this is 'default', authentication + is removed. auth_type: description: - Authentication type. - required: false - default: null choices: ['text','md5'] state: description: - Specify desired state of the resource. - required: false choices: ['present','absent'] default: 'present' ''' @@ -100,6 +94,7 @@ EXAMPLES = ''' host: 68.170.147.165 - name: Ensure HSRP is configured with following params on a SVI + with clear text authentication nxos_hsrp: group: 10 vip: 10.1.1.1 @@ -110,6 +105,30 @@ EXAMPLES = ''' auth_type: text auth_string: CISCO +- name: Ensure HSRP is configured with md5 authentication and clear + authentication string + nxos_hsrp: + group: 10 + vip: 10.1.1.1 + priority: 150 + interface: vlan10 + preempt: enabled + host: 68.170.147.165 + auth_type: md5 + auth_string: "0 1234" + +- name: Ensure HSRP is configured with md5 authentication and hidden + authentication string + nxos_hsrp: + group: 10 + vip: 10.1.1.1 + priority: 150 + interface: vlan10 + preempt: enabled + host: 68.170.147.165 + auth_type: md5 + auth_string: "7 1234" + - name: Remove HSRP config for given interface, group, and VIP nxos_hsrp: group: 10 @@ -132,6 +151,14 @@ from ansible.module_utils.network.nxos.nxos import get_capabilities, nxos_argume from ansible.module_utils.basic import AnsibleModule +PARAM_TO_DEFAULT_KEYMAP = { + 'vip': None, + 'priority': '100', + 'auth_type': 'text', + 'auth_string': 'cisco', +} + + def execute_show_command(command, module): device_info = get_capabilities(module) network_api = device_info.get('network_api', 'nxapi') @@ -196,29 +223,8 @@ def get_interface_mode(interface, intf_type, module): return mode -def get_hsrp_groups_on_interfaces(device, module): - command = 'show hsrp all' - hsrp = {} - - try: - body = execute_show_command(command, module)[0] - get_data = body['TABLE_grp_detail']['ROW_grp_detail'] - except (IndexError, KeyError, AttributeError): - return {} - - for entry in get_data: - interface = str(entry['sh_if_index'].lower()) - value = hsrp.get(interface, 'new') - if value == 'new': - hsrp[interface] = [] - group = str(entry['sh_group_num']) - hsrp[interface].append(group) - - return hsrp - - def get_hsrp_group(group, interface, module): - command = 'show hsrp group {0}'.format(group) + command = 'show hsrp group {0} all'.format(group) hsrp = {} hsrp_key = { @@ -229,6 +235,7 @@ def get_hsrp_group(group, interface, module): 'sh_preempt': 'preempt', 'sh_vip': 'vip', 'sh_authentication_type': 'auth_type', + 'sh_keystring_attr': 'auth_enc', 'sh_authentication_data': 'auth_string' } @@ -251,6 +258,12 @@ def get_hsrp_group(group, interface, module): elif parsed_hsrp['version'] == 'v2': parsed_hsrp['version'] = '2' + if parsed_hsrp['auth_type'] == 'md5': + if parsed_hsrp['auth_enc'] == 'hidden': + parsed_hsrp['auth_enc'] = '7' + else: + parsed_hsrp['auth_enc'] = '0' + if parsed_hsrp['interface'] == interface: return parsed_hsrp @@ -262,24 +275,45 @@ def get_commands_remove_hsrp(group, interface): return commands -def get_commands_config_hsrp(delta, interface, args): +def get_commands_config_hsrp(delta, interface, args, existing): commands = [] config_args = { 'group': 'hsrp {group}', - 'priority': 'priority {priority}', + 'priority': '{priority}', 'preempt': '{preempt}', - 'vip': 'ip {vip}' + 'vip': '{vip}' } preempt = delta.get('preempt', None) group = delta.get('group', None) + vip = delta.get('vip', None) + priority = delta.get('priority', None) + if preempt: if preempt == 'enabled': delta['preempt'] = 'preempt' elif preempt == 'disabled': delta['preempt'] = 'no preempt' + if priority: + if priority == 'default': + if existing and existing.get('priority') != PARAM_TO_DEFAULT_KEYMAP.get('priority'): + delta['priority'] = 'no priority' + else: + del(delta['priority']) + else: + delta['priority'] = 'priority {0}'.format(delta['priority']) + + if vip: + if vip == 'default': + if existing and existing.get('vip') != PARAM_TO_DEFAULT_KEYMAP.get('vip'): + delta['vip'] = 'no ip' + else: + del(delta['vip']) + else: + delta['vip'] = 'ip {0}'.format(delta['vip']) + for key in delta: command = config_args.get(key, 'DNE').format(**delta) if command and command != 'DNE': @@ -291,17 +325,22 @@ def get_commands_config_hsrp(delta, interface, args): auth_type = delta.get('auth_type', None) auth_string = delta.get('auth_string', None) + auth_enc = delta.get('auth_enc', None) if auth_type or auth_string: if not auth_type: auth_type = args['auth_type'] elif not auth_string: auth_string = args['auth_string'] - if auth_type == 'md5': - command = 'authentication md5 key-string {0}'.format(auth_string) - commands.append(command) - elif auth_type == 'text': - command = 'authentication text {0}'.format(auth_string) - commands.append(command) + if auth_string != 'default': + if auth_type == 'md5': + command = 'authentication md5 key-string {0} {1}'.format(auth_enc, auth_string) + commands.append(command) + elif auth_type == 'text': + command = 'authentication text {0}'.format(auth_string) + commands.append(command) + else: + if existing and existing.get('auth_string') != PARAM_TO_DEFAULT_KEYMAP.get('auth_string'): + commands.append('no authentication') if commands and not group: commands.insert(0, 'hsrp {0}'.format(args['group'])) @@ -346,35 +385,11 @@ def validate_config(body, vip, module): vip=vip) -def validate_params(param, module): - value = module.params[param] - version = module.params['version'] - - if param == 'group': - try: - if (int(value) < 0 or int(value) > 255) and version == '1': - raise ValueError - elif int(value) < 0 or int(value) > 4095: - raise ValueError - except ValueError: - module.fail_json(msg="Warning! 'group' must be an integer between" - " 0 and 255 when version 1 and up to 4095 " - "when version 2.", group=value, - version=version) - elif param == 'priority': - try: - if (int(value) < 0 or int(value) > 255): - raise ValueError - except ValueError: - module.fail_json(msg="Warning! 'priority' must be an integer " - "between 0 and 255", priority=value) - - def main(): argument_spec = dict( group=dict(required=True, type='str'), interface=dict(required=True), - version=dict(choices=['1', '2'], default='2', required=False), + version=dict(choices=['1', '2'], default='1', required=False), priority=dict(type='str', required=False), preempt=dict(type='str', choices=['disabled', 'enabled'], required=False), vip=dict(type='str', required=False), @@ -398,18 +413,24 @@ def main(): preempt = module.params['preempt'] vip = module.params['vip'] auth_type = module.params['auth_type'] - auth_string = module.params['auth_string'] + auth_full_string = module.params['auth_string'] + auth_enc = '0' + auth_string = None + if auth_full_string: + kstr = auth_full_string.split() + if len(kstr) == 2: + auth_enc = kstr[0] + auth_string = kstr[1] + elif len(kstr) == 1: + auth_string = kstr[0] + else: + module.fail_json(msg='Inavlid auth_string') + if auth_enc != '0' and auth_enc != '7': + module.fail_json(msg='Inavlid auth_string, only 0 or 7 allowed') device_info = get_capabilities(module) network_api = device_info.get('network_api', 'nxapi') - if state == 'present' and not vip: - module.fail_json(msg='the "vip" param is required when state=present') - - for param in ['group', 'priority']: - if module.params[param] is not None: - validate_params(param, module) - intf_type = get_interface_type(interface) if (intf_type != 'ethernet' and network_api == 'cliconf'): if is_default(interface, module) == 'DNE': @@ -431,7 +452,7 @@ def main(): args = dict(group=group, version=version, priority=priority, preempt=preempt, vip=vip, auth_type=auth_type, - auth_string=auth_string) + auth_string=auth_string, auth_enc=auth_enc) proposed = dict((k, v) for k, v in args.items() if v is not None) @@ -445,7 +466,7 @@ def main(): elif not proposed.get('auth_type', None) and existing: if (proposed['version'] == '1' and - existing['auth_type'] == 'md5'): + existing['auth_type'] == 'md5') and state == 'present': module.fail_json(msg="Existing auth_type is md5. It's recommended " "to use HSRP v2 when using md5") @@ -454,7 +475,7 @@ def main(): delta = dict( set(proposed.items()).difference(existing.items())) if delta: - command = get_commands_config_hsrp(delta, interface, args) + command = get_commands_config_hsrp(delta, interface, args, existing) commands.extend(command) elif state == 'absent': diff --git a/lib/ansible/modules/network/nxos/nxos_igmp.py b/lib/ansible/modules/network/nxos/nxos_igmp.py index a1a023c1550..8694b172473 100644 --- a/lib/ansible/modules/network/nxos/nxos_igmp.py +++ b/lib/ansible/modules/network/nxos/nxos_igmp.py @@ -139,10 +139,12 @@ def main(): commands.append('no ip igmp enforce-router-alert') elif state == 'present': - if desired['flush_routes'] and not current['flush_routes']: - commands.append('ip igmp flush-routes') - if desired['enforce_rtr_alert'] and not current['enforce_rtr_alert']: - commands.append('ip igmp enforce-router-alert') + ldict = {'flush_routes': 'flush-routes', 'enforce_rtr_alert': 'enforce-router-alert'} + for arg in ['flush_routes', 'enforce_rtr_alert']: + if desired[arg] and not current[arg]: + commands.append('ip igmp {0}'.format(ldict.get(arg))) + elif current[arg] and not desired[arg]: + commands.append('no ip igmp {0}'.format(ldict.get(arg))) result = {'changed': False, 'updates': commands, 'warnings': warnings} @@ -152,7 +154,8 @@ def main(): result['changed'] = True if module.params['restart']: - run_commands(module, 'restart igmp') + cmd = {'command': 'restart igmp', 'output': 'text'} + run_commands(module, cmd) module.exit_json(**result) diff --git a/test/integration/targets/nxos_hsrp/tests/common/sanity.yaml b/test/integration/targets/nxos_hsrp/tests/common/sanity.yaml index 0b7cc5885e3..365fbe7eca5 100644 --- a/test/integration/targets/nxos_hsrp/tests/common/sanity.yaml +++ b/test/integration/targets/nxos_hsrp/tests/common/sanity.yaml @@ -4,7 +4,8 @@ when: ansible_connection == "local" # Select interface for test -- set_fact: intname="{{ nxos_int1 }}" +- set_fact: intname1="{{ nxos_int1 }}" +- set_fact: intname2="{{ nxos_int2 }}" - block: - name: "Enable feature hsrp" @@ -13,25 +14,34 @@ state: enabled provider: "{{ connection }}" - - name: "change interface mode" + - name: "change int1 mode" nxos_config: commands: - no switchport parents: - - "interface {{ intname }}" + - "interface {{ intname1 }}" + match: none + provider: "{{ connection }}" + + - name: "change int2 mode" + nxos_config: + commands: + - no switchport + parents: + - "interface {{ intname2 }}" match: none provider: "{{ connection }}" - name: "configure nxos_hsrp" - nxos_hsrp: &configure - group: 10 + nxos_hsrp: &conf1000 + group: 1000 version: 2 vip: 10.1.1.1 priority: 150 - interface: "{{ intname }}" + interface: "{{ intname1 }}" preempt: enabled - auth_type: text - auth_string: CISCO + auth_type: md5 + auth_string: "7 1234" provider: "{{ connection }}" register: result @@ -40,50 +50,109 @@ - "result.changed == true" - name: "Conf Idempotence" - nxos_hsrp: *configure + nxos_hsrp: *conf1000 register: result - assert: &false that: - "result.changed == false" - - name: "remove nxos_hsrp" - nxos_hsrp: &remove - group: 10 + - name: "configure group 100" + nxos_hsrp: &conf100 + group: 100 version: 2 - vip: 10.1.1.1 - priority: 150 - interface: "{{ intname }}" + vip: 2.2.2.2 + priority: 25 + interface: "{{ intname1 }}" preempt: enabled + auth_type: md5 + auth_string: "0 1234" + provider: "{{ connection }}" + register: result + + - assert: *true + + - name: "Conf Idempotence" + nxos_hsrp: *conf100 + register: result + + - assert: *false + + - name: "change group 100" + nxos_hsrp: &chg100 + group: 100 + version: 2 + vip: default + priority: default + interface: "{{ intname1 }}" + preempt: disabled + auth_type: md5 + auth_string: "0 1234" + provider: "{{ connection }}" + register: result + + - assert: *true + + - name: "Conf Idempotence" + nxos_hsrp: *chg100 + register: result + + - assert: *false + + - name: "configure group 200" + nxos_hsrp: &conf200 + group: 200 + vip: 3.3.3.3 + version: 1 + interface: "{{ intname2 }}" auth_type: text - auth_string: CISCO + auth_string: "1234" provider: "{{ connection }}" - state: absent register: result - assert: *true - - name: "Remove Idempotence" - nxos_hsrp: *remove + - name: "Conf Idempotence" + nxos_hsrp: *conf200 register: result - assert: *false - always: - - name: "remove nxos_hsrp" - nxos_hsrp: - group: 10 + - name: "change group 200" + nxos_hsrp: &chg200 + group: 200 + vip: 3.3.3.3 version: 2 - vip: 10.1.1.1 - priority: 150 - interface: "{{ intname }}" - preempt: enabled + interface: "{{ intname2 }}" auth_type: text - auth_string: CISCO + auth_string: default provider: "{{ connection }}" + register: result + + - assert: *true + + - name: "Conf Idempotence" + nxos_hsrp: *chg200 + register: result + + - assert: *false + + - name: "remove nxos_hsrp" + nxos_hsrp: &remove + group: 1000 + interface: "{{ intname1 }}" state: absent - ignore_errors: yes + register: result + + - assert: *true + + - name: "Remove Idempotence" + nxos_hsrp: *remove + register: result + + - assert: *false + always: - name: "Disable feature hsrp" nxos_feature: feature: hsrp diff --git a/test/integration/targets/nxos_igmp/tests/common/sanity.yaml b/test/integration/targets/nxos_igmp/tests/common/sanity.yaml index da8bce3c887..5689d996bd2 100644 --- a/test/integration/targets/nxos_igmp/tests/common/sanity.yaml +++ b/test/integration/targets/nxos_igmp/tests/common/sanity.yaml @@ -3,6 +3,9 @@ - debug: msg="Using provider={{ connection.transport }}" when: ansible_connection == "local" +- set_fact: restart="true" + when: platform is not match("N35") + - block: - name: Configure igmp with non-default values @@ -26,23 +29,45 @@ that: - "result.changed == false" - - name: Configure igmp with default values + - name: Configure igmp defaults nxos_igmp: &default - state: default + flush_routes: false + enforce_rtr_alert: false + restart: "{{restart|default(omit)}}" + state: present provider: "{{ connection }}" register: result - assert: *true - - name: "Check Idempotence - Configure igmp with default values" + - name: "Check Idempotence - Configure igmp with defaults" nxos_igmp: *default register: result - assert: *false + - name: Configure igmp non-defaults again + nxos_igmp: *non-default + register: result + + - name: Configure igmp state as values + nxos_igmp: &sdefault + state: default + provider: "{{ connection }}" + register: result + + - assert: *true + + - name: "Check Idempotence - Configure igmp with state default" + nxos_igmp: *sdefault + register: result + + - assert: *false + always: - name: Configure igmp with default values - nxos_igmp: *default + nxos_igmp: *sdefault register: result + ignore_errors: yes - debug: msg="END connection={{ ansible_connection }} nxos_igmp sanity test" diff --git a/test/units/modules/network/nxos/test_nxos_hsrp.py b/test/units/modules/network/nxos/test_nxos_hsrp.py index 382a0c97035..4bf53f24232 100644 --- a/test/units/modules/network/nxos/test_nxos_hsrp.py +++ b/test/units/modules/network/nxos/test_nxos_hsrp.py @@ -59,7 +59,7 @@ class TestNxosHsrpModule(TestNxosModule): result = self.execute_module(changed=True) self.assertEqual(sorted(result['commands']), sorted(['config t', 'interface ethernet1/2', - 'hsrp version 2', + 'hsrp version 1', 'hsrp 10', 'priority 150', 'ip 192.0.2.2/8',