diff --git a/openstackclient/common/client_config.py b/openstackclient/common/client_config.py new file mode 100644 index 0000000000..cc6cad5899 --- /dev/null +++ b/openstackclient/common/client_config.py @@ -0,0 +1,186 @@ +# 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. +# + +"""OpenStackConfig subclass for argument compatibility""" + +import logging + +from os_client_config.config import OpenStackConfig + + +LOG = logging.getLogger(__name__) + + +# Sublcass OpenStackConfig in order to munge config values +# before auth plugins are loaded +class OSC_Config(OpenStackConfig): + + def _auth_select_default_plugin(self, config): + """Select a default plugin based on supplied arguments + + Migrated from auth.select_auth_plugin() + """ + + identity_version = config.get('identity_api_version', '') + + if config.get('username', None) and not config.get('auth_type', None): + if identity_version == '3': + config['auth_type'] = 'v3password' + elif identity_version.startswith('2'): + config['auth_type'] = 'v2password' + else: + # let keystoneauth figure it out itself + config['auth_type'] = 'password' + elif config.get('token', None) and not config.get('auth_type', None): + if identity_version == '3': + config['auth_type'] = 'v3token' + elif identity_version.startswith('2'): + config['auth_type'] = 'v2token' + else: + # let keystoneauth figure it out itself + config['auth_type'] = 'token' + else: + # The ultimate default is similar to the original behaviour, + # but this time with version discovery + if not config.get('auth_type', None): + config['auth_type'] = 'password' + + LOG.debug("Auth plugin %s selected" % config['auth_type']) + return config + + def _auth_v2_arguments(self, config): + """Set up v2-required arguments from v3 info + + Migrated from auth.build_auth_params() + """ + + if ('auth_type' in config and config['auth_type'].startswith("v2")): + if 'project_id' in config['auth']: + config['auth']['tenant_id'] = config['auth']['project_id'] + if 'project_name' in config['auth']: + config['auth']['tenant_name'] = config['auth']['project_name'] + return config + + def _auth_v2_ignore_v3(self, config): + """Remove v3 arguemnts if present for v2 plugin + + Migrated from clientmanager.setup_auth() + """ + + # NOTE(hieulq): If USER_DOMAIN_NAME, USER_DOMAIN_ID, PROJECT_DOMAIN_ID + # or PROJECT_DOMAIN_NAME is present and API_VERSION is 2.0, then + # ignore all domain related configs. + if (config.get('identity_api_version', '').startswith('2') and + config.get('auth_type', None).endswith('password')): + domain_props = [ + 'project_domain_id', + 'project_domain_name', + 'user_domain_id', + 'user_domain_name', + ] + for prop in domain_props: + if config['auth'].pop(prop, None) is not None: + LOG.warning("Ignoring domain related config " + + prop + " because identity API version is 2.0") + return config + + def _auth_default_domain(self, config): + """Set a default domain from available arguments + + Migrated from clientmanager.setup_auth() + """ + + identity_version = config.get('identity_api_version', '') + auth_type = config.get('auth_type', None) + + # TODO(mordred): This is a usability improvement that's broadly useful + # We should port it back up into os-client-config. + default_domain = config.get('default_domain', None) + if (identity_version == '3' and + not auth_type.startswith('v2') and + default_domain): + + # NOTE(stevemar): If PROJECT_DOMAIN_ID or PROJECT_DOMAIN_NAME is + # present, then do not change the behaviour. Otherwise, set the + # PROJECT_DOMAIN_ID to 'OS_DEFAULT_DOMAIN' for better usability. + if ( + not config['auth'].get('project_domain_id') and + not config['auth'].get('project_domain_name') + ): + config['auth']['project_domain_id'] = default_domain + + # NOTE(stevemar): If USER_DOMAIN_ID or USER_DOMAIN_NAME is present, + # then do not change the behaviour. Otherwise, set the + # USER_DOMAIN_ID to 'OS_DEFAULT_DOMAIN' for better usability. + # NOTE(aloga): this should only be set if there is a username. + # TODO(dtroyer): Move this to os-client-config after the plugin has + # been loaded so we can check directly if the options are accepted. + if ( + auth_type in ("password", "v3password", "v3totp") and + not config['auth'].get('user_domain_id') and + not config['auth'].get('user_domain_name') + ): + config['auth']['user_domain_id'] = default_domain + return config + + def auth_config_hook(self, config): + """Allow examination of config values before loading auth plugin + + OpenStackClient will override this to perform additional chacks + on auth_type. + """ + + config = self._auth_select_default_plugin(config) + config = self._auth_v2_arguments(config) + config = self._auth_v2_ignore_v3(config) + config = self._auth_default_domain(config) + + LOG.debug("auth_config_hook(): %s" % config) + return config + + def _validate_auth_ksc(self, config, cloud): + """Old compatibility hack for OSC, no longer needed/wanted""" + return config + + def _validate_auth(self, config, loader): + """Validate auth plugin arguments""" + # May throw a keystoneauth1.exceptions.NoMatchingPlugin + + plugin_options = loader.get_options() + + for p_opt in plugin_options: + # if it's in config, win, move it and kill it from config dict + # if it's in config.auth but not in config we're good + # deprecated loses to current + # provided beats default, deprecated or not + winning_value = self._find_winning_auth_value(p_opt, config) + if not winning_value: + winning_value = self._find_winning_auth_value( + p_opt, config['auth']) + + # Clean up after ourselves + for opt in [p_opt.name] + [o.name for o in p_opt.deprecated]: + opt = opt.replace('-', '_') + config.pop(opt, None) + config['auth'].pop(opt, None) + + if winning_value: + # Prefer the plugin configuration dest value if the value's key + # is marked as depreciated. + if p_opt.dest is None: + config['auth'][p_opt.name.replace('-', '_')] = ( + winning_value) + else: + config['auth'][p_opt.dest] = winning_value + + return config diff --git a/openstackclient/identity/v3/identity_provider.py b/openstackclient/identity/v3/identity_provider.py index 0453e8883e..b6b0318870 100644 --- a/openstackclient/identity/v3/identity_provider.py +++ b/openstackclient/identity/v3/identity_provider.py @@ -204,11 +204,10 @@ class SetIdentityProvider(command.Command): if parsed_args.remote_id_file or parsed_args.remote_id: kwargs['remote_ids'] = remote_ids - identity_provider = federation_client.identity_providers.update( - parsed_args.identity_provider, **kwargs) - - identity_provider._info.pop('links', None) - return zip(*sorted(six.iteritems(identity_provider._info))) + federation_client.identity_providers.update( + parsed_args.identity_provider, + **kwargs + ) class ShowIdentityProvider(command.ShowOne): diff --git a/openstackclient/identity/v3/service_provider.py b/openstackclient/identity/v3/service_provider.py index 440eba4090..8548ae1fbd 100644 --- a/openstackclient/identity/v3/service_provider.py +++ b/openstackclient/identity/v3/service_provider.py @@ -182,12 +182,13 @@ class SetServiceProvider(command.Command): elif parsed_args.disable is True: enabled = False - service_provider = federation_client.service_providers.update( - parsed_args.service_provider, enabled=enabled, + federation_client.service_providers.update( + parsed_args.service_provider, + enabled=enabled, description=parsed_args.description, auth_url=parsed_args.auth_url, - sp_url=parsed_args.service_provider_url) - return zip(*sorted(six.iteritems(service_provider._info))) + sp_url=parsed_args.service_provider_url, + ) class ShowServiceProvider(command.ShowOne): diff --git a/openstackclient/shell.py b/openstackclient/shell.py index 7a042e1ec7..67c51998c8 100644 --- a/openstackclient/shell.py +++ b/openstackclient/shell.py @@ -25,6 +25,7 @@ from oslo_utils import importutils import six import openstackclient +from openstackclient.common import client_config as cloud_config from openstackclient.common import clientmanager from openstackclient.common import commandmanager @@ -127,9 +128,33 @@ class OpenStackShell(shell.OpenStackShell): def initialize_app(self, argv): super(OpenStackShell, self).initialize_app(argv) - # For now we need to build our own ClientManager so re-do what - # has already been done :( - # TODO(dtroyer): remove when osc-lib is fixed + # Argument precedence is really broken in multiple places + # so we're just going to fix it here until o-c-c and osc-lib + # get sorted out. + # TODO(dtroyer): remove when os-client-config and osc-lib are fixed + + # First, throw away what has already been done with o-c-c and + # use our own. + try: + cc = cloud_config.OSC_Config( + override_defaults={ + 'interface': None, + 'auth_type': self._auth_type, + }, + ) + except (IOError, OSError) as e: + self.log.critical("Could not read clouds.yaml configuration file") + self.print_help_if_requested() + raise e + + if not self.options.debug: + self.options.debug = None + self.cloud = cc.get_one_cloud( + cloud=self.options.cloud, + argparse=self.options, + ) + + # Then, re-create the client_manager with the correct arguments self.client_manager = clientmanager.ClientManager( cli_options=self.cloud, api_version=self.api_version, diff --git a/openstackclient/tests/identity/v3/test_identity_provider.py b/openstackclient/tests/identity/v3/test_identity_provider.py index b5d784ef32..d86ac11e3f 100644 --- a/openstackclient/tests/identity/v3/test_identity_provider.py +++ b/openstackclient/tests/identity/v3/test_identity_provider.py @@ -356,19 +356,11 @@ class TestIdentityProviderSet(TestIdentityProvider): ('remote_id', None) ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) + self.cmd.take_action(parsed_args) self.identity_providers_mock.update.assert_called_with( identity_fakes.idp_id, description=new_description, ) - self.assertEqual(self.columns, columns) - datalist = ( - identity_fakes.idp_description, - False, - identity_fakes.idp_id, - identity_fakes.idp_remote_ids - ) - self.assertEqual(datalist, data) def test_identity_provider_disable(self): """Disable Identity Provider @@ -402,22 +394,13 @@ class TestIdentityProviderSet(TestIdentityProvider): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) + self.cmd.take_action(parsed_args) self.identity_providers_mock.update.assert_called_with( identity_fakes.idp_id, enabled=False, remote_ids=identity_fakes.idp_remote_ids ) - self.assertEqual(self.columns, columns) - datalist = ( - identity_fakes.idp_description, - False, - identity_fakes.idp_id, - identity_fakes.idp_remote_ids - ) - self.assertEqual(datalist, data) - def test_identity_provider_enable(self): """Enable Identity Provider. @@ -448,12 +431,10 @@ class TestIdentityProviderSet(TestIdentityProvider): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) + self.cmd.take_action(parsed_args) self.identity_providers_mock.update.assert_called_with( identity_fakes.idp_id, enabled=True, remote_ids=identity_fakes.idp_remote_ids) - self.assertEqual(self.columns, columns) - self.assertEqual(self.datalist, data) def test_identity_provider_replace_remote_ids(self): """Enable Identity Provider. @@ -488,18 +469,10 @@ class TestIdentityProviderSet(TestIdentityProvider): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) + self.cmd.take_action(parsed_args) self.identity_providers_mock.update.assert_called_with( identity_fakes.idp_id, enabled=True, remote_ids=[self.new_remote_id]) - self.assertEqual(self.columns, columns) - datalist = ( - identity_fakes.idp_description, - True, - identity_fakes.idp_id, - [self.new_remote_id] - ) - self.assertEqual(datalist, data) def test_identity_provider_replace_remote_ids_file(self): """Enable Identity Provider. @@ -538,18 +511,10 @@ class TestIdentityProviderSet(TestIdentityProvider): mocker.return_value = self.new_remote_id with mock.patch("openstackclient.identity.v3.identity_provider." "utils.read_blob_file_contents", mocker): - columns, data = self.cmd.take_action(parsed_args) + self.cmd.take_action(parsed_args) self.identity_providers_mock.update.assert_called_with( identity_fakes.idp_id, enabled=True, remote_ids=[self.new_remote_id]) - self.assertEqual(self.columns, columns) - datalist = ( - identity_fakes.idp_description, - True, - identity_fakes.idp_id, - [self.new_remote_id] - ) - self.assertEqual(datalist, data) def test_identity_provider_no_options(self): def prepare(self): @@ -580,12 +545,7 @@ class TestIdentityProviderSet(TestIdentityProvider): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - - # expect take_action() to return (None, None) as - # neither --enable nor --disable was specified - self.assertEqual(self.columns, columns) - self.assertEqual(self.datalist, data) + self.cmd.take_action(parsed_args) class TestIdentityProviderShow(TestIdentityProvider): diff --git a/openstackclient/tests/identity/v3/test_service_provider.py b/openstackclient/tests/identity/v3/test_service_provider.py index f5270d839c..873ab1e759 100644 --- a/openstackclient/tests/identity/v3/test_service_provider.py +++ b/openstackclient/tests/identity/v3/test_service_provider.py @@ -289,7 +289,7 @@ class TestServiceProviderSet(TestServiceProvider): ('disable', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) + self.cmd.take_action(parsed_args) self.service_providers_mock.update.assert_called_with( service_fakes.sp_id, enabled=False, @@ -298,9 +298,6 @@ class TestServiceProviderSet(TestServiceProvider): sp_url=None ) - self.assertEqual(self.columns, columns) - self.assertEqual(self.datalist, data) - def test_service_provider_enable(self): """Enable Service Provider. @@ -327,19 +324,10 @@ class TestServiceProviderSet(TestServiceProvider): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) + self.cmd.take_action(parsed_args) self.service_providers_mock.update.assert_called_with( service_fakes.sp_id, enabled=True, description=None, auth_url=None, sp_url=None) - self.assertEqual(self.columns, columns) - datalist = ( - service_fakes.sp_auth_url, - service_fakes.sp_description, - True, - service_fakes.sp_id, - service_fakes.service_provider_url - ) - self.assertEqual(datalist, data) def test_service_provider_no_options(self): def prepare(self): @@ -372,20 +360,7 @@ class TestServiceProviderSet(TestServiceProvider): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - - # expect take_action() to return (None, None) as none of --disabled, - # --enabled, --description, --service-provider-url, --auth_url option - # was set. - self.assertEqual(self.columns, columns) - datalist = ( - service_fakes.sp_auth_url, - service_fakes.sp_description, - True, - service_fakes.sp_id, - service_fakes.service_provider_url - ) - self.assertEqual(datalist, data) + self.cmd.take_action(parsed_args) class TestServiceProviderShow(TestServiceProvider): diff --git a/tox.ini b/tox.ini index 29854b6775..9eb0a1a0af 100644 --- a/tox.ini +++ b/tox.ini @@ -55,8 +55,6 @@ setenv = OS_TEST_PATH=./functional/tests passenv = OS_* [testenv:venv] -# TODO(ihrachys): remove once infra supports constraints for this target -install_command = pip install -U {opts} {packages} commands = {posargs} [testenv:cover] @@ -71,8 +69,6 @@ commands = oslo_debug_helper -t openstackclient/tests {posargs} commands = python setup.py build_sphinx [testenv:releasenotes] -# TODO(ihrachys): remove once infra supports constraints for this target -install_command = pip install -U {opts} {packages} commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html [flake8]