Updates to ontap_user

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

@@ -26,24 +26,24 @@ description:
 - Create or destroy users.
 
 options:
-
   state:
     description:
     - Whether the specified user should exist or not.
     choices: ['present', 'absent']
     default: 'present'
-
   name:
     description:
     - The name of the user to manage.
     required: true
-
+  applications:
-  application:
     description:
-    - Application to grant access to.
+    - List of application to grant access to.
     required: true
+    type: list
     choices: ['console', 'http','ontapi','rsh','snmp','service-processor','sp','ssh','telnet']
-
+    version_added: "2.8"
+    aliases:
+      - application
   authentication_method:
     description:
     - Authentication method for the application.
@@ -60,27 +60,22 @@ options:
     - Password, publickey, domain, nsswitch for ssh application.
     required: true
     choices: ['community', 'password', 'publickey', 'domain', 'nsswitch', 'usm']
-
   set_password:
     description:
     - Password for the user account.
     - It is ignored for creating snmp users, but is required for creating non-snmp users.
     - For an existing user, this value will be used as the new password.
-
   role_name:
     description:
     - The name of the role. Required when C(state=present)
-
   lock_user:
     description:
     - Whether the specified user account is locked.
     type: bool
-
   vserver:
     description:
     - The name of the vserver to use.
     required: true
