From cad70e20cf8a04b809be54f122bff82aae8f4137 Mon Sep 17 00:00:00 2001 From: Andrea Frittoli Date: Wed, 16 Aug 2017 13:19:04 +0100 Subject: [PATCH] Don't read config in cred_factory module The credentials_factory module loads a few bit configuration at module import time when the DEFAULT_PARAMS variable is defined. This is not a good idea as it forces configuration to be loaded even during test discovery. Besides DEFAULT_PARAMS was out of date as it did not include http_timeout. Replacing the DEFAULT_PARAMS variable with calls to config.service_client_config(). Not loading CONF at test discovery time uncovered the fact that the account generator unit tests were depending on oslo log configuration parameters to be there. Mocking the log setup step fixed the issue. Change-Id: I6e0eb85b3749baedb6035f59ed1c66850f6c95fb --- .../notes/drop-DEFAULT_PARAMS-bfcc2e7b74ef880b.yaml | 13 +++++++++++++ tempest/common/credentials_factory.py | 11 ++--------- tempest/tests/cmd/test_account_generator.py | 1 + tempest/tests/common/test_credentials_factory.py | 10 +++++----- tempest/tests/test_base_test.py | 3 +++ 5 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/drop-DEFAULT_PARAMS-bfcc2e7b74ef880b.yaml diff --git a/releasenotes/notes/drop-DEFAULT_PARAMS-bfcc2e7b74ef880b.yaml b/releasenotes/notes/drop-DEFAULT_PARAMS-bfcc2e7b74ef880b.yaml new file mode 100644 index 0000000000..c9a49a7377 --- /dev/null +++ b/releasenotes/notes/drop-DEFAULT_PARAMS-bfcc2e7b74ef880b.yaml @@ -0,0 +1,13 @@ +--- +upgrade: + - | + Replace any call in your code to credentials_factory.DEFAULT_PARAMS with + a call to config.service_client_config(). +fixes: + - | + The credentials_factory module used to load configuration at import time + which caused configuration being loaded at test discovery time. + This was fixed by removing the DEFAULT_PARAMS variable. This variable + was redundant (and outdated), the same dictionary (but up to date) can + be obtained via invoking config.service_client_config() with no service + parameter. diff --git a/tempest/common/credentials_factory.py b/tempest/common/credentials_factory.py index a340531486..da349755d9 100644 --- a/tempest/common/credentials_factory.py +++ b/tempest/common/credentials_factory.py @@ -219,13 +219,6 @@ CREDENTIAL_TYPES = { 'alt_user': ('identity', 'alt') } -DEFAULT_PARAMS = { - 'disable_ssl_certificate_validation': - CONF.identity.disable_ssl_certificate_validation, - 'ca_certs': CONF.identity.ca_certificates_file, - 'trace_requests': CONF.debug.trace_requests -} - def get_configured_admin_credentials(fill_in=True, identity_version=None): """Get admin credentials from the config file @@ -252,7 +245,7 @@ def get_configured_admin_credentials(fill_in=True, identity_version=None): if identity_version == 'v3': conf_attributes.append('domain_name') # Read the parts of credentials from config - params = DEFAULT_PARAMS.copy() + params = config.service_client_config() for attr in conf_attributes: params[attr] = getattr(CONF.auth, 'admin_' + attr) # Build and validate credentials. We are reading configured credentials, @@ -282,7 +275,7 @@ def get_credentials(fill_in=True, identity_version=None, **kwargs): :param kwargs: Attributes to be used to build the Credentials object. :returns: An object of a sub-type of `auth.Credentials` """ - params = dict(DEFAULT_PARAMS, **kwargs) + params = dict(config.service_client_config(), **kwargs) identity_version = identity_version or CONF.identity.auth_version # In case of "v3" add the domain from config if not specified # To honour the "default_credentials_domain_name", if not domain diff --git a/tempest/tests/cmd/test_account_generator.py b/tempest/tests/cmd/test_account_generator.py index f907bd0814..8bf4c5b600 100644 --- a/tempest/tests/cmd/test_account_generator.py +++ b/tempest/tests/cmd/test_account_generator.py @@ -44,6 +44,7 @@ class MockHelpersMixin(object): self.patchobject(config, 'TempestConfigPrivate', fake_config.FakePrivate) self.opts = FakeOpts(version=identity_version) + self.patch('oslo_log.log.setup', autospec=True) def mock_resource_creation(self): fake_resource = dict(id='id', name='name') diff --git a/tempest/tests/common/test_credentials_factory.py b/tempest/tests/common/test_credentials_factory.py index 020818e69d..7cf87f8e18 100644 --- a/tempest/tests/common/test_credentials_factory.py +++ b/tempest/tests/common/test_credentials_factory.py @@ -183,7 +183,7 @@ class TestCredentialsFactory(base.TestCase): # Build the expected params expected_params = dict( [(field, value) for _, field, value in all_params]) - expected_params.update(cf.DEFAULT_PARAMS) + expected_params.update(config.service_client_config()) admin_creds = cf.get_configured_admin_credentials() mock_get_credentials.assert_called_once_with( fill_in=True, identity_version='v3', **expected_params) @@ -205,7 +205,7 @@ class TestCredentialsFactory(base.TestCase): # Build the expected params expected_params = dict( [(field, value) for _, field, value in all_params]) - expected_params.update(cf.DEFAULT_PARAMS) + expected_params.update(config.service_client_config()) admin_creds = cf.get_configured_admin_credentials( fill_in=False, identity_version='v3') mock_get_credentials.assert_called_once_with( @@ -232,7 +232,7 @@ class TestCredentialsFactory(base.TestCase): cfg.CONF.set_default('uri', expected_uri, 'identity') params = {'foo': 'bar'} expected_params = params.copy() - expected_params.update(cf.DEFAULT_PARAMS) + expected_params.update(config.service_client_config()) result = cf.get_credentials(identity_version='v2', **params) self.assertEqual(expected_result, result) mock_auth_get_credentials.assert_called_once_with( @@ -251,7 +251,7 @@ class TestCredentialsFactory(base.TestCase): params = {'foo': 'bar'} expected_params = params.copy() expected_params['domain_name'] = expected_domain - expected_params.update(cf.DEFAULT_PARAMS) + expected_params.update(config.service_client_config()) result = cf.get_credentials(fill_in=False, identity_version='v3', **params) self.assertEqual(expected_result, result) @@ -270,7 +270,7 @@ class TestCredentialsFactory(base.TestCase): expected_domain, 'auth') params = {'foo': 'bar', 'user_domain_name': expected_domain} expected_params = params.copy() - expected_params.update(cf.DEFAULT_PARAMS) + expected_params.update(config.service_client_config()) result = cf.get_credentials(fill_in=False, identity_version='v3', **params) self.assertEqual(expected_result, result) diff --git a/tempest/tests/test_base_test.py b/tempest/tests/test_base_test.py index 3ece11d0b3..011bc9bebc 100644 --- a/tempest/tests/test_base_test.py +++ b/tempest/tests/test_base_test.py @@ -17,6 +17,7 @@ from oslo_config import cfg from tempest import clients from tempest.common import credentials_factory as credentials +from tempest import config from tempest.lib.common import fixed_network from tempest import test from tempest.tests import base @@ -27,6 +28,8 @@ class TestBaseTestCase(base.TestCase): def setUp(self): super(TestBaseTestCase, self).setUp() self.useFixture(fake_config.ConfigFixture()) + self.patchobject(config, 'TempestConfigPrivate', + fake_config.FakePrivate) self.fixed_network_name = 'fixed-net' cfg.CONF.set_default('fixed_network_name', self.fixed_network_name, 'compute')