From 58c39b7a80583dd54165cf292ae5dc621e9da361 Mon Sep 17 00:00:00 2001 From: Vladyslav Drok Date: Tue, 23 Aug 2016 12:13:23 +0300 Subject: [PATCH] Switch to none auth for standalone mode Currently, during the ironic shell client construction, if only os_auth_token and endpoint arguments are passed, custom HTTPClient class based on requests' sessions is used. This is unnecessary, as there is admin_token auth type in keystoneauth that does basically the same, eliminating the need for our custom implementation. Apart from that, there is a none auth, which requires only passing the desired endpoint to use, so we can use it too without having to specify fake token strings anymore. Let's use these auth methods instead and deprecate HTTPClient. Also this patch deprecates a bunch of arguments to client.get_client function, changing them to the standard keystoneauth naming. DocImpact Story: 1696791 Task: 11836 Depends-On: https://review.openstack.org/559116 Change-Id: Ifc7b45d047c8882a41021e1604b74d17eac2e6e8 --- ironicclient/client.py | 208 ++++++-------- ironicclient/common/http.py | 35 ++- ironicclient/osc/plugin.py | 2 +- ironicclient/shell.py | 79 ++++-- ironicclient/tests/unit/common/test_http.py | 12 + ironicclient/tests/unit/osc/test_plugin.py | 6 +- ironicclient/tests/unit/test_client.py | 263 +++++++++--------- ironicclient/tests/unit/test_shell.py | 155 ++++++----- ironicclient/tests/unit/v1/test_client.py | 13 +- ironicclient/v1/client.py | 27 +- lower-constraints.txt | 2 +- ...eprecate-http-client-8d664e5ec50ec403.yaml | 73 +++++ requirements.txt | 2 +- 13 files changed, 520 insertions(+), 357 deletions(-) create mode 100644 releasenotes/notes/deprecate-http-client-8d664e5ec50ec403.yaml diff --git a/ironicclient/client.py b/ironicclient/client.py index 0c4b11159..b785fafa3 100644 --- a/ironicclient/client.py +++ b/ironicclient/client.py @@ -10,146 +10,120 @@ # License for the specific language governing permissions and limitations # under the License. +import logging + from keystoneauth1 import loading as kaloading from oslo_utils import importutils from ironicclient.common.i18n import _ from ironicclient import exc +LOG = logging.getLogger(__name__) -def get_client(api_version, os_auth_token=None, ironic_url=None, - os_username=None, os_password=None, os_auth_url=None, - os_project_id=None, os_project_name=None, os_tenant_id=None, - os_tenant_name=None, os_region_name=None, - os_user_domain_id=None, os_user_domain_name=None, - os_project_domain_id=None, os_project_domain_name=None, - os_service_type=None, os_endpoint_type=None, - insecure=None, timeout=None, os_cacert=None, ca_file=None, - os_cert=None, cert_file=None, os_key=None, key_file=None, - os_ironic_api_version=None, max_retries=None, - retry_interval=None, session=None, **ignored_kwargs): + +# TODO(vdrok): remove in Stein +def convert_keystoneauth_opts(kwargs): + old_to_new_names = { + ('os_auth_token',): 'token', + ('os_username',): 'username', + ('os_password',): 'password', + ('os_auth_url',): 'auth_url', + ('os_project_id',): 'project_id', + ('os_project_name',): 'project_name', + ('os_tenant_id',): 'tenant_id', + ('os_tenant_name',): 'tenant_name', + ('os_region_name',): 'region_name', + ('os_user_domain_id',): 'user_domain_id', + ('os_user_domain_name',): 'user_domain_name', + ('os_project_domain_id',): 'project_domain_id', + ('os_project_domain_name',): 'project_domain_name', + ('os_service_type',): 'service_type', + ('os_endpoint_type',): 'interface', + ('ironic_url',): 'endpoint', + ('os_cacert', 'ca_file'): 'cafile', + ('os_cert', 'cert_file'): 'certfile', + ('os_key', 'key_file'): 'keyfile' + } + for olds, new in old_to_new_names.items(): + for old in olds: + if kwargs.get(old): + LOG.warning('The argument "%s" passed to get_client is ' + 'deprecated and will be removed in Stein release, ' + 'please use "%s" instead.', old, new) + kwargs.setdefault(new, kwargs[old]) + + +def get_client(api_version, auth_type=None, os_ironic_api_version=None, + max_retries=None, retry_interval=None, **kwargs): """Get an authenticated client, based on the credentials. :param api_version: the API version to use. Valid value: '1'. - :param os_auth_token: pre-existing token to re-use - :param ironic_url: ironic API endpoint - :param os_username: name of a user - :param os_password: user's password - :param os_auth_url: endpoint to authenticate against - :param os_tenant_name: name of a tenant (deprecated in favour of - os_project_name) - :param os_tenant_id: ID of a tenant (deprecated in favour of - os_project_id) - :param os_project_name: name of a project - :param os_project_id: ID of a project - :param os_region_name: name of a keystone region - :param os_user_domain_name: name of a domain the user belongs to - :param os_user_domain_id: ID of a domain the user belongs to - :param os_project_domain_name: name of a domain the project belongs to - :param os_project_domain_id: ID of a domain the project belongs to - :param os_service_type: the type of service to lookup the endpoint for - :param os_endpoint_type: the type (exposure) of the endpoint - :param insecure: allow insecure SSL (no cert verification) - :param timeout: allows customization of the timeout for client HTTP - requests - :param os_cacert: path to cacert file - :param ca_file: path to cacert file, deprecated in favour of os_cacert - :param os_cert: path to cert file - :param cert_file: path to cert file, deprecated in favour of os_cert - :param os_key: path to key file - :param key_file: path to key file, deprecated in favour of os_key - :param os_ironic_api_version: ironic API version to use or a list of - available API versions to attempt to negotiate. + :param auth_type: type of keystoneauth auth plugin loader to use. + :param os_ironic_api_version: ironic API version to use. :param max_retries: Maximum number of retries in case of conflict error :param retry_interval: Amount of time (in seconds) between retries in case - of conflict error - :param session: Keystone session to use - :param ignored_kwargs: all the other params that are passed. Left for - backwards compatibility. They are ignored. + of conflict error. + :param kwargs: all the other params that are passed to keystoneauth. """ # TODO(TheJulia): At some point, we should consider possibly noting # the "latest" flag for os_ironic_api_version to cause the client to # auto-negotiate to the greatest available version, however we do not # have the ability yet for a caller to cap the version, and will hold # off doing so until then. - os_service_type = os_service_type or 'baremetal' - os_endpoint_type = os_endpoint_type or 'publicURL' - project_id = (os_project_id or os_tenant_id) - project_name = (os_project_name or os_tenant_name) - kwargs = { + convert_keystoneauth_opts(kwargs) + if auth_type is None: + if 'endpoint' in kwargs: + if 'token' in kwargs: + auth_type = 'admin_token' + else: + auth_type = 'none' + elif 'token' in kwargs and 'auth_url' in kwargs: + auth_type = 'token' + else: + auth_type = 'password' + session = kwargs.get('session') + if not session: + loader = kaloading.get_plugin_loader(auth_type) + loader_options = loader.get_options() + # option.name looks like 'project-name', while dest will be the actual + # argument name to which the value will be passed to (project_name) + auth_options = [o.dest for o in loader_options] + # Include deprecated names as well + auth_options.extend([d.dest for o in loader_options + for d in o.deprecated]) + auth_kwargs = {k: v for (k, v) in kwargs.items() if k in auth_options} + auth_plugin = loader.load_from_options(**auth_kwargs) + # Let keystoneauth do the necessary parameter conversions + session_loader = kaloading.session.Session() + session_opts = {k: v for (k, v) in kwargs.items() if k in + [o.dest for o in session_loader.get_conf_options()]} + session = session_loader.load_from_options(auth=auth_plugin, + **session_opts) + + endpoint = kwargs.get('endpoint') + if not endpoint: + try: + # endpoint will be used to get hostname + # and port that will be used for API version caching. + endpoint = session.get_endpoint( + service_type=kwargs.get('service_type') or 'baremetal', + interface=kwargs.get('interface') or 'publicURL', + region_name=kwargs.get('region_name') + ) + except Exception as e: + raise exc.AmbiguousAuthSystem( + _('Must provide Keystone credentials or user-defined ' + 'endpoint, error was: %s') % e) + + ironicclient_kwargs = { 'os_ironic_api_version': os_ironic_api_version, 'max_retries': max_retries, 'retry_interval': retry_interval, + 'session': session, + 'endpoint_override': endpoint } - endpoint = ironic_url - cacert = os_cacert or ca_file - cert = os_cert or cert_file - key = os_key or key_file - if os_auth_token and endpoint: - kwargs.update({ - 'token': os_auth_token, - 'insecure': insecure, - 'ca_file': cacert, - 'cert_file': cert, - 'key_file': key, - 'timeout': timeout, - }) - elif os_auth_url: - auth_type = 'password' - auth_kwargs = { - 'auth_url': os_auth_url, - 'project_id': project_id, - 'project_name': project_name, - 'user_domain_id': os_user_domain_id, - 'user_domain_name': os_user_domain_name, - 'project_domain_id': os_project_domain_id, - 'project_domain_name': os_project_domain_name, - } - if os_username and os_password: - auth_kwargs.update({ - 'username': os_username, - 'password': os_password, - }) - elif os_auth_token: - auth_type = 'token' - auth_kwargs.update({ - 'token': os_auth_token, - }) - # Create new session only if it was not passed in - if not session: - loader = kaloading.get_plugin_loader(auth_type) - auth_plugin = loader.load_from_options(**auth_kwargs) - # Let keystoneauth do the necessary parameter conversions - session = kaloading.session.Session().load_from_options( - auth=auth_plugin, insecure=insecure, cacert=cacert, - cert=cert, key=key, timeout=timeout, - ) - exception_msg = _('Must provide Keystone credentials or user-defined ' - 'endpoint and token') - if not endpoint: - if session: - try: - # Pass the endpoint, it will be used to get hostname - # and port that will be used for API version caching. It will - # be also set as endpoint_override. - endpoint = session.get_endpoint( - service_type=os_service_type, - interface=os_endpoint_type, - region_name=os_region_name - ) - except Exception as e: - raise exc.AmbiguousAuthSystem( - _('%(message)s, error was: %(error)s') % - {'message': exception_msg, 'error': e}) - else: - # Neither session, nor valid auth parameters provided - raise exc.AmbiguousAuthSystem(exception_msg) - - # Always pass the session - kwargs['session'] = session - - return Client(api_version, endpoint, **kwargs) + return Client(api_version, **ironicclient_kwargs) def Client(version, *args, **kwargs): diff --git a/ironicclient/common/http.py b/ironicclient/common/http.py index 1d293a202..fbba7c0e4 100644 --- a/ironicclient/common/http.py +++ b/ironicclient/common/http.py @@ -81,11 +81,11 @@ def _extract_error_json(body): return error_json -def get_server(endpoint): - """Extract and return the server & port that we're connecting to.""" - if endpoint is None: +def get_server(url): + """Extract and return the server & port.""" + if url is None: return None, None - parts = urlparse.urlparse(endpoint) + parts = urlparse.urlparse(url) return parts.hostname, str(parts.port) @@ -205,7 +205,10 @@ class VersionNegotiationMixin(object): LOG.debug('Negotiated API version is %s', negotiated_ver) # Cache the negotiated version for this server - host, port = get_server(self.endpoint) + # TODO(vdrok): get rid of self.endpoint attribute in Stein + endpoint_override = (getattr(self, 'endpoint_override', None) or + getattr(self, 'endpoint', None)) + host, port = get_server(endpoint_override) filecache.save_data(host=host, port=port, data=negotiated_ver) return negotiated_ver @@ -266,6 +269,8 @@ def with_retries(func): class HTTPClient(VersionNegotiationMixin): def __init__(self, endpoint, **kwargs): + LOG.warning('HTTPClient class is deprecated and will be removed ' + 'in Stein release, please use SessionClient instead.') self.endpoint = endpoint self.endpoint_trimmed = _trim_endpoint_api_version(endpoint) self.auth_token = kwargs.get('token') @@ -556,13 +561,19 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter): api_version_select_state, max_retries, retry_interval, - endpoint, + endpoint=None, **kwargs): self.os_ironic_api_version = os_ironic_api_version self.api_version_select_state = api_version_select_state self.conflict_max_retries = max_retries self.conflict_retry_interval = retry_interval - self.endpoint = endpoint + # TODO(vdrok): remove this conditional in Stein + if endpoint and not kwargs.get('endpoint_override'): + LOG.warning('Passing "endpoint" argument to SessionClient ' + 'constructor is deprecated, such possibility will be ' + 'removed in Stein. Please use "endpoint_override" ' + 'instead.') + self.endpoint = endpoint super(SessionClient, self).__init__(**kwargs) @@ -662,8 +673,7 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter): return self._http_request(url, method, **kwargs) -def _construct_http_client(endpoint=None, - session=None, +def _construct_http_client(session=None, token=None, auth_ref=None, os_ironic_api_version=DEFAULT_VER, @@ -679,8 +689,8 @@ def _construct_http_client(endpoint=None, if session: kwargs.setdefault('service_type', 'baremetal') kwargs.setdefault('user_agent', 'python-ironicclient') - kwargs.setdefault('interface', kwargs.pop('endpoint_type', None)) - kwargs.setdefault('endpoint_override', endpoint) + kwargs.setdefault('interface', kwargs.pop('endpoint_type', + 'publicURL')) ignored = {'token': token, 'auth_ref': auth_ref, @@ -702,10 +712,11 @@ def _construct_http_client(endpoint=None, api_version_select_state=api_version_select_state, max_retries=max_retries, retry_interval=retry_interval, - endpoint=endpoint, **kwargs) else: + endpoint = None if kwargs: + endpoint = kwargs.pop('endpoint_override', None) LOG.warning('The following arguments are being ignored when ' 'constructing the client: %s'), ', '.join(kwargs) diff --git a/ironicclient/osc/plugin.py b/ironicclient/osc/plugin.py index 848aa37e8..832578e94 100644 --- a/ironicclient/osc/plugin.py +++ b/ironicclient/osc/plugin.py @@ -75,7 +75,7 @@ def make_client(instance): region_name=instance._region_name, # NOTE(vdrok): This will be set as endpoint_override, and the Client # class will be able to do the version stripping if needed - endpoint=instance.get_endpoint_for_service_type( + endpoint_override=instance.get_endpoint_for_service_type( API_NAME, interface=instance.interface, region_name=instance._region_name ) diff --git a/ironicclient/shell.py b/ironicclient/shell.py index b1a08cb0b..8343216e8 100644 --- a/ironicclient/shell.py +++ b/ironicclient/shell.py @@ -98,59 +98,75 @@ class IronicShell(object): help=_('DEPRECATED! Use --os-cacert.')) parser.add_argument('--os-username', + dest='username', default=cliutils.env('OS_USERNAME'), help=_('Defaults to env[OS_USERNAME]')) parser.add_argument('--os_username', + dest='username', help=argparse.SUPPRESS) parser.add_argument('--os-password', + dest='password', default=cliutils.env('OS_PASSWORD'), help=_('Defaults to env[OS_PASSWORD]')) parser.add_argument('--os_password', + dest='password', help=argparse.SUPPRESS) parser.add_argument('--os-tenant-id', + dest='tenant_id', default=cliutils.env('OS_TENANT_ID'), help=_('Defaults to env[OS_TENANT_ID]')) parser.add_argument('--os_tenant_id', + dest='tenant_id', help=argparse.SUPPRESS) parser.add_argument('--os-tenant-name', + dest='tenant_name', default=cliutils.env('OS_TENANT_NAME'), help=_('Defaults to env[OS_TENANT_NAME]')) parser.add_argument('--os_tenant_name', + dest='tenant_name', help=argparse.SUPPRESS) parser.add_argument('--os-auth-url', + dest='auth_url', default=cliutils.env('OS_AUTH_URL'), help=_('Defaults to env[OS_AUTH_URL]')) parser.add_argument('--os_auth_url', + dest='auth_url', help=argparse.SUPPRESS) parser.add_argument('--os-region-name', + dest='region_name', default=cliutils.env('OS_REGION_NAME'), help=_('Defaults to env[OS_REGION_NAME]')) parser.add_argument('--os_region_name', + dest='region_name', help=argparse.SUPPRESS) parser.add_argument('--os-auth-token', + dest='token', default=cliutils.env('OS_AUTH_TOKEN'), help=_('Defaults to env[OS_AUTH_TOKEN]')) parser.add_argument('--os_auth_token', + dest='token', help=argparse.SUPPRESS) parser.add_argument('--ironic-url', + dest='endpoint', default=cliutils.env('IRONIC_URL'), help=_('Defaults to env[IRONIC_URL]')) parser.add_argument('--ironic_url', + dest='endpoint', help=argparse.SUPPRESS) parser.add_argument('--ironic-api-version', @@ -164,15 +180,17 @@ class IronicShell(object): help=argparse.SUPPRESS) parser.add_argument('--os-service-type', + dest='service_type', default=cliutils.env('OS_SERVICE_TYPE'), help=_('Defaults to env[OS_SERVICE_TYPE] or ' '"baremetal"')) parser.add_argument('--os_service_type', + dest='service_type', help=argparse.SUPPRESS) parser.add_argument('--os-endpoint', - dest='ironic_url', + dest='endpoint', default=cliutils.env('OS_SERVICE_ENDPOINT'), help=_('Specify an endpoint to use instead of ' 'retrieving one from the service catalog ' @@ -180,26 +198,31 @@ class IronicShell(object): 'Defaults to env[OS_SERVICE_ENDPOINT].')) parser.add_argument('--os_endpoint', - dest='ironic_url', + dest='endpoint', help=argparse.SUPPRESS) parser.add_argument('--os-endpoint-type', + dest='interface', default=cliutils.env('OS_ENDPOINT_TYPE'), help=_('Defaults to env[OS_ENDPOINT_TYPE] or ' '"publicURL"')) parser.add_argument('--os_endpoint_type', + dest='interface', help=argparse.SUPPRESS) parser.add_argument('--os-user-domain-id', + dest='user_domain_id', default=cliutils.env('OS_USER_DOMAIN_ID'), help=_('Defaults to env[OS_USER_DOMAIN_ID].')) parser.add_argument('--os-user-domain-name', + dest='user_domain_name', default=cliutils.env('OS_USER_DOMAIN_NAME'), help=_('Defaults to env[OS_USER_DOMAIN_NAME].')) parser.add_argument('--os-project-id', + dest='project_id', default=cliutils.env('OS_PROJECT_ID'), help=_('Another way to specify tenant ID. ' 'This option is mutually exclusive with ' @@ -207,6 +230,7 @@ class IronicShell(object): 'Defaults to env[OS_PROJECT_ID].')) parser.add_argument('--os-project-name', + dest='project_name', default=cliutils.env('OS_PROJECT_NAME'), help=_('Another way to specify tenant name. ' 'This option is mutually exclusive with ' @@ -214,10 +238,12 @@ class IronicShell(object): 'Defaults to env[OS_PROJECT_NAME].')) parser.add_argument('--os-project-domain-id', + dest='project_domain_id', default=cliutils.env('OS_PROJECT_DOMAIN_ID'), help=_('Defaults to env[OS_PROJECT_DOMAIN_ID].')) parser.add_argument('--os-project-domain-name', + dest='project_domain_name', default=cliutils.env('OS_PROJECT_DOMAIN_NAME'), help=_('Defaults to env[OS_PROJECT_DOMAIN_NAME].')) @@ -354,38 +380,39 @@ class IronicShell(object): self.do_bash_completion() return 0 - if not (args.os_auth_token and (args.ironic_url or args.os_auth_url)): - if not args.os_username: + # Assume password auth if it does not seem like none, admin_token or + # token auth + if not args.endpoint and not (args.token and args.auth_url): + if not args.username: raise exc.CommandError(_("You must provide a username via " "either --os-username or via " "env[OS_USERNAME]")) - if not args.os_password: + if not args.password: # No password, If we've got a tty, try prompting for it if hasattr(sys.stdin, 'isatty') and sys.stdin.isatty(): # Check for Ctl-D try: - args.os_password = getpass.getpass( + args.password = getpass.getpass( 'OpenStack Password: ') except EOFError: pass # No password because we didn't have a tty or the # user Ctl-D when prompted. - if not args.os_password: + if not args.password: raise exc.CommandError(_("You must provide a password via " "either --os-password, " "env[OS_PASSWORD], " "or prompted response")) - if not (args.os_tenant_id or args.os_tenant_name or - args.os_project_id or args.os_project_name): + if not (args.tenant_id or args.tenant_name or + args.project_id or args.project_name): raise exc.CommandError( _("You must provide a project name or" " project id via --os-project-name, --os-project-id," - " env[OS_PROJECT_ID] or env[OS_PROJECT_NAME]. You may" - " use os-project and os-tenant interchangeably.")) + " env[OS_PROJECT_ID] or env[OS_PROJECT_NAME].")) - if not args.os_auth_url: + if not args.auth_url: raise exc.CommandError(_("You must provide an auth url via " "either --os-auth-url or via " "env[OS_AUTH_URL]")) @@ -397,17 +424,29 @@ class IronicShell(object): raise exc.CommandError(_("You must provide value >= 1 for " "--retry-interval")) client_args = ( - 'os_auth_token', 'ironic_url', 'os_username', 'os_password', - 'os_auth_url', 'os_project_id', 'os_project_name', 'os_tenant_id', - 'os_tenant_name', 'os_region_name', 'os_user_domain_id', - 'os_user_domain_name', 'os_project_domain_id', - 'os_project_domain_name', 'os_service_type', 'os_endpoint_type', - 'os_cacert', 'os_cert', 'os_key', 'max_retries', 'retry_interval', - 'timeout', 'insecure' + 'token', 'endpoint', 'username', 'password', 'auth_url', + 'project_id', 'project_name', 'tenant_id', 'tenant_name', + 'region_name', 'user_domain_id', 'user_domain_name', + 'project_domain_id', 'project_domain_name', 'service_type', + 'interface', 'max_retries', 'retry_interval', 'timeout', 'insecure' ) kwargs = {} for key in client_args: - kwargs[key] = getattr(args, key) + value = getattr(args, key) + # NOTE(vdrok): check for both None and ''. If the default value + # for option is set using cliutils.env function, default empty + # value is ''. If the default is not set explicitly, it is None. + if value not in (None, ''): + kwargs[key] = value + # NOTE(vdrok): this is to workaround the fact that these options are + # named differently in keystoneauth, depending on whether they are + # provided through CLI or loaded from conf options, here we unify them. + for cli_ssl_opt, conf_ssl_opt in [ + ('os_cacert', 'cafile'), ('os_cert', 'certfile'), + ('os_key', 'keyfile')]: + value = getattr(args, cli_ssl_opt) + if value not in (None, ''): + kwargs[conf_ssl_opt] = value kwargs['os_ironic_api_version'] = os_ironic_api_version client = ironicclient.client.get_client(api_major_version, **kwargs) if options.ironic_api_version in ('1', 'latest'): diff --git a/ironicclient/tests/unit/common/test_http.py b/ironicclient/tests/unit/common/test_http.py index be0fd6adc..8d4adb3d0 100644 --- a/ironicclient/tests/unit/common/test_http.py +++ b/ironicclient/tests/unit/common/test_http.py @@ -327,6 +327,11 @@ class VersionNegotiationMixinTest(utils.BaseTestCase): class HttpClientTest(utils.BaseTestCase): + @mock.patch.object(http.LOG, 'warning', autospec=True) + def test_http_client_deprecation(self, log_mock): + http.HTTPClient('http://localhost') + self.assertIn('deprecated', log_mock.call_args[0][0]) + def test_url_generation_trailing_slash_in_base(self): client = http.HTTPClient('http://localhost/') url = client._make_connection_url('/v1/resources') @@ -594,6 +599,13 @@ class HttpClientTest(utils.BaseTestCase): class SessionClientTest(utils.BaseTestCase): + @mock.patch.object(http.LOG, 'warning', autospec=True) + def test_session_client_endpoint_deprecation(self, log_mock): + http.SessionClient(os_ironic_api_version=1, session=mock.Mock(), + api_version_select_state='user', max_retries=5, + retry_interval=5, endpoint='abc') + self.assertIn('deprecated', log_mock.call_args[0][0]) + def test_server_exception_empty_body(self): error_body = _get_error_body() diff --git a/ironicclient/tests/unit/osc/test_plugin.py b/ironicclient/tests/unit/osc/test_plugin.py index f4cff9952..728c7f83d 100644 --- a/ironicclient/tests/unit/osc/test_plugin.py +++ b/ironicclient/tests/unit/osc/test_plugin.py @@ -34,7 +34,7 @@ class MakeClientTest(testtools.TestCase): allow_api_version_downgrade=False, session=instance.session, region_name=instance._region_name, - endpoint='endpoint') + endpoint_override='endpoint') instance.get_endpoint_for_service_type.assert_called_once_with( 'baremetal', region_name=instance._region_name, interface=instance.interface) @@ -54,7 +54,7 @@ class MakeClientTest(testtools.TestCase): allow_api_version_downgrade=True, session=instance.session, region_name=instance._region_name, - endpoint='endpoint') + endpoint_override='endpoint') instance.get_endpoint_for_service_type.assert_called_once_with( 'baremetal', region_name=instance._region_name, interface=instance.interface) @@ -72,7 +72,7 @@ class MakeClientTest(testtools.TestCase): allow_api_version_downgrade=True, session=instance.session, region_name=instance._region_name, - endpoint='endpoint') + endpoint_override='endpoint') instance.get_endpoint_for_service_type.assert_called_once_with( 'baremetal', region_name=instance._region_name, interface=instance.interface) diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index 17b51d569..c7ddafb15 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/ironicclient/tests/unit/test_client.py @@ -12,7 +12,9 @@ import mock +from keystoneauth1 import identity from keystoneauth1 import loading as kaloading +from keystoneauth1 import token_endpoint from ironicclient import client as iroclient from ironicclient.common import filecache @@ -24,39 +26,42 @@ from ironicclient.v1 import client as v1 class ClientTest(utils.BaseTestCase): - def test_get_client_with_auth_token_ironic_url(self): - kwargs = { - 'ironic_url': 'http://ironic.example.org:6385/', - 'os_auth_token': 'USER_AUTH_TOKEN', - } - client = iroclient.get_client('1', **kwargs) - - self.assertEqual('USER_AUTH_TOKEN', client.http_client.auth_token) - self.assertEqual('http://ironic.example.org:6385/', - client.http_client.endpoint) - + @mock.patch.object(iroclient.LOG, 'warning', autospec=True) @mock.patch.object(filecache, 'retrieve_data', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) @mock.patch.object(kaloading, 'get_plugin_loader', autospec=True) def _test_get_client(self, mock_ks_loader, mock_ks_session, - mock_retrieve_data, version=None, - auth='password', **kwargs): + mock_retrieve_data, warn_mock, version=None, + auth='password', warn_mock_call_count=0, **kwargs): session = mock_ks_session.return_value.load_from_options.return_value session.get_endpoint.return_value = 'http://localhost:6385/v1/f14b4123' + + class Opt(object): + def __init__(self, name): + self.dest = name + + session_loader_options = [ + Opt('insecure'), Opt('cafile'), Opt('certfile'), Opt('keyfile'), + Opt('timeout')] + mock_ks_session.return_value.get_conf_options.return_value = ( + session_loader_options) mock_ks_loader.return_value.load_from_options.return_value = 'auth' mock_retrieve_data.return_value = version client = iroclient.get_client('1', **kwargs) + self.assertEqual(warn_mock_call_count, warn_mock.call_count) + iroclient.convert_keystoneauth_opts(kwargs) mock_ks_loader.assert_called_once_with(auth) + session_opts = {k: v for (k, v) in kwargs.items() if k in + [o.dest for o in session_loader_options]} mock_ks_session.return_value.load_from_options.assert_called_once_with( - auth='auth', timeout=kwargs.get('timeout'), - insecure=kwargs.get('insecure'), cert=kwargs.get('cert'), - cacert=kwargs.get('cacert'), key=kwargs.get('key')) - session.get_endpoint.assert_called_once_with( - service_type=kwargs.get('os_service_type') or 'baremetal', - interface=kwargs.get('os_endpoint_type') or 'publicURL', - region_name=kwargs.get('os_region_name')) + auth='auth', **session_opts) + if not {'endpoint', 'ironic_url'}.intersection(kwargs): + session.get_endpoint.assert_called_once_with( + service_type=kwargs.get('service_type') or 'baremetal', + interface=kwargs.get('interface') or 'publicURL', + region_name=kwargs.get('region_name')) if 'os_ironic_api_version' in kwargs: # NOTE(TheJulia): This does not test the negotiation logic # as a request must be triggered in order for any verison @@ -71,6 +76,35 @@ class ClientTest(utils.BaseTestCase): port='6385') self.assertEqual(version or v1.DEFAULT_VER, client.http_client.os_ironic_api_version) + return client + + def test_get_client_only_ironic_url(self): + kwargs = {'ironic_url': 'http://localhost:6385/v1'} + client = self._test_get_client(auth='none', + warn_mock_call_count=1, **kwargs) + self.assertIsInstance(client.http_client, http.SessionClient) + self.assertEqual('http://localhost:6385/v1', + client.http_client.endpoint_override) + + def test_get_client_only_endpoint(self): + kwargs = {'endpoint': 'http://localhost:6385/v1'} + client = self._test_get_client(auth='none', **kwargs) + self.assertIsInstance(client.http_client, http.SessionClient) + self.assertEqual('http://localhost:6385/v1', + client.http_client.endpoint_override) + + def test_get_client_with_auth_token_ironic_url(self): + kwargs = { + 'ironic_url': 'http://localhost:6385/v1', + 'os_auth_token': 'USER_AUTH_TOKEN', + } + + client = self._test_get_client(auth='admin_token', + warn_mock_call_count=2, **kwargs) + + self.assertIsInstance(client.http_client, http.SessionClient) + self.assertEqual('http://localhost:6385/v1', + client.http_client.endpoint_override) def test_get_client_no_auth_token(self): kwargs = { @@ -80,7 +114,7 @@ class ClientTest(utils.BaseTestCase): 'os_auth_url': 'http://localhost:35357/v2.0', 'os_auth_token': '', } - self._test_get_client(**kwargs) + self._test_get_client(warn_mock_call_count=4, **kwargs) def test_get_client_service_and_endpoint_type_defaults(self): kwargs = { @@ -92,7 +126,7 @@ class ClientTest(utils.BaseTestCase): 'os_service_type': '', 'os_endpoint_type': '' } - self._test_get_client(**kwargs) + self._test_get_client(warn_mock_call_count=4, **kwargs) def test_get_client_with_region_no_auth_token(self): kwargs = { @@ -103,20 +137,7 @@ class ClientTest(utils.BaseTestCase): 'os_auth_url': 'http://localhost:35357/v2.0', 'os_auth_token': '', } - self._test_get_client(**kwargs) - - def test_get_client_no_url(self): - kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': '', - } - self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client, - '1', **kwargs) - # test the alias as well to ensure backwards compatibility - self.assertRaises(exc.AmbigiousAuthSystem, iroclient.get_client, - '1', **kwargs) + self._test_get_client(warn_mock_call_count=5, **kwargs) def test_get_client_incorrect_auth_params(self): kwargs = { @@ -125,37 +146,35 @@ class ClientTest(utils.BaseTestCase): 'os_auth_url': 'http://localhost:35357/v2.0', } self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client, - '1', **kwargs) + '1', warn_mock_call_count=3, **kwargs) def test_get_client_with_api_version_latest(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', 'os_ironic_api_version': "latest", } self._test_get_client(**kwargs) def test_get_client_with_api_version_list(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', + 'auth_token': '', 'os_ironic_api_version': ['1.1', '1.99'], } self._test_get_client(**kwargs) def test_get_client_with_api_version_numeric(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', 'os_ironic_api_version': "1.4", } self._test_get_client(**kwargs) @@ -165,26 +184,25 @@ class ClientTest(utils.BaseTestCase): # Make sure we don't coincidentally succeed self.assertNotEqual(v1.DEFAULT_VER, version) kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', } self._test_get_client(version=version, **kwargs) def test_get_client_with_auth_token(self): kwargs = { - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': 'USER_AUTH_TOKEN', + 'auth_url': 'http://localhost:35357/v2.0', + 'token': 'USER_AUTH_TOKEN', } self._test_get_client(auth='token', **kwargs) def test_get_client_with_region_name_auth_token(self): kwargs = { - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_region_name': 'REGIONONE', - 'os_auth_token': 'USER_AUTH_TOKEN', + 'auth_url': 'http://localhost:35357/v2.0', + 'region_name': 'REGIONONE', + 'token': 'USER_AUTH_TOKEN', } self._test_get_client(auth='token', **kwargs) @@ -209,46 +227,60 @@ class ClientTest(utils.BaseTestCase): '1', **kwargs) @mock.patch.object(kaloading.session, 'Session', autospec=True) - @mock.patch.object(kaloading, 'get_plugin_loader', autospec=True) def _test_loader_arguments_passed_correctly( - self, mock_ks_loader, mock_ks_session, - passed_kwargs, expected_kwargs): + self, mock_ks_session, passed_kwargs, expected_kwargs, + loader_class): session = mock_ks_session.return_value.load_from_options.return_value session.get_endpoint.return_value = 'http://localhost:6385/v1/f14b4123' - mock_ks_loader.return_value.load_from_options.return_value = 'auth' - iroclient.get_client('1', **passed_kwargs) + with mock.patch.object(loader_class, '__init__', + autospec=True) as init_mock: + init_mock.return_value = None + iroclient.get_client('1', **passed_kwargs) + iroclient.convert_keystoneauth_opts(passed_kwargs) - mock_ks_loader.return_value.load_from_options.assert_called_once_with( - **expected_kwargs) + init_mock.assert_called_once_with(mock.ANY, **expected_kwargs) + session_opts = {k: v for (k, v) in passed_kwargs.items() if k in + ['insecure', 'cacert', 'cert', 'key', 'timeout']} mock_ks_session.return_value.load_from_options.assert_called_once_with( - auth='auth', timeout=passed_kwargs.get('timeout'), - insecure=passed_kwargs.get('insecure'), - cert=passed_kwargs.get('cert'), - cacert=passed_kwargs.get('cacert'), key=passed_kwargs.get('key')) - session.get_endpoint.assert_called_once_with( - service_type=passed_kwargs.get('os_service_type') or 'baremetal', - interface=passed_kwargs.get('os_endpoint_type') or 'publicURL', - region_name=passed_kwargs.get('os_region_name')) + auth=mock.ANY, **session_opts) + if 'ironic_url' not in passed_kwargs: + service_type = passed_kwargs.get('service_type') or 'baremetal' + interface = passed_kwargs.get('interface') or 'publicURL' + session.get_endpoint.assert_called_once_with( + service_type=service_type, interface=interface, + region_name=passed_kwargs.get('region_name')) + + def test_loader_arguments_admin_token(self): + passed_kwargs = { + 'ironic_url': 'http://localhost:6385/v1', + 'os_auth_token': 'USER_AUTH_TOKEN', + } + expected_kwargs = { + 'endpoint': 'http://localhost:6385/v1', + 'token': 'USER_AUTH_TOKEN' + } + self._test_loader_arguments_passed_correctly( + passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, + loader_class=token_endpoint.Token + ) def test_loader_arguments_token(self): passed_kwargs = { 'os_auth_url': 'http://localhost:35357/v3', 'os_region_name': 'REGIONONE', 'os_auth_token': 'USER_AUTH_TOKEN', + 'os_project_name': 'admin' } expected_kwargs = { 'auth_url': 'http://localhost:35357/v3', - 'project_id': None, - 'project_name': None, - 'user_domain_id': None, - 'user_domain_name': None, - 'project_domain_id': None, - 'project_domain_name': None, + 'project_name': 'admin', 'token': 'USER_AUTH_TOKEN' } self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs) + passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, + loader_class=identity.Token + ) def test_loader_arguments_password_tenant_name(self): passed_kwargs = { @@ -262,17 +294,16 @@ class ClientTest(utils.BaseTestCase): } expected_kwargs = { 'auth_url': 'http://localhost:35357/v3', - 'project_id': None, 'project_name': 'PROJECT', 'user_domain_id': 'DEFAULT', - 'user_domain_name': None, 'project_domain_id': 'DEFAULT', - 'project_domain_name': None, 'username': 'user', 'password': '1234' } self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs) + passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, + loader_class=identity.Password + ) def test_loader_arguments_password_project_id(self): passed_kwargs = { @@ -287,47 +318,35 @@ class ClientTest(utils.BaseTestCase): expected_kwargs = { 'auth_url': 'http://localhost:35357/v3', 'project_id': '1000', - 'project_name': None, - 'user_domain_id': None, 'user_domain_name': 'domain1', - 'project_domain_id': None, 'project_domain_name': 'domain1', 'username': 'user', 'password': '1234' } self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs) + passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, + loader_class=identity.Password + ) @mock.patch.object(iroclient, 'Client', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) def test_correct_arguments_passed_to_client_constructor_noauth_mode( self, mock_ks_session, mock_client): + session = mock_ks_session.return_value.load_from_options.return_value kwargs = { 'ironic_url': 'http://ironic.example.org:6385/', 'os_auth_token': 'USER_AUTH_TOKEN', 'os_ironic_api_version': 'latest', - 'insecure': True, - 'max_retries': 10, - 'retry_interval': 10, - 'os_cacert': 'data' } iroclient.get_client('1', **kwargs) mock_client.assert_called_once_with( - '1', 'http://ironic.example.org:6385/', - **{ - 'os_ironic_api_version': 'latest', - 'max_retries': 10, - 'retry_interval': 10, - 'token': 'USER_AUTH_TOKEN', - 'insecure': True, - 'ca_file': 'data', - 'cert_file': None, - 'key_file': None, - 'timeout': None, - 'session': None - } + '1', **{'os_ironic_api_version': 'latest', + 'max_retries': None, + 'retry_interval': None, + 'session': session, + 'endpoint_override': 'http://ironic.example.org:6385/'} ) - self.assertFalse(mock_ks_session.called) + self.assertFalse(session.get_endpoint.called) @mock.patch.object(iroclient, 'Client', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) @@ -345,13 +364,11 @@ class ClientTest(utils.BaseTestCase): } iroclient.get_client('1', **kwargs) mock_client.assert_called_once_with( - '1', session.get_endpoint.return_value, - **{ - 'os_ironic_api_version': None, - 'max_retries': None, - 'retry_interval': None, - 'session': session, - } + '1', **{'os_ironic_api_version': None, + 'max_retries': None, + 'retry_interval': None, + 'session': session, + 'endpoint_override': session.get_endpoint.return_value} ) @mock.patch.object(iroclient, 'Client', autospec=True) @@ -364,13 +381,11 @@ class ClientTest(utils.BaseTestCase): } iroclient.get_client('1', **kwargs) mock_client.assert_called_once_with( - '1', session.get_endpoint.return_value, - **{ - 'os_ironic_api_version': None, - 'max_retries': None, - 'retry_interval': None, - 'session': session, - } + '1', **{'os_ironic_api_version': None, + 'max_retries': None, + 'retry_interval': None, + 'session': session, + 'endpoint_override': session.get_endpoint.return_value} ) self.assertFalse(mock_ks_session.called) diff --git a/ironicclient/tests/unit/test_shell.py b/ironicclient/tests/unit/test_shell.py index 672c4f29d..74d0084ac 100644 --- a/ironicclient/tests/unit/test_shell.py +++ b/ironicclient/tests/unit/test_shell.py @@ -39,6 +39,13 @@ FAKE_ENV = {'OS_USERNAME': 'username', 'OS_PROJECT_NAME': 'project_name', 'OS_AUTH_URL': V2_URL} +FAKE_ENV_WITH_SSL = FAKE_ENV.copy() +FAKE_ENV_WITH_SSL.update({ + 'OS_CACERT': 'cacert', + 'OS_CERT': 'cert', + 'OS_KEY': 'key', +}) + FAKE_ENV_KEYSTONE_V2 = { 'OS_USERNAME': 'username', 'OS_PASSWORD': 'password', @@ -162,16 +169,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, 'node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': ironic_shell.LATEST_VERSION, @@ -181,6 +182,32 @@ class ShellTest(utils.BaseTestCase): # Make sure we are actually prompted. mock_getpass.assert_called_with('OpenStack Password: ') + @mock.patch.object(client, 'get_client', autospec=True, + side_effect=keystone_exc.ConnectFailure) + @mock.patch('sys.stdin', side_effect=mock.MagicMock, autospec=True) + @mock.patch('getpass.getpass', return_value='password', autospec=True) + def test_password(self, mock_getpass, mock_stdin, mock_client): + self.make_env(environ_dict=FAKE_ENV_WITH_SSL) + # We will get a ConnectFailure because there is no keystone. + self.assertRaises(keystone_exc.ConnectFailure, + self.shell, 'node-list') + expected_kwargs = { + 'auth_url': FAKE_ENV_WITH_SSL['OS_AUTH_URL'], + 'username': FAKE_ENV_WITH_SSL['OS_USERNAME'], + 'password': FAKE_ENV_WITH_SSL['OS_PASSWORD'], + 'project_name': FAKE_ENV_WITH_SSL['OS_PROJECT_NAME'], + 'cafile': FAKE_ENV_WITH_SSL['OS_CACERT'], + 'certfile': FAKE_ENV_WITH_SSL['OS_CERT'], + 'keyfile': FAKE_ENV_WITH_SSL['OS_KEY'], + 'max_retries': http.DEFAULT_MAX_RETRIES, + 'retry_interval': http.DEFAULT_RETRY_INTERVAL, + 'timeout': 600, + 'os_ironic_api_version': ironic_shell.LATEST_VERSION, + 'insecure': False + } + mock_client.assert_called_once_with(1, **expected_kwargs) + self.assertFalse(mock_getpass.called) + @mock.patch.object(client, 'get_client', autospec=True, side_effect=keystone_exc.ConnectFailure) @mock.patch('getpass.getpass', return_value='password', autospec=True) @@ -190,23 +217,49 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, 'node-list') expected_kwargs = { - 'ironic_url': '', - 'os_auth_url': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_URL'], - 'os_tenant_id': '', - 'os_tenant_name': '', - 'os_username': '', 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': '', - 'os_auth_token': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_TOKEN'], - 'os_project_id': '', - 'os_project_name': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_PROJECT_NAME'], - 'os_project_domain_id': '', 'os_project_domain_name': '', - 'os_region_name': '', 'os_service_type': '', - 'os_endpoint_type': '', 'os_cacert': None, 'os_cert': None, - 'os_key': None, 'max_retries': http.DEFAULT_MAX_RETRIES, + 'auth_url': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_URL'], + 'token': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_TOKEN'], + 'project_name': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_PROJECT_NAME'], + 'max_retries': http.DEFAULT_MAX_RETRIES, + 'retry_interval': http.DEFAULT_RETRY_INTERVAL, + 'timeout': 600, + 'os_ironic_api_version': ironic_shell.LATEST_VERSION, + 'insecure': False + } + mock_client.assert_called_once_with(1, **expected_kwargs) + self.assertFalse(mock_getpass.called) + + @mock.patch.object(client, 'get_client', autospec=True) + @mock.patch('getpass.getpass', return_value='password', autospec=True) + def test_admin_token_auth(self, mock_getpass, mock_client): + self.make_env(environ_dict={ + 'IRONIC_URL': 'http://192.168.1.1/v1', + 'OS_AUTH_TOKEN': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_TOKEN']}) + expected_kwargs = { + 'endpoint': 'http://192.168.1.1/v1', + 'token': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_TOKEN'], + 'max_retries': http.DEFAULT_MAX_RETRIES, + 'retry_interval': http.DEFAULT_RETRY_INTERVAL, + 'timeout': 600, + 'os_ironic_api_version': ironic_shell.LATEST_VERSION, + 'insecure': False + } + self.shell('node-list') + mock_client.assert_called_once_with(1, **expected_kwargs) + self.assertFalse(mock_getpass.called) + + @mock.patch.object(client, 'get_client', autospec=True) + @mock.patch('getpass.getpass', return_value='password', autospec=True) + def test_none_auth(self, mock_getpass, mock_client): + self.make_env(environ_dict={'IRONIC_URL': 'http://192.168.1.1/v1'}) + expected_kwargs = { + 'endpoint': 'http://192.168.1.1/v1', + 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': ironic_shell.LATEST_VERSION, 'timeout': 600, 'insecure': False } + self.shell('node-list') mock_client.assert_called_once_with(1, **expected_kwargs) self.assertFalse(mock_getpass.called) @@ -274,16 +327,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, 'node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': '1.10', @@ -300,16 +347,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, 'node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': ironic_shell.LATEST_VERSION, @@ -326,16 +367,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, '--ironic-api-version 1.11 node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': '1.11', @@ -352,16 +387,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, '--ironic-api-version 1 node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': ironic_shell.LATEST_VERSION, diff --git a/ironicclient/tests/unit/v1/test_client.py b/ironicclient/tests/unit/v1/test_client.py index 33f4de329..40414d2a5 100644 --- a/ironicclient/tests/unit/v1/test_client.py +++ b/ironicclient/tests/unit/v1/test_client.py @@ -31,7 +31,7 @@ class ClientTest(utils.BaseTestCase): os_ironic_api_version=os_ironic_api_version) http_client_mock.assert_called_once_with( - endpoint, token=token, + endpoint_override=endpoint, token=token, os_ironic_api_version=os_ironic_api_version, api_version_select_state='user') @@ -45,7 +45,7 @@ class ClientTest(utils.BaseTestCase): allow_api_version_downgrade=True) http_client_mock.assert_called_once_with( - endpoint, token=token, + token=token, endpoint_override=endpoint, os_ironic_api_version=os_ironic_api_version, api_version_select_state='default') @@ -70,7 +70,7 @@ class ClientTest(utils.BaseTestCase): cache_mock.assert_called_once_with(host='ironic', port='6385') http_client_mock.assert_called_once_with( - endpoint, token=token, + endpoint_override=endpoint, token=token, os_ironic_api_version=os_ironic_api_version, api_version_select_state='cached') @@ -84,7 +84,7 @@ class ClientTest(utils.BaseTestCase): cache_mock.assert_called_once_with(host='ironic', port='6385') http_client_mock.assert_called_once_with( - endpoint, token=token, + endpoint_override=endpoint, token=token, os_ironic_api_version=client.DEFAULT_VER, api_version_select_state='default') @@ -92,8 +92,7 @@ class ClientTest(utils.BaseTestCase): self.assertRaises(exc.EndpointException, client.Client, session='fake_session', - insecure=True, - endpoint_override='http://ironic:6385') + insecure=True) def test_client_initialized_managers(self, http_client_mock): cl = client.Client('http://ironic:6385', token='safe_token', @@ -113,7 +112,7 @@ class ClientTest(utils.BaseTestCase): cl.negotiate_api_version() http_client_mock.assert_called_once_with( - endpoint, api_version_select_state='user', + api_version_select_state='user', endpoint_override=endpoint, os_ironic_api_version='latest', token=token) # TODO(TheJulia): We should verify that negotiate_version # is being called in the client and returns a version, diff --git a/ironicclient/v1/client.py b/ironicclient/v1/client.py index 975355702..d7dd55381 100644 --- a/ironicclient/v1/client.py +++ b/ironicclient/v1/client.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import logging + from ironicclient.common import filecache from ironicclient.common import http from ironicclient.common.http import DEFAULT_VER @@ -26,20 +28,29 @@ from ironicclient.v1 import portgroup from ironicclient.v1 import volume_connector from ironicclient.v1 import volume_target +LOG = logging.getLogger(__name__) + class Client(object): """Client for the Ironic v1 API. :param string endpoint: A user-supplied endpoint URL for the ironic - service. + service. DEPRECATED, use endpoint_override instead. + :param string endpoint_override: A user-supplied endpoint URL for the + ironic service. :param function token: Provides token for authentication. :param integer timeout: Allows customization of the timeout for client http requests. (optional) """ - def __init__(self, endpoint=None, *args, **kwargs): + def __init__(self, endpoint=None, endpoint_override=None, *args, **kwargs): """Initialize a new client for the Ironic v1 API.""" allow_downgrade = kwargs.pop('allow_api_version_downgrade', False) + if endpoint_override is None and endpoint is not None: + LOG.warning('Passing "endpoint" parameter to Client constructor ' + 'is deprecated, and it will be removed in Stein ' + 'release. Please use "endpoint_override" instead.') + endpoint_override = endpoint if kwargs.get('os_ironic_api_version'): # TODO(TheJulia): We should sanity check os_ironic_api_version # against our maximum suported version, so the client fails @@ -58,14 +69,14 @@ class Client(object): else: kwargs['api_version_select_state'] = "user" else: - if not endpoint: + if not endpoint_override: raise exc.EndpointException( - _("Must provide 'endpoint' if os_ironic_api_version " - "isn't specified")) + _("Must provide 'endpoint_override' if " + "'os_ironic_api_version' isn't specified")) # If the user didn't specify a version, use a cached version if # one has been stored - host, netport = http.get_server(endpoint) + host, netport = http.get_server(endpoint_override) saved_version = filecache.retrieve_data(host=host, port=netport) if saved_version: kwargs['api_version_select_state'] = "cached" @@ -74,8 +85,8 @@ class Client(object): kwargs['api_version_select_state'] = "default" kwargs['os_ironic_api_version'] = DEFAULT_VER - self.http_client = http._construct_http_client( - endpoint, *args, **kwargs) + kwargs['endpoint_override'] = endpoint_override + self.http_client = http._construct_http_client(*args, **kwargs) self.chassis = chassis.ChassisManager(self.http_client) self.node = node.NodeManager(self.http_client) diff --git a/lower-constraints.txt b/lower-constraints.txt index 43ecb45dc..59a058681 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -46,7 +46,7 @@ openstacksdk==0.11.2 os-client-config==1.28.0 os-service-types==1.2.0 os-testr==1.0.0 -osc-lib==1.8.0 +osc-lib==1.10.0 oslo.concurrency==3.25.0 oslo.config==5.2.0 oslo.context==2.19.2 diff --git a/releasenotes/notes/deprecate-http-client-8d664e5ec50ec403.yaml b/releasenotes/notes/deprecate-http-client-8d664e5ec50ec403.yaml new file mode 100644 index 000000000..c38ae7c09 --- /dev/null +++ b/releasenotes/notes/deprecate-http-client-8d664e5ec50ec403.yaml @@ -0,0 +1,73 @@ +--- +features: + - | + The client now supports ``none`` authorization method, which should be + used if the Identity service is not present in the deployment that the + client talks to. To use it: + + - openstack baremetal CLI -- supported starting with ``osc-lib`` version + ``1.10.0``, by providing ``--os-auth-type none`` and ``--os-endpoint`` + argument to ``openstack`` command + + - ironic CLI -- just specify the ``--ironic-url`` or ``--os-endpoint`` + argument in the ``ironic`` command (or set the corresponding environment + variable) + + - python API -- specify the ``endpoint_override`` argument to the + ``client.get_client()`` method (in addition to the required + ``api_version``) +deprecations: + - | + ``common.http.HTTPClient`` class is deprecated and will be removed in + the Stein release. If you initialize the ironic client via + ``v1.client.Client`` class directly, please pass the `keystoneauth + `_ session to the Client + constructor, so that ``common.http.SessionClient`` is used instead. + - | + As part of standardizing argument naming to the one used by `keystoneauth + `_, the following + arguments to ``client.get_client`` method are deprecated and will be + removed in Stein release: + + * ``os_auth_token``: use ``token`` instead + + * ``os_username``: use ``username`` instead + + * ``os_password``: use ``password`` instead + + * ``os_auth_url``: use ``auth_url`` instead + + * ``os_project_id``: use ``project_id`` instead + + * ``os_project_name``: use ``project_name`` instead + + * ``os_tenant_id``: use ``tenant_id`` instead + + * ``os_tenant_name``: use ``tenant_name`` instead + + * ``os_region_name``: use ``region_name`` instead + + * ``os_user_domain_id``: use ``user_domain_id`` instead + + * ``os_user_domain_name``: use ``user_domain_name`` instead + + * ``os_project_domain_id``: use ``project_domain_id`` instead + + * ``os_project_domain_name``: use ``project_domain_name`` instead + + * ``os_service_type``: use ``service_type`` instead + + * ``os_endpoint_type``: use ``interface`` instead + + * ``ironic_url``: use ``endpoint`` instead + + * ``os_cacert``, ``ca_file``: use ``cafile`` instead + + * ``os_cert``, ``cert_file``: use ``certfile`` instead + + * ``os_key``, ``key_file``: use ``keyfile`` instead + - | + The ``endpoint`` argument to the ``v1.client.Client`` constructor is + deprecated and will be removed in Stein release. Instead, please use the + standard `keystoneauth `_ + argument name ``endpoint_override``. diff --git a/requirements.txt b/requirements.txt index 9277e3263..918958573 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ appdirs>=1.3.0 # MIT License dogpile.cache>=0.6.2 # BSD jsonschema<3.0.0,>=2.6.0 # MIT keystoneauth1>=3.4.0 # Apache-2.0 -osc-lib>=1.8.0 # Apache-2.0 +osc-lib>=1.10.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0