From 2c11e9dd8221325a1071d42c069dc6c806d3ce60 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 1 May 2020 16:45:04 +0100 Subject: [PATCH] Resolve UnboundLocalError As discussed in bug 1841038, configuring a StrOpt with a regex parameter and providing an invalid environment variable will result in an ugly stacktrace due to an undefined 'alt_loc' variable. Resolve this by creating said variable. Change-Id: Id0fb6efdc435a2bd88d5dfea2b4720df453fbfec Signed-off-by: Stephen Finucane Closes-Bug: #1841038 --- oslo_config/cfg.py | 5 +++++ oslo_config/sources/_environment.py | 11 +++++++++-- oslo_config/tests/test_sources.py | 16 ++++++++++++++-- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py index 638e4ff2..5324e528 100644 --- a/oslo_config/cfg.py +++ b/oslo_config/cfg.py @@ -2678,6 +2678,7 @@ class ConfigOpts(abc.Mapping): namespace = self._namespace if namespace is not None: try: + alt_loc = None try: val, alt_loc = opt._get_from_namespace(namespace, group_name) @@ -2692,6 +2693,10 @@ class ConfigOpts(abc.Mapping): if val != sources._NoValue: return (convert(val), alt_loc) except KeyError: # nosec: Valid control flow instruction + alt_loc = LocationInfo( + Locations.environment, + self._env_driver.get_name(group_name, name), + ) # If there was a KeyError looking at config files or # command line, retry the env_val. if env_val[0] != sources._NoValue: diff --git a/oslo_config/sources/_environment.py b/oslo_config/sources/_environment.py index 19dc093c..7caaa43a 100644 --- a/oslo_config/sources/_environment.py +++ b/oslo_config/sources/_environment.py @@ -77,12 +77,19 @@ class EnvironmentConfigurationSource(sources.ConfigurationSource): """A configuration source for options in the environment.""" @staticmethod - def _make_name(group_name, option_name): + def get_name(group_name, option_name): + """Return the expected environment variable name for the given option. + + :param group_name: The group name or None. Defaults to 'DEFAULT' if + None. + :param option_name: The option name. + :returns: Th expected environment variable name. + """ group_name = group_name or 'DEFAULT' return 'OS_{}__{}'.format(group_name.upper(), option_name.upper()) def get(self, group_name, option_name, opt): - env_name = self._make_name(group_name, option_name) + env_name = self.get_name(group_name, option_name) try: value = os.environ[env_name] loc = oslo_config.cfg.LocationInfo( diff --git a/oslo_config/tests/test_sources.py b/oslo_config/tests/test_sources.py index 656f61ef..2da73da1 100644 --- a/oslo_config/tests/test_sources.py +++ b/oslo_config/tests/test_sources.py @@ -15,6 +15,7 @@ import os from oslotest import base from requests import HTTPError import requests_mock +import testtools from oslo_config import _list_opts from oslo_config import cfg @@ -120,10 +121,13 @@ class TestEnvironmentConfigurationSource(base.BaseTestCase): self.conf = cfg.ConfigOpts() self.conf_fixture = self.useFixture(fixture.Config(self.conf)) self.conf.register_opt(cfg.StrOpt('bar'), 'foo') + self.conf.register_opt(cfg.StrOpt('baz', regex='^[a-z].*$'), 'foo') def cleanup(): - if 'OS_FOO__BAR' in os.environ: - del os.environ['OS_FOO__BAR'] + for env in ('OS_FOO__BAR', 'OS_FOO__BAZ'): + if env in os.environ: + del os.environ[env] + self.addCleanup(cleanup) def test_simple_environment_get(self): @@ -171,6 +175,14 @@ class TestEnvironmentConfigurationSource(base.BaseTestCase): self.conf(args=[], use_env=True) self.assertEqual(env_value, self.conf['foo']['bar']) + def test_invalid_env(self): + self.conf(args=[]) + env_value = 'ABC' + os.environ['OS_FOO__BAZ'] = env_value + + with testtools.ExpectedException(cfg.ConfigSourceValueError): + self.conf['foo']['baz'] + def make_uri(name): return "https://oslo.config/{}.conf".format(name)