Fix multiple code and test issues on iosxr (#27267)

* Fix multiple code and test issues on iosxr

It passes the integration tests now.
Fixes #27123

* Fix pep8 issue

* Fix unit tests

lib/ansible/modules/network/iosxr/iosxr_user.py

@@ -132,58 +132,71 @@ from ansible.module_utils.six import iteritems
 from ansible.module_utils.iosxr import iosxr_argument_spec, check_args
 
 
+def search_obj_in_list(name, lst):
+    for o in lst:
+        if o['name'] == name:
+            return o
+
+    return None
+
+
 def map_obj_to_commands(updates, module):
     commands = list()
-    state = module.params['state']
+    want, have = updates
-    update_password = module.params['update_password']
 
-    def needs_update(want, have, x):
+    for w in want:
-        return want.get(x) and (want.get(x) != have.get(x))
+        name = w['name']
+        state = w['state']
 
-    def add(command, want, x):
+        obj_in_have = search_obj_in_list(name, have)
-        command.append('username %s %s' % (want['name'], x))
 
-    for update in updates:
+        if state == 'absent' and obj_in_have:
-        want, have = update
+            commands.append('no username ' + name)
+        elif state == 'present' and not obj_in_have:
+            user_cmd = 'username ' + name
+            commands.append(user_cmd)
 
-        if want['state'] == 'absent':
+            if w['password']:
-            commands.append('no username %s' % want['name'])
+                commands.append(user_cmd + ' secret ' + w['password'])
-            continue
+            if w['group']:
+                commands.append(user_cmd + ' group ' + w['group'])
 
-        if needs_update(want, have, 'group'):
+        elif state == 'present' and obj_in_have:
-            add(commands, want, 'group %s' % want['group'])
+            user_cmd = 'username ' + name
 
-        if needs_update(want, have, 'password'):
+            if module.params['update_password'] == 'always' and w['password']:
-            if update_password == 'always' or not have:
+                commands.append(user_cmd + ' secret ' + w['password'])
-                add(commands, want, 'secret %s' % want['password'])
+            if w['group'] and w['group'] != obj_in_have['group']:
+                commands.append(user_cmd + ' group ' + w['group'])
 
     return commands
 
 
-def parse_group(data):
-    match = re.search(r'\n group (\S+)', data, re.M)
-    if match:
-        return match.group(1)
-
-
 def map_config_to_obj(module):
     data = get_config(module, flags=['username'])
+    users = data.strip().rstrip('!').split('!')
 
-    match = re.findall(r'^username (\S+)', data, re.M)
+    if not users:
-    if not match:
         return list()
 
     instances = list()
 
-    for user in set(match):
+    for user in users:
-        regex = r'username %s .+$' % user
+        user_config = user.strip().splitlines()
-        cfg = re.findall(regex, data, re.M)
+
-        cfg = '\n'.join(cfg)
+        name = user_config[0].strip().split()[1]
+        group = None
+
+        if len(user_config) > 1:
+            group_or_secret = user_config[1].strip().split()
+            if group_or_secret[0] == 'group':
+                group = group_or_secret[1]
+
         obj = {
-            'name': user,
+            'name': name,
             'state': 'present',
             'password': None,
-            'group': parse_group(cfg)
+            'group': group
         }
         instances.append(obj)
 
@@ -241,19 +254,6 @@ def map_params_to_obj(module):
     return objects
 
 
-def update_objects(want, have):
-    updates = list()
-    for entry in want:
-        item = next((i for i in have if i['name'] == entry['name']), None)
-        if all((item is None, entry['state'] == 'present')):
-            updates.append((entry, {}))
-        elif item:
-            for key, value in iteritems(entry):
-                if value and value != item[key]:
-                    updates.append((entry, item))
-    return updates
-
-
 def main():
     """ main entry point for module execution
     """
@@ -285,7 +285,7 @@ def main():
     want = map_params_to_obj(module)
     have = map_config_to_obj(module)
 
-    commands = map_obj_to_commands(update_objects(want, have), module)
+    commands = map_obj_to_commands((want, have), module)
 
     if module.params['purge']:
         want_users = [x['name'] for x in want]

test/integration/targets/iosxr_user/tests/cli/basic.yaml

@@ -19,7 +19,7 @@
     that:
       - 'result.changed == true'
       - '"username" in result.commands[0]'
-      - '"secret" in result.commands[0]'
+      - '"secret" in result.commands[1]'
 
 - name: Create user with update_password always (not idempotent)
   iosxr_user:
@@ -50,11 +50,43 @@
       - 'result.changed == false'
       - 'result.commands | length == 0'
 
+- name: Modify user group
+  iosxr_user:
+    name: ansibletest1
+    password: test
+    update_password: on_create
+    group: sysadmin
+    state: present
+    provider: "{{ cli }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"username" in result.commands[0]'
+      - '"group" in result.commands[0]'
+
+- name: Modify user group again (idempotent)
+  iosxr_user:
+    name: ansibletest1
+    password: test
+    update_password: on_create
+    group: sysadmin
+    state: present
+    provider: "{{ cli }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == false'
+      - 'result.commands | length == 0'
+
 - name: Collection of users (SetUp)
   iosxr_user:
     users:
       - name: ansibletest2
       - name: ansibletest3
+    password: test
     state: present
     group: sysadmin
     provider: "{{ cli }}"
@@ -63,13 +95,19 @@
 - assert:
     that:
       - 'result.changed == true'
-      - 'result.commands == ["username ansibletest2 group sysadmin", "username ansibletest3 group sysadmin"]'
+      - '"username" in result.commands[0]'
+      - '"secret" in result.commands[1]'
+      - '"group sysadmin" in result.commands[2]'
+      - '"username" in result.commands[3]'
+      - '"secret" in result.commands[4]'
+      - '"group sysadmin" in result.commands[5]'
 
 - name: Add collection of users again with update_password always (not idempotent)
   iosxr_user:
     users:
       - name: ansibletest2
       - name: ansibletest3
+    password: test
     state: present
     group: sysadmin
     provider: "{{ cli }}"
@@ -78,13 +116,17 @@
 - assert:
     that:
       - 'result.changed == true'
-      - 'result.commands == ["username ansibletest2 group sysadmin", "username ansibletest3 group sysadmin"]'
+      - '"username" in result.commands[0]'
+      - '"secret" in result.commands[0]'
+      - '"username" in result.commands[1]'
+      - '"secret" in result.commands[1]'
 
 - name: Add collection of users again with update_password on_create (idempotent)
   iosxr_user:
     users:
       - name: ansibletest2
       - name: ansibletest3
+    password: test
     update_password: on_create
     state: present
     group: sysadmin

test/units/modules/network/iosxr/fixtures/iosxr_user_config.cfg

@@ -1,2 +1,8 @@
-username admin secret 5 $1$mdQIUxjg$3t3lzBpfKfITKvFm1uEIY. group sysadmin
+username admin
-username ansible secret 5 $1$3yWSXiIi$VdzV59ChiurrNdGxlDeAW/ group sysadmin
+ secret 5 $1$mdQIUxjg$3t3lzBpfKfITKvFm1uEIY.
+ group sysadmin
+!
+username ansible
+ secret 5 $1$3yWSXiIi$VdzV59ChiurrNdGxlDeAW/
+ group sysadmin
+!

test/units/modules/network/iosxr/test_iosxr_user.py

@@ -68,7 +68,8 @@ class TestIosxrUserModule(TestIosxrModule):
     def test_iosxr_user_update_password_changed(self):
         set_module_args(dict(name='test', password='test', update_password='on_create'))
         result = self.execute_module(changed=True)
-        self.assertEqual(result['commands'], ['username test secret test'])
+        self.assertEqual(result['commands'],
+                         ['username test', 'username test secret test'])
 
     def test_iosxr_user_update_password_on_create_ok(self):
         set_module_args(dict(name='ansible', password='test', update_password='on_create'))