From 7226ad44aafc0a4399a62e7ebbd26f69bade8c21 Mon Sep 17 00:00:00 2001 From: Vladyslav Drok Date: Wed, 3 Feb 2016 16:20:06 +0200 Subject: [PATCH] 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 | 212 ++++++---- ironicclient/common/http.py | 2 +- ironicclient/shell.py | 248 +++-------- ironicclient/tests/unit/test_client.py | 399 ++++++++++++------ ironicclient/tests/unit/test_shell.py | 40 +- ironicclient/tests/unit/utils.py | 30 -- .../keystone-token-auth-661a0c0d53c1b4de.yaml | 5 + requirements.txt | 2 +- 8 files changed, 470 insertions(+), 468 deletions(-) create mode 100644 releasenotes/notes/keystone-token-auth-661a0c0d53c1b4de.yaml diff --git a/ironicclient/client.py b/ironicclient/client.py index 0811619a9..b5bea8196 100644 --- a/ironicclient/client.py +++ b/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): - """Get an endpoint and auth token from Keystone. - - :param kwargs: keyword args containing credentials: - * username: name of user - * password: user's password - * auth_url: endpoint to authenticate against - * insecure: allow insecure SSL (no cert verification) - * tenant_{name|id}: name or ID of tenant - """ - - client = keystoneclient.client.Client( - 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. +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): + """Get an authenticated client, based on the credentials. :param api_version: the API version to use. Valid value: '1'. - :param kwargs: keyword args containing credentials, either: - * os_auth_token: pre-existing token to re-use - * ironic_url: ironic API endpoint - or: - * os_username: name of user - * os_password: user's password - * os_auth_url: endpoint to authenticate against - * insecure: allow insecure SSL (no cert verification) - * os_tenant_{name|id}: name or ID of tenant + :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 + :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. """ - - if kwargs.get('os_auth_token') and kwargs.get('ironic_url'): - token = kwargs.get('os_auth_token') - endpoint = kwargs.get('ironic_url') - auth_ref = None - elif (kwargs.get('os_username') and - kwargs.get('os_password') and - kwargs.get('os_auth_url') and - (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, + 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 = { + 'os_ironic_api_version': os_ironic_api_version, + 'max_retries': max_retries, + 'retry_interval': retry_interval, } - for key in ('insecure', 'timeout', 'ca_file', 'cert_file', 'key_file', - 'os_ironic_api_version', 'max_retries', 'retry_interval'): - cli_kwargs[key] = kwargs.get(key) + 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, + ) - 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): diff --git a/ironicclient/common/http.py b/ironicclient/common/http.py index 14db41369..cde10d026 100644 --- a/ironicclient/common/http.py +++ b/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 diff --git a/ironicclient/shell.py b/ironicclient/shell.py index 54fbaeb04..a386c8efb 100644 --- a/ironicclient/shell.py +++ b/ironicclient/shell.py @@ -22,15 +22,9 @@ import logging import sys import httplib2 -from keystoneclient.auth.identity import v2 as v2_auth -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 keystoneauth1.loading import session as kasession 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 - kwargs['retry_interval'] = args.retry_interval - client = iroclient.Client(api_major_version, endpoint, **kwargs) + 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' + ) + 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) diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index aa92f8cfa..b601037be 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/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 ironicclient.client import get_client +from keystoneauth1 import loading as kaloading + +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) - 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_service_and_endpoint_type_defaults(self): + kwargs = { + 'os_tenant_name': 'TENANT_NAME', + 'os_username': 'USERNAME', + '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) - 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)) + def test_get_client_no_url(self): 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): - ksclient = fake_get_ksclient - self.useFixture(fixtures.MonkeyPatch( - 'ironicclient.client._get_ksclient', ksclient)) + def test_get_client_incorrect_auth_params(self): 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.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) + self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client, + '1', **kwargs) 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) - 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) + self._test_get_client(**kwargs) - @mock.patch.object(filecache, 'retrieve_data', autospec=True) - @mock.patch.object(ironicclient.client, '_get_ksclient', autospec=True) - def test_get_client_default_version_set_cached(self, mock_ksclient, - mock_retrieve_data): + def test_get_client_with_api_version_numeric(self): + 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': "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) - mock_retrieve_data.assert_called_once_with(host=mock.ANY, - port=mock.ANY) - self.assertEqual(version, client.http_client.os_ironic_api_version) - self.assertEqual('cached', client.http_client.api_version_select_state) + 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', + } + 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') diff --git a/ironicclient/tests/unit/test_shell.py b/ironicclient/tests/unit/test_shell.py index 67c854037..1d4b93f10 100644 --- a/ironicclient/tests/unit/test_shell.py +++ b/ironicclient/tests/unit/test_shell.py @@ -16,14 +16,16 @@ import uuid import fixtures import httplib2 -from keystoneclient import exceptions as keystone_exc -from keystoneclient import fixture as ks_fixture +from keystoneauth1 import exceptions as keystone_exc +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', - side_effect=keystone_exc.ConnectionRefused) + @mock.patch.object(client, 'get_client', + 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. - self.assertRaises(keystone_exc.ConnectionRefused, + # We will get a ConnectFailure because there is no keystone. + self.assertRaises(keystone_exc.ConnectFailure, self.shell, 'node-list') expected_kwargs = { - 'username': FAKE_ENV['OS_USERNAME'], - 'user_domain_id': '', - 'user_domain_name': '', - 'password': FAKE_ENV['OS_PASSWORD'], - 'auth_token': '', - 'project_id': '', - 'project_name': FAKE_ENV['OS_TENANT_NAME'], - 'project_domain_id': '', - 'project_domain_name': '', + 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], + 'os_tenant_id': '', 'os_tenant_name': FAKE_ENV['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': '', '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, + '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'], - **expected_kwargs) + mock_client.assert_called_once_with(1, **expected_kwargs) # Make sure we are actually prompted. mock_getpass.assert_called_with('OpenStack Password: ') diff --git a/ironicclient/tests/unit/utils.py b/ironicclient/tests/unit/utils.py index cb31e1230..10aa2bfef 100644 --- a/ironicclient/tests/unit/utils.py +++ b/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): diff --git a/releasenotes/notes/keystone-token-auth-661a0c0d53c1b4de.yaml b/releasenotes/notes/keystone-token-auth-661a0c0d53c1b4de.yaml new file mode 100644 index 000000000..7268117da --- /dev/null +++ b/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. diff --git a/requirements.txt b/requirements.txt index ae093a209..177d39090 100644 --- a/requirements.txt +++ b/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