From: Rushil Chugh Date: Tue, 18 Nov 2014 19:37:17 +0000 (-0500) Subject: Ensure that lun_id is an int for NetApp Drivers X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=2d0204ff3ca3a7d42fac5722d577e18edd34ea83;p=openstack-build%2Fcinder-build.git Ensure that lun_id is an int for NetApp Drivers Various NetApp drivers were treating the lun_id as a string rather than an int. This was causing attempts to mount NetApp volumes to Hyper-V nodes to fail as they were checking an integer type against a unicode type which would fail. This change casts result_lun to an integer after the value has gone through the ssh injection attack check. This way Hyper-V is able to verify if the found LUN is the target LUN, enabling mounting of NetApp volumes to Hyper-V. This change also refactors initialize_connection into some helper methods so as to enable simpler unit testing of the patchset. Closes-bug: 1372808 Change-Id: I308b3b2dff315ec33451fb45a30ecd53d5d4c353 --- diff --git a/cinder/tests/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/volume/drivers/netapp/dataontap/fakes.py index 7ff493ee3..b55d1a90f 100644 --- a/cinder/tests/volume/drivers/netapp/dataontap/fakes.py +++ b/cinder/tests/volume/drivers/netapp/dataontap/fakes.py @@ -58,7 +58,7 @@ FC_FABRIC_MAP = {'fabricB': 'initiator_port_wwn_list': ['21000024ff406cc3']}} FC_TARGET_INFO = {'driver_volume_type': 'fibre_channel', - 'data': {'target_lun': '1', + 'data': {'target_lun': 1, 'initiator_target_map': FC_I_T_MAP, 'access_mode': 'rw', 'target_wwn': FC_TARGET_WWPNS, diff --git a/cinder/tests/volume/drivers/netapp/fakes.py b/cinder/tests/volume/drivers/netapp/fakes.py index d39a52d83..b797613c3 100644 --- a/cinder/tests/volume/drivers/netapp/fakes.py +++ b/cinder/tests/volume/drivers/netapp/fakes.py @@ -1,4 +1,5 @@ -# Copyright (c) - 2014, Clinton Knight. All rights reserved. +# Copyright (c) - 2014, Clinton Knight All rights reserved. +# Copyright (c) - 2014, Rushil Chugh All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -42,3 +43,27 @@ def create_configuration_eseries(): config = create_configuration() config.append_config_values(na_opts.netapp_eseries_opts) return config + +ISCSI_FAKE_LUN_ID = 1 + +ISCSI_FAKE_IQN = 'iqn.1993-08.org.debian:01:10' + +ISCSI_FAKE_ADDRESS = '10.63.165.216' + +ISCSI_FAKE_PORT = '2232' + +ISCSI_FAKE_VOLUME = {'id': 'fake_id'} + +ISCSI_FAKE_TARGET = {} +ISCSI_FAKE_TARGET['address'] = ISCSI_FAKE_ADDRESS +ISCSI_FAKE_TARGET['port'] = ISCSI_FAKE_PORT + +ISCSI_FAKE_VOLUME = {'id': 'fake_id', 'provider_auth': 'None stack password'} + +FC_ISCSI_TARGET_INFO_DICT = {'target_discovered': False, + 'target_portal': '10.63.165.216:2232', + 'target_iqn': ISCSI_FAKE_IQN, + 'target_lun': ISCSI_FAKE_LUN_ID, + 'volume_id': ISCSI_FAKE_VOLUME['id'], + 'auth_method': 'None', 'auth_username': 'stack', + 'auth_password': 'password'} diff --git a/cinder/tests/volume/drivers/netapp/test_utils.py b/cinder/tests/volume/drivers/netapp/test_utils.py index 271bca97f..a368b7da6 100644 --- a/cinder/tests/volume/drivers/netapp/test_utils.py +++ b/cinder/tests/volume/drivers/netapp/test_utils.py @@ -24,6 +24,7 @@ from oslo.concurrency import processutils as putils from cinder import exception from cinder import test +import cinder.tests.volume.drivers.netapp.fakes as fake from cinder import version import cinder.volume.drivers.netapp.utils as na_utils @@ -350,3 +351,34 @@ class OpenStackInfoTestCase(test.TestCase): info._update_openstack_info() self.assertTrue(mock_updt_from_dpkg.called) + + def test_iscsi_connection_properties(self): + + actual_properties = na_utils.get_iscsi_connection_properties( + fake.ISCSI_FAKE_LUN_ID, fake.ISCSI_FAKE_VOLUME, + fake.ISCSI_FAKE_IQN, fake.ISCSI_FAKE_ADDRESS, + fake.ISCSI_FAKE_PORT) + + actual_properties_mapped = actual_properties['data'] + + self.assertDictEqual(actual_properties_mapped, + fake.FC_ISCSI_TARGET_INFO_DICT) + + def test_iscsi_connection_lun_id_type_str(self): + FAKE_LUN_ID = '1' + + actual_properties = na_utils.get_iscsi_connection_properties( + FAKE_LUN_ID, fake.ISCSI_FAKE_VOLUME, fake.ISCSI_FAKE_IQN, + fake.ISCSI_FAKE_ADDRESS, fake.ISCSI_FAKE_PORT) + + actual_properties_mapped = actual_properties['data'] + + self.assertIs(type(actual_properties_mapped['target_lun']), int) + + def test_iscsi_connection_lun_id_type_dict(self): + FAKE_LUN_ID = {'id': 'fake_id'} + + self.assertRaises(TypeError, na_utils.get_iscsi_connection_properties, + FAKE_LUN_ID, fake.ISCSI_FAKE_VOLUME, + fake.ISCSI_FAKE_IQN, fake.ISCSI_FAKE_ADDRESS, + fake.ISCSI_FAKE_PORT) diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py index ecbed6eb8..ba8a52409 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_base.py +++ b/cinder/volume/drivers/netapp/dataontap/block_base.py @@ -519,7 +519,6 @@ class NetAppBlockStorageLibrary(object): msg = _("Mapped LUN %(name)s to the initiator %(initiator_name)s") msg_fmt = {'name': name, 'initiator_name': initiator_name} LOG.debug(msg % msg_fmt) - iqn = self.zapi_client.get_iscsi_service_details() target_details_list = self.zapi_client.get_iscsi_target_details() msg = _("Successfully fetched target details for LUN %(name)s and " "initiator %(initiator_name)s") @@ -537,32 +536,22 @@ class NetAppBlockStorageLibrary(object): if not target_details: target_details = target_details_list[0] + (address, port) = (target_details['address'], target_details['port']) + if not target_details['address'] and target_details['port']: msg = _('Failed to get target portal for the LUN %s') raise exception.VolumeBackendAPIException(data=msg % name) + + iqn = self.zapi_client.get_iscsi_service_details() if not iqn: msg = _('Failed to get target IQN for the LUN %s') raise exception.VolumeBackendAPIException(data=msg % name) - properties = {} - properties['target_discovered'] = False - (address, port) = (target_details['address'], target_details['port']) - properties['target_portal'] = '%s:%s' % (address, port) - properties['target_iqn'] = iqn - properties['target_lun'] = lun_id - properties['volume_id'] = volume['id'] - - auth = volume['provider_auth'] - if auth: - (auth_method, auth_username, auth_secret) = auth.split() - properties['auth_method'] = auth_method - properties['auth_username'] = auth_username - properties['auth_password'] = auth_secret - - return { - 'driver_volume_type': 'iscsi', - 'data': properties, - } + properties = na_utils.get_iscsi_connection_properties(lun_id, volume, + iqn, address, + port) + + return properties def terminate_connection_iscsi(self, volume, connector, **kwargs): """Driver entry point to unattach a volume from an instance. @@ -648,7 +637,7 @@ class NetAppBlockStorageLibrary(object): target_info = {'driver_volume_type': 'fibre_channel', 'data': {'target_discovered': True, - 'target_lun': lun_id, + 'target_lun': int(lun_id), 'target_wwn': target_wwpns, 'access_mode': 'rw', 'initiator_target_map': initiator_target_map}} diff --git a/cinder/volume/drivers/netapp/eseries/iscsi.py b/cinder/volume/drivers/netapp/eseries/iscsi.py index b0b7798e8..3fe1574d5 100644 --- a/cinder/volume/drivers/netapp/eseries/iscsi.py +++ b/cinder/volume/drivers/netapp/eseries/iscsi.py @@ -556,7 +556,7 @@ class NetAppEseriesISCSIDriver(driver.ISCSIDriver): initiator_name = connector['initiator'] vol = self._get_latest_volume(volume['name_id']) iscsi_details = self._get_iscsi_service_details() - iscsi_det = self._get_iscsi_portal_for_vol(vol, iscsi_details) + iscsi_portal = self._get_iscsi_portal_for_vol(vol, iscsi_details) mapping = self._map_volume_to_host(vol, initiator_name) lun_id = mapping['lun'] self._cache_vol_mapping(mapping) @@ -566,23 +566,13 @@ class NetAppEseriesISCSIDriver(driver.ISCSIDriver): msg = _("Successfully fetched target details for volume %(id)s and " "initiator %(initiator_name)s.") LOG.debug(msg % msg_fmt) - properties = {} - properties['target_discovered'] = False - properties['target_portal'] = '%s:%s' % (iscsi_det['ip'], - iscsi_det['tcp_port']) - properties['target_iqn'] = iscsi_det['iqn'] - properties['target_lun'] = lun_id - properties['volume_id'] = volume['id'] - auth = volume['provider_auth'] - if auth: - (auth_method, auth_username, auth_secret) = auth.split() - properties['auth_method'] = auth_method - properties['auth_username'] = auth_username - properties['auth_password'] = auth_secret - return { - 'driver_volume_type': 'iscsi', - 'data': properties, - } + iqn = iscsi_portal['iqn'] + address = iscsi_portal['ip'] + port = iscsi_portal['tcp_port'] + properties = na_utils.get_iscsi_connection_properties(lun_id, volume, + iqn, address, + port) + return properties def _get_iscsi_service_details(self): """Gets iscsi iqn, ip and port information.""" diff --git a/cinder/volume/drivers/netapp/utils.py b/cinder/volume/drivers/netapp/utils.py index 2fc522725..2269a35f3 100644 --- a/cinder/volume/drivers/netapp/utils.py +++ b/cinder/volume/drivers/netapp/utils.py @@ -139,6 +139,27 @@ def log_extra_spec_warnings(extra_specs): LOG.warning(msg % args) +def get_iscsi_connection_properties(lun_id, volume, iqn, + address, port): + + properties = {} + properties['target_discovered'] = False + properties['target_portal'] = '%s:%s' % (address, port) + properties['target_iqn'] = iqn + properties['target_lun'] = int(lun_id) + properties['volume_id'] = volume['id'] + auth = volume['provider_auth'] + if auth: + (auth_method, auth_username, auth_secret) = auth.split() + properties['auth_method'] = auth_method + properties['auth_username'] = auth_username + properties['auth_password'] = auth_secret + return { + 'driver_volume_type': 'iscsi', + 'data': properties, + } + + class hashabledict(dict): """A hashable dictionary that is comparable (i.e. in unit tests, etc.)""" def __hash__(self):