From 3ebefda51574b44349279d9421b22a593b14dd10 Mon Sep 17 00:00:00 2001 From: Bhaa Shakur Date: Sun, 14 Jul 2019 11:08:28 +0300 Subject: [PATCH] Zadara VPSA: Move to API access key authentication Add 'access_key' API authentication login method given as input and move access_key to HTTP request header. Fix unittest accordingly. Change-Id: I11df21a7a213a0700d277d57ceee40f636955c50 --- .../tests/unit/volume/drivers/test_zadara.py | 208 ++++++++++-------- cinder/volume/drivers/zadara.py | 180 ++++++++------- ...change-to-access-key-b16bdaa9d8460b57.yaml | 11 + 3 files changed, 229 insertions(+), 170 deletions(-) create mode 100644 releasenotes/notes/Zadara-change-to-access-key-b16bdaa9d8460b57.yaml diff --git a/cinder/tests/unit/volume/drivers/test_zadara.py b/cinder/tests/unit/volume/drivers/test_zadara.py index 622b03cbe8e..bc4536ac922 100644 --- a/cinder/tests/unit/volume/drivers/test_zadara.py +++ b/cinder/tests/unit/volume/drivers/test_zadara.py @@ -26,6 +26,16 @@ from cinder.volume import configuration as conf from cinder.volume.drivers import zadara +def check_access_key(func): + """A decorator for all operations that needed an API before executing""" + def wrap(self, *args, **kwargs): + if not self._is_correct_access_key(): + return RUNTIME_VARS['bad_login'] + return func(self, *args, **kwargs) + + return wrap + + DEFAULT_RUNTIME_VARS = { 'status': 200, 'user': 'test', @@ -83,12 +93,20 @@ RUNTIME_VARS = None class FakeResponse(object): - def __init__(self, method, url, body): + def __init__(self, method, url, params, body, headers, **kwargs): + # kwargs include: verify, timeout self.method = method self.url = url self.body = body + self.params = params + self.headers = headers self.status = RUNTIME_VARS['status'] + @property + def access_key(self): + """Returns Response Access Key""" + return self.headers["X-Access-Key"] + def read(self): ops = {'POST': [('/api/users/login.xml', self._login), ('/api/volumes.xml', self._create_volume), @@ -113,13 +131,13 @@ class FakeResponse(object): } ops_list = ops[self.method] - modified_url = self.url.split('?')[0] for (templ_url, func) in ops_list: - if self._compare_url(modified_url, templ_url): + if self._compare_url(self.url, templ_url): result = func() return result - def _compare_url(self, url, template_url): + @staticmethod + def _compare_url(url, template_url): items = url.split('/') titems = template_url.split('/') for (i, titem) in enumerate(titems): @@ -127,36 +145,26 @@ class FakeResponse(object): return False return True - def _get_parameters(self, data): - items = data.split('&') - params = {} - for item in items: - if item: - (k, v) = item.split('=') - params[k] = v - return params - - def _get_counter(self): + @staticmethod + def _get_counter(): cnt = RUNTIME_VARS['counter'] RUNTIME_VARS['counter'] += 1 return cnt def _login(self): - params = self._get_parameters(self.body) + params = self.body if (params['user'] == RUNTIME_VARS['user'] and params['password'] == RUNTIME_VARS['password']): return RUNTIME_VARS['login'] % RUNTIME_VARS['access_key'] else: return RUNTIME_VARS['bad_login'] - def _incorrect_access_key(self, params): - return (params['access_key'] != RUNTIME_VARS['access_key']) + def _is_correct_access_key(self): + return self.access_key == RUNTIME_VARS['access_key'] + @check_access_key def _create_volume(self): - params = self._get_parameters(self.body) - if self._incorrect_access_key(params): - return RUNTIME_VARS['bad_login'] - + params = self.body params['display-name'] = params['name'] params['cg-name'] = params['name'] params['snapshots'] = [] @@ -165,22 +173,21 @@ class FakeResponse(object): RUNTIME_VARS['volumes'].append((vpsa_vol, params)) return RUNTIME_VARS['good'] + @check_access_key def _create_server(self): - params = self._get_parameters(self.body) - if self._incorrect_access_key(params): - return RUNTIME_VARS['bad_login'] + params = self.body params['display-name'] = params['display_name'] vpsa_srv = 'srv-%07d' % self._get_counter() RUNTIME_VARS['servers'].append((vpsa_srv, params)) return RUNTIME_VARS['server_created'] % vpsa_srv + @check_access_key def _attach(self): - params = self._get_parameters(self.body) - if self._incorrect_access_key(params): - return RUNTIME_VARS['bad_login'] - srv = self.url.split('/')[3] + + params = self.body + vol = params['volume_name[]'] for (vol_name, params) in RUNTIME_VARS['volumes']: @@ -195,11 +202,9 @@ class FakeResponse(object): return RUNTIME_VARS['bad_volume'] + @check_access_key def _detach(self): - params = self._get_parameters(self.body) - if self._incorrect_access_key(params): - return RUNTIME_VARS['bad_login'] - + params = self.body vol = self.url.split('/')[3] srv = params['server_name[]'] @@ -214,11 +219,9 @@ class FakeResponse(object): return RUNTIME_VARS['bad_volume'] + @check_access_key def _expand(self): - params = self._get_parameters(self.body) - if self._incorrect_access_key(params): - return RUNTIME_VARS['bad_login'] - + params = self.body vol = self.url.split('/')[3] capacity = params['capacity'] @@ -229,11 +232,9 @@ class FakeResponse(object): return RUNTIME_VARS['bad_volume'] + @check_access_key def _create_snapshot(self): - params = self._get_parameters(self.body) - if self._incorrect_access_key(params): - return RUNTIME_VARS['bad_login'] - + params = self.body cg_name = self.url.split('/')[3] snap_name = params['display_name'] @@ -249,6 +250,7 @@ class FakeResponse(object): return RUNTIME_VARS['bad_volume'] + @check_access_key def _delete_snapshot(self): snap = self.url.split('/')[3].split('.')[0] @@ -259,11 +261,9 @@ class FakeResponse(object): return RUNTIME_VARS['bad_volume'] + @check_access_key def _create_clone(self): - params = self._get_parameters(self.body) - if self._incorrect_access_key(params): - return RUNTIME_VARS['bad_login'] - + params = self.body params['display-name'] = params['name'] params['cg-name'] = params['name'] params['capacity'] = 1 @@ -439,16 +439,24 @@ class FakeResponse(object): class FakeRequests(object): """A fake requests for zadara volume driver tests.""" - def __init__(self, method, api_url, data, verify): + def __init__(self, method, api_url, params=None, data=None, + headers=None, **kwargs): url = parse.urlparse(api_url).path - res = FakeResponse(method, url, data) + res = FakeResponse(method, url, params, data, headers, **kwargs) self.content = res.read() self.status_code = res.status class ZadaraVPSADriverTestCase(test.TestCase): + + def __init__(self, *args, **kwargs): + super(ZadaraVPSADriverTestCase, self).__init__(*args, **kwargs) + + self.configuration = None + self.driver = None + """Test case for Zadara VPSA volume driver.""" - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def setUp(self): super(ZadaraVPSADriverTestCase, self).setUp() @@ -462,6 +470,7 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.configuration.zadara_vpsa_port = '80' self.configuration.zadara_user = 'test' self.configuration.zadara_password = 'test_password' + self.configuration.zadara_access_key = '0123456789ABCDEF' self.configuration.zadara_vpsa_poolname = 'pool-0001' self.configuration.zadara_vol_encrypt = False self.configuration.zadara_vol_name_template = 'OS_%s' @@ -472,14 +481,14 @@ class ZadaraVPSADriverTestCase(test.TestCase): configuration=self.configuration)) self.driver.do_setup(None) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_create_destroy(self): """Create/Delete volume.""" volume = {'name': 'test_volume_01', 'size': 1} self.driver.create_volume(volume) self.driver.delete_volume(volume) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_create_destroy_multiple(self): """Create/Delete multiple volumes.""" self.driver.create_volume({'name': 'test_volume_01', 'size': 1}) @@ -490,13 +499,13 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.driver.delete_volume({'name': 'test_volume_01'}) self.driver.delete_volume({'name': 'test_volume_04'}) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_destroy_non_existent(self): """Delete non-existent volume.""" volume = {'name': 'test_volume_02', 'size': 1} self.driver.delete_volume(volume) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_empty_apis(self): """Test empty func (for coverage only).""" context = None @@ -509,7 +518,7 @@ class ZadaraVPSADriverTestCase(test.TestCase): None) self.driver.check_for_setup_error() - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_volume_attach_detach(self): """Test volume attachment and detach.""" volume = {'name': 'test_volume_01', 'size': 1, 'id': 123} @@ -529,25 +538,7 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.driver.terminate_connection(volume, connector) self.driver.delete_volume(volume) - @mock.patch.object(requests, 'request', FakeRequests) - def test_volume_attach_multiple_detach(self): - """Test multiple volume attachment and detach.""" - volume = {'name': 'test_volume_01', 'size': 1, 'id': 123} - connector1 = dict(initiator='test_iqn.1') - connector2 = dict(initiator='test_iqn.2') - connector3 = dict(initiator='test_iqn.3') - - self.driver.create_volume(volume) - self.driver.initialize_connection(volume, connector1) - self.driver.initialize_connection(volume, connector2) - self.driver.initialize_connection(volume, connector3) - - self.driver.terminate_connection(volume, connector1) - self.driver.terminate_connection(volume, connector3) - self.driver.terminate_connection(volume, connector2) - self.driver.delete_volume(volume) - - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_wrong_attach_params(self): """Test different wrong attach scenarios.""" volume1 = {'name': 'test_volume_01', 'size': 1, 'id': 101} @@ -556,32 +547,49 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.driver.initialize_connection, volume1, connector1) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_wrong_detach_params(self): """Test different wrong detachment scenarios.""" volume1 = {'name': 'test_volume_01', 'size': 1, 'id': 101} + # Volume is not created. + self.assertRaises(exception.VolumeNotFound, + self.driver.terminate_connection, + volume1, None) + + self.driver.create_volume(volume1) + connector1 = dict(initiator='test_iqn.1') + # Server is not found. Volume is found + self.assertRaises(zadara.ZadaraServerNotFound, + self.driver.terminate_connection, + volume1, connector1) + volume2 = {'name': 'test_volume_02', 'size': 1, 'id': 102} volume3 = {'name': 'test_volume_03', 'size': 1, 'id': 103} - connector1 = dict(initiator='test_iqn.1') connector2 = dict(initiator='test_iqn.2') connector3 = dict(initiator='test_iqn.3') - self.driver.create_volume(volume1) self.driver.create_volume(volume2) self.driver.initialize_connection(volume1, connector1) self.driver.initialize_connection(volume2, connector2) + # volume is found. Server not found self.assertRaises(zadara.ZadaraServerNotFound, self.driver.terminate_connection, volume1, connector3) + # Server is found. volume not found self.assertRaises(exception.VolumeNotFound, self.driver.terminate_connection, volume3, connector1) + # Server and volume exits but not attached self.assertRaises(exception.FailedCmdWithDump, self.driver.terminate_connection, volume1, connector2) - @mock.patch.object(requests, 'request', FakeRequests) + self.driver.terminate_connection(volume1, connector1) + self.driver.terminate_connection(volume2, connector2) + + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_wrong_login_reply(self): """Test wrong login reply.""" + self.configuration.zadara_access_key = None RUNTIME_VARS['login'] = """ %s @@ -605,16 +613,18 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.assertRaises(exception.MalformedResponse, self.driver.do_setup, None) - @mock.patch.object(requests, 'request') + @mock.patch.object(requests.Session, 'request') def test_ssl_use(self, request): """Coverage test for SSL connection.""" self.configuration.zadara_ssl_cert_verify = True self.configuration.zadara_vpsa_use_ssl = True self.configuration.driver_ssl_cert_path = '/path/to/cert' + fake_request_ctrls = FakeRequests("GET", "/api/vcontrollers.xml") + raw_controllers = fake_request_ctrls.content good_response = mock.MagicMock() good_response.status_code = RUNTIME_VARS['status'] - good_response.content = RUNTIME_VARS['login'] + good_response.content = raw_controllers def request_verify_cert(*args, **kwargs): self.assertEqual(kwargs['verify'], '/path/to/cert') @@ -623,7 +633,30 @@ class ZadaraVPSADriverTestCase(test.TestCase): request.side_effect = request_verify_cert self.driver.do_setup(None) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request') + def test_wrong_access_key(self, request): + """Wrong Access Key""" + fake_ak = 'FAKEACCESSKEY' + self.configuration.zadara_access_key = fake_ak + + bad_response = mock.MagicMock() + bad_response.status_code = RUNTIME_VARS['status'] + bad_response.content = RUNTIME_VARS['bad_login'] + + def request_verify_access_key(*args, **kwargs): + # Checks if the fake access_key was sent to driver + token = kwargs['headers']['X-Access-Key'] + self.assertEqual(token, fake_ak, "access_key wasn't delivered") + return bad_response + + request.side_effect = request_verify_access_key + # when access key is invalid, driver will raise + # ZadaraInvalidAccessKey exception + self.assertRaises(zadara.ZadaraInvalidAccessKey, + self.driver.do_setup, + None) + + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_bad_http_response(self): """Coverage test for non-good HTTP response.""" RUNTIME_VARS['status'] = 400 @@ -632,8 +665,8 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.assertRaises(exception.BadHTTPResponseStatus, self.driver.create_volume, volume) - @mock.patch.object(requests, 'request', FakeRequests) - def test_termiante_connection_force_detach(self): + @mock.patch.object(requests.Session, 'request', FakeRequests) + def test_terminate_connection_force_detach(self): """Test terminate connection for os-force_detach """ volume = {'name': 'test_volume_01', 'size': 1, 'id': 101} connector = dict(initiator='test_iqn.1') @@ -650,7 +683,7 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.driver.delete_volume(volume) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_delete_without_detach(self): """Test volume deletion without detach.""" @@ -664,18 +697,19 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.driver.initialize_connection(volume1, connector3) self.driver.delete_volume(volume1) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_no_active_ctrl(self): - RUNTIME_VARS['controllers'] = [] volume = {'name': 'test_volume_01', 'size': 1, 'id': 123} connector = dict(initiator='test_iqn.1') self.driver.create_volume(volume) + + RUNTIME_VARS['controllers'] = [] self.assertRaises(zadara.ZadaraVPSANoActiveController, self.driver.initialize_connection, volume, connector) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_create_destroy_snapshot(self): """Create/Delete snapshot test.""" volume = {'name': 'test_volume_01', 'size': 1} @@ -700,7 +734,7 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.driver.delete_snapshot(snapshot) self.driver.delete_volume(volume) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_expand_volume(self): """Expand volume test.""" volume = {'name': 'test_volume_01', 'size': 10} @@ -718,7 +752,7 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.driver.extend_volume(volume, 15) self.driver.delete_volume(volume) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_create_destroy_clones(self): """Create/Delete clones test.""" volume1 = {'name': 'test_volume_01', 'id': '01', 'size': 1} @@ -757,7 +791,7 @@ class ZadaraVPSADriverTestCase(test.TestCase): self.driver.delete_snapshot(snapshot) self.driver.delete_volume(volume1) - @mock.patch.object(requests, 'request', FakeRequests) + @mock.patch.object(requests.Session, 'request', FakeRequests) def test_get_volume_stats(self): """Get stats test.""" self.configuration.safe_get.return_value = 'ZadaraVPSAISCSIDriver' diff --git a/cinder/volume/drivers/zadara.py b/cinder/volume/drivers/zadara.py index 48d1e44a0b0..30a7fb3f47a 100644 --- a/cinder/volume/drivers/zadara.py +++ b/cinder/volume/drivers/zadara.py @@ -12,8 +12,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -""" -Volume driver for Zadara Virtual Private Storage Array (VPSA). +"""Volume driver for Zadara Virtual Private Storage Array (VPSA). This driver requires VPSA with API version 15.07 or higher. """ @@ -52,10 +51,16 @@ zadara_opts = [ 'certificate of the VPSA endpoint.'), cfg.StrOpt('zadara_user', default=None, + deprecated_for_removal=True, help='VPSA - Username'), cfg.StrOpt('zadara_password', default=None, help='VPSA - Password', + deprecated_for_removal=True, + secret=True), + cfg.StrOpt('zadara_access_key', + default=None, + help='VPSA access key', secret=True), cfg.StrOpt('zadara_vpsa_poolname', default=None, @@ -69,7 +74,6 @@ zadara_opts = [ cfg.BoolOpt('zadara_default_snap_policy', default=False, help="VPSA - Attach snapshot policy for volumes")] - CONF = cfg.CONF CONF.register_opts(zadara_opts, group=configuration.SHARED_CONF_GROUP) @@ -98,24 +102,22 @@ class ZadaraVolumeNotFound(exception.VolumeDriverException): message = "%(reason)s" +class ZadaraInvalidAccessKey(exception.VolumeDriverException): + message = "Invalid VPSA access key" + + class ZadaraVPSAConnection(object): """Executes volume driver commands on VPSA.""" def __init__(self, conf): self.conf = conf - self.access_key = None + self.access_key = conf.zadara_access_key self.ensure_connection() def _generate_vpsa_cmd(self, cmd, **kwargs): """Generate command to be sent to VPSA.""" - def _joined_params(params): - param_str = [] - for k, v in params.items(): - param_str.append("%s=%s" % (k, v)) - return '&'.join(param_str) - # Dictionary of applicable VPSA commands in the following format: # 'command': (method, API_URL, {optional parameters}) vpsa_commands = { @@ -123,7 +125,6 @@ class ZadaraVPSAConnection(object): '/api/users/login.xml', {'user': self.conf.zadara_user, 'password': self.conf.zadara_password}), - # Volume operations 'create_volume': ('POST', '/api/volumes.xml', @@ -143,7 +144,6 @@ class ZadaraVPSAConnection(object): '/api/volumes/%s/expand.xml' % kwargs.get('vpsa_vol'), {'capacity': kwargs.get('size')}), - # Snapshot operations # Snapshot request is triggered for a single volume though the # API call implies that snapshot is triggered for CG (legacy API). @@ -155,24 +155,20 @@ class ZadaraVPSAConnection(object): '/api/snapshots/%s.xml' % kwargs.get('snap_id'), {}), - 'create_clone_from_snap': ('POST', '/api/consistency_groups/%s/clone.xml' % kwargs.get('cg_name'), {'name': kwargs.get('name'), 'snapshot': kwargs.get('snap_id')}), - 'create_clone': ('POST', '/api/consistency_groups/%s/clone.xml' % kwargs.get('cg_name'), {'name': kwargs.get('name')}), - # Server operations 'create_server': ('POST', '/api/servers.xml', {'display_name': kwargs.get('initiator'), 'iqn': kwargs.get('initiator')}), - # Attach/Detach operations 'attach_volume': ('POST', '/api/servers/%s/volumes.xml' @@ -184,7 +180,6 @@ class ZadaraVPSAConnection(object): % kwargs.get('vpsa_vol'), {'server_name[]': kwargs.get('vpsa_srv'), 'force': 'YES'}), - # Get operations 'list_volumes': ('GET', '/api/volumes.xml', @@ -207,28 +202,18 @@ class ZadaraVPSAConnection(object): % kwargs.get('cg_name'), {})} - if cmd not in vpsa_commands: + try: + method, url, params = vpsa_commands[cmd] + except KeyError: raise exception.UnknownCmd(cmd=cmd) - else: - (method, url, params) = vpsa_commands[cmd] if method == 'GET': - # For GET commands add parameters to the URL - params.update(dict(access_key=self.access_key, - page=1, start=0, limit=0)) - url += '?' + _joined_params(params) - body = '' + params = dict(page=1, start=0, limit=0) + body = None - elif method == 'DELETE': - # For DELETE commands add parameters to the URL - params.update(dict(access_key=self.access_key)) - url += '?' + _joined_params(params) - body = '' - - elif method == 'POST': - if self.access_key: - params.update(dict(access_key=self.access_key)) - body = _joined_params(params) + elif method in ['DELETE', 'POST']: + body = params + params = None else: msg = (_('Method %(method)s is not defined') % @@ -236,7 +221,11 @@ class ZadaraVPSAConnection(object): LOG.error(msg) raise AssertionError(msg) - return (method, url, body) + # 'access_key' was generated using username and password + # or it was taken from the input file + headers = {'X-Access-Key': self.access_key} + + return method, url, params, body, headers def ensure_connection(self, cmd=None): """Retrieve access key for VPSA connection.""" @@ -248,12 +237,12 @@ class ZadaraVPSAConnection(object): xml_tree = self.send_cmd(cmd) user = xml_tree.find('user') if user is None: - raise (exception.MalformedResponse(cmd=cmd, - reason=_('no "user" field'))) + raise (exception.MalformedResponse( + cmd=cmd, reason=_('no "user" field'))) access_key = user.findtext('access-key') if access_key is None: - raise (exception.MalformedResponse(cmd=cmd, - reason=_('no "access-key" field'))) + raise (exception.MalformedResponse( + cmd=cmd, reason=_('no "access-key" field'))) self.access_key = access_key def send_cmd(self, cmd, **kwargs): @@ -261,7 +250,8 @@ class ZadaraVPSAConnection(object): self.ensure_connection(cmd) - (method, url, body) = self._generate_vpsa_cmd(cmd, **kwargs) + method, url, params, body, headers = self._generate_vpsa_cmd(cmd, + **kwargs) LOG.debug('Invoking %(cmd)s using %(method)s request.', {'cmd': cmd, 'method': method}) @@ -284,8 +274,11 @@ class ZadaraVPSAConnection(object): api_url = "%s://%s%s" % (protocol, host, url) try: - response = requests.request(method, api_url, data=body, - verify=verify) + with requests.Session() as session: + session.headers.update(headers) + response = session.request(method, api_url, params=params, + data=body, headers=headers, + verify=verify) except requests.exceptions.RequestException as e: message = (_('Exception: %s') % six.text_type(e)) raise exception.VolumeDriverException(message=message) @@ -296,6 +289,10 @@ class ZadaraVPSAConnection(object): data = response.content xml_tree = lxml.fromstring(data) status = xml_tree.findtext('status') + if status == '5': + # Invalid Credentials + raise ZadaraInvalidAccessKey() + if status != '0': raise exception.FailedCmdWithDump(status=status, data=data) @@ -314,15 +311,17 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): Version history: 15.07 - Initial driver 16.05 - Move from httplib to requests + 19.08 - Add API access key authentication option """ - VERSION = '16.05' + VERSION = '19.08' # ThirdPartySystems wiki page CI_WIKI_NAME = "ZadaraStorage_VPSA_CI" def __init__(self, *args, **kwargs): super(ZadaraVPSAISCSIDriver, self).__init__(*args, **kwargs) + self.vpsa = None self.configuration.append_config_values(zadara_opts) @staticmethod @@ -335,10 +334,20 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): Establishes initial connection with VPSA and retrieves access_key. """ self.vpsa = ZadaraVPSAConnection(self.configuration) + self._check_access_key_validity() + + def _check_access_key_validity(self): + """Check VPSA access key""" + self.vpsa.ensure_connection() + if not self.vpsa.access_key: + raise ZadaraInvalidAccessKey() + active_ctrl = self._get_active_controller_details() + if active_ctrl is None: + raise ZadaraInvalidAccessKey() def check_for_setup_error(self): """Returns an error (exception) if prerequisites aren't met.""" - self.vpsa.ensure_connection() + self._check_access_key_validity() def local_path(self, volume): """Return local path to existing local volume.""" @@ -358,14 +367,14 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): return None result_list = [] - (key, value) = search_tuple - for object in objects.getchildren(): - found_value = object.findtext(key) + key, value = search_tuple + for child_object in objects.getchildren(): + found_value = child_object.findtext(key) if found_value and (found_value == value or value is None): if first: - return object + return child_object else: - result_list.append(object) + result_list.append(child_object) return result_list if result_list else None def _get_vpsa_volume_name_and_size(self, name): @@ -377,7 +386,7 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): return (volume.findtext('name'), int(volume.findtext('virtual-capacity'))) - return (None, None) + return None, None def _get_vpsa_volume_name(self, name): """Return VPSA's name for the volume.""" @@ -419,9 +428,9 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): free = int(float(pool.findtext('available-capacity'))) LOG.debug('Pool %(name)s: %(total)sGB total, %(free)sGB free', {'name': pool_name, 'total': total, 'free': free}) - return (total, free) + return total, free - return ('unknown', 'unknown') + return 'unknown', 'unknown' def _get_active_controller_details(self): """Return details of VPSA's active controller.""" @@ -435,14 +444,18 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): chap_passwd=ctrl.findtext('vpsa-chap-secret')) return None - def _detach_vpsa_volume(self, vpsa_vol): + def _detach_vpsa_volume(self, vpsa_vol, vpsa_srv=None): """Detach volume from all attached servers.""" - list_servers = self._get_servers_attached_to_volume(vpsa_vol) - for server in list_servers: + if vpsa_srv: + list_servers_ids = [vpsa_srv] + else: + list_servers = self._get_servers_attached_to_volume(vpsa_vol) + list_servers_ids = [s.findtext('name') for s in list_servers] + + for server_id in list_servers_ids: # Detach volume from server - vpsa_srv = server.findtext('name') self.vpsa.send_cmd('detach_volume', - vpsa_srv=vpsa_srv, + vpsa_srv=server_id, vpsa_vol=vpsa_vol) def _get_server_name(self, initiator): @@ -482,7 +495,7 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): 'It might be already deleted', name) return - self._detach_vpsa_volume(vpsa_vol) + self._detach_vpsa_volume(vpsa_vol=vpsa_vol) # Delete volume self.vpsa.send_cmd('delete_volume', vpsa_vol=vpsa_vol) @@ -556,7 +569,7 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): % volume['name'], snap_id=snap_id) - if (volume['size'] > snapshot['volume_size']): + if volume['size'] > snapshot['volume_size']: self.extend_volume(volume, volume['size']) def create_cloned_volume(self, volume, src_vref): @@ -577,7 +590,7 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): name=self.configuration.zadara_vol_name_template % volume['name']) - if (volume['size'] > src_vref['size']): + if volume['size'] > src_vref['size']: self.extend_volume(volume, volume['size']) def extend_volume(self, volume, new_size): @@ -618,10 +631,13 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): During this call VPSA exposes volume to particular Initiator. It also creates a 'server' entity for Initiator (if it was not created before) - All necessary connection information is returned, including auth data. Connection data (target, LUN) is not stored in the DB. """ + # First: Check Active controller: if not valid, raise exception + ctrl = self._get_active_controller_details() + if not ctrl: + raise ZadaraVPSANoActiveController() # Get/Create server name for IQN initiator_name = connector['initiator'] @@ -635,11 +651,6 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): if not vpsa_vol: raise exception.VolumeNotFound(volume_id=volume['id']) - # Get Active controller details - ctrl = self._get_active_controller_details() - if not ctrl: - raise ZadaraVPSANoActiveController() - xml_tree = self.vpsa.send_cmd('list_vol_attachments', vpsa_vol=vpsa_vol) attach = self._xml_parse_helper(xml_tree, 'servers', @@ -649,30 +660,30 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): self.vpsa.send_cmd('attach_volume', vpsa_srv=vpsa_srv, vpsa_vol=vpsa_vol) - # Get connection info + xml_tree = self.vpsa.send_cmd('list_vol_attachments', vpsa_vol=vpsa_vol) server = self._xml_parse_helper(xml_tree, 'servers', ('iqn', initiator_name)) if server is None: raise ZadaraAttachmentsNotFound(name=name) + target = server.findtext('target') lun = int(server.findtext('lun')) - if target is None or lun is None: + if None in [target, lun]: raise ZadaraInvalidAttachmentInfo( name=name, reason=_('target=%(target)s, lun=%(lun)s') % {'target': target, 'lun': lun}) - properties = {} - properties['target_discovered'] = False - properties['target_portal'] = '%s:%s' % (ctrl['ip'], '3260') - properties['target_iqn'] = target - properties['target_lun'] = lun - properties['volume_id'] = volume['id'] - properties['auth_method'] = 'CHAP' - properties['auth_username'] = ctrl['chap_user'] - properties['auth_password'] = ctrl['chap_passwd'] + properties = {'target_discovered': False, + 'target_portal': '%s:%s' % (ctrl['ip'], '3260'), + 'target_iqn': target, + 'target_lun': lun, + 'volume_id': volume['id'], + 'auth_method': 'CHAP', + 'auth_username': ctrl['chap_user'], + 'auth_password': ctrl['chap_passwd']} LOG.debug('Attach properties: %(properties)s', {'properties': strutils.mask_password(properties)}) @@ -682,14 +693,19 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): def terminate_connection(self, volume, connector, **kwargs): """Detach volume from the initiator.""" + # Get server name for IQN if connector is None: # Detach volume from all servers # Get volume name name = self.configuration.zadara_vol_name_template % volume['name'] vpsa_vol = self._get_vpsa_volume_name(name) - self._detach_vpsa_volume(vpsa_vol) - return + if vpsa_vol: + self._detach_vpsa_volume(vpsa_vol=vpsa_vol) + return + else: + LOG.warning('Volume %s could not be found', name) + raise exception.VolumeNotFound(volume_id=volume['id']) initiator_name = connector['initiator'] @@ -704,9 +720,7 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver): raise exception.VolumeNotFound(volume_id=volume['id']) # Detach volume from server - self.vpsa.send_cmd('detach_volume', - vpsa_srv=vpsa_srv, - vpsa_vol=vpsa_vol) + self._detach_vpsa_volume(vpsa_vol=vpsa_vol, vpsa_srv=vpsa_srv) def get_volume_stats(self, refresh=False): """Get volume stats. diff --git a/releasenotes/notes/Zadara-change-to-access-key-b16bdaa9d8460b57.yaml b/releasenotes/notes/Zadara-change-to-access-key-b16bdaa9d8460b57.yaml new file mode 100644 index 00000000000..9b7e33944d0 --- /dev/null +++ b/releasenotes/notes/Zadara-change-to-access-key-b16bdaa9d8460b57.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + Zadara VPSA Driver: Added new driver authentication method to use VPSA + API access key, and deprecate exisiting authentication method that used + username and password combination. The deprecated config inputs will be + removed in the next official release after Train. + +upgrade: + - | + Add a new config option 'zadara_access_key': Zadara VPSA access key.