From 975e9a364afb6ee6000f8a38805f6c9a84957e25 Mon Sep 17 00:00:00 2001 From: "Erlon R. Cruz" Date: Tue, 17 Nov 2015 15:43:01 -0200 Subject: [PATCH] Fixing HNAS XML parser HNAS XML parser does not handles situations where the user could accidentally insert a blank space in the parameter value breaking the driver in execution time. With this fix, an error is raised in the driver setup. Closes-bug: #1537758 Change-Id: If553647563638a5c9a09441ad2ec40ed13a0ece2 --- cinder/tests/unit/test_hitachi_hnas_nfs.py | 84 +++++++++++++++++++++ cinder/volume/drivers/hitachi/hnas_iscsi.py | 50 ++++++------ cinder/volume/drivers/hitachi/hnas_nfs.py | 55 +++++++------- 3 files changed, 141 insertions(+), 48 deletions(-) diff --git a/cinder/tests/unit/test_hitachi_hnas_nfs.py b/cinder/tests/unit/test_hitachi_hnas_nfs.py index b4c9c54db..12b403e9e 100644 --- a/cinder/tests/unit/test_hitachi_hnas_nfs.py +++ b/cinder/tests/unit/test_hitachi_hnas_nfs.py @@ -76,6 +76,71 @@ HNAS_WRONG_CONF2 = """ """ +HNAS_WRONG_CONF3 = """ + + ssc + 172.17.44.15 + + supervisor + + default + 172.17.39.132:/cinder + + + silver + 172.17.39.133:/cinder + + +""" + +HNAS_WRONG_CONF4 = """ + + ssc + 172.17.44.15 + super + supervisor + + default + 172.17.39.132:/cinder + + + silver + 172.17.39.133:/cinder + + +""" + +HNAS_FULL_CONF = """ + + ssc + 172.17.44.15 + super + supervisor + True + 2222 + True + /etc/cinder/ssh_priv + 10.0.0.1 + + default + 172.17.39.132:/cinder + + + silver + 172.17.39.133:/cinder/silver + + + gold + 172.17.39.133:/cinder/gold + + + platinum + 172.17.39.133:/cinder/platinum + + +""" + + # The following information is passed on to tests, when creating a volume _SERVICE = ('Test_hdp', 'Test_path', 'Test_label') _SHARE = '172.17.39.132:/cinder' @@ -217,6 +282,25 @@ class HDSNFSDriverTest(test.TestCase): self.configuration.hds_hnas_iscsi_config_file = '' self.assertRaises(exception.ParameterNotFound, nfs._read_config, '') + # Test exception when config file has parsing errors + # due to blank tag + m_open.return_value = six.StringIO(HNAS_WRONG_CONF3) + self.configuration.hds_hnas_iscsi_config_file = '' + self.assertRaises(exception.ParameterNotFound, nfs._read_config, '') + + # Test when config file has parsing errors due invalid svc_number + m_open.return_value = six.StringIO(HNAS_WRONG_CONF4) + self.configuration.hds_hnas_iscsi_config_file = '' + config = nfs._read_config('') + self.assertEqual(1, len(config['services'])) + + # Test config with full options + # due invalid svc_number + m_open.return_value = six.StringIO(HNAS_FULL_CONF) + self.configuration.hds_hnas_iscsi_config_file = '' + config = nfs._read_config('') + self.assertEqual(4, len(config['services'])) + @mock.patch.object(nfs.HDSNFSDriver, '_id_to_vol') @mock.patch.object(nfs.HDSNFSDriver, '_get_provider_location') @mock.patch.object(nfs.HDSNFSDriver, '_get_export_path') diff --git a/cinder/volume/drivers/hitachi/hnas_iscsi.py b/cinder/volume/drivers/hitachi/hnas_iscsi.py index 39a3297cb..43c10f322 100644 --- a/cinder/volume/drivers/hitachi/hnas_iscsi.py +++ b/cinder/volume/drivers/hitachi/hnas_iscsi.py @@ -18,13 +18,13 @@ iSCSI Cinder Volume driver for Hitachi Unified Storage (HUS-HNAS) platform. """ import os +import re import six from xml.etree import ElementTree as ETree from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging -from oslo_utils import excutils from oslo_utils import units @@ -36,7 +36,7 @@ from cinder.volume.drivers.hitachi import hnas_backend from cinder.volume import utils from cinder.volume import volume_types -HDS_HNAS_ISCSI_VERSION = '4.0.0' +HDS_HNAS_ISCSI_VERSION = '4.1.0' LOG = logging.getLogger(__name__) @@ -75,23 +75,30 @@ def _loc_info(loc): def _xml_read(root, element, check=None): """Read an xml element.""" - try: - val = root.findtext(element) - LOG.info(_LI("%(element)s: %(val)s"), - {'element': element, - 'val': val if element != 'password' else '***'}) - if val: - return val.strip() - if check: - raise exception.ParameterNotFound(param=element) + val = root.findtext(element) + + # mandatory parameter not found + if val is None and check: + raise exception.ParameterNotFound(param=element) + + # tag not found + if val is None: return None - except ETree.ParseError: - if check: - with excutils.save_and_reraise_exception(): - LOG.error(_LE("XML exception reading parameter: %s"), element) + + svc_tag_pattern = re.compile("svc_[0-3]$") + # tag found but empty parameter. + if not val.strip(): + # Service tags are empty + if svc_tag_pattern.search(element): + return "" else: - LOG.info(_LI("XML exception reading parameter: %s"), element) - return None + raise exception.ParameterNotFound(param=element) + + LOG.debug(_LI("%(element)s: %(val)s"), + {'element': element, + 'val': val if element != 'password' else '***'}) + + return val.strip() def _read_config(xml_config_file): @@ -111,7 +118,7 @@ def _read_config(xml_config_file): config = {} arg_prereqs = ['mgmt_ip0', 'username'] for req in arg_prereqs: - config[req] = _xml_read(root, req, 'check') + config[req] = _xml_read(root, req, True) # optional parameters opt_parameters = ['hnas_cmd', 'ssh_enabled', 'chap_enabled', @@ -123,14 +130,14 @@ def _read_config(xml_config_file): config['chap_enabled'] = HNAS_DEFAULT_CONFIG['chap_enabled'] if config['ssh_enabled'] == 'True': - config['ssh_private_key'] = _xml_read(root, 'ssh_private_key', 'check') + config['ssh_private_key'] = _xml_read(root, 'ssh_private_key', True) config['ssh_port'] = _xml_read(root, 'ssh_port') config['password'] = _xml_read(root, 'password') if config['ssh_port'] is None: config['ssh_port'] = HNAS_DEFAULT_CONFIG['ssh_port'] else: # password is mandatory when not using SSH - config['password'] = _xml_read(root, 'password', 'check') + config['password'] = _xml_read(root, 'password', True) if config['hnas_cmd'] is None: config['hnas_cmd'] = HNAS_DEFAULT_CONFIG['hnas_cmd'] @@ -146,7 +153,7 @@ def _read_config(xml_config_file): # none optional for arg in ['volume_type', 'hdp', 'iscsi_ip']: - service[arg] = _xml_read(root, svc + '/' + arg, 'check') + service[arg] = _xml_read(root, svc + '/' + arg, True) config['services'][service['volume_type']] = service config['hdp'][service['hdp']] = service['hdp'] @@ -166,6 +173,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): Fixed concurrency errors Version 3.3.0: Fixed iSCSI target limitation error Version 4.0.0: Added manage/unmanage features + Version 4.1.0: Fixed XML parser checks on blank options """ def __init__(self, *args, **kwargs): diff --git a/cinder/volume/drivers/hitachi/hnas_nfs.py b/cinder/volume/drivers/hitachi/hnas_nfs.py index 1edaed311..e13e085ea 100644 --- a/cinder/volume/drivers/hitachi/hnas_nfs.py +++ b/cinder/volume/drivers/hitachi/hnas_nfs.py @@ -19,6 +19,7 @@ Volume driver for HDS HNAS NFS storage. import math import os +import re import six import socket import time @@ -27,7 +28,6 @@ from xml.etree import ElementTree as ETree from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging -from oslo_utils import excutils from oslo_utils import units from cinder import exception @@ -40,7 +40,7 @@ from cinder.volume import utils from cinder.volume import volume_types -HDS_HNAS_NFS_VERSION = '4.0.0' +HDS_HNAS_NFS_VERSION = '4.1.0' LOG = logging.getLogger(__name__) @@ -56,30 +56,30 @@ HNAS_DEFAULT_CONFIG = {'hnas_cmd': 'ssc', 'ssh_port': '22'} def _xml_read(root, element, check=None): - """Read an xml element. + """Read an xml element.""" - :param root: XML object - :param element: string desired tag - :param check: string if present, throw exception if element missing - """ + val = root.findtext(element) - try: - val = root.findtext(element) - LOG.info(_LI("%(element)s: %(val)s"), - {'element': element, - 'val': val if element != 'password' else '***'}) - if val: - return val.strip() - if check: - raise exception.ParameterNotFound(param=element) + # mandatory parameter not found + if val is None and check: + raise exception.ParameterNotFound(param=element) + + # tag not found + if val is None: return None - except ETree.ParseError: - if check: - with excutils.save_and_reraise_exception(): - LOG.error(_LE("XML exception reading parameter: %s"), element) - else: - LOG.info(_LI("XML exception reading parameter: %s"), element) - return None + + svc_tag_pattern = re.compile("svc_.$") + # tag found but empty parameter. + if not val.strip(): + if svc_tag_pattern.search(element): + return "" + raise exception.ParameterNotFound(param=element) + + LOG.debug(_LI("%(element)s: %(val)s"), + {'element': element, + 'val': val if element != 'password' else '***'}) + + return val.strip() def _read_config(xml_config_file): @@ -102,7 +102,7 @@ def _read_config(xml_config_file): config = {} arg_prereqs = ['mgmt_ip0', 'username'] for req in arg_prereqs: - config[req] = _xml_read(root, req, 'check') + config[req] = _xml_read(root, req, True) # optional parameters opt_parameters = ['hnas_cmd', 'ssh_enabled', 'cluster_admin_ip0'] @@ -110,14 +110,14 @@ def _read_config(xml_config_file): config[req] = _xml_read(root, req) if config['ssh_enabled'] == 'True': - config['ssh_private_key'] = _xml_read(root, 'ssh_private_key', 'check') + config['ssh_private_key'] = _xml_read(root, 'ssh_private_key', True) config['password'] = _xml_read(root, 'password') config['ssh_port'] = _xml_read(root, 'ssh_port') if config['ssh_port'] is None: config['ssh_port'] = HNAS_DEFAULT_CONFIG['ssh_port'] else: # password is mandatory when not using SSH - config['password'] = _xml_read(root, 'password', 'check') + config['password'] = _xml_read(root, 'password', True) if config['hnas_cmd'] is None: config['hnas_cmd'] = HNAS_DEFAULT_CONFIG['hnas_cmd'] @@ -133,7 +133,7 @@ def _read_config(xml_config_file): # none optional for arg in ['volume_type', 'hdp']: - service[arg] = _xml_read(root, svc + '/' + arg, 'check') + service[arg] = _xml_read(root, svc + '/' + arg, True) config['services'][service['volume_type']] = service config['hdp'][service['hdp']] = service['hdp'] @@ -159,6 +159,7 @@ class HDSNFSDriver(nfs.NfsDriver): Version 2.2.0: Added support to SSH authentication Version 3.0.0: Added pool aware scheduling Version 4.0.0: Added manage/unmanage features + Version 4.1.0: Fixed XML parser checks on blank options """ def __init__(self, *args, **kwargs): -- 2.45.2