Use keystoneauth instead of keystoneclient

This change also enables client to be authenticated via 'token'
keystone auth method, allowing to pass os_auth_token instead of
os_username and os_password.

Closes-bug: #1541411
Closes-bug: #1548907
Change-Id: I9739c95069ebf8d60f8362be154844e74334beb2

ironicclient/client.py

@@ -10,110 +10,136 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import keystoneclient
+from keystoneauth1 import loading as kaloading
 
 from ironicclient.common.i18n import _
 from ironicclient.common import utils
 from ironicclient import exc
 
 
-def _get_ksclient(**kwargs):
+def get_client(api_version, os_auth_token=None, ironic_url=None,
-    """Get an endpoint and auth token from Keystone.
+               os_username=None, os_password=None, os_auth_url=None,
-
+               os_project_id=None, os_project_name=None, os_tenant_id=None,
-    :param kwargs: keyword args containing credentials:
+               os_tenant_name=None, os_region_name=None,
-            * username: name of user
+               os_user_domain_id=None, os_user_domain_name=None,
-            * password: user's password
+               os_project_domain_id=None, os_project_domain_name=None,
-            * auth_url: endpoint to authenticate against
+               os_service_type=None, os_endpoint_type=None,
-            * insecure: allow insecure SSL (no cert verification)
+               insecure=None, timeout=None, os_cacert=None, ca_file=None,
-            * tenant_{name|id}: name or ID of tenant
+               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):
-    client = keystoneclient.client.Client(
+    """Get an authenticated client, based on the credentials.
-        username=kwargs.get('username'),
-        password=kwargs.get('password'),
-        tenant_id=kwargs.get('tenant_id'),
-        tenant_name=kwargs.get('tenant_name'),
-        auth_url=kwargs.get('auth_url'),
-        insecure=kwargs.get('insecure'))
-    # FIXME(jlvillal): Work around for Bug 1539839 as client.authenticate is
-    # not called.
-    client.authenticate()
-    return client
-
-
-def _get_endpoint(client, **kwargs):
-    """Get an endpoint using the provided keystone client."""
-    attr = None
-    filter_value = None
-    if kwargs.get('region_name'):
-        attr = 'region'
-        filter_value = kwargs.get('region_name')
-    return client.service_catalog.url_for(
-        service_type=kwargs.get('service_type') or 'baremetal',
-        attr=attr,
-        filter_value=filter_value,
-        endpoint_type=kwargs.get('endpoint_type') or 'publicURL')
-
-
-def get_client(api_version, **kwargs):
-    """Get an authenticated client, based on the credentials in args.
 
     :param api_version: the API version to use. Valid value: '1'.
-    :param kwargs: keyword args containing credentials, either:
+    :param os_auth_token: pre-existing token to re-use
-            * os_auth_token: pre-existing token to re-use
+    :param ironic_url: ironic API endpoint
-            * ironic_url: ironic API endpoint
+    :param os_username: name of a user
-            or:
+    :param os_password: user's password
-            * os_username: name of user
+    :param os_auth_url: endpoint to authenticate against
-            * os_password: user's password
+    :param os_tenant_name: name of a tenant (deprecated in favour of
-            * os_auth_url: endpoint to authenticate against
+        os_project_name)
-            * insecure: allow insecure SSL (no cert verification)
+    :param os_tenant_id: ID of a tenant (deprecated in favour of
-            * os_tenant_{name|id}: name or ID of tenant
+        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
+    :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.
     """
-
+    os_service_type = os_service_type or 'baremetal'
-    if kwargs.get('os_auth_token') and kwargs.get('ironic_url'):
+    os_endpoint_type = os_endpoint_type or 'publicURL'
-        token = kwargs.get('os_auth_token')
+    project_id = (os_project_id or os_tenant_id)
-        endpoint = kwargs.get('ironic_url')
+    project_name = (os_project_name or os_tenant_name)
-        auth_ref = None
+    kwargs = {
-    elif (kwargs.get('os_username') and
+        'os_ironic_api_version': os_ironic_api_version,
-          kwargs.get('os_password') and
+        'max_retries': max_retries,
-          kwargs.get('os_auth_url') and
+        'retry_interval': retry_interval,
-          (kwargs.get('os_tenant_id') or kwargs.get('os_tenant_name'))):
-
-        ks_kwargs = {
-            'username': kwargs.get('os_username'),
-            'password': kwargs.get('os_password'),
-            'tenant_id': kwargs.get('os_tenant_id'),
-            'tenant_name': kwargs.get('os_tenant_name'),
-            'auth_url': kwargs.get('os_auth_url'),
-            'service_type': kwargs.get('os_service_type'),
-            'endpoint_type': kwargs.get('os_endpoint_type'),
-            'insecure': kwargs.get('insecure'),
-        }
-        _ksclient = _get_ksclient(**ks_kwargs)
-        token = (kwargs.get('os_auth_token')
-                 if kwargs.get('os_auth_token')
-                 else _ksclient.auth_token)
-
-        ks_kwargs['region_name'] = kwargs.get('os_region_name')
-        endpoint = (kwargs.get('ironic_url') or
-                    _get_endpoint(_ksclient, **ks_kwargs))
-
-        auth_ref = _ksclient.auth_ref
-
-    else:
-        e = (_('Must provide Keystone credentials or user-defined endpoint '
-               'and token'))
-        raise exc.AmbiguousAuthSystem(e)
-
-    cli_kwargs = {
-        'token': token,
-        'auth_ref': auth_ref,
     }
-    for key in ('insecure', 'timeout', 'ca_file', 'cert_file', 'key_file',
+    endpoint = ironic_url
-                'os_ironic_api_version', 'max_retries', 'retry_interval'):
+    cacert = os_cacert or ca_file
-        cli_kwargs[key] = kwargs.get(key)
+    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,
+            )
 
