Multiple fixs for na_ontap_user (#54610)

* Fix ontap user for 9.1

* fix bugs:

* update unit tests

lib/ansible/modules/storage/netapp/na_ontap_user.py

@@ -164,9 +164,8 @@ class NetAppOntapUser(object):
         Checks if the user exists.
         :param: application: application to grant access to
         :return:
-            True if user found
+            Dictionary if user found
-            False if user is not found
+            None if user is not found
-        :rtype: bool
         """
         security_login_get_iter = netapp_utils.zapi.NaElement('security-login-get-iter')
         query_details = netapp_utils.zapi.NaElement.create_node_with_children(
@@ -186,17 +185,18 @@ class NetAppOntapUser(object):
                 interface_attributes = result.get_child_by_name('attributes-list').\
                     get_child_by_name('security-login-account-info')
                 return_value = {
-                    'lock_user': interface_attributes.get_child_content('is-locked')
+                    'lock_user': interface_attributes.get_child_content('is-locked'),
+                    'role_name': interface_attributes.get_child_content('role-name')
                 }
                 return return_value
             return None
         except netapp_utils.zapi.NaApiError as error:
             # Error 16034 denotes a user not being found.
             if to_native(error.code) == "16034":
-                return False
+                return None
             # Error 16043 denotes the user existing, but the application missing
             elif to_native(error.code) == "16043":
-                return False
+                return None
             else:
                 self.module.fail_json(msg='Error getting user %s: %s' % (self.parameters['name'], to_native(error)),
                                       exception=traceback.format_exc())
@@ -305,7 +305,9 @@ class NetAppOntapUser(object):
             if to_native(error.code) == '13114':
                 return False
             # if the user give the same password, instead of returning an error, return ok
-            if to_native(error.code) == '13214' and error.message.startswith('New password must be different than the old password.'):
+            if to_native(error.code) == '13214' and \
+                    (error.message.startswith('New password must be different than last 6 passwords.')
+                     or error.message.startswith('New password must be different than the old password.')):
                 return False
             self.module.fail_json(msg='Error setting password for user %s: %s' % (self.parameters['name'], to_native(error)),
                                       exception=traceback.format_exc())
@@ -313,39 +315,68 @@ class NetAppOntapUser(object):
         self.server.set_vserver(None)
         return True
 
+    def modify_user(self, application):
+        """
+        Modify user
+        """
+        user_modify = netapp_utils.zapi.NaElement.create_node_with_children(
+            'security-login-modify', **{'vserver': self.parameters['vserver'],
+                                        'user-name': self.parameters['name'],
+                                        'application': application,
+                                        'authentication-method': self.parameters['authentication_method'],
+                                        'role-name': self.parameters.get('role_name')})
+
+        try:
+            self.server.invoke_successfully(user_modify,
+                                            enable_tunneling=False)
+        except netapp_utils.zapi.NaApiError as error:
+            self.module.fail_json(msg='Error modifying user %s: %s' % (self.parameters['name'], to_native(error)),
+                                  exception=traceback.format_exc())
+
     def apply(self):
         create_delete_decision = {}
-        modify = {}
+        modify_decision = {}
+
         netapp_utils.ems_log_event("na_ontap_user", self.server)
         for application in self.parameters['applications']:
             current = self.get_user(application)
+            if current is not None:
+                current['lock_user'] = self.na_helper.get_value_for_bool(True, current['lock_user'])
+
             cd_action = self.na_helper.get_cd_action(current, self.parameters)
+
             if cd_action is not None:
                 create_delete_decision[application] = cd_action
+            else:
+                modify_decision[application] = self.na_helper.get_modified_attributes(current, self.parameters)
+
         if not create_delete_decision and self.parameters.get('state') == 'present':
             if self.parameters.get('set_password') is not None:
                 self.na_helper.changed = True
-            current = self.get_user()
-            if current is not None:
-                current['lock_user'] = self.na_helper.get_value_for_bool(True, current['lock_user'])
-            modify = self.na_helper.get_modified_attributes(current, self.parameters)
 
         if self.na_helper.changed:
+
             if self.module.check_mode:
                 pass
             else:
-                if create_delete_decision:
+                for application in create_delete_decision:
-                    for cd_action in create_delete_decision:
+                    if create_delete_decision[application] == 'create':
-                        if create_delete_decision[cd_action] == 'create':
+                        self.create_user(application)
-                            self.create_user(cd_action)
+                    elif create_delete_decision[application] == 'delete':
-                        elif create_delete_decision[cd_action] == 'delete':
+                        self.delete_user(application)
-                            self.delete_user(cd_action)
+                lock_user = False
-                elif modify:
+                for application in modify_decision:
+                    if 'role_name' in modify_decision[application]:
+                        self.modify_user(application)
+                    if 'lock_user' in modify_decision[application]:
+                        lock_user = True
+
+                if lock_user:
                     if self.parameters.get('lock_user'):
                         self.lock_given_user()
                     else:
                         self.unlock_given_user()
-                elif not create_delete_decision and self.parameters.get('set_password') is not None:
+                if not create_delete_decision and self.parameters.get('set_password') is not None:
                     # if change password return false nothing has changed so we need to set changed to False
                     self.na_helper.changed = self.change_password()
         self.module.exit_json(changed=self.na_helper.changed)

test/units/modules/storage/netapp/test_na_ontap_user.py

@@ -52,10 +52,11 @@ def fail_json(*args, **kwargs):  # pylint: disable=unused-argument
 class MockONTAPConnection(object):
     ''' mock server connection to ONTAP host '''
 
-    def __init__(self, kind=None, parm1=None):
+    def __init__(self, kind=None, parm1=None, parm2=None):
         ''' save arguments '''
         self.type = kind
         self.parm1 = parm1
+        self.parm2 = parm2
         self.xml_in = None
         self.xml_out = None
 
@@ -63,7 +64,7 @@ class MockONTAPConnection(object):
         ''' mock invoke_successfully returning xml data '''
         self.xml_in = xml
         if self.type == 'user':
-            xml = self.build_user_info(self.parm1)
+            xml = self.build_user_info(self.parm1, self.parm2)
         self.xml_out = xml
         return xml
 
@@ -73,11 +74,11 @@ class MockONTAPConnection(object):
         pass
 
     @staticmethod
-    def build_user_info(locked):
+    def build_user_info(locked, role_name):
         ''' build xml data for user-info '''
         xml = netapp_utils.zapi.NaElement('xml')
         data = {'num-records': 1,
-                'attributes-list': {'security-login-account-info': {'is-locked': locked}}}
+                'attributes-list': {'security-login-account-info': {'is-locked': locked, 'role-name': role_name}}}
 
         xml.translate_struct(data)
         print(xml.to_string())
@@ -104,6 +105,7 @@ class TestMyModule(unittest.TestCase):
             user_name = 'test'
             vserver = 'ansible_test'
             application = 'console'
+            lock_user = 'False'
             authentication_method = 'password'
         else:
             hostname = 'hostname'
@@ -173,7 +175,7 @@ class TestMyModule(unittest.TestCase):
         set_module_args(module_args)
         my_obj = my_module()
         if not self.use_vsim:
-            my_obj.server = MockONTAPConnection('user', 'false')
+            my_obj.server = MockONTAPConnection('user', 'false', 'test')
         with pytest.raises(AnsibleExitJson) as exc:
             my_obj.apply()
         print('Info: test_user_apply: %s' % repr(exc.value))
@@ -182,7 +184,7 @@ class TestMyModule(unittest.TestCase):
         set_module_args(module_args)
         my_obj = my_module()
         if not self.use_vsim:
-            my_obj.server = MockONTAPConnection('user', 'false')
+            my_obj.server = MockONTAPConnection('user', 'false', 'test')
         with pytest.raises(AnsibleExitJson) as exc:
             my_obj.apply()
         print('Info: test_user_delete: %s' % repr(exc.value))
@@ -198,7 +200,7 @@ class TestMyModule(unittest.TestCase):
         set_module_args(module_args)
         my_obj = my_module()
         if not self.use_vsim:
-            my_obj.server = MockONTAPConnection('user', 'false')
+            my_obj.server = MockONTAPConnection('user', 'false', 'test')
         with pytest.raises(AnsibleExitJson) as exc:
             my_obj.apply()
         print('Info: test_user_apply: %s' % repr(exc.value))
@@ -219,11 +221,11 @@ class TestMyModule(unittest.TestCase):
         module_args.update(self.set_default_args())
         module_args.update({'name': 'create'})
         module_args.update({'role_name': 'test'})
-        module_args.update({'lock_user': 'true'})
+        module_args.update({'lock_user': 'false'})
         set_module_args(module_args)
         my_obj = my_module()
         if not self.use_vsim:
-            my_obj.server = MockONTAPConnection('user', 'true')
+            my_obj.server = MockONTAPConnection('user', 'false', 'test')
         with pytest.raises(AnsibleExitJson) as exc:
             my_obj.apply()
         print('Info: test_user_apply: %s' % repr(exc.value))
@@ -232,7 +234,7 @@ class TestMyModule(unittest.TestCase):
         set_module_args(module_args)
         my_obj = my_module()
         if not self.use_vsim:
-            my_obj.server = MockONTAPConnection('user', 'true')
+            my_obj.server = MockONTAPConnection('user', 'true', 'test')
         with pytest.raises(AnsibleExitJson) as exc:
             my_obj.apply()
         print('Info: test_user_unlock: %s' % repr(exc.value))
@@ -253,3 +255,36 @@ class TestMyModule(unittest.TestCase):
             my_obj.apply()
         print('Info: test_user_apply: %s' % repr(exc.value))
         assert exc.value.args[0]['changed']
+
+    def test_ensure_user_role_update_called(self):
+        ''' set password '''
+        module_args = {}
+        module_args.update(self.set_default_args())
+        module_args.update({'name': 'create'})
+        module_args.update({'role_name': 'test123'})
+        module_args.update({'set_password': '123456'})
+        set_module_args(module_args)
+        my_obj = my_module()
+        if not self.use_vsim:
+            my_obj.server = MockONTAPConnection('user', 'true')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        print('Info: test_user_apply: %s' % repr(exc.value))
+        assert exc.value.args[0]['changed']
+
+    def test_ensure_user_role_update_additional_application_called(self):
+        ''' set password '''
+        module_args = {}
+        module_args.update(self.set_default_args())
+        module_args.update({'name': 'create'})
+        module_args.update({'role_name': 'test123'})
+        module_args.update({'application': 'http'})
+        module_args.update({'set_password': '123456'})
+        set_module_args(module_args)
+        my_obj = my_module()
+        if not self.use_vsim:
+            my_obj.server = MockONTAPConnection('user', 'true')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        print('Info: test_user_apply: %s' % repr(exc.value))
+        assert exc.value.args[0]['changed']