From 81ec69116a5252c1f90c0a1742f88b409851b0a8 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Fri, 28 Jul 2017 13:45:25 -0500 Subject: [PATCH] Protect against missing interface attribute If deprecated options aren't registered, interface will not exist, resulting in NoSuchOptError. Add safeguards around accessing the interface opt, and appropriate test cases. Co-Authored-By: Eric Fried Closes-Bug: #1707273 Change-Id: Ic3df9817f0038f8f610db70d7e34fe9d458606b6 --- keystoneauth1/loading/adapter.py | 4 +- .../tests/unit/loading/test_adapter.py | 86 ++++++++++++------- 2 files changed, 56 insertions(+), 34 deletions(-) diff --git a/keystoneauth1/loading/adapter.py b/keystoneauth1/loading/adapter.py index b305c673..c25f0901 100644 --- a/keystoneauth1/loading/adapter.py +++ b/keystoneauth1/loading/adapter.py @@ -213,7 +213,7 @@ class Adapter(base.BaseLoader): """ c = conf[group] - if c.valid_interfaces and c.interface: + if c.valid_interfaces and hasattr(c, 'interface') and c.interface: raise TypeError("interface and valid_interfaces are mutually" " exclusive. Please use valid_interfaces.") if c.valid_interfaces: @@ -224,7 +224,7 @@ class Adapter(base.BaseLoader): " public, internal or admin".format( iface=iface)) kwargs.setdefault('interface', c.valid_interfaces) - else: + elif hasattr(c, 'interface'): kwargs.setdefault('interface', c.interface) kwargs.setdefault('service_type', c.service_type) kwargs.setdefault('service_name', c.service_name) diff --git a/keystoneauth1/tests/unit/loading/test_adapter.py b/keystoneauth1/tests/unit/loading/test_adapter.py index 9bc8906c..54d452f6 100644 --- a/keystoneauth1/tests/unit/loading/test_adapter.py +++ b/keystoneauth1/tests/unit/loading/test_adapter.py @@ -27,7 +27,8 @@ class ConfLoadingTests(utils.TestCase): super(ConfLoadingTests, self).setUp() self.conf_fx = self.useFixture(config.Config()) - loading.register_adapter_conf_options(self.conf_fx.conf, self.GROUP) + loading.register_adapter_conf_options(self.conf_fx.conf, self.GROUP, + include_deprecated=False) def test_load(self): self.conf_fx.config( @@ -86,25 +87,6 @@ class ConfLoadingTests(utils.TestCase): self.assertIsNone(adap.min_version) self.assertIsNone(adap.max_version) - def test_load_old_interface(self): - self.conf_fx.config( - service_type='type', service_name='name', - interface='internal', - region_name='region', endpoint_override='endpoint', - version='2.0', group=self.GROUP) - adap = loading.load_adapter_from_conf_options( - self.conf_fx.conf, self.GROUP, session='session', auth='auth') - self.assertEqual('type', adap.service_type) - self.assertEqual('name', adap.service_name) - self.assertEqual('internal', adap.interface) - self.assertEqual('region', adap.region_name) - self.assertEqual('endpoint', adap.endpoint_override) - self.assertEqual('session', adap.session) - self.assertEqual('auth', adap.auth) - self.assertEqual('2.0', adap.version) - self.assertIsNone(adap.min_version) - self.assertIsNone(adap.max_version) - def test_load_bad_valid_interfaces_value(self): self.conf_fx.config( service_type='type', service_name='name', @@ -135,18 +117,6 @@ class ConfLoadingTests(utils.TestCase): self.assertEqual('2.0', adap.min_version) self.assertEqual('3.0', adap.max_version) - def test_interface_conflict(self): - self.conf_fx.config( - service_type='type', service_name='name', interface='iface', - valid_interfaces='internal,public', - region_name='region', endpoint_override='endpoint', - group=self.GROUP) - - self.assertRaises( - TypeError, - loading.load_adapter_from_conf_options, - self.conf_fx.conf, self.GROUP, session='session', auth='auth') - def test_load_bad_version(self): self.conf_fx.config( service_type='type', service_name='name', @@ -186,6 +156,11 @@ class ConfLoadingTests(utils.TestCase): {opt.name for opt in opts}) def test_deprecated(self): + """Test external options that are deprecated by Adapter options. + + Not to be confused with ConfLoadingDeprecatedTests, which tests conf + options in Adapter which are themselves deprecated. + """ def new_deprecated(): return cfg.DeprecatedOpt(uuid.uuid4().hex, group=uuid.uuid4().hex) @@ -196,3 +171,50 @@ class ConfLoadingTests(utils.TestCase): for opt in opts: if opt.name in opt_names: self.assertIn(depr[opt.name][0], opt.deprecated_opts) + + +class ConfLoadingLegacyTests(ConfLoadingTests): + """Tests with inclusion of deprecated conf options. + + Not to be confused with ConfLoadingTests.test_deprecated, which tests + external options that are deprecated in favor of Adapter options. + """ + + GROUP = 'adaptergroup' + + def setUp(self): + super(ConfLoadingLegacyTests, self).setUp() + + self.conf_fx = self.useFixture(config.Config()) + loading.register_adapter_conf_options(self.conf_fx.conf, self.GROUP) + + def test_load_old_interface(self): + self.conf_fx.config( + service_type='type', service_name='name', + interface='internal', + region_name='region', endpoint_override='endpoint', + version='2.0', group=self.GROUP) + adap = loading.load_adapter_from_conf_options( + self.conf_fx.conf, self.GROUP, session='session', auth='auth') + self.assertEqual('type', adap.service_type) + self.assertEqual('name', adap.service_name) + self.assertEqual('internal', adap.interface) + self.assertEqual('region', adap.region_name) + self.assertEqual('endpoint', adap.endpoint_override) + self.assertEqual('session', adap.session) + self.assertEqual('auth', adap.auth) + self.assertEqual('2.0', adap.version) + self.assertIsNone(adap.min_version) + self.assertIsNone(adap.max_version) + + def test_interface_conflict(self): + self.conf_fx.config( + service_type='type', service_name='name', interface='iface', + valid_interfaces='internal,public', + region_name='region', endpoint_override='endpoint', + group=self.GROUP) + + self.assertRaises( + TypeError, + loading.load_adapter_from_conf_options, + self.conf_fx.conf, self.GROUP, session='session', auth='auth')