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

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,
         }

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.

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)

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
         }

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,