-    return Client(api_version, endpoint, **cli_kwargs)
+    exception_msg = _('Must provide Keystone credentials or user-defined '
+                      'endpoint and token')
+    if not endpoint:
+        if session:
+            try:
+                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(
+                    exception_msg + _(', error was: %s') % 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)
 
 
 def Client(version, *args, **kwargs):

ironicclient/common/http.py

@@ -25,7 +25,7 @@ import ssl
 import textwrap
 import time
 
-from keystoneclient import adapter
+from keystoneauth1 import adapter
 from oslo_utils import strutils
 import requests
 import six

ironicclient/shell.py

@@ -22,15 +22,9 @@ import logging
 import sys
 
 import httplib2
-from keystoneclient.auth.identity import v2 as v2_auth
+from keystoneauth1.loading import session as kasession
-from keystoneclient.auth.identity import v3 as v3_auth
-from keystoneclient import discover
-from keystoneclient import exceptions as ks_exc
-from keystoneclient import session as kssession
 from oslo_utils import encodeutils
 import six
-import six.moves.urllib.parse as urlparse
-
 
 import ironicclient
 from ironicclient import client as iroclient
@@ -46,46 +40,6 @@ LATEST_API_VERSION = ('1', 'latest')
 
 class IronicShell(object):
 
-    def _append_global_identity_args(self, parser):
-        # FIXME(dhu): these are global identity (Keystone) arguments which
-        # should be consistent and shared by all service clients. Therefore,
-        # they should be provided by python-keystoneclient. We will need to
-        # refactor this code once this functionality is avaible in
-        # python-keystoneclient.
-
-        # Register arguments needed for a Session
-        kssession.Session.register_cli_options(parser)
-
-        parser.add_argument('--os-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',
-                            default=cliutils.env('OS_USER_DOMAIN_NAME'),
-                            help='Defaults to env[OS_USER_DOMAIN_NAME].')
-
-        parser.add_argument('--os-project-id',
-                            default=cliutils.env('OS_PROJECT_ID'),
-                            help='Another way to specify tenant ID. '
-                                 'This option is mutually exclusive with '
-                                 ' --os-tenant-id. '
-                                 'Defaults to env[OS_PROJECT_ID].')
-
-        parser.add_argument('--os-project-name',
-                            default=cliutils.env('OS_PROJECT_NAME'),
-                            help='Another way to specify tenant name. '
-                                 'This option is mutually exclusive with '
-                                 ' --os-tenant-name. '
-                                 'Defaults to env[OS_PROJECT_NAME].')
-
-        parser.add_argument('--os-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',
-                            default=cliutils.env('OS_PROJECT_DOMAIN_NAME'),
-                            help='Defaults to env[OS_PROJECT_DOMAIN_NAME].')
-
     def get_base_parser(self):
         parser = argparse.ArgumentParser(
             prog='ironic',
@@ -96,13 +50,9 @@ class IronicShell(object):
             formatter_class=HelpFormatter,
         )
 
-        # FIXME(gyee): this method should come from python-keystoneclient.
-        # Will refactor this code once it is available.
-        # https://bugs.launchpad.net/python-keystoneclient/+bug/1332337
-
         # Register global Keystone args first so their defaults are respected.
         # See https://bugs.launchpad.net/python-ironicclient/+bug/1463581