-
 '''
 
 EXAMPLES = """
@@ -89,7 +84,7 @@ EXAMPLES = """
       na_ontap_user:
         state: present
         name: SampleUser
-        application: ssh
+        applications: ssh,console
         authentication_method: password
         set_password: apn1242183u1298u41
         lock_user: True
@@ -99,6 +94,17 @@ EXAMPLES = """
         username: "{{ netapp_username }}"
         password: "{{ netapp_password }}"
 
+    - name: Delete User
+      na_ontap_user:
+        state: absent
+        name: SampleUser
+        applications: ssh
+        authentication_method: password
+        vserver: ansibleVServer
+        hostname: "{{ netapp_hostname }}"
+        username: "{{ netapp_username }}"
+        password: "{{ netapp_password }}"
+
 """
 
 RETURN = """
@@ -109,7 +115,7 @@ import traceback
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_native
 import ansible.module_utils.netapp as netapp_utils
-
+from ansible.module_utils.netapp_module import NetAppModule
 
 HAS_NETAPP_LIB = netapp_utils.has_netapp_lib()
 
@@ -125,14 +131,14 @@ class NetAppOntapUser(object):
             state=dict(required=False, choices=['present', 'absent'], default='present'),
             name=dict(required=True, type='str'),
 
-            application=dict(required=True, type='str', choices=[
+            applications=dict(required=True, type='list', aliases=['application'],
-                'console', 'http', 'ontapi', 'rsh',
+                              choices=['console', 'http', 'ontapi', 'rsh', 'snmp',
-                'snmp', 'sp', 'service-processor', 'ssh', 'telnet']),
+                                       'sp', 'service-processor', 'ssh', 'telnet'],),
             authentication_method=dict(required=True, type='str',
                                        choices=['community', 'password',
                                                 'publickey', 'domain',
                                                 'nsswitch', 'usm']),
-            set_password=dict(required=False, type='str'),
+            set_password=dict(required=False, type='str', no_log=True),
             role_name=dict(required=False, type='str'),
             lock_user=dict(required=False, type='bool'),
             vserver=dict(required=True, type='str'),
@@ -146,44 +152,33 @@ class NetAppOntapUser(object):
             supports_check_mode=True
         )
 
-        parameters = self.module.params
+        self.na_helper = NetAppModule()
-
+        self.parameters = self.na_helper.set_parameters(self.module.params)
-        # set up state variables
-        self.state = parameters['state']
-        self.name = parameters['name']
-        self.application = parameters['application']
-        self.authentication_method = parameters['authentication_method']
-        self.set_password = parameters['set_password']
-        self.role_name = parameters['role_name']
-        self.lock_user = parameters['lock_user']
-        self.vserver = parameters['vserver']
 
         if HAS_NETAPP_LIB is False:
             self.module.fail_json(msg="the python NetApp-Lib module is required")
         else:
-            self.server = netapp_utils.setup_na_ontap_zapi(module=self.module, vserver=self.vserver)
+            self.server = netapp_utils.setup_na_ontap_zapi(module=self.module, vserver=self.parameters['vserver'])
 
-    def get_user(self):
+    def get_user(self, application=None):
         """
         Checks if the user exists.
-
+        :param: application: application to grant access to
         :return:
             True if user found
             False 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(
-            'security-login-account-info', **{'vserver': self.vserver,
+            'security-login-account-info', **{'vserver': self.parameters['vserver'],
-                                              'user-name': self.name,
+                                              'user-name': self.parameters['name'],
-                                              'application': self.application,
+                                              'authentication-method': self.parameters['authentication_method']})
-                                              'authentication-method':
+        if application is not None:
-                                                  self.authentication_method})
+            query_details.add_new_child('application', application)
         query = netapp_utils.zapi.NaElement('query')
         query.add_child_elem(query_details)
         security_login_get_iter.add_child_elem(query)
-        return_value = None
         try:
             result = self.server.invoke_successfully(security_login_get_iter,
                                                      enable_tunneling=False)
@@ -192,9 +187,10 @@ class NetAppOntapUser(object):
                 interface_attributes = result.get_child_by_name('attributes-list').\
                     get_child_by_name('security-login-account-info')
                 return_value = {
-                    'is_locked': interface_attributes.get_child_content('is-locked')
+                    'lock_user': interface_attributes.get_child_content('is-locked')
                 }
                 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":
@@ -203,55 +199,28 @@ class NetAppOntapUser(object):
             elif to_native(error.code) == "16043":
                 return False
             else:
-                self.module.fail_json(msg='Error getting user %s: %s' % (self.name, to_native(error)),
+                self.module.fail_json(msg='Error getting user %s: %s' % (self.parameters['name'], to_native(error)),
                                       exception=traceback.format_exc())
 
-    def get_user_lock_info(self):
+    def create_user(self, application):
         """
-        gets details of the user.
+        creates the user for the given application and authentication_method
+        :param: application: application to grant access to
         """
-        security_login_get_iter = netapp_utils.zapi.NaElement('security-login-get-iter')
-        query_details = netapp_utils.zapi.NaElement.create_node_with_children(
-            'security-login-account-info', **{'vserver': self.vserver,
-                                              'user-name': self.name,
-                                              'application': self.application,
-                                              # 'role-name': self.role_name,
-                                              'authentication-method':
-                                                  self.authentication_method})
-
-        query = netapp_utils.zapi.NaElement('query')
-        query.add_child_elem(query_details)
-        security_login_get_iter.add_child_elem(query)
-
-        result = self.server.invoke_successfully(security_login_get_iter, True)
-        return_value = None
-
-        if result.get_child_by_name('num-records') and \
-                int(result.get_child_content('num-records')) == 1:
-
-            interface_attributes = result.get_child_by_name('attributes-list').\
-                get_child_by_name('security-login-account-info')
-            return_value = {
-                'is_locked': interface_attributes.get_child_content('is-locked')
-            }
-        return return_value
-
-    def create_user(self):
         user_create = netapp_utils.zapi.NaElement.create_node_with_children(
-            'security-login-create', **{'vserver': self.vserver,
+            'security-login-create', **{'vserver': self.parameters['vserver'],
-                                        'user-name': self.name,
+                                        'user-name': self.parameters['name'],
-                                        'application': self.application,
+                                        'application': application,
-                                        'authentication-method':
+                                        'authentication-method': self.parameters['authentication_method'],
-                                            self.authentication_method,
+                                        'role-name': self.parameters.get('role_name')})
-                                        'role-name': self.role_name})
+        if self.parameters.get('set_password') is not None:
-        if self.set_password is not None:
+            user_create.add_new_child('password', self.parameters.get('set_password'))
-            user_create.add_new_child('password', self.set_password)
 
         try:
             self.server.invoke_successfully(user_create,
                                             enable_tunneling=False)
         except netapp_utils.zapi.NaApiError as error:
-            self.module.fail_json(msg='Error creating user %s: %s' % (self.name, to_native(error)),
+            self.module.fail_json(msg='Error creating user %s: %s' % (self.parameters['name'], to_native(error)),
                                   exception=traceback.format_exc())
 
     def lock_given_user(self):
@@ -264,14 +233,14 @@ class NetAppOntapUser(object):
         :rtype: bool
         """
         user_lock = netapp_utils.zapi.NaElement.create_node_with_children(
-            'security-login-lock', **{'vserver': self.vserver,
+            'security-login-lock', **{'vserver': self.parameters['vserver'],
-                                      'user-name': self.name})
+                                      'user-name': self.parameters['name']})
 
         try:
             self.server.invoke_successfully(user_lock,
                                             enable_tunneling=False)
         except netapp_utils.zapi.NaApiError as error:
-            self.module.fail_json(msg='Error locking user %s: %s' % (self.name, to_native(error)),
+            self.module.fail_json(msg='Error locking user %s: %s' % (self.parameters['name'], to_native(error)),
                                   exception=traceback.format_exc())
 
     def unlock_given_user(self):
@@ -284,8 +253,8 @@ class NetAppOntapUser(object):
         :rtype: bool
         """
         user_unlock = netapp_utils.zapi.NaElement.create_node_with_children(
-            'security-login-unlock', **{'vserver': self.vserver,
+            'security-login-unlock', **{'vserver': self.parameters['vserver'],
-                                        'user-name': self.name})
+                                        'user-name': self.parameters['name']})
 
         try:
             self.server.invoke_successfully(user_unlock,
@@ -294,23 +263,26 @@ class NetAppOntapUser(object):
             if to_native(error.code) == '13114':
                 return False
             else:
-                self.module.fail_json(msg='Error unlocking user %s: %s' % (self.name, to_native(error)),
+                self.module.fail_json(msg='Error unlocking user %s: %s' % (self.parameters['name'], to_native(error)),
                                       exception=traceback.format_exc())
         return True
 
-    def delete_user(self):
+    def delete_user(self, application):
+        """
+        deletes the user for the given application and authentication_method
+        :param: application: application to grant access to
+        """
         user_delete = netapp_utils.zapi.NaElement.create_node_with_children(
-            'security-login-delete', **{'vserver': self.vserver,
+            'security-login-delete', **{'vserver': self.parameters['vserver'],
-                                        'user-name': self.name,
+                                        'user-name': self.parameters['name'],
-                                        'application': self.application,
+                                        'application': application,
-                                        'authentication-method':
+                                        'authentication-method': self.parameters['authentication_method']})
-                                            self.authentication_method})
 
         try:
             self.server.invoke_successfully(user_delete,
                                             enable_tunneling=False)
         except netapp_utils.zapi.NaApiError as error:
-            self.module.fail_json(msg='Error removing user %s: %s' % (self.name, to_native(error)),
+            self.module.fail_json(msg='Error removing user %s: %s' % (self.parameters['name'], to_native(error)),
                                   exception=traceback.format_exc())
 
     def change_password(self):
@@ -322,11 +294,11 @@ class NetAppOntapUser(object):
             False if password is not updated
         :rtype: bool
         """
-        self.server.set_vserver(self.vserver)
+        # self.server.set_vserver(self.parameters['vserver'])
         modify_password = netapp_utils.zapi.NaElement.create_node_with_children(
             'security-login-modify-password', **{
-                'new-password': str(self.set_password),
+                'new-password': str(self.parameters.get('set_password')),
-                'user-name': self.name})
+                'user-name': self.parameters['name']})
         try:
             self.server.invoke_successfully(modify_password,
                                             enable_tunneling=True)
@@ -334,55 +306,46 @@ class NetAppOntapUser(object):
             if to_native(error.code) == '13114':
                 return False
             else:
-                self.module.fail_json(msg='Error setting password for user %s: %s' % (self.name, to_native(error)),
+                self.module.fail_json(msg='Error setting password for user %s: %s' % (self.parameters['name'], to_native(error)),
                                       exception=traceback.format_exc())
 
         self.server.set_vserver(None)
         return True
 
     def apply(self):
-        property_changed = False
+        create_delete_decision = {}
-        password_changed = False
+        modify = {}
-        lock_user_changed = False
         netapp_utils.ems_log_event("na_ontap_user", self.server)
-        user_exists = self.get_user()
+        for application in self.parameters['applications']:
+            current = self.get_user(application)
+            cd_action = self.na_helper.get_cd_action(current, self.parameters)
+            if cd_action is not None:
+                create_delete_decision[application] = cd_action
+        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()
+            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 user_exists:
+        if self.na_helper.changed:
-            if self.state == 'absent':
-                property_changed = True
-            elif self.state == 'present':
-                if self.set_password is not None:
-                    password_changed = True
-                if self.lock_user is not None:
-                    if self.lock_user is True and user_exists['is_locked'] != 'true':
-                        lock_user_changed = True
-                    elif self.lock_user is False and user_exists['is_locked'] != 'false':
-                        lock_user_changed = True
-        else:
-            if self.state == 'present':
-                # Check if anything needs to be updated
-                property_changed = True
-
-        changed = property_changed or password_changed or lock_user_changed
-
-        if changed:
             if self.module.check_mode:
                 pass
             else:
-                if self.state == 'present':
+                if create_delete_decision:
-                    if not user_exists:
+                    for cd_action in create_delete_decision:
-                        self.create_user()
+                        if create_delete_decision[cd_action] == 'create':
-                    else:
+                            self.create_user(cd_action)
-                        if password_changed:
+                        elif create_delete_decision[cd_action] == 'delete':
-                            self.change_password()
+                            self.delete_user(cd_action)
-                        if lock_user_changed:
+                elif modify:
-                            if self.lock_user:
+                    if self.parameters.get('lock_user'):
                         self.lock_given_user()
                     else:
                         self.unlock_given_user()
-                elif self.state == 'absent':
+                elif not create_delete_decision and self.parameters.get('set_password') is not None:
-                    self.delete_user()
+                    self.change_password()
-        self.module.exit_json(changed=changed)
+        self.module.exit_json(changed=self.na_helper.changed)
 
 
 def main():

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

@@ -0,0 +1,255 @@
+# (c) 2018, NetApp, Inc
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+''' unit tests ONTAP Ansible module: na_ontap_user '''
+
+from __future__ import print_function
+import json
+import pytest
+
+from units.compat import unittest
+from units.compat.mock import patch
+from ansible.module_utils import basic
+from ansible.module_utils._text import to_bytes
+import ansible.module_utils.netapp as netapp_utils
+
+from ansible.modules.storage.netapp.na_ontap_user \
+    import NetAppOntapUser as my_module  # module under test
+
+if not netapp_utils.has_netapp_lib():
+    pytestmark = pytest.skip('skipping as missing required netapp_lib')
+
+
+def set_module_args(args):
+    """prepare arguments so that they will be picked up during module creation"""
+    args = json.dumps({'ANSIBLE_MODULE_ARGS': args})
+    basic._ANSIBLE_ARGS = to_bytes(args)  # pylint: disable=protected-access
+
+
+class AnsibleExitJson(Exception):
+    """Exception class to be raised by module.exit_json and caught by the test case"""
+    pass
+
+
+class AnsibleFailJson(Exception):
+    """Exception class to be raised by module.fail_json and caught by the test case"""
+    pass
+
+
+def exit_json(*args, **kwargs):  # pylint: disable=unused-argument
+    """function to patch over exit_json; package return data into an exception"""
+    if 'changed' not in kwargs:
+        kwargs['changed'] = False
+    raise AnsibleExitJson(kwargs)
+
+
+def fail_json(*args, **kwargs):  # pylint: disable=unused-argument
+    """function to patch over fail_json; package return data into an exception"""
+    kwargs['failed'] = True
+    raise AnsibleFailJson(kwargs)
+
+
+class MockONTAPConnection(object):
+    ''' mock server connection to ONTAP host '''
+
+    def __init__(self, kind=None, parm1=None):
+        ''' save arguments '''
+        self.type = kind
+        self.parm1 = parm1
+        self.xml_in = None
+        self.xml_out = None
+
+    def invoke_successfully(self, xml, enable_tunneling):  # pylint: disable=unused-argument
+        ''' mock invoke_successfully returning xml data '''
+        self.xml_in = xml
+        if self.type == 'user':
+            xml = self.build_user_info(self.parm1)
+        self.xml_out = xml
+        return xml
+
+    @staticmethod
+    def set_vserver(vserver):
+        '''mock set vserver'''
+        pass
+
+    @staticmethod
+    def build_user_info(locked):
+        ''' 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}}}
+
+        xml.translate_struct(data)
+        print(xml.to_string())
+        return xml
+
+
+class TestMyModule(unittest.TestCase):
+    ''' a group of related Unit Tests '''
+
+    def setUp(self):
+        self.mock_module_helper = patch.multiple(basic.AnsibleModule,
+                                                 exit_json=exit_json,
+                                                 fail_json=fail_json)
+        self.mock_module_helper.start()
+        self.addCleanup(self.mock_module_helper.stop)
+        self.server = MockONTAPConnection()
+        self.use_vsim = False
+
+    def set_default_args(self):
+        if self.use_vsim:
+            hostname = '10.193.77.154'
+            username = 'admin'
+            password = 'netapp1!'
+            user_name = 'test'
+            vserver = 'ansible_test'
+            application = 'console'
+            authentication_method = 'password'
+        else:
+            hostname = 'hostname'
+            username = 'username'
+            password = 'password'
+            user_name = 'name'
+            vserver = 'vserver'
+            application = 'console'
+            authentication_method = 'password'
+
+        return dict({
+            'hostname': hostname,
+            'username': username,
+            'password': password,
+            'name': user_name,
+            'vserver': vserver,
+            'applications': application,
+            'authentication_method': authentication_method
+        })
+
+    def test_module_fail_when_required_args_missing(self):
+        ''' required arguments are reported as errors '''
+        with pytest.raises(AnsibleFailJson) as exc:
+            set_module_args({})
+            my_module()
+        print('Info: %s' % exc.value.args[0]['msg'])
+
+    def test_ensure_user_get_called(self):
+        ''' a more interesting test '''
+        module_args = {}
+        module_args.update(self.set_default_args())
+        module_args.update({'role_name': 'test'})
+        set_module_args(module_args)
+        my_obj = my_module()
+        my_obj.server = self.server
+        user_info = my_obj.get_user()
+        print('Info: test_user_get: %s' % repr(user_info))
+        assert user_info is None
+
+    def test_ensure_user_apply_called(self):
+        ''' creating user and checking idempotency '''
+        module_args = {}
+        module_args.update(self.set_default_args())
+        module_args.update({'name': 'create'})
+        module_args.update({'role_name': 'test'})
+        set_module_args(module_args)
+        my_obj = my_module()
+        if not self.use_vsim:
+            my_obj.server = self.server
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        print('Info: test_user_apply: %s' % repr(exc.value))
+        assert exc.value.args[0]['changed']
+        if not self.use_vsim:
+            my_obj.server = MockONTAPConnection('user', 'false')
+        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_apply_for_delete_called(self):
+        ''' deleting user and checking idempotency '''
+        module_args = {}
+        module_args.update(self.set_default_args())
+        module_args.update({'name': 'create'})
+        module_args.update({'role_name': 'test'})
+        set_module_args(module_args)
+        my_obj = my_module()
+        if not self.use_vsim:
+            my_obj.server = MockONTAPConnection('user', 'false')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        print('Info: test_user_apply: %s' % repr(exc.value))
+        assert not exc.value.args[0]['changed']
+        module_args.update({'state': 'absent'})
+        set_module_args(module_args)
+        my_obj = my_module()
+        if not self.use_vsim:
+            my_obj.server = MockONTAPConnection('user', 'false')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        print('Info: test_user_delete: %s' % repr(exc.value))
+        assert exc.value.args[0]['changed']
+
+    def test_ensure_user_lock_called(self):
+        ''' changing user_lock to True and checking idempotency'''
+        module_args = {}
+        module_args.update(self.set_default_args())
+        module_args.update({'name': 'create'})
+        module_args.update({'role_name': 'test'})
+        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', 'false')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        print('Info: test_user_apply: %s' % repr(exc.value))
+        assert not exc.value.args[0]['changed']
+        module_args.update({'lock_user': 'true'})
+        set_module_args(module_args)
+        my_obj = my_module()
+        if not self.use_vsim:
+            my_obj.server = MockONTAPConnection('user', 'false')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        print('Info: test_user_lock: %s' % repr(exc.value))
+        assert exc.value.args[0]['changed']
+
+    def test_ensure_user_unlock_called(self):
+        ''' changing user_lock to False and checking idempotency'''
+        module_args = {}
+        module_args.update(self.set_default_args())
+        module_args.update({'name': 'create'})
+        module_args.update({'role_name': 'test'})
+        module_args.update({'lock_user': 'true'})
+        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 not exc.value.args[0]['changed']
+        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')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        print('Info: test_user_unlock: %s' % repr(exc.value))
+        assert exc.value.args[0]['changed']
+
+    def test_ensure_user_set_password_called(self):
+        ''' set password '''
+        module_args = {}
+        module_args.update(self.set_default_args())
+        module_args.update({'name': 'create'})
+        module_args.update({'role_name': 'test'})
+        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']