From d6b1c143b496af90c896e9ca2cf68d2140806d60 Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Wed, 25 Nov 2015 10:28:08 -0800 Subject: [PATCH] NetApp cDOT driver should support read-only CIFS shares Read-only shares are now a required Manila feature. The cDOT driver supports read-only access for NFS shares but not CIFS. This commit adds CIFS read-only support. Closes-Bug: #1513509 Change-Id: Ib7367d80c263331341ca136b61fa8185e894db3b --- .../netapp/dataontap/client/client_cmode.py | 4 +-- .../netapp/dataontap/protocols/cifs_cmode.py | 17 ++++++++-- .../netapp/dataontap/protocols/nfs_cmode.py | 5 +-- .../dataontap/client/test_client_cmode.py | 9 ++++-- .../dataontap/protocols/test_cifs_cmode.py | 32 +++++++++++++++++-- 5 files changed, 56 insertions(+), 11 deletions(-) diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode.py b/manila/share/drivers/netapp/dataontap/client/client_cmode.py index 45314fe7bd..2afa7b2ac6 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py @@ -1562,9 +1562,9 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): self.send_request('cifs-share-create', api_args) @na_utils.trace - def add_cifs_share_access(self, share_name, user_name): + def add_cifs_share_access(self, share_name, user_name, readonly): api_args = { - 'permission': 'full_control', + 'permission': 'read' if readonly else 'full_control', 'share': share_name, 'user-or-group': user_name, } diff --git a/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py b/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py index 9f022d44de..6bf546839c 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py +++ b/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py @@ -19,6 +19,7 @@ import re from oslo_log import log +from manila.common import constants from manila import exception from manila.i18n import _, _LE from manila.share.drivers.netapp.dataontap.client import api as netapp_api @@ -52,11 +53,23 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper): if access['access_type'] != 'user': msg = _("Cluster Mode supports only 'user' type for share access" " rules with CIFS protocol.") - raise exception.NetAppException(msg) + raise exception.InvalidShareAccess(reason=msg) + + user_name = access['access_to'] + + if access['access_level'] == constants.ACCESS_LEVEL_RW: + readonly = False + elif access['access_level'] == constants.ACCESS_LEVEL_RO: + readonly = True + else: + raise exception.InvalidShareAccessLevel( + level=access['access_level']) target, share_name = self._get_export_location(share) try: - self._client.add_cifs_share_access(share_name, access['access_to']) + self._client.add_cifs_share_access(share_name, + user_name, + readonly) except netapp_api.NaApiError as e: if e.code == netapp_api.EDUPLICATEENTRY: # Duplicate entry, so use specific exception. diff --git a/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py b/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py index f6c77398c6..b480ec1521 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py +++ b/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py @@ -51,8 +51,9 @@ class NetAppCmodeNFSHelper(base.NetAppBaseHelper): def allow_access(self, context, share, share_name, access): """Allows access to a given NFS share.""" if access['access_type'] != 'ip': - reason = _('Only ip access type allowed.') - raise exception.InvalidShareAccess(reason) + msg = _("Cluster Mode supports only 'ip' type for share access" + " rules with NFS protocol.") + raise exception.InvalidShareAccess(reason=msg) self._ensure_export_policy(share, share_name) export_policy_name = self._get_export_policy_name(share) diff --git a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py index cea0d27a10..143ec65fc0 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py @@ -2765,14 +2765,17 @@ class NetAppClientCmodeTestCase(test.TestCase): self.client.send_request.assert_has_calls([ mock.call('cifs-share-create', cifs_share_create_args)]) - def test_add_cifs_share_access(self): + @ddt.data(True, False) + def test_add_cifs_share_access(self, readonly): self.mock_object(self.client, 'send_request') - self.client.add_cifs_share_access(fake.SHARE_NAME, fake.USER_NAME) + self.client.add_cifs_share_access(fake.SHARE_NAME, + fake.USER_NAME, + readonly) cifs_share_access_control_create_args = { - 'permission': 'full_control', + 'permission': 'read' if readonly else 'full_control', 'share': fake.SHARE_NAME, 'user-or-group': fake.USER_NAME } diff --git a/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py b/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py index 84eadba298..7e952fbc36 100644 --- a/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py @@ -15,10 +15,13 @@ Mock unit tests for the NetApp driver protocols CIFS class module. """ +import copy + import ddt import mock from oslo_log import log +from manila.common import constants from manila import exception from manila.share.drivers.netapp.dataontap.client import api as netapp_api from manila.share.drivers.netapp.dataontap.protocols import cifs_cmode @@ -88,7 +91,20 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase): fake.USER_ACCESS) self.mock_client.add_cifs_share_access.assert_called_once_with( - fake.SHARE_NAME, fake.USER_ACCESS['access_to']) + fake.SHARE_NAME, fake.USER_ACCESS['access_to'], False) + + def test_allow_access_readonly(self): + + user_access = copy.deepcopy(fake.USER_ACCESS) + user_access['access_level'] = constants.ACCESS_LEVEL_RO + + self.helper.allow_access(self.mock_context, + fake.CIFS_SHARE, + fake.SHARE_NAME, + user_access) + + self.mock_client.add_cifs_share_access.assert_called_once_with( + fake.SHARE_NAME, fake.USER_ACCESS['access_to'], True) def test_allow_access_preexisting(self): @@ -114,11 +130,23 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase): fake.SHARE_NAME, fake.USER_ACCESS) + def test_allow_access_invalid_level(self): + + user_access = copy.deepcopy(fake.USER_ACCESS) + user_access['access_level'] = 'fake_level' + + self.assertRaises(exception.InvalidShareAccessLevel, + self.helper.allow_access, + self.mock_context, + fake.NFS_SHARE, + fake.SHARE_NAME, + user_access) + def test_allow_access_invalid_type(self): fake_access = fake.USER_ACCESS.copy() fake_access['access_type'] = 'group' - self.assertRaises(exception.NetAppException, + self.assertRaises(exception.InvalidShareAccess, self.helper.allow_access, self.mock_context, fake.CIFS_SHARE,