-        self._append_global_identity_args(parser)
+        kasession.register_argparse_arguments(parser)
 
         # Global arguments
         parser.add_argument('-h', '--help',
@@ -213,6 +163,7 @@ class IronicShell(object):
                             help=argparse.SUPPRESS)
 
         parser.add_argument('--os-endpoint',
+                            dest='ironic_url',
                             default=cliutils.env('OS_SERVICE_ENDPOINT'),
                             help='Specify an endpoint to use instead of '
                                  'retrieving one from the service catalog '
@@ -220,6 +171,7 @@ class IronicShell(object):
                                  'Defaults to env[OS_SERVICE_ENDPOINT].')
 
         parser.add_argument('--os_endpoint',
+                            dest='ironic_url',
                             help=argparse.SUPPRESS)
 
         parser.add_argument('--os-endpoint-type',
@@ -230,6 +182,36 @@ class IronicShell(object):
         parser.add_argument('--os_endpoint_type',
                             help=argparse.SUPPRESS)
 
+        parser.add_argument('--os-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',
+                            default=cliutils.env('OS_USER_DOMAIN_NAME'),
+                            help='Defaults to env[OS_USER_DOMAIN_NAME].')
+
+        parser.add_argument('--os-project-id',
+                            default=cliutils.env('OS_PROJECT_ID'),
+                            help='Another way to specify tenant ID. '
+                                 'This option is mutually exclusive with '
+                                 ' --os-tenant-id. '
+                                 'Defaults to env[OS_PROJECT_ID].')
+
+        parser.add_argument('--os-project-name',
+                            default=cliutils.env('OS_PROJECT_NAME'),
+                            help='Another way to specify tenant name. '
+                                 'This option is mutually exclusive with '
+                                 ' --os-tenant-name. '
+                                 'Defaults to env[OS_PROJECT_NAME].')
+
+        parser.add_argument('--os-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',
+                            default=cliutils.env('OS_PROJECT_DOMAIN_NAME'),
+                            help='Defaults to env[OS_PROJECT_DOMAIN_NAME].')
+
         parser.add_argument('--max-retries', type=int,
                             help='Maximum number of retries in case of '
                             'conflict error (HTTP 409). '
@@ -286,96 +268,6 @@ class IronicShell(object):
         commands.remove('bash-completion')
         print(' '.join(commands | options))
 
-    def _discover_auth_versions(self, session, auth_url):
-        # discover the API versions the server is supporting base on the
-        # given URL
-        v2_auth_url = None
-        v3_auth_url = None
-        try:
-            ks_discover = discover.Discover(session=session, auth_url=auth_url)
-            v2_auth_url = ks_discover.url_for('2.0')
-            v3_auth_url = ks_discover.url_for('3.0')
-        except ks_exc.ClientException:
-            # Identity service may not support discover API version.
-            # Let's try to figure out the API version from the original URL.
-            url_parts = urlparse.urlparse(auth_url)
-            (scheme, netloc, path, params, query, fragment) = url_parts
-            path = path.lower()
-            if path.startswith('/v3'):
-                v3_auth_url = auth_url
-            elif path.startswith('/v2'):
-                v2_auth_url = auth_url
-            else:
-                # not enough information to determine the auth version
-                msg = _('Unable to determine the Keystone version '
-                        'to authenticate with using the given '
-                        'auth_url. Identity service may not support API '
-                        'version discovery. Please provide a versioned '
-                        'auth_url instead. %s') % auth_url
-                raise exc.CommandError(msg)
-
-        return (v2_auth_url, v3_auth_url)
-
-    def _get_keystone_v3_auth(self, v3_auth_url, **kwargs):
-        auth_token = kwargs.pop('auth_token', None)
-        if auth_token:
-            return v3_auth.Token(v3_auth_url, auth_token)
-        else:
-            return v3_auth.Password(v3_auth_url, **kwargs)
-
-    def _get_keystone_v2_auth(self, v2_auth_url, **kwargs):
-        auth_token = kwargs.pop('auth_token', None)
-        if auth_token:
-            return v2_auth.Token(v2_auth_url, auth_token,
-                                 tenant_id=kwargs.pop('project_id', None),
-                                 tenant_name=kwargs.pop('project_name', None))
-        else:
-            return v2_auth.Password(
-                v2_auth_url,
-                username=kwargs.pop('username', None),
-                password=kwargs.pop('password', None),
-                tenant_id=kwargs.pop('project_id', None),
-                tenant_name=kwargs.pop('project_name', None))
-
-    def _get_keystone_auth(self, session, auth_url, **kwargs):
-        # FIXME(dhu): this code should come from keystoneclient
-
-        # discover the supported keystone versions using the given url
-        (v2_auth_url, v3_auth_url) = self._discover_auth_versions(
-            session=session,
-            auth_url=auth_url)
-
-        # Determine which authentication plugin to use. First inspect the
-        # auth_url to see the supported version. If both v3 and v2 are
-        # supported, then use the highest version if possible.
-        auth = None
-        if v3_auth_url and v2_auth_url:
-            user_domain_name = kwargs.get('user_domain_name', None)
-            user_domain_id = kwargs.get('user_domain_id', None)
-            project_domain_name = kwargs.get('project_domain_name', None)
-            project_domain_id = kwargs.get('project_domain_id', None)
-
-            # support both v2 and v3 auth. Use v3 if domain information is
-            # provided.
-            if (user_domain_name or user_domain_id or project_domain_name or
-                    project_domain_id):
-                auth = self._get_keystone_v3_auth(v3_auth_url, **kwargs)
-            else:
-                auth = self._get_keystone_v2_auth(v2_auth_url, **kwargs)
-        elif v3_auth_url:
-            # support only v3
-            auth = self._get_keystone_v3_auth(v3_auth_url, **kwargs)
-        elif v2_auth_url:
-            # support only v2
-            auth = self._get_keystone_v2_auth(v2_auth_url, **kwargs)
-        else:
-            msg = _('Unable to determine the Keystone version '
-                    'to authenticate with using the given '
-                    'auth_url.')
-            raise exc.CommandError(msg)
-
-        return auth
-
     def _check_version(self, api_version):
         if api_version == 'latest':
             return LATEST_API_VERSION
@@ -434,7 +326,8 @@ class IronicShell(object):
             self.do_bash_completion()
             return 0
 
-        if not (args.os_auth_token and (args.ironic_url or args.os_endpoint)):
+        if not (args.os_auth_token and (args.ironic_url or args.os_endpoint or
+                                        args.os_auth_url)):
             if not args.os_username:
                 raise exc.CommandError(_("You must provide a username via "
                                          "either --os-username or via "
@@ -470,67 +363,26 @@ class IronicShell(object):
                                          "either --os-auth-url or via "
                                          "env[OS_AUTH_URL]"))
 
-        endpoint = args.ironic_url or args.os_endpoint
-        service_type = args.os_service_type or 'baremetal'
-        project_id = args.os_project_id or args.os_tenant_id
-        project_name = args.os_project_name or args.os_tenant_name
-
-        if (args.os_auth_token and (args.ironic_url or args.os_endpoint)):
-            kwargs = {
-                'token': args.os_auth_token,
-                'insecure': args.insecure,
-                'timeout': args.timeout,
-                'ca_file': args.os_cacert,
-                'cert_file': args.os_cert,
-                'key_file': args.os_key,
-                'auth_ref': None,
-            }
-        elif (args.os_username and
-              args.os_password and
-              args.os_auth_url and
-              (project_id or project_name)):
-
-            keystone_session = kssession.Session.load_from_cli_options(args)
-
-            kwargs = {
-                'username': args.os_username,
-                'user_domain_id': args.os_user_domain_id,
-                'user_domain_name': args.os_user_domain_name,
-                'password': args.os_password,
-                'auth_token': args.os_auth_token,
-                'project_id': project_id,
-                'project_name': project_name,
-                'project_domain_id': args.os_project_domain_id,
-                'project_domain_name': args.os_project_domain_name,
-            }
-            keystone_auth = self._get_keystone_auth(keystone_session,
-                                                    args.os_auth_url,
-                                                    **kwargs)
-            if not endpoint:
-                svc_type = args.os_service_type
-                region_name = args.os_region_name
-                endpoint = keystone_auth.get_endpoint(keystone_session,
-                                                      service_type=svc_type,
-                                                      region_name=region_name)
-
-            endpoint_type = args.os_endpoint_type or 'publicURL'
-            kwargs = {
-                'session': keystone_session,
-                'auth': keystone_auth,
-                'service_type': service_type,
-                'endpoint_type': endpoint_type,
-                'region_name': args.os_region_name,
-            }
-        kwargs['os_ironic_api_version'] = os_ironic_api_version
         if args.max_retries < 0:
             raise exc.CommandError(_("You must provide value >= 0 for "
                                      "--max-retries"))
         if args.retry_interval < 1:
             raise exc.CommandError(_("You must provide value >= 1 for "
                                      "--retry-interval"))
-        kwargs['max_retries'] = args.max_retries
+        client_args = (
-        kwargs['retry_interval'] = args.retry_interval
+            'os_auth_token', 'ironic_url', 'os_username', 'os_password',
-        client = iroclient.Client(api_major_version, endpoint, **kwargs)
+            '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'
+        )
+        kwargs = {}
+        for key in client_args:
+            kwargs[key] = getattr(args, key)
+        kwargs['os_ironic_api_version'] = os_ironic_api_version
+        client = iroclient.get_client(api_major_version, **kwargs)
 
         try:
             args.func(client, args)

ironicclient/tests/unit/test_client.py

@@ -10,11 +10,11 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import fixtures
 import mock
 
-import ironicclient
+from keystoneauth1 import loading as kaloading
-from ironicclient.client import get_client
+
+from ironicclient import client as iroclient
 from ironicclient.common import filecache
 from ironicclient.common import http
 from ironicclient import exc
@@ -22,28 +22,51 @@ from ironicclient.tests.unit import utils
 from ironicclient.v1 import client as v1
 
 
-def fake_get_ksclient(**kwargs):
-    return utils.FakeKeystone('KSCLIENT_AUTH_TOKEN')
-
-
 class ClientTest(utils.BaseTestCase):
 
     def test_get_client_with_auth_token_ironic_url(self):
-        self.useFixture(fixtures.MonkeyPatch(
-            'ironicclient.client._get_ksclient', fake_get_ksclient))
         kwargs = {
             'ironic_url': 'http://ironic.example.org:6385/',
             'os_auth_token': 'USER_AUTH_TOKEN',
         }
-        client = get_client('1', **kwargs)
+        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(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):
+        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'
+        mock_retrieve_data.return_value = version
+
+        client = iroclient.get_client('1', **kwargs)
+
+        mock_ks_loader.assert_called_once_with(auth)
+        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'))
+        if 'os_ironic_api_version' in kwargs:
+            self.assertEqual(0, mock_retrieve_data.call_count)
+        else:
+            mock_retrieve_data.assert_called_once_with(
+                host='localhost',
+                port='6385')
+            self.assertEqual(version or v1.DEFAULT_VER,
+                             client.http_client.os_ironic_api_version)
+
     def test_get_client_no_auth_token(self):
-        self.useFixture(fixtures.MonkeyPatch(
-            'ironicclient.client._get_ksclient', fake_get_ksclient))
         kwargs = {
             'os_tenant_name': 'TENANT_NAME',
             'os_username': 'USERNAME',
@@ -51,17 +74,21 @@ class ClientTest(utils.BaseTestCase):
             'os_auth_url': 'http://localhost:35357/v2.0',
             'os_auth_token': '',
         }
-        client = get_client('1', **kwargs)
+        self._test_get_client(**kwargs)
 
-        self.assertEqual('KSCLIENT_AUTH_TOKEN', client.http_client.auth_token)
+    def test_get_client_service_and_endpoint_type_defaults(self):
-        self.assertEqual('http://localhost:6385/v1/f14b41234',
+        kwargs = {
-                         client.http_client.endpoint)
+            'os_tenant_name': 'TENANT_NAME',
-        self.assertEqual(fake_get_ksclient().auth_ref,
+            'os_username': 'USERNAME',
-                         client.http_client.auth_ref)
+            'os_password': 'PASSWORD',
+            'os_auth_url': 'http://localhost:35357/v2.0',
+            'os_auth_token': '',
+            'os_service_type': '',
+            'os_endpoint_type': ''
+        }
+        self._test_get_client(**kwargs)
 
     def test_get_client_with_region_no_auth_token(self):
-        self.useFixture(fixtures.MonkeyPatch(
-            'ironicclient.client._get_ksclient', fake_get_ksclient))
         kwargs = {
             'os_tenant_name': 'TENANT_NAME',
             'os_username': 'USERNAME',
@@ -70,135 +97,56 @@ class ClientTest(utils.BaseTestCase):
             'os_auth_url': 'http://localhost:35357/v2.0',
             'os_auth_token': '',
         }
-        client = get_client('1', **kwargs)
+        self._test_get_client(**kwargs)
 
-        self.assertEqual('KSCLIENT_AUTH_TOKEN', client.http_client.auth_token)
+    def test_get_client_no_url(self):
-        self.assertEqual('http://regionhost:6385/v1/f14b41234',
-                         client.http_client.endpoint)
-        self.assertEqual(fake_get_ksclient().auth_ref,
-                         client.http_client.auth_ref)
-
-    def test_get_client_with_auth_token(self):
-        self.useFixture(fixtures.MonkeyPatch(
-            'ironicclient.client._get_ksclient', fake_get_ksclient))
-        kwargs = {
-            'os_tenant_name': 'TENANT_NAME',
-            'os_username': 'USERNAME',
-            'os_password': 'PASSWORD',
-            'os_auth_url': 'http://localhost:35357/v2.0',
-            'os_auth_token': 'USER_AUTH_TOKEN',
-        }
-        client = get_client('1', **kwargs)
-
-        self.assertEqual('USER_AUTH_TOKEN', client.http_client.auth_token)
-        self.assertEqual('http://localhost:6385/v1/f14b41234',
-                         client.http_client.endpoint)
-        self.assertEqual(fake_get_ksclient().auth_ref,
-                         client.http_client.auth_ref)
-
-    def test_get_client_with_region_name_auth_token(self):
-        self.useFixture(fixtures.MonkeyPatch(
-            'ironicclient.client._get_ksclient', fake_get_ksclient))
-        kwargs = {
-            'os_tenant_name': 'TENANT_NAME',
-            'os_username': 'USERNAME',
-            'os_password': 'PASSWORD',
-            'os_auth_url': 'http://localhost:35357/v2.0',
-            'os_region_name': 'REGIONONE',
-            'os_auth_token': 'USER_AUTH_TOKEN',
-        }
-        client = get_client('1', **kwargs)
-
-        self.assertEqual('USER_AUTH_TOKEN', client.http_client.auth_token)
-        self.assertEqual('http://regionhost:6385/v1/f14b41234',
-                         client.http_client.endpoint)
-        self.assertEqual(fake_get_ksclient().auth_ref,
-                         client.http_client.auth_ref)
-
-    def test_get_client_no_url_and_no_token(self):
-        self.useFixture(fixtures.MonkeyPatch(
-            'ironicclient.client._get_ksclient', fake_get_ksclient))
         kwargs = {
             'os_tenant_name': 'TENANT_NAME',
             'os_username': 'USERNAME',
             'os_password': 'PASSWORD',
             'os_auth_url': '',
-            'os_auth_token': '',
         }
-        self.assertRaises(exc.AmbiguousAuthSystem, get_client, '1', **kwargs)
+        self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client,
+                          '1', **kwargs)
         # test the alias as well to ensure backwards compatibility
-        self.assertRaises(exc.AmbigiousAuthSystem, get_client, '1', **kwargs)
+        self.assertRaises(exc.AmbigiousAuthSystem, iroclient.get_client,
+                          '1', **kwargs)
 
-    def test_ensure_auth_ref_propagated(self):
+    def test_get_client_incorrect_auth_params(self):
-        ksclient = fake_get_ksclient
-        self.useFixture(fixtures.MonkeyPatch(
-            'ironicclient.client._get_ksclient', ksclient))
         kwargs = {
             'os_tenant_name': 'TENANT_NAME',
             'os_username': 'USERNAME',
-            'os_password': 'PASSWORD',
             'os_auth_url': 'http://localhost:35357/v2.0',
-            'os_auth_token': '',
         }
-        client = get_client('1', **kwargs)
+        self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client,
-
+                          '1', **kwargs)
-        self.assertEqual(ksclient().auth_ref, client.http_client.auth_ref)
-
-    @mock.patch.object(filecache, 'retrieve_data', autospec=True)
-    @mock.patch.object(ironicclient.client, '_get_ksclient', autospec=True)
-    def _get_client_with_api_version(self, version, mock_ksclient,
-                                     mock_retrieve_data):
-        mock_ksclient.return_value = fake_get_ksclient()
-        kwargs = {
-            'os_tenant_name': 'TENANT_NAME',
-            'os_username': 'USERNAME',
-            'os_password': 'PASSWORD',
-            'os_auth_url': 'http://localhost:35357/v2.0',
-            'os_auth_token': '',
-            'os_ironic_api_version': version,
-        }
-        client = get_client('1', **kwargs)
-        self.assertEqual(0, mock_retrieve_data.call_count)
-        self.assertEqual(version, client.http_client.os_ironic_api_version)
-        self.assertEqual('user', client.http_client.api_version_select_state)
 
     def test_get_client_with_api_version_latest(self):
