HNAS: avoid mismatch access level for managed shares

A CIFS managed share can contain some rules added in backend, and when
we try to add an already existent rule through manila, but with
different access level, the driver will return success with message
"user already allowed". So, manila will show an access level for this
rule that is different from the one in backend. This behaviour does
not apply to NFS shares.

In this case, it will call an update access rule for CIFS shares to
assure that rules in backend and in manila are always with same access
level.

Change-Id: I3c97ff7a5fee241685d54ac4cf7269ed55f1f602
Closes-bug: #1660401

manila/share/drivers/hitachi/hnas/ssh.py

@@ -162,23 +162,35 @@ class HNASSSHBackend(object):
         command = ['cifs-saa', 'add', '--target-label', self.fs_name,
                    name, user, permission]
 
-        entity_type = "share"
-        if is_snapshot:
-            entity_type = "snapshot"
-
         try:
             self._execute(command)
         except processutils.ProcessExecutionError as e:
             if 'already listed as a user' in e.stderr:
-                LOG.debug('User %(user)s already allowed to access '
-                          '%(entity_type)s %(share)s.',
-                          {'entity_type': entity_type, 'user': user,
-                           'share': name})
+                if is_snapshot:
+                    LOG.debug('User %(user)s already allowed to access '
+                              'snapshot %(snapshot)s.',
+                              {'user': user, 'snapshot': name})
+                else:
+                    self._update_cifs_rule(name, user, permission)
             else:
                 msg = six.text_type(e)
                 LOG.exception(msg)
                 raise exception.InvalidShareAccess(reason=msg)
 
+    def _update_cifs_rule(self, name, user, permission):
+        LOG.debug('User %(user)s already allowed to access '
+                  'share %(share)s. Updating access level...',
+                  {'user': user, 'share': name})
+
+        command = ['cifs-saa', 'change', '--target-label', self.fs_name,
+                   name, user, permission]
+        try:
+            self._execute(command)
+        except processutils.ProcessExecutionError:
+            msg = _("Could not update CIFS rule access for user %s.") % user
+            LOG.exception(msg)
+            raise exception.HNASBackendException(msg=msg)
+
     def cifs_deny_access(self, name, user, is_snapshot=False):
         command = ['cifs-saa', 'delete', '--target-label', self.fs_name,
                    name, user]

manila/tests/share/drivers/hitachi/hnas/test_ssh.py

@@ -713,13 +713,20 @@ class HNASSSHTestCase(test.TestCase):
         fake_cifs_allow_command = ['cifs-saa', 'add', '--target-label',
                                    self.fs_name, 'vvol_test',
                                    'fake_user', 'acr']
+        fake_cifs_allow_command2 = ['cifs-saa', 'change', '--target-label',
+                                    'file_system', 'vvol_test', 'fake_user',
+                                    'acr']
+
         self.mock_object(ssh.HNASSSHBackend, '_execute',
                          mock.Mock(side_effect=[putils.ProcessExecutionError(
-                             stderr='already listed as a user')]))
+                             stderr='already listed as a user'),
+                             "Rule modified."]))
 
         self._driver_ssh.cifs_allow_access('vvol_test', 'fake_user', 'acr')
 
-        self._driver_ssh._execute.assert_called_with(fake_cifs_allow_command)
+        self._driver_ssh._execute.assert_has_calls(
+            [mock.call(fake_cifs_allow_command),
+             mock.call(fake_cifs_allow_command2)])
         self.assertTrue(self.mock_log.debug.called)
 
     def test_cifs_allow_access_exception(self):
@@ -737,6 +744,29 @@ class HNASSSHTestCase(test.TestCase):
 
         self._driver_ssh._execute.assert_called_with(fake_cifs_allow_command)
 
+    def test_cifs_update_access_level_exception(self):
+        fake_cifs_allow_command = ['cifs-saa', 'add', '--target-label',
+                                   self.fs_name, 'vvol_test',
+                                   'fake_user', 'acr']
+        fake_cifs_allow_command2 = ['cifs-saa', 'change', '--target-label',
+                                    'file_system', 'vvol_test', 'fake_user',
+                                    'acr']
+
+        self.mock_object(ssh.HNASSSHBackend, '_execute',
+                         mock.Mock(side_effect=[putils.ProcessExecutionError(
+                             stderr='already listed as a user'),
+                             putils.ProcessExecutionError(
+                             stderr='Error when trying to modify rule.')]))
+
+        self.assertRaises(exception.HNASBackendException,
+                          self._driver_ssh.cifs_allow_access, 'vvol_test',
+                          'fake_user', 'acr')
+
+        self._driver_ssh._execute.assert_has_calls(
+            [mock.call(fake_cifs_allow_command),
+             mock.call(fake_cifs_allow_command2)])
+        self.assertTrue(self.mock_log.debug.called)
+
     def test_cifs_deny_access(self):
         fake_cifs_deny_command = ['cifs-saa', 'delete', '--target-label',
                                   self.fs_name, 'vvol_test', 'fake_user']

releasenotes/notes/fix_access_level_managed_shares_hnas-c76a09beed365b46.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - HNAS driver correctly handles rule updates to pre-existing access rules
+    on a managed CIFS share.