diff --git a/manila/share/drivers/qnap/api.py b/manila/share/drivers/qnap/api.py index 7d70208568..8a8a355e43 100644 --- a/manila/share/drivers/qnap/api.py +++ b/manila/share/drivers/qnap/api.py @@ -177,7 +177,10 @@ class QnapAPIExecutor(object): for key in params: value = params[key] if value is not None: - sanitized_params[key] = six.text_type(value) + if isinstance(value, list): + sanitized_params[key] = [six.text_type(v) for v in value] + else: + sanitized_params[key] = six.text_type(value) return sanitized_params @_connection_checker @@ -580,6 +583,52 @@ class QnapAPIExecutor(object): if root.find('result').text < '0': raise exception.ShareBackendException(msg=MSG_UNEXPECT_RESP) + @_connection_checker + def edit_host(self, hostname, ipv4_list): + """Execute edit_host API.""" + params = { + 'module': 'hosts', + 'func': 'apply_sethost', + 'name': hostname, + 'ipaddr_v4': ipv4_list, + 'sid': self.sid, + } + sanitized_params = self._sanitize_params(params) + + # urlencode with True parameter to parse ipv4_list + sanitized_params = urllib.parse.urlencode(sanitized_params, True) + url = ('/cgi-bin/accessrights/accessrightsRequest.cgi?%s' % + sanitized_params) + + res_details = self._execute_and_get_response_details(self.ip, url) + root = ET.fromstring(res_details['data']) + if root.find('authPassed').text == '0': + raise exception.ShareBackendException(msg=MSG_SESSION_EXPIRED) + if root.find('result').text < '0': + raise exception.ShareBackendException(msg=MSG_UNEXPECT_RESP) + + @_connection_checker + def delete_host(self, hostname): + """Execute delete_host API.""" + params = { + 'module': 'hosts', + 'func': 'apply_delhost', + 'host_name': hostname, + 'sid': self.sid, + } + sanitized_params = self._sanitize_params(params) + + sanitized_params = urllib.parse.urlencode(sanitized_params) + url = ('/cgi-bin/accessrights/accessrightsRequest.cgi?%s' % + sanitized_params) + + res_details = self._execute_and_get_response_details(self.ip, url) + root = ET.fromstring(res_details['data']) + if root.find('authPassed').text == '0': + raise exception.ShareBackendException(msg=MSG_SESSION_EXPIRED) + if root.find('result').text < '0': + raise exception.ShareBackendException(msg=MSG_UNEXPECT_RESP) + @_connection_checker def set_nfs_access(self, sharename, access, host_name): """Execute set_nfs_access API.""" diff --git a/manila/share/drivers/qnap/qnap.py b/manila/share/drivers/qnap/qnap.py index fedde37c10..ec4ab1b3eb 100644 --- a/manila/share/drivers/qnap/qnap.py +++ b/manila/share/drivers/qnap/qnap.py @@ -16,7 +16,9 @@ Share driver for QNAP Storage. This driver supports QNAP Storage for NFS. """ +import datetime import re +import time from oslo_config import cfg from oslo_log import log as logging @@ -62,9 +64,11 @@ class QnapShareDriver(driver.ShareDriver): Version history: 1.0.0 - Initial driver (Only NFS) 1.0.1 - Add support for QES fw 1.1.4. + 1.0.2 - Fix bug #1736370, QNAP Manila driver: Access rule setting is + override by the another access rule. """ - DRIVER_VERSION = '1.0.1' + DRIVER_VERSION = '1.0.2' def __init__(self, *args, **kwargs): """Initialize QnapShareDriver.""" @@ -193,6 +197,16 @@ class QnapShareDriver(driver.ShareDriver): {'ifx': infix, 'time': timeutils.utcnow().strftime('%Y%m%d%H%M%S%f')}) + def _gen_host_name(self, vol_name_timestamp, access_level): + # host_name will be manila-{vol_name_timestamp}-ro or + # manila-{vol_name_timestamp}-rw + return 'manila-{}-{}'.format(vol_name_timestamp, access_level) + + def _get_timestamp_from_vol_name(self, vol_name): + vol_name_split = vol_name.split('-') + dt = datetime.datetime.strptime(vol_name_split[2], '%Y%m%d%H%M%S%f') + return int(time.mktime(dt.timetuple())) + def _get_location_path(self, share_name, share_proto, ip, vol_id): if share_proto == 'NFS': vol = self.api_executor.get_specific_volinfo(vol_id) @@ -503,30 +517,31 @@ class QnapShareDriver(driver.ShareDriver): self.configuration.qnap_share_ip, create_volID) - def _get_manila_hostIPv4s(self, hostlist): - host_dict_IPs = [] - if hostlist is None: - return host_dict_IPs - for host in hostlist: - # Check host alias name with prefix "manila-hst-" to verify this - # host is created/managed by Manila or not. - if (re.match("^manila-hst-[0-9]+$", host.find('name').text) - is not None): - LOG.debug('host netaddrs text: %s', host.find('netaddrs').text) - if host.find('netaddrs').text is not None: - # Because Manila supports only IPv4 now, check "netaddrs" - # have "ipv4" tag to verify this host is created/managed - # by Manila or not. - if host.find('netaddrs/ipv4').text is not None: - host_dict = { - 'index': host.find('index').text, - 'hostid': host.find('hostid').text, - 'name': host.find('name').text, - 'netaddrs': host.find('netaddrs').find('ipv4').text - } - host_dict_IPs.append(host_dict) - return host_dict_IPs + def _get_vol_host(self, host_list, vol_name_timestamp): + vol_host_list = [] + if host_list is None: + return vol_host_list + for host in host_list: + # Check host alias name with prefix "manila-{vol_name_timestamp}" + # to find the host of this manila share. + LOG.debug('_get_vol_host name:%s', host.find('name').text) + # Because driver supports only IPv4 now, check "netaddrs" + # have "ipv4" tag to get address. + if re.match("^manila-{}".format(vol_name_timestamp), + host.find('name').text): + host_dict = { + 'index': host.find('index').text, + 'hostid': host.find('hostid').text, + 'name': host.find('name').text, + 'ipv4': [], + } + for ipv4 in host.findall('netaddrs/ipv4'): + host_dict['ipv4'].append(ipv4.text) + vol_host_list.append(host_dict) + LOG.debug('_get_vol_host vol_host_list:%s', vol_host_list) + return vol_host_list + @utils.synchronized('qnap-update_access') def update_access(self, context, share, access_rules, add_rules, delete_rules, share_server=None): if not (add_rules or delete_rules): @@ -540,6 +555,15 @@ class QnapShareDriver(driver.ShareDriver): # Clear all current ACLs self.api_executor.set_nfs_access(volName, 2, "all") + vol_name_timestamp = self._get_timestamp_from_vol_name(volName) + host_list = self.api_executor.get_host_list() + LOG.debug('host_list:%s', host_list) + vol_host_list = self._get_vol_host(host_list, vol_name_timestamp) + # If host already exist, delete the host + if len(vol_host_list) > 0: + for vol_host in vol_host_list: + self.api_executor.delete_host(vol_host['name']) + # Add each one through all rules. for access in access_rules: self._allow_access(context, share, access, share_server) @@ -556,45 +580,71 @@ class QnapShareDriver(driver.ShareDriver): access_type = access['access_type'] access_level = access['access_level'] access_to = access['access_to'] + LOG.debug('share_proto: %(share_proto)s ' + 'access_type: %(access_type)s' + 'access_level: %(access_level)s' + 'access_to: %(access_to)s', + {'share_proto': share_proto, + 'access_type': access_type, + 'access_level': access_level, + 'access_to': access_to}) self._check_share_access(share_proto, access_type) - hostlist = self.api_executor.get_host_list() - host_dict_IPs = self._get_manila_hostIPv4s(hostlist) - LOG.debug('host_dict_IPs: %s', host_dict_IPs) - if len(host_dict_IPs) == 0: - host_name = self._gen_random_name("host") + vol_name = self.private_storage.get(share['id'], 'volName') + vol_name_timestamp = self._get_timestamp_from_vol_name(vol_name) + host_name = self._gen_host_name(vol_name_timestamp, access_level) + + host_list = self.api_executor.get_host_list() + LOG.debug('vol_name: %(vol_name)s ' + 'access_level: %(access_level)s ' + 'host_name: %(host_name)s ' + 'host_list: %(host_list)s ', + {'vol_name': vol_name, + 'access_level': access_level, + 'host_name': host_name, + 'host_list': host_list}) + filter_host_list = self._get_vol_host(host_list, vol_name_timestamp) + if len(filter_host_list) == 0: + # if host does not exist, create a host for the share self.api_executor.add_host(host_name, access_to) + elif (len(filter_host_list) == 1 and + filter_host_list[0]['name'] == host_name): + # if the host exist, and this host is for the same access right, + # add ip to the host. + ipv4_list = filter_host_list[0]['ipv4'] + if access_to not in ipv4_list: + ipv4_list.append(access_to) + LOG.debug('vol_host["ipv4"]: %s', filter_host_list[0]['ipv4']) + LOG.debug('ipv4_list: %s', ipv4_list) + self.api_executor.edit_host(host_name, ipv4_list) else: - for host in host_dict_IPs: - LOG.debug('host[netaddrs]: %s', host['netaddrs']) - LOG.debug('access_to: %s', access_to) - if host['netaddrs'] == access_to: - LOG.debug('in match ip') - host_name = host['name'] - break - if host is host_dict_IPs[-1]: - host_name = self._gen_random_name("host") - self.api_executor.add_host(host_name, access_to) - - volName = self.private_storage.get(share['id'], 'volName') - LOG.debug('volName: %(volName)s for share: %(share)s', - {'volName': volName, 'share': share['id']}) - - LOG.debug('access_level: %(access)s for share: %(share)s', - {'access': access_level, 'share': share['id']}) - LOG.debug('host_name: %(host)s for share: %(share)s', - {'host': host_name, 'share': share['id']}) - if access_level == constants.ACCESS_LEVEL_RO: - self.api_executor.set_nfs_access(volName, 1, host_name) - elif access_level == constants.ACCESS_LEVEL_RW: - self.api_executor.set_nfs_access(volName, 0, host_name) + # Until now, share of QNAP NAS can only apply one access level for + # all ips. "rw" for some ips and "ro" for else is not allowed. + support_level = (constants.ACCESS_LEVEL_RW if + access_level == constants.ACCESS_LEVEL_RO + else constants.ACCESS_LEVEL_RO) + reason = _('Share only supports one access ' + 'level: %s') % support_level + LOG.error(reason) + raise exception.InvalidShareAccess(reason=reason) + access = 1 if access_level == constants.ACCESS_LEVEL_RO else 0 + self.api_executor.set_nfs_access(vol_name, access, host_name) def _deny_access(self, context, share, access, share_server=None): """Deny access to the share.""" share_proto = share['share_proto'] access_type = access['access_type'] + access_level = access['access_level'] access_to = access['access_to'] + LOG.debug('share_proto: %(share_proto)s ' + 'access_type: %(access_type)s' + 'access_level: %(access_level)s' + 'access_to: %(access_to)s', + {'share_proto': share_proto, + 'access_type': access_type, + 'access_level': access_level, + 'access_to': access_to}) try: self._check_share_access(share_proto, access_type) @@ -602,23 +652,34 @@ class QnapShareDriver(driver.ShareDriver): LOG.warning('The denied rule is invalid and does not exist.') return - hostlist = self.api_executor.get_host_list() - host_dict_IPs = self._get_manila_hostIPv4s(hostlist) - LOG.debug('host_dict_IPs: %s', host_dict_IPs) - if len(host_dict_IPs) == 0: - return - else: - for host in host_dict_IPs: - if (host['netaddrs'] == access_to): - host_name = host['name'] - break - if (host is host_dict_IPs[-1]): - return - - volName = self.private_storage.get(share['id'], 'volName') - LOG.debug('volName: %s', volName) - - self.api_executor.set_nfs_access(volName, 2, host_name) + vol_name = self.private_storage.get(share['id'], 'volName') + vol_name_timestamp = self._get_timestamp_from_vol_name(vol_name) + host_name = self._gen_host_name(vol_name_timestamp, access_level) + host_list = self.api_executor.get_host_list() + LOG.debug('vol_name: %(vol_name)s ' + 'access_level: %(access_level)s ' + 'host_name: %(host_name)s ' + 'host_list: %(host_list)s ', + {'vol_name': vol_name, + 'access_level': access_level, + 'host_name': host_name, + 'host_list': host_list}) + filter_host_list = self._get_vol_host(host_list, vol_name_timestamp) + # if share already have host, remove ip from host + for vol_host in filter_host_list: + if vol_host['name'] == host_name: + ipv4_list = vol_host['ipv4'] + if access_to in ipv4_list: + ipv4_list.remove(access_to) + LOG.debug('vol_host["ipv4"]: %s', vol_host['ipv4']) + LOG.debug('ipv4_list: %s', ipv4_list) + if len(ipv4_list) == 0: # if list empty, remove the host + self.api_executor.set_nfs_access( + vol_name, 2, host_name) + self.api_executor.delete_host(host_name) + else: + self.api_executor.edit_host(host_name, ipv4_list) + break def _check_share_access(self, share_proto, access_type): if share_proto == 'NFS' and access_type != 'ip': diff --git a/manila/tests/share/drivers/qnap/fakes.py b/manila/tests/share/drivers/qnap/fakes.py index 22b514d1f0..90fa364136 100644 --- a/manila/tests/share/drivers/qnap/fakes.py +++ b/manila/tests/share/drivers/qnap/fakes.py @@ -192,7 +192,7 @@ FAKE_RES_DETAIL_DATA_GET_HOST_LIST = """ - + @@ -287,6 +287,15 @@ FAKE_RES_DETAIL_DATA_GET_HOST_LIST_API = """ """ +FAKE_RES_DETAIL_DATA_GET_NO_HOST_LIST_API = """ + + + + + + + """ + FAKE_RES_DETAIL_DATA_CREATE_SNAPSHOT = """ @@ -546,7 +555,7 @@ class FakeDeleteSnapshotResponseShareNotExist(object): class FakeGetHostListResponse(object): - """Fake pool info response.""" + """Fake host info response.""" status = 'fackStatus' @@ -555,6 +564,16 @@ class FakeGetHostListResponse(object): return FAKE_RES_DETAIL_DATA_GET_HOST_LIST_API +class FakeGetNoHostListResponse(object): + """Fake host info response.""" + + status = 'fackStatus' + + def read(self): + """Mock response.read.""" + return FAKE_RES_DETAIL_DATA_GET_NO_HOST_LIST_API + + class FakeAuthPassFailResponse(object): """Fake pool info response.""" diff --git a/manila/tests/share/drivers/qnap/test_api.py b/manila/tests/share/drivers/qnap/test_api.py index 693c7e038c..19c404a86c 100644 --- a/manila/tests/share/drivers/qnap/test_api.py +++ b/manila/tests/share/drivers/qnap/test_api.py @@ -91,14 +91,17 @@ class QnapAPITestCase(QnapShareDriverBaseTestCase): host='QnapShareDriver', size=10) - def _sanitize_params(self, params): + def _sanitize_params(self, params, doseq=False): sanitized_params = {} for key in params: value = params[key] if value is not None: - sanitized_params[key] = six.text_type(value) + if isinstance(value, list): + sanitized_params[key] = [six.text_type(v) for v in value] + else: + sanitized_params[key] = six.text_type(value) - sanitized_params = urllib.parse.urlencode(sanitized_params) + sanitized_params = urllib.parse.urlencode(sanitized_params, doseq) return sanitized_params @ddt.data('fake_share_name', 'fakeLabel') @@ -493,14 +496,16 @@ class QnapAPITestCase(QnapShareDriverBaseTestCase): expected_call_list, mock_http_connection.return_value.request.call_args_list) - def test_get_host_list(self): + @ddt.data(fakes.FakeGetHostListResponse(), + fakes.FakeGetNoHostListResponse()) + def test_get_host_list(self, fakeGetHostListResponse): """Test get host list api.""" mock_http_connection = six.moves.http_client.HTTPConnection mock_http_connection.return_value.getresponse.side_effect = [ fakes.FakeLoginResponse(), fakes.FakeGetBasicInfoResponseEs_1_1_3(), fakes.FakeLoginResponse(), - fakes.FakeGetHostListResponse()] + fakeGetHostListResponse] self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin', 'qnapadmin', 'Storage Pool 1') @@ -560,8 +565,8 @@ class QnapAPITestCase(QnapShareDriverBaseTestCase): expected_call_list, mock_http_connection.return_value.request.call_args_list) - def test_set_nfs_access(self): - """Test get host list api.""" + def test_edit_host(self): + """Test edit host api.""" mock_http_connection = six.moves.http_client.HTTPConnection mock_http_connection.return_value.getresponse.side_effect = [ fakes.FakeLoginResponse(), @@ -569,6 +574,75 @@ class QnapAPITestCase(QnapShareDriverBaseTestCase): fakes.FakeLoginResponse(), fakes.FakeGetHostListResponse()] + self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin', + 'qnapadmin', 'Storage Pool 1') + self.driver.api_executor.edit_host( + 'fakeHostName', ['fakeIpV4']) + + fake_params = { + 'module': 'hosts', + 'func': 'apply_sethost', + 'name': 'fakeHostName', + 'ipaddr_v4': ['fakeIpV4'], + 'sid': 'fakeSid', + } + sanitized_params = self._sanitize_params(fake_params, doseq=True) + fake_url = ( + ('/cgi-bin/accessrights/accessrightsRequest.cgi?%s') % + sanitized_params) + + expected_call_list = [ + mock.call('GET', self.login_url), + mock.call('GET', self.get_basic_info_url), + mock.call('GET', self.login_url), + mock.call('GET', fake_url)] + self.assertEqual( + expected_call_list, + mock_http_connection.return_value.request.call_args_list) + + def test_delete_host(self): + """Test delete host api.""" + mock_http_connection = six.moves.http_client.HTTPConnection + mock_http_connection.return_value.getresponse.side_effect = [ + fakes.FakeLoginResponse(), + fakes.FakeGetBasicInfoResponseEs_1_1_3(), + fakes.FakeLoginResponse(), + fakes.FakeGetHostListResponse()] + + self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin', + 'qnapadmin', 'Storage Pool 1') + self.driver.api_executor.delete_host('fakeHostName') + + fake_params = { + 'module': 'hosts', + 'func': 'apply_delhost', + 'host_name': 'fakeHostName', + 'sid': 'fakeSid', + } + sanitized_params = self._sanitize_params(fake_params) + fake_url = ( + ('/cgi-bin/accessrights/accessrightsRequest.cgi?%s') % + sanitized_params) + + expected_call_list = [ + mock.call('GET', self.login_url), + mock.call('GET', self.get_basic_info_url), + mock.call('GET', self.login_url), + mock.call('GET', fake_url)] + self.assertEqual( + expected_call_list, + mock_http_connection.return_value.request.call_args_list) + + @ddt.data(fakes.FakeGetHostListResponse()) + def test_set_nfs_access(self, fakeGetHostListResponse): + """Test get host list api.""" + mock_http_connection = six.moves.http_client.HTTPConnection + mock_http_connection.return_value.getresponse.side_effect = [ + fakes.FakeLoginResponse(), + fakes.FakeGetBasicInfoResponseEs_1_1_3(), + fakes.FakeLoginResponse(), + fakeGetHostListResponse] + self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin', 'qnapadmin', 'Storage Pool 1') self.driver.api_executor.set_nfs_access( @@ -749,6 +823,24 @@ class QnapAPITestCase(QnapShareDriverBaseTestCase): 'ipv4': 'fakeIpV4'}, fakes.FakeAuthPassFailResponse(), fakes.FakeGetBasicInfoResponseEs_1_1_3()], + ['self.driver.api_executor.edit_host', + {'hostname': 'fakeHostName', + 'ipv4_list': 'fakeIpV4List'}, + fakes.FakeResultNegativeResponse(), + fakes.FakeGetBasicInfoResponseEs_1_1_3()], + ['self.driver.api_executor.edit_host', + {'hostname': 'fakeHostName', + 'ipv4_list': 'fakeIpV4List'}, + fakes.FakeAuthPassFailResponse(), + fakes.FakeGetBasicInfoResponseEs_1_1_3()], + ['self.driver.api_executor.delete_host', + {'hostname': 'fakeHostName'}, + fakes.FakeResultNegativeResponse(), + fakes.FakeGetBasicInfoResponseEs_1_1_3()], + ['self.driver.api_executor.delete_host', + {'hostname': 'fakeHostName'}, + fakes.FakeAuthPassFailResponse(), + fakes.FakeGetBasicInfoResponseEs_1_1_3()], ['self.driver.api_executor.get_host_list', {}, fakes.FakeResultNegativeResponse(), diff --git a/manila/tests/share/drivers/qnap/test_qnap.py b/manila/tests/share/drivers/qnap/test_qnap.py index 41b81f3da5..e5a861fc4f 100644 --- a/manila/tests/share/drivers/qnap/test_qnap.py +++ b/manila/tests/share/drivers/qnap/test_qnap.py @@ -721,15 +721,22 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): snapshot=fake_snapshot, share_server=None) + @mock.patch.object(qnap.QnapShareDriver, '_get_timestamp_from_vol_name') @mock.patch.object(qnap.QnapShareDriver, '_allow_access') + @ddt.data('fakeHostName', 'fakeHostNameNotMatch') def test_update_access_allow_access( - self, mock_allow_access): + self, fakeHostName, mock_allow_access, + mock_get_timestamp_from_vol_name): """Test update access with allow access rules.""" mock_private_storage = mock.Mock() mock_private_storage.get.return_value = 'fakeVolName' mock_api_executor = qnap.QnapShareDriver._create_api_executor + mock_api_executor.return_value.get_host_list.return_value = ( + self.get_host_list_return_value()) mock_api_executor.return_value.set_nfs_access.return_value = None + mock_api_executor.return_value.delete_host.return_value = None mock_allow_access.return_value = None + mock_get_timestamp_from_vol_name.return_value = fakeHostName self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin', 'qnapadmin', 'Storage Pool 1', @@ -1044,7 +1051,7 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): mock_api_return.get_specific_poolinfo.assert_called_once_with( self.driver.configuration.qnap_poolname) - def test_get_manila_host_ipv4s(self): + def test_get_vol_host(self): """Test get manila host IPV4s.""" mock_private_storage = mock.Mock() @@ -1059,28 +1066,31 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): 'index': host.find('index').text, 'hostid': host.find('hostid').text, 'name': host.find('name').text, - 'netaddrs': host.find('netaddrs').find('ipv4').text + 'ipv4': [host.find('netaddrs').find('ipv4').text] } expect_host_dict_ips.append(host_dict) self.assertEqual( - expect_host_dict_ips, self.driver._get_manila_hostIPv4s( - host_list)) + expect_host_dict_ips, self.driver._get_vol_host( + host_list, 'fakeHostName')) - @mock.patch.object(qnap.QnapShareDriver, '_gen_random_name') + @mock.patch.object(qnap.QnapShareDriver, '_gen_host_name') + @mock.patch.object(qnap.QnapShareDriver, '_get_timestamp_from_vol_name') @mock.patch.object(qnap.QnapShareDriver, '_check_share_access') def test_allow_access_ro( self, mock_check_share_access, - mock_gen_random_name): + mock_get_timestamp_from_vol_name, + mock_gen_host_name): """Test allow_access with access type ro.""" fake_access = fakes.AccessClass('fakeAccessType', 'ro', 'fakeIp') mock_private_storage = mock.Mock() + mock_private_storage.get.return_value = 'fakeVolName' mock_api_executor = qnap.QnapShareDriver._create_api_executor - mock_api_executor.return_value.get_host_list.return_value = ( - self.get_host_list_return_value()) - mock_gen_random_name.return_value = 'fakeHostName' + mock_api_executor.return_value.get_host_list.return_value = [] + mock_get_timestamp_from_vol_name.return_value = 'fakeHostName' + mock_gen_host_name.return_value = 'manila-fakeHostName-ro' mock_api_executor.return_value.add_host.return_value = None mock_api_executor.return_value.set_nfs_access.return_value = None @@ -1092,14 +1102,17 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): mock_check_share_access.assert_called_once_with( 'NFS', 'fakeAccessType') - mock_gen_random_name.assert_called_once_with('host') mock_api_executor.return_value.add_host.assert_called_once_with( - 'fakeHostName', 'fakeIp') + 'manila-fakeHostName-ro', 'fakeIp') + @mock.patch.object(qnap.QnapShareDriver, '_gen_host_name') + @mock.patch.object(qnap.QnapShareDriver, '_get_timestamp_from_vol_name') @mock.patch.object(qnap.QnapShareDriver, '_check_share_access') def test_allow_access_ro_with_hostlist( self, - mock_check_share_access): + mock_check_share_access, + mock_get_timestamp_from_vol_name, + mock_gen_host_name): """Test allow_access_ro_with_hostlist.""" host_dict_ips = [] for host in self.get_host_list_return_value(): @@ -1108,18 +1121,21 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): 'index': host.find('index').text, 'hostid': host.find('hostid').text, 'name': host.find('name').text, - 'netaddrs': host.find('netaddrs').find('ipv4').text} + 'ipv4': [host.find('netaddrs').find('ipv4').text]} host_dict_ips.append(host_dict) for host in host_dict_ips: - fake_access_to = host['netaddrs'] + fake_access_to = host['ipv4'] fake_access = fakes.AccessClass( 'fakeAccessType', 'ro', fake_access_to) mock_private_storage = mock.Mock() + mock_private_storage.get.return_value = 'fakeVolName' mock_api_executor = qnap.QnapShareDriver._create_api_executor mock_api_executor.return_value.get_host_list.return_value = ( self.get_host_list_return_value()) + mock_get_timestamp_from_vol_name.return_value = 'fakeHostName' + mock_gen_host_name.return_value = 'manila-fakeHostName' mock_api_executor.return_value.set_nfs_access.return_value = None self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin', @@ -1131,20 +1147,64 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): mock_check_share_access.assert_called_once_with( 'NFS', 'fakeAccessType') - @mock.patch.object(qnap.QnapShareDriver, '_gen_random_name') + @mock.patch.object(qnap.QnapShareDriver, '_gen_host_name') + @mock.patch.object(qnap.QnapShareDriver, '_get_timestamp_from_vol_name') + @mock.patch.object(qnap.QnapShareDriver, '_check_share_access') + def test_allow_access_rw_with_hostlist_invalid_access( + self, + mock_check_share_access, + mock_get_timestamp_from_vol_name, + mock_gen_host_name): + """Test allow_access_rw_invalid_access.""" + host_dict_ips = [] + for host in self.get_host_list_return_value(): + if host.find('netaddrs/ipv4').text is not None: + host_dict = { + 'index': host.find('index').text, + 'hostid': host.find('hostid').text, + 'name': host.find('name').text, + 'ipv4': [host.find('netaddrs').find('ipv4').text]} + host_dict_ips.append(host_dict) + + for host in host_dict_ips: + fake_access_to = host['ipv4'] + fake_access = fakes.AccessClass( + 'fakeAccessType', 'rw', fake_access_to) + + mock_private_storage = mock.Mock() + mock_private_storage.get.return_value = 'fakeVolName' + mock_api_executor = qnap.QnapShareDriver._create_api_executor + mock_api_executor.return_value.get_host_list.return_value = ( + self.get_host_list_return_value()) + mock_get_timestamp_from_vol_name.return_value = 'fakeHostName' + mock_gen_host_name.return_value = 'manila-fakeHostName-rw' + + self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin', + 'qnapadmin', 'Storage Pool 1', + private_storage=mock_private_storage) + + self.assertRaises( + exception.InvalidShareAccess, + self.driver._allow_access, + context='context', + share=self.share, + access=fake_access, + share_server=None) + + @mock.patch.object(qnap.QnapShareDriver, '_get_timestamp_from_vol_name') @mock.patch.object(qnap.QnapShareDriver, '_check_share_access') def test_allow_access_rw( self, mock_check_share_access, - mock_gen_random_name): + mock_get_timestamp_from_vol_name): """Test allow_access with access type rw.""" fake_access = fakes.AccessClass('fakeAccessType', 'rw', 'fakeIp') mock_private_storage = mock.Mock() + mock_private_storage.get.return_value = 'fakeVolName' mock_api_executor = qnap.QnapShareDriver._create_api_executor - mock_api_executor.return_value.get_host_list.return_value = ( - self.get_host_list_return_value()) - mock_gen_random_name.return_value = 'fakeHostName' + mock_api_executor.return_value.get_host_list.return_value = [] + mock_get_timestamp_from_vol_name.return_value = 'fakeHostName' mock_api_executor.return_value.add_host.return_value = None mock_api_executor.return_value.set_nfs_access.return_value = None @@ -1156,44 +1216,50 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): mock_check_share_access.assert_called_once_with( 'NFS', 'fakeAccessType') - mock_gen_random_name.assert_called_once_with('host') mock_api_executor.return_value.add_host.assert_called_once_with( - 'fakeHostName', 'fakeIp') + 'manila-fakeHostName-rw', 'fakeIp') - @mock.patch.object(qnap.QnapShareDriver, '_get_manila_hostIPv4s') - @mock.patch.object(qnap.QnapShareDriver, '_gen_random_name') + @mock.patch.object(qnap.QnapShareDriver, '_gen_host_name') @mock.patch.object(qnap.QnapShareDriver, '_check_share_access') - def test_allow_access_without_hostlist( + def test_allow_access_ro_without_hostlist( self, mock_check_share_access, - mock_gen_random_name, - mock_get_manila_hostipv4s): + mock_gen_host_name): """Test allow access without host list.""" fake_access = fakes.AccessClass('fakeAccessType', 'ro', 'fakeIp') mock_private_storage = mock.Mock() + mock_api_executor = qnap.QnapShareDriver._create_api_executor mock_api_executor.return_value.get_host_list.return_value = None - mock_gen_random_name.return_value = 'fakeHostName' + mock_gen_host_name.return_value = 'fakeHostName' mock_api_executor.return_value.add_host.return_value = None mock_api_executor.return_value.set_nfs_access.return_value = None self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin', 'qnapadmin', 'Storage Pool 1', private_storage=mock_private_storage) + share_name = self.driver._gen_random_name('share') + mock_private_storage.get.return_value = share_name self.driver._allow_access( 'context', self.share, fake_access, share_server=None) mock_check_share_access.assert_called_once_with( 'NFS', 'fakeAccessType') - mock_gen_random_name.assert_called_once_with('host') mock_api_executor.return_value.add_host.assert_called_once_with( 'fakeHostName', 'fakeIp') + @mock.patch.object(qnap.QnapShareDriver, '_get_vol_host') + @mock.patch.object(qnap.QnapShareDriver, '_gen_host_name') + @mock.patch.object(qnap.QnapShareDriver, '_get_timestamp_from_vol_name') @mock.patch.object(qnap.QnapShareDriver, '_check_share_access') def test_deny_access_with_hostlist( self, - mock_check_share_access): + mock_check_share_access, + mock_get_timestamp_from_vol_name, + mock_gen_host_name, + mock_get_vol_host): + """Test deny access.""" host_dict_ips = [] for host in self.get_host_list_return_value(): @@ -1202,11 +1268,11 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): 'index': host.find('index').text, 'hostid': host.find('hostid').text, 'name': host.find('name').text, - 'netaddrs': host.find('netaddrs').find('ipv4').text} + 'ipv4': [host.find('netaddrs').find('ipv4').text]} host_dict_ips.append(host_dict) for host in host_dict_ips: - fake_access_to = host['netaddrs'] + fake_access_to = host['ipv4'][0] fake_access = fakes.AccessClass('fakeAccessType', 'ro', fake_access_to) mock_private_storage = mock.Mock() @@ -1216,6 +1282,9 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): qnap.QnapShareDriver._create_api_executor.return_value) mock_api_return.get_host_list.return_value = ( self.get_host_list_return_value()) + mock_get_timestamp_from_vol_name.return_value = 'fakeTimeStamp' + mock_gen_host_name.return_value = 'manila-fakeHostName' + mock_get_vol_host.return_value = host_dict_ips mock_api_return.add_host.return_value = None mock_api_return.set_nfs_access.return_value = None @@ -1227,13 +1296,13 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): mock_check_share_access.assert_called_once_with( 'NFS', 'fakeAccessType') - mock_api_return.set_nfs_access.assert_called_once_with( - 'vol_name', 2, 'manila-hst-123') + @mock.patch.object(qnap.QnapShareDriver, '_get_timestamp_from_vol_name') @mock.patch.object(qnap.QnapShareDriver, '_check_share_access') def test_deny_access_with_hostlist_not_equel_access_to( self, - mock_check_share_access): + mock_check_share_access, + mock_get_timestamp_from_vol_name): """Test deny access.""" fake_access = fakes.AccessClass('fakeAccessType', 'ro', 'fakeIp') @@ -1254,18 +1323,20 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase): mock_check_share_access.assert_called_once_with( 'NFS', 'fakeAccessType') - @mock.patch.object(qnap.QnapShareDriver, '_get_manila_hostIPv4s') + @mock.patch.object(qnap.QnapShareDriver, '_get_timestamp_from_vol_name') @mock.patch.object(qnap.QnapShareDriver, '_check_share_access') def test_deny_access_without_hostlist( self, mock_check_share_access, - mock_get_manila_hostipv4s): + mock_get_timestamp_from_vol_name): """Test deny access without hostlist.""" fake_access = fakes.AccessClass('fakeAccessType', 'ro', 'fakeIp') mock_private_storage = mock.Mock() + mock_private_storage.get.return_value = 'fakeVolName' mock_api_executor = qnap.QnapShareDriver._create_api_executor mock_api_executor.return_value.get_host_list.return_value = None + mock_get_timestamp_from_vol_name.return_value = 'fakeHostName' mock_api_executor.return_value.add_host.return_value = None mock_api_executor.return_value.set_nfs_access.return_value = None diff --git a/releasenotes/notes/bug-1736370-qnap-fix-access-rule-override-1b79b70ae48ad9e6.yaml b/releasenotes/notes/bug-1736370-qnap-fix-access-rule-override-1b79b70ae48ad9e6.yaml new file mode 100644 index 0000000000..6b0a55be74 --- /dev/null +++ b/releasenotes/notes/bug-1736370-qnap-fix-access-rule-override-1b79b70ae48ad9e6.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed the QNAP driver that the access rule setting is overridden by the later + access rule setting.