-        self._get_client_with_api_version("latest")
-
-    def test_get_client_with_api_version_numeric(self):
-        self._get_client_with_api_version("1.4")
-
-    @mock.patch.object(filecache, 'retrieve_data', autospec=True)
-    @mock.patch.object(ironicclient.client, '_get_ksclient', autospec=True)
-    def test_get_client_default_version_set_no_cache(self, mock_ksclient,
-                                                     mock_retrieve_data):
-        mock_ksclient.return_value = fake_get_ksclient()
-        mock_retrieve_data.return_value = None
         kwargs = {
             'os_tenant_name': 'TENANT_NAME',
             'os_username': 'USERNAME',
             'os_password': 'PASSWORD',
             'os_auth_url': 'http://localhost:35357/v2.0',
             'os_auth_token': '',
+            'os_ironic_api_version': "latest",
         }
-        client = get_client('1', **kwargs)
+        self._test_get_client(**kwargs)
-        mock_retrieve_data.assert_called_once_with(
-            host=utils.DEFAULT_TEST_HOST,
-            port=utils.DEFAULT_TEST_PORT)
-        self.assertEqual(v1.DEFAULT_VER,
-                         client.http_client.os_ironic_api_version)
-        self.assertEqual('default',
-                         client.http_client.api_version_select_state)
 
