From 753bf63dc53eb661c6b9aa301062b1e97cf9b0eb Mon Sep 17 00:00:00 2001 From: Shaun Edwards Date: Mon, 1 May 2017 09:05:07 -0700 Subject: [PATCH] Implement update_access in Isilon Driver. Implemented the required "update_access" method in the Isilon driver. Change-Id: Iedef4b88c6cdd8f39d88cac40a66146bf666e84b Implements: blueprint dellemc-isilon-driver-update-access --- manila/share/drivers/dell_emc/driver.py | 6 + .../drivers/dell_emc/plugins/isilon/isilon.py | 114 ++++- .../dell_emc/plugins/isilon/test_isilon.py | 419 ++++++++++++++++++ 3 files changed, 538 insertions(+), 1 deletion(-) diff --git a/manila/share/drivers/dell_emc/driver.py b/manila/share/drivers/dell_emc/driver.py index f1e5a81705..ec6f9ef398 100644 --- a/manila/share/drivers/dell_emc/driver.py +++ b/manila/share/drivers/dell_emc/driver.py @@ -120,6 +120,12 @@ class EMCShareDriver(driver.ShareDriver): """Deny access to the share.""" self.plugin.deny_access(context, share, access, share_server) + def update_access(self, context, share, access_rules, add_rules, + delete_rules, share_server=None): + """Update access to the share.""" + self.plugin.update_access(context, share, access_rules, add_rules, + delete_rules, share_server) + def check_for_setup_error(self): """Check for setup error.""" self.plugin.check_for_setup_error() diff --git a/manila/share/drivers/dell_emc/plugins/isilon/isilon.py b/manila/share/drivers/dell_emc/plugins/isilon/isilon.py index 52266895d4..3043c330d7 100644 --- a/manila/share/drivers/dell_emc/plugins/isilon/isilon.py +++ b/manila/share/drivers/dell_emc/plugins/isilon/isilon.py @@ -20,6 +20,7 @@ import os from oslo_config import cfg from oslo_log import log from oslo_utils import units +from requests.exceptions import HTTPError import six from manila.common import constants as const @@ -253,7 +254,7 @@ class IsilonStorageConnection(base.StorageConnection): host_acl.append(allowed_ip) data = {'host_acl': host_acl} url = ('{0}/platform/1/protocols/smb/shares/{1}' - .format(self._server_url, smb_share['name'])) + .format(self._server_url, share['name'])) r = self._isilon_api.request('PUT', url, data=data) r.raise_for_status() @@ -402,3 +403,114 @@ class IsilonStorageConnection(base.StorageConnection): def teardown_server(self, server_details, security_services=None): """Teardown share server.""" # TODO(Shaun Edwards): Look into supporting share servers + + def update_access(self, context, share, access_rules, add_rules, + delete_rules, share_server=None): + """Update share access.""" + if share['share_proto'] == 'NFS': + state_map = self._update_access_nfs(share, access_rules) + if share['share_proto'] == 'CIFS': + state_map = self._update_access_cifs(share, access_rules) + return state_map + + def _update_access_nfs(self, share, access_rules): + """Updates access on a NFS share.""" + nfs_rw_ips = set() + nfs_ro_ips = set() + rule_state_map = {} + for rule in access_rules: + rule_state_map[rule['access_id']] = { + 'state': 'error' + } + + for rule in access_rules: + if rule['access_level'] == const.ACCESS_LEVEL_RW: + nfs_rw_ips.add(rule['access_to']) + elif rule['access_level'] == const.ACCESS_LEVEL_RO: + nfs_ro_ips.add(rule['access_to']) + + export_id = self._isilon_api.lookup_nfs_export( + self._get_container_path(share)) + if export_id is None: + # share does not exist on backend (set all rules to error state) + return rule_state_map + data = { + 'clients': list(nfs_rw_ips), + 'read_only_clients': list(nfs_ro_ips) + } + url = ('{0}/platform/1/protocols/nfs/exports/{1}' + .format(self._server_url, six.text_type(export_id))) + r = self._isilon_api.request('PUT', url, data=data) + try: + r.raise_for_status() + except HTTPError: + return rule_state_map + + # if we finish the bulk rule update with no error set rules to active + for rule in access_rules: + rule_state_map[rule['access_id']]['state'] = 'active' + return rule_state_map + + def _update_access_cifs(self, share, access_rules): + """Clear access on a CIFS share.""" + cifs_ip_set = set() + users = set() + for rule in access_rules: + if rule['access_type'] == 'ip': + cifs_ip_set.add('allow:' + rule['access_to']) + elif rule['access_type'] == 'user': + users.add(rule['access_to']) + + smb_share = self._isilon_api.lookup_smb_share(share['name']) + + backend_smb_user_permissions = smb_share['permissions'] + perms_to_remove = [] + for perm in backend_smb_user_permissions: + if perm['trustee']['name'] not in users: + perms_to_remove.append(perm) + for perm in perms_to_remove: + backend_smb_user_permissions.remove(perm) + + data = { + 'host_acl': list(cifs_ip_set), + 'permissions': backend_smb_user_permissions, + } + + url = ('{0}/platform/1/protocols/smb/shares/{1}' + .format(self._server_url, share['name'])) + r = self._isilon_api.request('PUT', url, data=data) + try: + r.raise_for_status() + except HTTPError: + # clear access rules failed so set all access rules to error state + rule_state_map = {} + for rule in access_rules: + rule_state_map[rule['access_id']] = { + 'state': 'error' + } + return rule_state_map + + # add access rules that don't exist on backend + rule_state_map = {} + for rule in access_rules: + rule_state_map[rule['access_id']] = { + 'state': 'error' + } + try: + if rule['access_type'] == 'ip': + self._cifs_allow_access_ip(rule['access_to'], share, + rule['access_level']) + rule_state_map[rule['access_id']]['state'] = 'active' + elif rule['access_type'] == 'user': + backend_users = set() + for perm in backend_smb_user_permissions: + backend_users.add(perm['trustee']['name']) + if rule['access_to'] not in backend_users: + self._cifs_allow_access_user( + rule['access_to'], share, rule['access_level']) + rule_state_map[rule['access_id']]['state'] = 'active' + else: + continue + except exception.ManilaException: + pass + return rule_state_map diff --git a/manila/tests/share/drivers/dell_emc/plugins/isilon/test_isilon.py b/manila/tests/share/drivers/dell_emc/plugins/isilon/test_isilon.py index 777a18bc60..d6acd52453 100644 --- a/manila/tests/share/drivers/dell_emc/plugins/isilon/test_isilon.py +++ b/manila/tests/share/drivers/dell_emc/plugins/isilon/test_isilon.py @@ -17,10 +17,12 @@ import ddt import mock from oslo_log import log from oslo_utils import units +from requests.exceptions import HTTPError import six from manila.common import constants as const from manila import exception +from manila.i18n import _ from manila.share.drivers.dell_emc.plugins.isilon import isilon from manila.share.drivers.dell_emc.plugins.isilon import isilon_api from manila import test @@ -764,3 +766,420 @@ class IsilonTest(test.TestCase): expected_quota_size = new_share_size * units.Gi self._mock_isilon_api.quota_set.assert_called_once_with( share_path, 'directory', expected_quota_size) + + def test_update_access_add_nfs(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'NFS', + } + fake_export_id = 4 + self._mock_isilon_api.lookup_nfs_export.return_value = fake_export_id + self._mock_isilon_api.get_nfs_export.return_value = { + 'clients': [], + 'read_only_clients': [] + } + nfs_access = { + 'access_type': 'ip', + 'access_to': '10.1.1.10', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '09960614-8574-4e03-89cf-7cf267b0bd08' + } + access_rules = [nfs_access] + add_rules = [nfs_access] + delete_rules = [] + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, add_rules, + delete_rules, share_server=None) + + expected_url = (self.API_URL + '/platform/1/protocols/nfs/exports/' + + str(fake_export_id)) + expected_data = {'clients': ['10.1.1.10'], 'read_only_clients': []} + expected_rule_map = { + '09960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'active' + } + } + self._mock_isilon_api.request.assert_called_once_with( + 'PUT', expected_url, data=expected_data) + self.assertEqual(expected_rule_map, rule_map) + + def test_update_access_add_cifs(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'CIFS', + } + access = { + 'access_type': 'user', + 'access_to': 'foo', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '09960614-8574-4e03-89cf-7cf267b0bd08' + } + add_rules = [access] + access_rules = [access] + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, add_rules, []) + + self._mock_isilon_api.smb_permissions_add.assert_called_once_with( + self.SHARE_NAME, 'foo', isilon_api.SmbPermission.rw) + expected_rule_map = { + '09960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'active' + } + } + self.assertEqual(expected_rule_map, rule_map) + + def test_update_access_delete_nfs(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'NFS', + } + fake_export_id = 4 + self._mock_isilon_api.lookup_nfs_export.return_value = fake_export_id + # simulate an IP added to the whitelist + ip_addr = '10.0.0.4' + ip_addr_ro = '10.0.0.50' + self._mock_isilon_api.get_nfs_export.return_value = { + 'clients': [ip_addr], 'read_only_clients': [ip_addr_ro]} + nfs_access_del_1 = { + 'access_type': 'ip', + 'access_to': ip_addr, + 'access_level': const.ACCESS_LEVEL_RW + } + nfs_access_del_2 = { + 'access_type': 'ip', + 'access_to': ip_addr, + 'access_level': const.ACCESS_LEVEL_RW + } + access_rules = [] + delete_rules = [nfs_access_del_1, nfs_access_del_2] + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, [], delete_rules) + + expected_url = (self.API_URL + '/platform/1/protocols/nfs/exports/' + + six.text_type(fake_export_id)) + expected_data = {'clients': [], 'read_only_clients': []} + self._mock_isilon_api.request.assert_called_once_with( + 'PUT', expected_url, data=expected_data) + self.assertEqual({}, rule_map) + + def test_update_access_delete_cifs(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'CIFS', + } + delete_rule = { + 'access_type': 'user', + 'access_to': 'newuser', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '29960614-8574-4e03-89cf-7cf267b0bd08' + } + access_rules = [] + delete_rules = [delete_rule] + self._mock_isilon_api.lookup_smb_share.return_value = { + 'permissions': [ + { + 'permission': 'change', + 'permission_type': 'allow', + 'trustee': { + 'id': 'SID:S-1-5-21', + 'name': 'newuser', + 'type': 'user', + } + + } + ] + } + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, [], delete_rules) + + expected_url = (self.API_URL + '/platform/1/protocols/smb/shares/' + + self.SHARE_NAME) + self._mock_isilon_api.request.assert_called_once_with( + 'PUT', expected_url, data={'permissions': [], 'host_acl': []}) + self.assertEqual({}, rule_map) + + def test_update_access_nfs_share_not_found(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'NFS', + } + access = { + 'access_type': 'user', + 'access_to': 'foouser', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '09960614-8574-4e03-89cf-7cf267b0bd08' + } + access_rules = [access] + add_rules = [access] + self._mock_isilon_api.lookup_nfs_export.return_value = None + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, add_rules, []) + + expected_rule_map = { + '09960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'error' + } + } + self.assertEqual(expected_rule_map, rule_map) + + def test_update_access_nfs_http_error_on_clear_rules(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'NFS', + } + access = { + 'access_type': 'user', + 'access_to': 'foouser', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '09960614-8574-4e03-89cf-7cf267b0bd08' + } + access_rules = [access] + add_rules = [access] + self._mock_isilon_api.request.return_value.raise_for_status.\ + side_effect = HTTPError + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, add_rules, []) + + expected_rule_map = { + '09960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'error' + } + } + self.assertEqual(expected_rule_map, rule_map) + + def test_update_access_cifs_http_error_on_clear_rules(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'CIFS', + } + access = { + 'access_type': 'user', + 'access_to': 'foo', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '09960614-8574-4e03-89cf-7cf267b0bd08' + } + add_rules = [access] + access_rules = [access] + self._mock_isilon_api.request.return_value.raise_for_status.\ + side_effect = HTTPError + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, add_rules, []) + + expected_rule_map = { + '09960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'error' + } + } + self.assertEqual(expected_rule_map, rule_map) + + def test_update_access_cifs_share_backend_error(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'CIFS', + } + access = { + 'access_type': 'user', + 'access_to': 'foo', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '09960614-8574-4e03-89cf-7cf267b0bd08' + } + add_rules = [access] + access_rules = [access] + message = _('Only "RW" and "RO" access levels are supported.') + self._mock_isilon_api.smb_permissions_add.side_effect = \ + exception.ShareBackendException(message=message) + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, add_rules, []) + + expected_rule_map = { + '09960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'error' + } + } + self.assertEqual(expected_rule_map, rule_map) + + def test_update_access_cifs_invalid_access_type(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'CIFS', + } + access = { + 'access_type': 'foo', + 'access_to': 'foo', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '09960614-8574-4e03-89cf-7cf267b0bd08' + } + add_rules = [access] + access_rules = [access] + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, add_rules, []) + + expected_rule_map = { + '09960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'error' + } + } + self.assertEqual(expected_rule_map, rule_map) + + def test_update_access_recover_nfs(self): + # verify that new ips are added and ips not in rules are removed + share = { + "name": self.SHARE_NAME, + "share_proto": 'NFS', + } + fake_export_id = 4 + self._mock_isilon_api.lookup_nfs_export.return_value = fake_export_id + self._mock_isilon_api.get_nfs_export.return_value = { + 'clients': ['10.1.1.8'], + 'read_only_clients': ['10.2.0.2'] + } + nfs_access_1 = { + 'access_type': 'ip', + 'access_to': '10.1.1.10', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '09960614-8574-4e03-89cf-7cf267b0bd08' + } + nfs_access_2 = { + 'access_type': 'ip', + 'access_to': '10.1.1.2', + 'access_level': const.ACCESS_LEVEL_RO, + 'access_id': '19960614-8574-4e03-89cf-7cf267b0bd08' + } + access_rules = [nfs_access_1, nfs_access_2] + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, [], []) + + expected_url = (self.API_URL + '/platform/1/protocols/nfs/exports/' + + six.text_type(fake_export_id)) + expected_data = { + 'clients': ['10.1.1.10'], + 'read_only_clients': ['10.1.1.2'] + } + expected_rule_map = { + '09960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'active' + }, + '19960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'active' + } + } + self._mock_isilon_api.request.assert_called_once_with( + 'PUT', expected_url, data=expected_data) + self.assertEqual(expected_rule_map, rule_map) + + def test_update_access_recover_cifs(self): + share = { + "name": self.SHARE_NAME, + "share_proto": 'CIFS', + } + mock_smb_share_1 = { + 'host_acl': ['allow:10.1.2.3', 'allow:10.1.1.12'], + 'permissions': [ + { + 'permission': 'change', + 'permission_type': 'allow', + 'trustee': { + 'id': 'SID:S-1-5-21', + 'name': 'foouser', + 'type': 'user', + } + }, + { + 'permission': 'ro', + 'permission_type': 'allow', + 'trustee': { + 'id': 'SID:S-1-5-22', + 'name': 'testuser', + 'type': 'user', + } + } + ] + } + mock_smb_share_2 = { + 'host_acl': ['allow:10.1.2.3', 'allow:10.1.1.12', + 'allow:10.1.1.10'], + 'permissions': [ + { + 'permission': 'change', + 'permission_type': 'allow', + 'trustee': { + 'id': 'SID:S-1-5-21', + 'name': 'foouser', + 'type': 'user', + } + }, + { + 'permission': 'ro', + 'permission_type': 'allow', + 'trustee': { + 'id': 'SID:S-1-5-22', + 'name': 'testuser', + 'type': 'user', + } + } + ] + } + self._mock_isilon_api.lookup_smb_share.side_effect = [ + mock_smb_share_1, mock_smb_share_2] + + access_1 = { + 'access_type': 'ip', + 'access_to': '10.1.1.10', + 'access_level': const.ACCESS_LEVEL_RW, + 'access_id': '09960614-8574-4e03-89cf-7cf267b0bd08' + } + access_2 = { + 'access_type': 'user', + 'access_to': 'testuser', + 'access_level': const.ACCESS_LEVEL_RO, + 'access_id': '19960614-8574-4e03-89cf-7cf267b0bd08' + } + access_rules = [access_1, access_2] + + rule_map = self.storage_connection.update_access( + self.mock_context, share, access_rules, [], []) + + expected_url = (self.API_URL + '/platform/1/protocols/smb/shares/' + + self.SHARE_NAME) + expected_data = { + 'host_acl': ['allow:10.1.1.10'], + 'permissions': [ + { + 'permission': 'ro', + 'permission_type': 'allow', + 'trustee': { + 'id': 'SID:S-1-5-22', + 'name': 'testuser', + 'type': 'user', + } + } + ] + } + expected_rule_map = { + '09960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'active' + }, + '19960614-8574-4e03-89cf-7cf267b0bd08': { + 'state': 'active' + } + } + self.assertEqual(2, self._mock_isilon_api.lookup_smb_share.call_count) + http_method, url = self._mock_isilon_api.request.call_args[0] + data = self._mock_isilon_api.request.call_args[1]['data'] + self.assertEqual('PUT', http_method) + self.assertEqual(expected_url, url) + self.assertEqual(expected_data['host_acl'], data['host_acl']) + self.assertEqual(1, len(data['permissions'])) + self.assertEqual(expected_data['permissions'][0], + data['permissions'][0]) + self.assertEqual(expected_rule_map, rule_map)