diff --git a/manila/share/drivers/hitachi/hnas/ssh.py b/manila/share/drivers/hitachi/hnas/ssh.py index b96ad345fa..39665bc0aa 100644 --- a/manila/share/drivers/hitachi/hnas/ssh.py +++ b/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] diff --git a/manila/tests/share/drivers/hitachi/hnas/test_ssh.py b/manila/tests/share/drivers/hitachi/hnas/test_ssh.py index 255bc419d7..b660fcdd23 100644 --- a/manila/tests/share/drivers/hitachi/hnas/test_ssh.py +++ b/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'] diff --git a/releasenotes/notes/fix_access_level_managed_shares_hnas-c76a09beed365b46.yaml b/releasenotes/notes/fix_access_level_managed_shares_hnas-c76a09beed365b46.yaml new file mode 100644 index 0000000000..d3a7f439d3 --- /dev/null +++ b/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.