-    @mock.patch.object(filecache, 'retrieve_data', autospec=True)
+    def test_get_client_with_api_version_numeric(self):
-    @mock.patch.object(ironicclient.client, '_get_ksclient', autospec=True)
+        kwargs = {
-    def test_get_client_default_version_set_cached(self, mock_ksclient,
+            'os_tenant_name': 'TENANT_NAME',
-                                                   mock_retrieve_data):
+            'os_username': 'USERNAME',
+            'os_password': 'PASSWORD',
+            'os_auth_url': 'http://localhost:35357/v2.0',
+            'os_auth_token': '',
+            'os_ironic_api_version': "1.4",
+        }
+        self._test_get_client(**kwargs)
+
+    def test_get_client_default_version_set_cached(self):
         version = '1.3'
         # Make sure we don't coincidentally succeed
         self.assertNotEqual(v1.DEFAULT_VER, version)
-        mock_ksclient.return_value = fake_get_ksclient()
-        mock_retrieve_data.return_value = version
         kwargs = {
             'os_tenant_name': 'TENANT_NAME',
             'os_username': 'USERNAME',
@@ -206,11 +154,208 @@ class ClientTest(utils.BaseTestCase):
             'os_auth_url': 'http://localhost:35357/v2.0',
             'os_auth_token': '',
         }
