From 18db2c939a6c3721a96a3510cd8d5334876d25ce Mon Sep 17 00:00:00 2001 From: Martin Kopec Date: Mon, 22 May 2017 20:09:00 +0000 Subject: [PATCH] Fix setting of admin_password admin_password wasn't obtained from the client's config file, so when it wasn't specified in CLI it led to an error. The patch moves obtaining values from the config file to a function, which is executed before override values (from CLI) are set, so they may be overridden. Change-Id: If20ae0ea1e38d1b670d5a395ac004f74466c404a --- config_tempest/config_tempest.py | 68 ++++++++++++++------- config_tempest/tests/base.py | 2 +- config_tempest/tests/test_config_tempest.py | 60 +++++++++++------- 3 files changed, 87 insertions(+), 43 deletions(-) diff --git a/config_tempest/config_tempest.py b/config_tempest/config_tempest.py index fdc6a6bd..6c92afe7 100755 --- a/config_tempest/config_tempest.py +++ b/config_tempest/config_tempest.py @@ -26,9 +26,13 @@ Some required options differ among deployed clouds but the right values cannot be discovered by the user. The file used here could be created by an installer, or manually if necessary. -3. Values provided on the command line. These override all other values. +3. Values provided in client's cloud config file or as an environment +variables, see documentation of os-client-config +https://docs.openstack.org/developer/os-client-config/ -4. Discovery. Values that have not been provided in steps [2-3] will be +4. Values provided on the command line. These override all other values. + +5. Discovery. Values that have not been provided in steps [2-4] will be obtained by querying the cloud. """ @@ -131,13 +135,12 @@ def main(): # There are no deployer input options in DEFAULT for (key, value) in deployer_input.items(section): conf.set(section, key, value, priority=True) + # get and set auth data from client's config (os-client-config support) + set_cloud_config_values(conf, args) + # set overrides - vales specified in CLI for section, key, value in args.overrides: conf.set(section, key, value, priority=True) - if conf.has_option("identity", "uri"): - uri = conf.get("identity", "uri") - else: - uri = args.config['auth'].get('auth_url') - conf.set("identity", "uri", uri) + uri = conf.get("identity", "uri") api_version = 2 v3_only = False if "v3" in uri and v3_only: @@ -166,7 +169,7 @@ def main(): # new way for running using accounts file conf.set("auth", "use_dynamic_credentials", "False") conf.set("auth", "test_accounts_file", "etc/accounts.yaml") - clients = ClientManager(conf, not args.non_admin, args) + clients = ClientManager(conf, not args.non_admin) swift_discover = conf.get_defaulted('object-storage-feature-enabled', 'discoverability') services = api_discovery.discover( @@ -311,6 +314,38 @@ def parse_overrides(overrides): return new_overrides +def set_cloud_config_values(conf, args): + """Set values from client's cloud config file. + + If the cloud config files was provided, set admin and non-admin credentials + and uri. + Note: the values may be later overriden by values specified in CLI. + + :conf TempestConf object + :args parsed arguments including client config values + """ + cloud_creds = args.config.get('auth') + if cloud_creds: + try: + if args.non_admin: + conf.set('identity', 'username', cloud_creds['username']) + conf.set('identity', + 'tenant_name', + cloud_creds['project_name']) + conf.set('identity', 'password', cloud_creds['password']) + else: + conf.set('identity', 'admin_username', cloud_creds['username']) + conf.set('identity', + 'admin_tenant_name', + cloud_creds['project_name']) + conf.set('identity', 'admin_password', cloud_creds['password']) + conf.set('identity', 'uri', cloud_creds['auth_url']) + + except cfg.NoSuchOptError: + LOG.warning( + 'Could not load some identity options from cloud config file') + + class ClientManager(object): """Manager of various OpenStack API clients. @@ -363,16 +398,11 @@ class ClientManager(object): else: return "v2" - def __init__(self, conf, admin, args): + def __init__(self, conf, admin): self.identity_version = self.get_identity_version(conf) username = None password = None tenant_name = None - os_client_creds = args.config.get('auth') - if os_client_creds: - username = os_client_creds.get('username') - password = os_client_creds.get('password') - tenant_name = os_client_creds.get('project_name') if admin: try: username = conf.get_defaulted('auth', 'admin_username') @@ -395,13 +425,9 @@ class ClientManager(object): DEFAULTS_FILE) else: try: - # override values only when were set in CLI - if conf.has_option('identity', 'username'): - username = conf.get_defaulted('identity', 'username') - if conf.has_option('identity', 'password'): - password = conf.get_defaulted('identity', 'password') - if conf.has_option('identity', 'tenant_name'): - tenant_name = conf.get_defaulted('identity', 'tenant_name') + username = conf.get_defaulted('identity', 'username') + password = conf.get_defaulted('identity', 'password') + tenant_name = conf.get_defaulted('identity', 'tenant_name') except cfg.NoSuchOptError: LOG.warning( diff --git a/config_tempest/tests/base.py b/config_tempest/tests/base.py index 9060f3ca..b1397fea 100644 --- a/config_tempest/tests/base.py +++ b/config_tempest/tests/base.py @@ -67,7 +67,7 @@ class BaseConfigTempestTest(base.BaseTestCase): mock_function = mock.Mock(return_value=False) func2mock = 'os_client_config.cloud_config.CloudConfig.config.get' self.useFixture(MonkeyPatch(func2mock, mock_function)) - return tool.ClientManager(conf, admin=admin, args=mock_args) + return tool.ClientManager(conf, admin=admin) class BaseServiceTest(base.BaseTestCase): diff --git a/config_tempest/tests/test_config_tempest.py b/config_tempest/tests/test_config_tempest.py index 25d5225e..0ef9d231 100644 --- a/config_tempest/tests/test_config_tempest.py +++ b/config_tempest/tests/test_config_tempest.py @@ -148,7 +148,8 @@ class TestOsClientConfigSupport(BaseConfigTempestTest): self.assertEqual(exp_pass, password) self.assertEqual(exp_tenant, tenant_name) - def _override_setup(self): + @mock.patch('os_client_config.cloud_config.CloudConfig') + def _override_setup(self, mock_args): cloud_args = { 'username': 'cloud_user', 'password': 'cloud_pass', @@ -161,53 +162,70 @@ class TestOsClientConfigSupport(BaseConfigTempestTest): func2mock = 'config_tempest.config_tempest.identity.get_tenant_by_name' self.useFixture(MonkeyPatch(func2mock, mock_function)) - @mock.patch('os_client_config.cloud_config.CloudConfig') - def test_init_manager_client_config(self, mock_args): + def _obtain_client_config_data(self, mock_args, admin): cloud_args = { 'username': 'cloud_user', 'password': 'cloud_pass', - 'project_name': 'cloud_project' + 'project_name': 'cloud_project', + 'auth_url': 'http://auth.url.com/' } mock_function = mock.Mock(return_value=cloud_args) func2mock = 'os_client_config.cloud_config.CloudConfig.config.get' self.useFixture(MonkeyPatch(func2mock, mock_function)) - # remove options, pretend like they aren't set in CLI - self.conf.remove_option('identity', 'username') - self.conf.remove_option('identity', 'password') - self.conf.remove_option('identity', 'tenant_name') - manager = tool.ClientManager(self.conf, admin=False, args=mock_args) - # check if cloud_args credentials were used - self._check_credentials(manager, - cloud_args['username'], - cloud_args['password'], - cloud_args['project_name']) + # create an empty conf + conf = tool.TempestConf() + conf.set('identity', 'uri', cloud_args['auth_url'], priority=True) + # call the function and check if data were obtained properly + tool.set_cloud_config_values(conf, mock_args) + if admin: + self.assertEqual(cloud_args['username'], + conf.get('identity', 'admin_username')) + self.assertEqual(cloud_args['password'], + conf.get('identity', 'admin_password')) + self.assertEqual(cloud_args['project_name'], + conf.get('identity', 'admin_tenant_name')) + else: + self.assertEqual(cloud_args['username'], + conf.get('identity', 'username')) + self.assertEqual(cloud_args['password'], + conf.get('identity', 'password')) + self.assertEqual(cloud_args['project_name'], + conf.get('identity', 'tenant_name')) + + @mock.patch('os_client_config.cloud_config.CloudConfig', + **{'non_admin': True}) + def test_init_manager_client_config(self, mock_args): + self._obtain_client_config_data(mock_args, False) + + @mock.patch('os_client_config.cloud_config.CloudConfig', + **{'non_admin': False}) + def test_init_manager_client_config_as_admin(self, mock_args): + self._obtain_client_config_data(mock_args, True) @mock.patch('os_client_config.cloud_config.CloudConfig') def test_init_manager_client_config_get_default(self, mock_args): mock_function = mock.Mock(return_value={}) func2mock = 'os_client_config.cloud_config.CloudConfig.config.get' self.useFixture(MonkeyPatch(func2mock, mock_function)) - manager = tool.ClientManager(self.conf, admin=False, args=mock_args) + manager = tool.ClientManager(self.conf, admin=False) # cloud_args is empty => check if default credentials were used self._check_credentials(manager, self.conf.get('identity', 'username'), self.conf.get('identity', 'password'), self.conf.get('identity', 'tenant_name')) - @mock.patch('os_client_config.cloud_config.CloudConfig') - def test_init_manager_client_config_override(self, mock_args): + def test_init_manager_client_config_override(self): self._override_setup() - manager = tool.ClientManager(self.conf, admin=False, args=mock_args) + manager = tool.ClientManager(self.conf, admin=False) # check if cloud_args credentials were overrided by the ones set in CLI self._check_credentials(manager, self.conf.get('identity', 'username'), self.conf.get('identity', 'password'), self.conf.get('identity', 'tenant_name')) - @mock.patch('os_client_config.cloud_config.CloudConfig') - def test_init_manager_client_config_admin_override(self, mock_args): + def test_init_manager_client_config_admin_override(self): self._override_setup() - manager = tool.ClientManager(self.conf, admin=True, args=mock_args) + manager = tool.ClientManager(self.conf, admin=True) # check if cloud_args credentials were overrided by admin ones self._check_credentials(manager, self.conf.get('identity', 'admin_username'),