From 099a2c38b99dff6a0909c0a3ba2909f1aea58644 Mon Sep 17 00:00:00 2001 From: Alvaro Lopez Garcia <aloga@ifca.unican.es> Date: Fri, 27 May 2016 11:03:15 +0200 Subject: [PATCH] Refactor setting defaults for some scope parameters The code is setting defaults for some scope parameters, cheking if the name ends with some specific substring (namely ending in "password") causing failures in some plugins that end with the same string, but do not allow those parameters (like "user_domain_id" in "v3oidcpassword"). Closes-Bug: #1582774 Change-Id: Id7036db3b783b135353d035dc4c1df7c808d6474 --- openstackclient/api/auth.py | 5 +- openstackclient/api/auth_plugin.py | 2 +- openstackclient/common/clientmanager.py | 78 ++++++++++--------- .../notes/bug-1582774-3bba709ef61e33b7.yaml | 5 ++ 4 files changed, 53 insertions(+), 37 deletions(-) create mode 100644 releasenotes/notes/bug-1582774-3bba709ef61e33b7.yaml diff --git a/openstackclient/api/auth.py b/openstackclient/api/auth.py index 9981f6d57c..d5412594a0 100644 --- a/openstackclient/api/auth.py +++ b/openstackclient/api/auth.py @@ -105,10 +105,12 @@ def select_auth_plugin(options): def build_auth_params(auth_plugin_name, cmd_options): - auth_params = dict(cmd_options.auth) if auth_plugin_name: LOG.debug('auth_type: %s', auth_plugin_name) auth_plugin_loader = base.get_plugin_loader(auth_plugin_name) + auth_params = {opt.dest: opt.default + for opt in base.get_plugin_options(auth_plugin_name)} + auth_params.update(dict(cmd_options.auth)) # grab tenant from project for v2.0 API compatibility if auth_plugin_name.startswith("v2"): if 'project_id' in auth_params: @@ -121,6 +123,7 @@ def build_auth_params(auth_plugin_name, cmd_options): LOG.debug('no auth_type') # delay the plugin choice, grab every option auth_plugin_loader = None + auth_params = dict(cmd_options.auth) plugin_options = set([o.replace('-', '_') for o in get_options_list()]) for option in plugin_options: LOG.debug('fetching option %s', option) diff --git a/openstackclient/api/auth_plugin.py b/openstackclient/api/auth_plugin.py index 36dc51605f..4434bc8ffc 100644 --- a/openstackclient/api/auth_plugin.py +++ b/openstackclient/api/auth_plugin.py @@ -38,7 +38,7 @@ class TokenEndpoint(token_endpoint.AdminToken): is for bootstrapping the Keystone database. """ - def load_from_options(self, url, token): + def load_from_options(self, url, token, **kwargs): """A plugin for static authentication with an existing token :param string url: Service endpoint diff --git a/openstackclient/common/clientmanager.py b/openstackclient/common/clientmanager.py index 5dbfb41712..3c35b52933 100644 --- a/openstackclient/common/clientmanager.py +++ b/openstackclient/common/clientmanager.py @@ -140,8 +140,49 @@ class ClientManager(object): # prior to dereferrencing auth_ref. self._auth_setup_completed = False + def _set_default_scope_options(self): + # TODO(mordred): This is a usability improvement that's broadly useful + # We should port it back up into os-client-config. + default_domain = self._cli_options.default_domain + + # 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 (self._api_version.get('identity') == '2.0' and + self.auth_plugin_name.endswith('password')): + domain_props = ['project_domain_name', 'project_domain_id', + 'user_domain_name', 'user_domain_id'] + for prop in domain_props: + if self._auth_params.pop(prop, None) is not None: + LOG.warning("Ignoring domain related configs " + + prop + " because identity API version is 2.0") + return + + # NOTE(aloga): The scope parameters below only apply to v3 and v3 + # related auth plugins, so we stop the parameter checking if v2 is + # being used. + if (self._api_version.get('identity') != '3' or + self.auth_plugin_name.startswith('v2')): + return + + # 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 ('project_domain_id' in self._auth_params and + not self._auth_params.get('project_domain_id') and + not self._auth_params.get('project_domain_name')): + self._auth_params['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. + if ('user_domain_id' in self._auth_params and + not self._auth_params.get('user_domain_id') and + not self._auth_params.get('user_domain_name')): + self._auth_params['user_domain_id'] = default_domain + def setup_auth(self): - """Set up authentication. + """Set up authentication This is deferred until authentication is actually attempted because it gets in the way of things that do not require auth. @@ -169,40 +210,7 @@ class ClientManager(object): self._cli_options, ) - # TODO(mordred): This is a usability improvement that's broadly useful - # We should port it back up into os-client-config. - default_domain = self._cli_options.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 (self._api_version.get('identity') == '3' and - self.auth_plugin_name.endswith('password') and - not self._auth_params.get('project_domain_id') and - not self.auth_plugin_name.startswith('v2') and - not self._auth_params.get('project_domain_name')): - self._auth_params['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. - if (self._api_version.get('identity') == '3' and - self.auth_plugin_name.endswith('password') and - not self.auth_plugin_name.startswith('v2') and - not self._auth_params.get('user_domain_id') and - not self._auth_params.get('user_domain_name')): - self._auth_params['user_domain_id'] = default_domain - - # 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 (self._api_version.get('identity') == '2.0' and - self.auth_plugin_name.endswith('password')): - domain_props = ['project_domain_name', 'project_domain_id', - 'user_domain_name', 'user_domain_id'] - for prop in domain_props: - if self._auth_params.pop(prop, None) is not None: - LOG.warning("Ignoring domain related configs " + - prop + " because identity API version is 2.0") + self._set_default_scope_options() # For compatibility until all clients can be updated if 'project_name' in self._auth_params: diff --git a/releasenotes/notes/bug-1582774-3bba709ef61e33b7.yaml b/releasenotes/notes/bug-1582774-3bba709ef61e33b7.yaml new file mode 100644 index 0000000000..35c72171f8 --- /dev/null +++ b/releasenotes/notes/bug-1582774-3bba709ef61e33b7.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Fix setting defaults for some scope parameters, that were putting invalid + scope parameters for some auth plugins. + [Bug `1582774 <https://bugs.launchpad.net/bugs/1582774>`_]