-        client = get_client('1', **kwargs)
+        self._test_get_client(version=version, **kwargs)
-        mock_retrieve_data.assert_called_once_with(host=mock.ANY,
+
-                                                   port=mock.ANY)
+    def test_get_client_with_auth_token(self):
-        self.assertEqual(version, client.http_client.os_ironic_api_version)
+        kwargs = {
-        self.assertEqual('cached', client.http_client.api_version_select_state)
+            'os_auth_url': 'http://localhost:35357/v2.0',
+            'os_auth_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',
+        }
+        self._test_get_client(auth='token', **kwargs)
+
+    def test_get_client_only_session_passed(self):
+        session = mock.Mock()
+        session.get_endpoint.return_value = 'http://localhost:35357/v2.0'
+        kwargs = {
+            'session': session,
+        }
+        iroclient.get_client('1', **kwargs)
+        session.get_endpoint.assert_called_once_with(service_type='baremetal',
+                                                     interface='publicURL',
+                                                     region_name=None)
+
+    def test_get_client_incorrect_session_passed(self):
+        session = mock.Mock()
+        session.get_endpoint.side_effect = Exception('boo')
+        kwargs = {
+            'session': session,
+        }
+        self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client,
+                          '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):
+        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)
+
+        mock_ks_loader.return_value.load_from_options.assert_called_once_with(
+            **expected_kwargs)
+        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'))
+
+    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',
+        }
+        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,
+            'token': 'USER_AUTH_TOKEN'
+        }
+        self._test_loader_arguments_passed_correctly(
+            passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs)
+
+    def test_loader_arguments_password_tenant_name(self):
+        passed_kwargs = {
+            'os_auth_url': 'http://localhost:35357/v3',
+            'os_region_name': 'REGIONONE',
+            'os_tenant_name': 'TENANT',
+            'os_username': 'user',
+            'os_password': '1234',
+            'os_project_domain_id': 'DEFAULT',
+            'os_user_domain_id': 'DEFAULT'
+        }
+        expected_kwargs = {
+            'auth_url': 'http://localhost:35357/v3',
+            'project_id': None,
+            'project_name': 'TENANT',
+            '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)
+
+    def test_loader_arguments_password_project_id(self):
+        passed_kwargs = {
+            'os_auth_url': 'http://localhost:35357/v3',
+            'os_region_name': 'REGIONONE',
+            'os_project_id': '1000',
+            'os_username': 'user',
+            'os_password': '1234',
+            'os_project_domain_name': 'domain1',
+            'os_user_domain_name': 'domain1'
+        }
+        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)
+
+    @mock.patch.object(iroclient, 'Client')
+    @mock.patch.object(kaloading.session, 'Session', autospec=True)
+    def test_correct_arguments_passed_to_client_constructor_noauth_mode(
+            self, mock_ks_session, mock_client):
+        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
+            }
+        )
+        self.assertFalse(mock_ks_session.called)
+
+    @mock.patch.object(iroclient, 'Client')
+    @mock.patch.object(kaloading.session, 'Session', autospec=True)
+    def test_correct_arguments_passed_to_client_constructor_session_created(
+            self, mock_ks_session, mock_client):
+        session = mock_ks_session.return_value.load_from_options.return_value
+        kwargs = {
+            'os_auth_url': 'http://localhost:35357/v3',
+            'os_region_name': 'REGIONONE',
+            'os_project_id': '1000',
+            'os_username': 'user',
+            'os_password': '1234',
+            'os_project_domain_name': 'domain1',
+            'os_user_domain_name': 'domain1'
+        }
+        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,
+            }
+        )
+
+    @mock.patch.object(iroclient, 'Client')
+    @mock.patch.object(kaloading.session, 'Session', autospec=True)
+    def test_correct_arguments_passed_to_client_constructor_session_passed(
+            self, mock_ks_session, mock_client):
+        session = mock.Mock()
+        kwargs = {
+            'session': session,
+        }
+        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,
+            }
+        )
+        self.assertFalse(mock_ks_session.called)
 
     def test_safe_header_with_auth_token(self):
         (name, value) = ('X-Auth-Token', u'3b640e2e64d946ac8f55615aff221dc1')

