From dc564548e14c5efbf81f8940a248085979f1e6b4 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Thu, 22 Feb 2018 16:44:05 -0600 Subject: [PATCH] Run normalize_keys on config for session codepath When we pass an authenticated session to Connection, we use the from_session codepath. That doesn't run through loader, and as a result the kwargs dict doesn't get passed through normalize_keys. That, in turn, means that int _api_version values are not turned in to strings, which in turn breaks a subscripting attempt in service_description. Run normalize_keys on the config dict and add some tests for this. Change-Id: I1c745c45512cb5d10cad36a3e2713a1dec9d494f --- openstack/config/_util.py | 31 +++++++++++++++++++ openstack/config/cloud_region.py | 4 +-- openstack/config/loader.py | 29 +++++------------ .../tests/unit/config/test_cloud_config.py | 6 ++-- openstack/tests/unit/test_connection.py | 28 +++++++++++++++++ 5 files changed, 71 insertions(+), 27 deletions(-) create mode 100644 openstack/config/_util.py diff --git a/openstack/config/_util.py b/openstack/config/_util.py new file mode 100644 index 000000000..8cfccc6fe --- /dev/null +++ b/openstack/config/_util.py @@ -0,0 +1,31 @@ +# Copyright (c) 2014 Hewlett-Packard Development Company, L.P. +# +# 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 +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +def normalize_keys(config): + new_config = {} + for key, value in config.items(): + key = key.replace('-', '_') + if isinstance(value, dict): + new_config[key] = normalize_keys(value) + elif isinstance(value, bool): + new_config[key] = value + elif isinstance(value, int) and key not in ( + 'verbose_level', 'api_timeout'): + new_config[key] = str(value) + elif isinstance(value, float): + new_config[key] = str(value) + else: + new_config[key] = value + return new_config diff --git a/openstack/config/cloud_region.py b/openstack/config/cloud_region.py index 4977e356d..62bacef36 100644 --- a/openstack/config/cloud_region.py +++ b/openstack/config/cloud_region.py @@ -23,6 +23,7 @@ from six.moves import urllib from openstack import version as openstack_version from openstack import _log +from openstack.config import _util from openstack.config import defaults as config_defaults from openstack import exceptions @@ -79,10 +80,9 @@ class CloudRegion(object): openstack_config=None, session_constructor=None, app_name=None, app_version=None, session=None, discovery_cache=None): - self._name = name self.region_name = region_name - self.config = config + self.config = _util.normalize_keys(config) self.log = _log.setup_logging('openstack.config') self._force_ipv4 = force_ipv4 self._auth = auth_plugin diff --git a/openstack/config/loader.py b/openstack/config/loader.py index aa766dd3c..1df0bc96e 100644 --- a/openstack/config/loader.py +++ b/openstack/config/loader.py @@ -29,6 +29,7 @@ from keystoneauth1 import loading import yaml from openstack import _log +from openstack.config import _util from openstack.config import cloud_region from openstack.config import defaults from openstack.config import vendors @@ -262,7 +263,7 @@ class OpenStackConfig(object): self._cache_arguments = {} self._cache_expiration = {} if 'cache' in self.cloud_config: - cache_settings = self._normalize_keys(self.cloud_config['cache']) + cache_settings = _util.normalize_keys(self.cloud_config['cache']) # expiration_time used to be 'max_age' but the dogpile setting # is expiration_time. Support max_age for backwards compat. @@ -327,12 +328,12 @@ class OpenStackConfig(object): :param dict defaults: (optional) default values to merge under the found config """ - defaults = self._normalize_keys(defaults or {}) + defaults = _util.normalize_keys(defaults or {}) if not key: return defaults return _merge_clouds( defaults, - self._normalize_keys(self.cloud_config.get(key, {}))) + _util.normalize_keys(self.cloud_config.get(key, {}))) def _load_config_file(self): return self._load_yaml_json_file(self._config_files) @@ -353,22 +354,6 @@ class OpenStackConfig(object): return path, yaml.safe_load(f) return (None, {}) - def _normalize_keys(self, config): - new_config = {} - for key, value in config.items(): - key = key.replace('-', '_') - if isinstance(value, dict): - new_config[key] = self._normalize_keys(value) - elif isinstance(value, bool): - new_config[key] = value - elif isinstance(value, int) and key != 'verbose_level': - new_config[key] = str(value) - elif isinstance(value, float): - new_config[key] = str(value) - else: - new_config[key] = value - return new_config - def get_cache_expiration_time(self): return int(self._cache_expiration_time) @@ -412,7 +397,7 @@ class OpenStackConfig(object): return regions def _get_known_regions(self, cloud): - config = self._normalize_keys(self.cloud_config['clouds'][cloud]) + config = _util.normalize_keys(self.cloud_config['clouds'][cloud]) if 'regions' in config: return self._expand_regions(config['regions']) elif 'region_name' in config: @@ -1075,7 +1060,7 @@ class OpenStackConfig(object): config[key] = val config = self.magic_fixes(config) - config = self._normalize_keys(config) + config = _util.normalize_keys(config) # NOTE(dtroyer): OSC needs a hook into the auth args before the # plugin is loaded in order to maintain backward- @@ -1203,7 +1188,7 @@ class OpenStackConfig(object): return self._cloud_region_class( name=cloud_name, region_name=config['region_name'], - config=self._normalize_keys(config), + config=config, force_ipv4=force_ipv4, auth_plugin=auth_plugin, openstack_config=self, diff --git a/openstack/tests/unit/config/test_cloud_config.py b/openstack/tests/unit/config/test_cloud_config.py index ee2741940..f0c86922a 100644 --- a/openstack/tests/unit/config/test_cloud_config.py +++ b/openstack/tests/unit/config/test_cloud_config.py @@ -45,14 +45,14 @@ class TestCloudRegion(base.TestCase): self.assertEqual("region-al", cc.region_name) # Look up straight value - self.assertEqual(1, cc.a) + self.assertEqual("1", cc.a) # Look up prefixed attribute, fail - returns None self.assertIsNone(cc.os_b) # Look up straight value, then prefixed value - self.assertEqual(3, cc.c) - self.assertEqual(3, cc.os_c) + self.assertEqual("3", cc.c) + self.assertEqual("3", cc.os_c) # Lookup mystery attribute self.assertIsNone(cc.x) diff --git a/openstack/tests/unit/test_connection.py b/openstack/tests/unit/test_connection.py index 1c77aa403..df28d1402 100644 --- a/openstack/tests/unit/test_connection.py +++ b/openstack/tests/unit/test_connection.py @@ -109,6 +109,34 @@ class TestConnection(base.TestCase): self.assertEqual('openstack.workflow.v2._proxy', conn.workflow.__class__.__module__) + def test_create_connection_version_param_default(self): + c1 = connection.Connection(cloud='sample') + conn = connection.Connection(session=c1.session) + self.assertEqual('openstack.identity.v2._proxy', + conn.identity.__class__.__module__) + + def test_create_connection_version_param_string(self): + c1 = connection.Connection(cloud='sample') + conn = connection.Connection( + session=c1.session, identity_api_version='3') + self.assertEqual('openstack.identity.v3._proxy', + conn.identity.__class__.__module__) + + def test_create_connection_version_param_int(self): + c1 = connection.Connection(cloud='sample') + conn = connection.Connection( + session=c1.session, identity_api_version=3) + self.assertEqual('openstack.identity.v3._proxy', + conn.identity.__class__.__module__) + + def test_create_connection_version_param_bogus(self): + c1 = connection.Connection(cloud='sample') + conn = connection.Connection( + session=c1.session, identity_api_version='red') + # TODO(mordred) This is obviously silly behavior + self.assertEqual('openstack.identity.v3._proxy', + conn.identity.__class__.__module__) + def test_from_config_given_config(self): cloud_region = openstack.config.OpenStackConfig().get_one("sample")