diff --git a/lib/ansible/modules/storage/netapp/na_ontap_cifs.py b/lib/ansible/modules/storage/netapp/na_ontap_cifs.py index 5570d72cfdc..edf6c883713 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_cifs.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_cifs.py @@ -34,13 +34,23 @@ options: The name of the CIFS share. The CIFS share name is a UTF-8 string with the following characters being illegal; control characters from 0x00 to 0x1F, both inclusive, 0x22 (double quotes) required: true + share_properties: + description: + - The list of properties for the CIFS share + required: false + version_added: '2.8' + symlink_properties: + description: + - The list of symlink properties for this CIFS share + required: false + version_added: '2.8' state: choices: ['present', 'absent'] description: - "Whether the specified CIFS share should exist or not." required: false default: present -short_description: "NetApp ONTAP manage cifs-share" +short_description: NetApp ONTAP Manage cifs-share version_added: "2.6" ''' @@ -52,6 +62,8 @@ EXAMPLES = """ share_name: cifsShareName path: / vserver: vserverName + share_properties: browsable,oplocks + symlink_properties: read_only,enable hostname: "{{ netapp_hostname }}" username: "{{ netapp_username }}" password: "{{ netapp_password }}" @@ -69,6 +81,8 @@ EXAMPLES = """ share_name: pb_test vserver: vserverName path: / + share_properties: show_previous_versions + symlink_properties: disable hostname: "{{ netapp_hostname }}" username: "{{ netapp_username }}" password: "{{ netapp_password }}" @@ -83,6 +97,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() @@ -99,31 +114,25 @@ class NetAppONTAPCifsShare(object): 'present', 'absent'], default='present'), share_name=dict(required=True, type='str'), path=dict(required=False, type='str'), - vserver=dict(required=True, type='str') + vserver=dict(required=True, type='str'), + share_properties=dict(required=False, type='list'), + symlink_properties=dict(required=False, type='list') )) self.module = AnsibleModule( argument_spec=self.argument_spec, - required_if=[ - ('state', 'present', ['share_name', 'path']) - ], supports_check_mode=True ) - parameters = self.module.params - - # set up state variables - self.state = parameters['state'] - self.share_name = parameters['share_name'] - self.path = parameters['path'] - self.vserver = parameters['vserver'] + self.na_helper = NetAppModule() + self.parameters = self.na_helper.set_parameters(self.module.params) 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) + module=self.module, vserver=self.parameters.get('vserver')) def get_cifs_share(self): """ @@ -135,7 +144,7 @@ class NetAppONTAPCifsShare(object): """ cifs_iter = netapp_utils.zapi.NaElement('cifs-share-get-iter') cifs_info = netapp_utils.zapi.NaElement('cifs-share') - cifs_info.add_new_child('share-name', self.share_name) + cifs_info.add_new_child('share-name', self.parameters.get('share_name')) query = netapp_utils.zapi.NaElement('query') query.add_child_elem(cifs_info) @@ -145,16 +154,26 @@ class NetAppONTAPCifsShare(object): result = self.server.invoke_successfully(cifs_iter, True) return_value = None - print(result.to_string()) # check if query returns the expected cifs-share if result.get_child_by_name('num-records') and \ int(result.get_child_content('num-records')) == 1: - - cifs_acl = result.get_child_by_name('attributes-list').\ + properties_list = [] + symlink_list = [] + cifs_attrs = result.get_child_by_name('attributes-list').\ get_child_by_name('cifs-share') + if cifs_attrs.get_child_by_name('share-properties'): + properties_attrs = cifs_attrs['share-properties'] + if properties_attrs is not None: + properties_list = [property.get_content() for property in properties_attrs.get_children()] + if cifs_attrs.get_child_by_name('symlink-properties'): + symlink_attrs = cifs_attrs['symlink-properties'] + if symlink_attrs is not None: + symlink_list = [symlink.get_content() for symlink in symlink_attrs.get_children()] return_value = { - 'share': cifs_acl.get_child_content('share-name'), - 'path': cifs_acl.get_child_content('path'), + 'share': cifs_attrs.get_child_content('share-name'), + 'path': cifs_attrs.get_child_content('path'), + 'share_properties': properties_list, + 'symlink_properties': symlink_list } return return_value @@ -163,9 +182,20 @@ class NetAppONTAPCifsShare(object): """ Create CIFS share """ + options = {'share-name': self.parameters.get('share_name'), + 'path': self.parameters.get('path')} cifs_create = netapp_utils.zapi.NaElement.create_node_with_children( - 'cifs-share-create', **{'share-name': self.share_name, - 'path': self.path}) + 'cifs-share-create', **options) + if self.parameters.get('share_properties'): + property_attrs = netapp_utils.zapi.NaElement('share-properties') + cifs_create.add_child_elem(property_attrs) + for property in self.parameters.get('share_properties'): + property_attrs.add_new_child('cifs-share-properties', property) + if self.parameters.get('symlink_properties'): + symlink_attrs = netapp_utils.zapi.NaElement('symlink-properties') + cifs_create.add_child_elem(symlink_attrs) + for symlink in self.parameters.get('symlink_properties'): + symlink_attrs.add_new_child('cifs-share-symlink-properties', symlink) try: self.server.invoke_successfully(cifs_create, @@ -173,7 +203,7 @@ class NetAppONTAPCifsShare(object): except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error creating cifs-share %s: %s' - % (self.share_name, to_native(error)), + % (self.parameters.get('share_name'), to_native(error)), exception=traceback.format_exc()) def delete_cifs_share(self): @@ -181,61 +211,61 @@ class NetAppONTAPCifsShare(object): Delete CIFS share """ cifs_delete = netapp_utils.zapi.NaElement.create_node_with_children( - 'cifs-share-delete', **{'share-name': self.share_name}) + 'cifs-share-delete', **{'share-name': self.parameters.get('share_name')}) try: self.server.invoke_successfully(cifs_delete, enable_tunneling=True) except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error deleting cifs-share %s: %s' - % (self.share_name, to_native(error)), + % (self.parameters.get('share_name'), to_native(error)), exception=traceback.format_exc()) def modify_cifs_share(self): """ modilfy path for the given CIFS share """ + options = {'share-name': self.parameters.get('share_name')} cifs_modify = netapp_utils.zapi.NaElement.create_node_with_children( - 'cifs-share-modify', **{'share-name': self.share_name, - 'path': self.path}) + 'cifs-share-modify', **options) + if self.parameters.get('path'): + cifs_modify.add_new_child('path', self.parameters.get('path')) + if self.parameters.get('share_properties'): + property_attrs = netapp_utils.zapi.NaElement('share-properties') + cifs_modify.add_child_elem(property_attrs) + for property in self.parameters.get('share_properties'): + property_attrs.add_new_child('cifs-share-properties', property) + if self.parameters.get('symlink_properties'): + symlink_attrs = netapp_utils.zapi.NaElement('symlink-properties') + cifs_modify.add_child_elem(symlink_attrs) + for property in self.parameters.get('symlink_properties'): + symlink_attrs.add_new_child('cifs-share-symlink-properties', property) try: self.server.invoke_successfully(cifs_modify, enable_tunneling=True) except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error modifying cifs-share %s:%s' - % (self.share_name, to_native(error)), + % (self.parameters.get('share_name'), to_native(error)), exception=traceback.format_exc()) def apply(self): '''Apply action to cifs share''' - changed = False - cifs_exists = False netapp_utils.ems_log_event("na_ontap_cifs", self.server) - cifs_details = self.get_cifs_share() - if cifs_details: - cifs_exists = True - if self.state == 'absent': # delete - changed = True - elif self.state == 'present': - if cifs_details['path'] != self.path: # modify path - changed = True - else: - if self.state == 'present': # create - changed = True - - if changed: + current = self.get_cifs_share() + cd_action = self.na_helper.get_cd_action(current, self.parameters) + if cd_action is None: + modify = self.na_helper.get_modified_attributes(current, self.parameters) + if self.na_helper.changed: if self.module.check_mode: pass else: - if self.state == 'present': # execute create - if not cifs_exists: - self.create_cifs_share() - else: # execute modify path - self.modify_cifs_share() - elif self.state == 'absent': # execute delete + if cd_action == 'create': + self.create_cifs_share() + elif cd_action == 'delete': self.delete_cifs_share() - - self.module.exit_json(changed=changed) + elif modify: + self.modify_cifs_share() + self.module.exit_json(changed=self.na_helper.changed) def main(): diff --git a/test/units/modules/storage/netapp/test_na_ontap_cifs.py b/test/units/modules/storage/netapp/test_na_ontap_cifs.py new file mode 100644 index 00000000000..a555a742166 --- /dev/null +++ b/test/units/modules/storage/netapp/test_na_ontap_cifs.py @@ -0,0 +1,221 @@ +# (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_cifs ''' + +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_cifs \ + import NetAppONTAPCifsShare 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): + ''' save arguments ''' + self.type = kind + 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 == 'cifs': + xml = self.build_cifs_info() + elif self.type == 'cifs_fail': + raise netapp_utils.zapi.NaApiError(code='TEST', message="This exception is from the unit test") + self.xml_out = xml + return xml + + @staticmethod + def build_cifs_info(): + ''' build xml data for cifs-info ''' + xml = netapp_utils.zapi.NaElement('xml') + data = {'num-records': 1, 'attributes-list': {'cifs-share': { + 'share-name': 'test', + 'path': '/test', + 'share-properties': {'cifs-share-properties': 'browsable'}, + 'symlink-properties': {'cifs-share-symlink-properties': 'enable'}, + }}} + 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.onbox = False + + def set_default_args(self): + if self.onbox: + hostname = '10.193.77.37' + username = 'admin' + password = 'netapp1!' + share_name = 'test' + path = '/test' + share_properties = 'browsable,oplocks' + symlink_properties = 'disable' + vserver = 'abc' + else: + hostname = '10.193.77.37' + username = 'admin' + password = 'netapp1!' + share_name = 'test' + path = '/test' + share_properties = 'show_previous_versions' + symlink_properties = 'disable' + vserver = 'abc' + return dict({ + 'hostname': hostname, + 'username': username, + 'password': password, + 'share_name': share_name, + 'path': path, + 'share_properties': share_properties, + 'symlink_properties': symlink_properties, + 'vserver': vserver + }) + + 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_cifs_get_called(self): + ''' fetching details of cifs ''' + set_module_args(self.set_default_args()) + my_obj = my_module() + my_obj.server = self.server + cifs_get = my_obj.get_cifs_share() + print('Info: test_cifs_share_get: %s' % repr(cifs_get)) + assert not bool(cifs_get) + + def test_ensure_apply_for_cifs_called(self): + ''' creating cifs share and checking idempotency ''' + module_args = {} + module_args.update(self.set_default_args()) + set_module_args(module_args) + my_obj = my_module() + if not self.onbox: + my_obj.server = self.server + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + print('Info: test_cifs_apply: %s' % repr(exc.value)) + assert exc.value.args[0]['changed'] + if not self.onbox: + my_obj.server = MockONTAPConnection('cifs') + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + print('Info: test_cifs_apply: %s' % repr(exc.value)) + assert exc.value.args[0]['changed'] + + @patch('ansible.modules.storage.netapp.na_ontap_cifs.NetAppONTAPCifsShare.create_cifs_share') + def test_cifs_create_called(self, create_cifs_share): + ''' creating cifs''' + module_args = {} + module_args.update(self.set_default_args()) + set_module_args(module_args) + my_obj = my_module() + if not self.onbox: + my_obj.server = MockONTAPConnection() + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + print('Info: test_cifs_apply: %s' % repr(exc.value)) + create_cifs_share.assert_called_with() + + @patch('ansible.modules.storage.netapp.na_ontap_cifs.NetAppONTAPCifsShare.delete_cifs_share') + def test_cifs_delete_called(self, delete_cifs_share): + ''' deleting cifs''' + module_args = {} + module_args.update(self.set_default_args()) + module_args['state'] = 'absent' + set_module_args(module_args) + my_obj = my_module() + if not self.onbox: + my_obj.server = MockONTAPConnection('cifs') + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + print('Info: test_cifs_apply: %s' % repr(exc.value)) + delete_cifs_share.assert_called_with() + + @patch('ansible.modules.storage.netapp.na_ontap_cifs.NetAppONTAPCifsShare.modify_cifs_share') + def test_cifs_modify_called(self, modify_cifs_share): + ''' modifying cifs''' + module_args = {} + module_args.update(self.set_default_args()) + set_module_args(module_args) + my_obj = my_module() + if not self.onbox: + my_obj.server = MockONTAPConnection('cifs') + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + print('Info: test_cifs_apply: %s' % repr(exc.value)) + modify_cifs_share.assert_called_with() + + def test_if_all_methods_catch_exception(self): + module_args = {} + module_args.update(self.set_default_args()) + set_module_args(module_args) + my_obj = my_module() + if not self.onbox: + my_obj.server = MockONTAPConnection('cifs_fail') + with pytest.raises(AnsibleFailJson) as exc: + my_obj.create_cifs_share() + assert 'Error creating cifs-share' in exc.value.args[0]['msg'] + with pytest.raises(AnsibleFailJson) as exc: + my_obj.delete_cifs_share() + assert 'Error deleting cifs-share' in exc.value.args[0]['msg'] + with pytest.raises(AnsibleFailJson) as exc: + my_obj.modify_cifs_share() + assert 'Error modifying cifs-share' in exc.value.args[0]['msg']