ironicclient/tests/unit/test_shell.py

@@ -16,14 +16,16 @@ import uuid
 
 import fixtures
 import httplib2
-from keystoneclient import exceptions as keystone_exc
+from keystoneauth1 import exceptions as keystone_exc
-from keystoneclient import fixture as ks_fixture
+from keystoneauth1 import fixture as ks_fixture
 import mock
 import requests_mock
 import six
 import testtools
 from testtools import matchers
 
+from ironicclient import client
+from ironicclient.common import http
 from ironicclient import exc
 from ironicclient import shell as ironic_shell
 from ironicclient.tests.unit import utils
@@ -120,28 +122,30 @@ class ShellTest(utils.BaseTestCase):
         self.make_env(exclude='OS_USERNAME')
         self.test_help()
 
-    @mock.patch.object(ironic_shell.IronicShell, '_get_keystone_auth',
+    @mock.patch.object(client, 'get_client',
-                       side_effect=keystone_exc.ConnectionRefused)
+                       side_effect=keystone_exc.ConnectFailure)
     @mock.patch('sys.stdin', side_effect=mock.MagicMock)
     @mock.patch('getpass.getpass', return_value='password')
-    def test_password_prompted(self, mock_getpass, mock_stdin, mock_ks):
+    def test_password_prompted(self, mock_getpass, mock_stdin, mock_client):
         self.make_env(exclude='OS_PASSWORD')
