fix nxos_aaa_server_host issues (#38188)

lib/ansible/modules/network/nxos/nxos_aaa_server_host.py

@@ -33,7 +33,7 @@ description:
 author: Jason Edelman (@jedelman8)
 notes:
     - Tested against NXOSv 7.3.(0)D1(1) on VIRL
-    - Changes to the AAA server host key (shared secret) are not idempotent.
+    - Changes to the host key (shared secret) are not idempotent for type 0.
     - If C(state=absent) removes the whole host configuration.
 options:
     server_type:
@@ -47,7 +47,7 @@ options:
         required: true
     key:
         description:
-            - Shared secret for the specified host.
+            - Shared secret for the specified host or keyword 'default'.
     encrypt_type:
         description:
             - The state of encryption applied to the entered key.
@@ -56,16 +56,17 @@ options:
         choices: ['0', '7']
     host_timeout:
         description:
-            - Timeout period for specified host, in seconds. Range is 1-60.
+            - Timeout period for specified host, in seconds or keyword 'default.
+              Range is 1-60.
     auth_port:
         description:
-            - Alternate UDP port for RADIUS authentication.
+            - Alternate UDP port for RADIUS authentication or keyword 'default'.
     acct_port:
         description:
-            - Alternate UDP port for RADIUS accounting.
+            - Alternate UDP port for RADIUS accounting or keyword 'default'.
     tacacs_port:
         description:
-            - Alternate TCP port TACACS Server.
+            - Alternate TCP port TACACS Server or keyword 'default'.
     state:
         description:
             - Manage the state of the resource.
@@ -147,13 +148,11 @@ from ansible.module_utils.network.nxos.nxos import get_capabilities, nxos_argume
 from ansible.module_utils.basic import AnsibleModule
 
 
-def execute_show_command(command, module, command_type='cli_show'):
+def execute_show_command(command, module):
     device_info = get_capabilities(module)
     network_api = device_info.get('network_api', 'nxapi')
 
     if network_api == 'cliconf':
-        if 'show run' not in command:
-            command += ' | json'
         cmds = [command]
         body = run_commands(module, cmds)
     elif network_api == 'nxapi':
@@ -173,40 +172,36 @@ def flatten_list(command_lists):
     return flat_command_list
 
 
-def _match_dict(match_list, key_map):
-    no_blanks = []
-    match_dict = {}
-
-    for match_set in match_list:
-        match_set = tuple(v for v in match_set if v)
-        no_blanks.append(match_set)
-
-    for info in no_blanks:
-        words = info[0].strip().split()
-        length = len(words)
-        alt_key = key_map.get(words[0])
-        first = alt_key or words[0]
-        last = words[length - 1]
-        match_dict[first] = last.replace('\"', '')
-
-    return match_dict
-
-
 def get_aaa_host_info(module, server_type, address):
     aaa_host_info = {}
     command = 'show run | inc {0}-server.host.{1}'.format(server_type, address)
 
-    body = execute_show_command(command, module, command_type='cli_show_ascii')
+    body = execute_show_command(command, module)[0]
-
+    if body:
-    if body[0]:
         try:
-            pattern = (r'(acct-port \d+)|(timeout \d+)|(auth-port \d+)|'
+            if 'radius' in body:
-                       r'(key 7 "\w+")|( port \d+)')
+                pattern = (r'\S+ host \S+(?:\s+key 7\s+(\S+))?(?:\s+auth-port (\d+))?'
-            raw_match = re.findall(pattern, body[0])
+                           r'(?:\s+acct-port (\d+))?(?:\s+authentication)?'
-            aaa_host_info = _match_dict(raw_match, {'acct-port': 'acct_port',
+                           r'(?:\s+accounting)?(?:\s+timeout (\d+))?')
-                                                    'auth-port': 'auth_port',
+                match = re.search(pattern, body)
-                                                    'port': 'tacacs_port',
+                aaa_host_info['key'] = match.group(1)
-                                                    'timeout': 'host_timeout'})
+                if aaa_host_info['key']:
+                    aaa_host_info['key'] = aaa_host_info['key'].replace('"', '')
+                    aaa_host_info['encrypt_type'] = '7'
+                aaa_host_info['auth_port'] = match.group(2)
+                aaa_host_info['acct_port'] = match.group(3)
+                aaa_host_info['host_timeout'] = match.group(4)
+            elif 'tacacs' in body:
+                pattern = (r'\S+ host \S+(?:\s+key 7\s+(\S+))?(?:\s+port (\d+))?'
+                           r'(?:\s+timeout (\d+))?')
+                match = re.search(pattern, body)
+                aaa_host_info['key'] = match.group(1)
+                if aaa_host_info['key']:
+                    aaa_host_info['key'] = aaa_host_info['key'].replace('"', '')
+                    aaa_host_info['encrypt_type'] = '7'
+                aaa_host_info['tacacs_port'] = match.group(2)
+                aaa_host_info['host_timeout'] = match.group(3)
+
             aaa_host_info['server_type'] = server_type
             aaa_host_info['address'] = address
         except TypeError:
@@ -217,35 +212,41 @@ def get_aaa_host_info(module, server_type, address):
     return aaa_host_info
 
 
-def config_aaa_host(server_type, address, params, clear=False):
+def config_aaa_host(server_type, address, params, existing):
     cmds = []
-
-    if clear:
-        cmds.append('no {0}-server host {1}'.format(server_type, address))
-
     cmd_str = '{0}-server host {1}'.format(server_type, address)
+    cmd_no_str = 'no ' + cmd_str
 
     key = params.get('key')
     enc_type = params.get('encrypt_type', '')
-    host_timeout = params.get('host_timeout')
-    auth_port = params.get('auth_port')
-    acct_port = params.get('acct_port')
-    port = params.get('tacacs_port')
 
-    if auth_port:
+    defval = False
-        cmd_str += ' auth-port {0}'.format(auth_port)
+    nondef = False
-    if acct_port:
+
-        cmd_str += ' acct-port {0}'.format(acct_port)
-    if port:
-        cmd_str += ' port {0}'.format(port)
-    if host_timeout:
-        cmd_str += ' timeout {0}'.format(host_timeout)
     if key:
-        cmds.append('{0}-server host {1} key {2} {3}'.format(server_type,
+        if key != 'default':
-                                                             address,
+            cmds.append(cmd_str + ' key {0} {1}'.format(enc_type, key))
-                                                             enc_type, key))
+        else:
+            cmds.append(cmd_no_str + ' key 7 {0}'.format(existing.get('key')))
+
+    locdict = {'auth_port': 'auth-port', 'acct_port': 'acct-port',
+               'tacacs_port': 'port', 'host_timeout': 'timeout'}
+
+    # platform CLI needs the keywords in the following order
+    for key in ['auth_port', 'acct_port', 'tacacs_port', 'host_timeout']:
+        item = params.get(key)
+        if item:
+            if item != 'default':
+                cmd_str += ' {0} {1}'.format(locdict.get(key), item)
+                nondef = True
+            else:
+                cmd_no_str += ' {0} 1'.format(locdict.get(key))
+                defval = True
+    if defval:
+        cmds.append(cmd_no_str)
+    if nondef or not existing:
+        cmds.append(cmd_str)
 
-    cmds.append(cmd_str)
     return cmds
 
 
@@ -302,24 +303,19 @@ def main():
     end_state = existing
 
     commands = []
+    delta = {}
     if state == 'present':
-        host_timeout = proposed.get('host_timeout')
+        if not existing:
-        if host_timeout:
+            delta = proposed
-            try:
+        else:
-                if int(host_timeout) < 1 or int(host_timeout) > 60:
+            for key, value in proposed.items():
-                    raise ValueError
+                if value != existing.get(key):
-            except ValueError:
+                    if value != 'default' or existing.get(key):
-                module.fail_json(
+                        delta[key] = value
-                    msg='host_timeout must be an integer between 1 and 60')
 
-        delta = dict(
+        command = config_aaa_host(server_type, address, delta, existing)
-            set(proposed.items()).difference(existing.items()))
+        if command:
-        if delta:
+            commands.append(command)
-            union = existing.copy()
-            union.update(delta)
-            command = config_aaa_host(server_type, address, union)
-            if command:
-                commands.append(command)
 
     elif state == 'absent':
         intersect = dict(

test/integration/targets/nxos_aaa_server_host/tests/common/radius.yaml

@@ -43,7 +43,7 @@
     register: result
 
   - assert: *false
-  
+ 
   - name: "Configure radius server non defaults"
     nxos_aaa_server_host: &configure_radius_non_default
       server_type: radius
@@ -54,7 +54,7 @@
       state: present
       provider: "{{ connection }}"
     register: result
-  
+ 
   - assert: *true
 
   - name: "Check Idempotence"
@@ -63,25 +63,38 @@
 
   - assert: *false
 
-  - name: "Remove radius server configuration"
+  - name: "Configure some defaults on radius server"
-    nxos_aaa_server_host: *remove
+    nxos_aaa_server_host: &configure_some_radius_default
+      server_type: radius
+      address: 8.8.8.8
+      host_timeout: default
+      auth_port: 1000
+      acct_port: default
+      state: present
+      provider: "{{ connection }}"
     register: result
 
   - assert: *true
 
+  - name: "Check Idempotence"
+    nxos_aaa_server_host: *configure_some_radius_default
+    register: result
+
+  - assert: *false
+
   - name: "Configure radius server with clear text pwd"
     nxos_aaa_server_host: &configure_radius_clear_text
       server_type: radius
       address: 8.8.8.8
       host_timeout: 25
-      auth_port: 2083
+      auth_port: default
       acct_port: 2084
       encrypt_type: 0
       key: hello
       state: present
       provider: "{{ connection }}"
     register: result
-   
+
   - assert: *true
 
   - name: "Check NOT Idempotent"
@@ -115,8 +128,49 @@
     nxos_aaa_server_host: *configure_radius_type7
     register: result
 
+  - assert: *false
+
+  - name: "Configure radius server with default key"
+    nxos_aaa_server_host: &configure_radius_defkey
+      server_type: radius
+      address: 8.8.8.8
+      host_timeout: default
+      auth_port: 1000
+      acct_port: default
+      encrypt_type: 7
+      key: default
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
   - assert: *true
 
+  - name: "Check Idempotence"
+    nxos_aaa_server_host: *configure_radius_defkey
+    register: result
+
+  - assert: *false
+
+  - name: "Configure radius server with all def"
+    nxos_aaa_server_host: &configure_radius_alldef
+      server_type: radius
+      address: 8.8.8.8
+      host_timeout: default
+      auth_port: default
+      acct_port: default
+      key: default
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_aaa_server_host: *configure_radius_alldef
+    register: result
+
+  - assert: *false
+
   rescue:
 
   - debug: msg="connection={{ ansible_connection }} nxos_aaa_server_host failure detected"
@@ -127,4 +181,4 @@
     nxos_aaa_server_host: *remove
     register: result
 
-  - debug: msg="END connection={{ ansible_connection }} nxos_aaa_server_host radius.yaml sanity test"  
+  - debug: msg="END connection={{ ansible_connection }} nxos_aaa_server_host radius.yaml sanity test"

test/integration/targets/nxos_aaa_server_host/tests/common/tacacs.yaml

@@ -60,7 +60,7 @@
       state: present
       provider: "{{ connection }}"
     register: result
-  
+ 
   - assert: *true
 
   - name: "Check Idempotence"
@@ -69,18 +69,30 @@
 
   - assert: *false
 
-  - name: "Remove tacacs server configuration"
+  - name: "Configure some defaults on tacacs server"
-    nxos_aaa_server_host: *remove
+    nxos_aaa_server_host: &configure_some_tacacs_default
+      server_type: tacacs
+      address: 8.8.8.8
+      host_timeout: default
+      tacacs_port: 100
+      state: present
+      provider: "{{ connection }}"
     register: result
 
   - assert: *true
 
+  - name: "Check Idempotence"
+    nxos_aaa_server_host: *configure_some_tacacs_default
+    register: result
+
+  - assert: *false
+
   - name: "Configure tacacs server with clear text pwd"
     nxos_aaa_server_host: &configure_tacacs_clear_text
       server_type: tacacs
       address: 8.8.8.8
       host_timeout: 25
-      tacacs_port: 89
+      tacacs_port: default
       encrypt_type: 0
       key: hello
       state: present
@@ -119,8 +131,47 @@
     nxos_aaa_server_host: *configure_tacacs_type7
     register: result
 
+  - assert: *false
+
+  - name: "Configure tacacs server with default key"
+    nxos_aaa_server_host: &configure_tacacs_defkey
+      server_type: tacacs
+      address: 8.8.8.8
+      host_timeout: default
+      tacacs_port: 89
+      encrypt_type: 7
+      key: default
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
   - assert: *true
 
+  - name: "Check Idempotence"
+    nxos_aaa_server_host: *configure_tacacs_defkey
+    register: result
+
+  - assert: *false
+
+  - name: "Configure tacacs server with all def"
+    nxos_aaa_server_host: &configure_tacacs_alldef
+      server_type: tacacs
+      address: 8.8.8.8
+      host_timeout: default
+      tacacs_port: default
+      key: default
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_aaa_server_host: *configure_tacacs_alldef
+    register: result
+
+  - assert: *false
+
   rescue:
 
   - debug: msg="connection={{ ansible_connection }} nxos_aaa_server_host failure detected"
@@ -131,7 +182,7 @@
     nxos_aaa_server_host: *remove
     register: result
 
-  - name: "Enable feature tacacs+"
+  - name: "Disable feature tacacs+"
     nxos_feature:
       feature: tacacs+
       state: disabled