-        # We will get a Connection Refused because there is no keystone.
+        # We will get a ConnectFailure because there is no keystone.
-        self.assertRaises(keystone_exc.ConnectionRefused,
+        self.assertRaises(keystone_exc.ConnectFailure,
                           self.shell, 'node-list')
         expected_kwargs = {
-            'username': FAKE_ENV['OS_USERNAME'],
+            'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'],
-            'user_domain_id': '',
+            'os_tenant_id': '', 'os_tenant_name': FAKE_ENV['OS_TENANT_NAME'],
-            'user_domain_name': '',
+            'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '',
-            'password': FAKE_ENV['OS_PASSWORD'],
+            'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'],
-            'auth_token': '',
+            'os_auth_token': '', 'os_project_id': '',
-            'project_id': '',
+            'os_project_name': '', 'os_project_domain_id': '',
-            'project_name': FAKE_ENV['OS_TENANT_NAME'],
+            'os_project_domain_name': '', 'os_region_name': '',
-            'project_domain_id': '',
+            'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None,
-            'project_domain_name': '',
+            'os_cert': None, 'os_key': None,
+            'max_retries': http.DEFAULT_MAX_RETRIES,
+            'retry_interval': http.DEFAULT_RETRY_INTERVAL,
+            'os_ironic_api_version': None, 'timeout': 600, 'insecure': False
         }
-        mock_ks.assert_called_once_with(mock.ANY, FAKE_ENV['OS_AUTH_URL'],
+        mock_client.assert_called_once_with(1, **expected_kwargs)
-                                        **expected_kwargs)
         # Make sure we are actually prompted.
         mock_getpass.assert_called_with('OpenStack Password: ')
 

ironicclient/tests/unit/utils.py

@@ -14,7 +14,6 @@
 #    under the License.
 
 import copy
-import datetime
 import os
 
 import fixtures
@@ -24,11 +23,6 @@ import six
 import testtools
 
 
-DEFAULT_TEST_HOST = 'localhost'
-DEFAULT_TEST_REGION = 'regionhost'
-DEFAULT_TEST_PORT = '6385'
-
-
 class BaseTestCase(testtools.TestCase):
 
     def setUp(self):
@@ -113,30 +107,6 @@ class FakeResponse(object):
                  self.reason))
 
 
-class FakeServiceCatalog(object):
-    def url_for(self, endpoint_type, service_type, attr=None,
-                filter_value=None):
-        if attr == 'region' and filter_value:
-            return 'http://%s:%s/v1/f14b41234' % (DEFAULT_TEST_REGION,
-                                                  DEFAULT_TEST_PORT)
-        else:
-            return 'http://%s:%s/v1/f14b41234' % (DEFAULT_TEST_HOST,
-                                                  DEFAULT_TEST_PORT)
-
-
-class FakeKeystone(object):
-    service_catalog = FakeServiceCatalog()
-    timestamp = datetime.datetime.utcnow() + datetime.timedelta(days=5)
-
-    def __init__(self, auth_token):
-        self.auth_token = auth_token
-        self.auth_ref = {
-            'token': {'expires': FakeKeystone.timestamp.strftime(
-                      '%Y-%m-%dT%H:%M:%S.%f'),
-                      'id': 'd1a541311782870742235'}
-        }
-
-
 class FakeSessionResponse(object):
 
     def __init__(self, headers, content=None, status_code=None, version=None):

releasenotes/notes/keystone-token-auth-661a0c0d53c1b4de.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - Added a possibility to pass os_auth_token instead of os_username and
+    os_password to authenticate in keystone both in CLI and in get_client
+    function.

requirements.txt

@@ -6,10 +6,10 @@ appdirs>=1.3.0 # MIT License
 dogpile.cache>=0.5.4 # BSD
 cliff!=1.16.0,>=1.15.0 # Apache-2.0
 httplib2>=0.7.5 # MIT
+keystoneauth1>=2.1.0  # Apache-2.0
 oslo.i18n>=2.1.0 # Apache-2.0
 oslo.utils>=3.5.0 # Apache-2.0
 PrettyTable<0.8,>=0.7 # BSD
-python-keystoneclient!=1.8.0,!=2.1.0,>=1.6.0 # Apache-2.0
 python-openstackclient>=2.1.0 # Apache-2.0
 requests>=2.8.1,!=2.9.0 # Apache-2.0
 six>=1.9.0 # MIT