From 5cbc5e722e4878e35399be575a6cc36d266b00ff Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Sat, 7 Nov 2015 09:29:23 -0500 Subject: [PATCH] Use keystoneauth to create a Session All client interactions should actually flow through a keystoneauth Session. Add the parameter for a user to pass one in, but support the old style of direct parameters too. A followup patch will add os-client-config support to the shell, so add that to requirements.txt instead of keystoneauth directly to look forward. Also, trim unneeded things from requirements.txt because we're adding os-client-config which pulls in various things. Closes-Bug: #1514733 Change-Id: I88b1d0e4e119429599dec75c5db24237a92edcec --- magnumclient/common/httpclient.py | 10 +- magnumclient/tests/test_shell.py | 2 +- magnumclient/tests/test_shell_args.py | 15 ++- magnumclient/tests/v1/test_client.py | 149 +++++++++++++++----------- magnumclient/v1/client.py | 122 +++++++++++---------- requirements.txt | 11 +- 6 files changed, 174 insertions(+), 135 deletions(-) diff --git a/magnumclient/common/httpclient.py b/magnumclient/common/httpclient.py index 95a3eecf..4a637c62 100644 --- a/magnumclient/common/httpclient.py +++ b/magnumclient/common/httpclient.py @@ -22,7 +22,7 @@ import os import socket import ssl -from keystoneclient import adapter +from keystoneauth1 import adapter import six import six.moves.urllib.parse as urlparse @@ -284,8 +284,14 @@ class VerifiedHTTPSConnection(six.moves.http_client.HTTPSConnection): class SessionClient(adapter.LegacyJsonAdapter): """HTTP client based on Keystone client session.""" + def __init__(self, user_agent=USER_AGENT, logger=LOG, *args, **kwargs): + super(SessionClient, self).__init__(*args, **kwargs) + def _http_request(self, url, method, **kwargs): - kwargs.setdefault('user_agent', USER_AGENT) + if url.startswith(API_VERSION): + url = url[len(API_VERSION):] + + kwargs.setdefault('user_agent', self.user_agent) kwargs.setdefault('auth', self.auth) endpoint_filter = kwargs.setdefault('endpoint_filter', {}) diff --git a/magnumclient/tests/test_shell.py b/magnumclient/tests/test_shell.py index 75ef52c4..c5aa40b4 100644 --- a/magnumclient/tests/test_shell.py +++ b/magnumclient/tests/test_shell.py @@ -16,7 +16,7 @@ import re import sys import fixtures -from keystoneclient import fixture +from keystoneauth1 import fixture import mock import six from testtools import matchers diff --git a/magnumclient/tests/test_shell_args.py b/magnumclient/tests/test_shell_args.py index 944beafe..f636def1 100644 --- a/magnumclient/tests/test_shell_args.py +++ b/magnumclient/tests/test_shell_args.py @@ -49,10 +49,17 @@ class TestCommandLineArgument(utils.TestCase): def setUp(self): super(TestCommandLineArgument, self).setUp() self.make_env(fake_env=FAKE_ENV) - keystone_mock = mock.patch( - 'magnumclient.v1.client.Client.get_keystone_client') - keystone_mock.start() - self.addCleanup(keystone_mock.stop) + session_client = mock.patch( + 'magnumclient.common.httpclient.SessionClient') + session_client.start() + loader = mock.patch('keystoneauth1.loading.get_plugin_loader') + loader.start() + session = mock.patch('keystoneauth1.session.Session') + session.start() + + self.addCleanup(session_client.stop) + self.addCleanup(loader.stop) + self.addCleanup(session.stop) def _test_arg_success(self, command): stdout, stderr = self.shell(command) diff --git a/magnumclient/tests/v1/test_client.py b/magnumclient/tests/v1/test_client.py index 7d562ae1..a95ac552 100644 --- a/magnumclient/tests/v1/test_client.py +++ b/magnumclient/tests/v1/test_client.py @@ -20,74 +20,99 @@ from magnumclient.v1 import client class ClientTest(testtools.TestCase): - @mock.patch('magnumclient.common.httpclient.HTTPClient') - @mock.patch.object(client.Client, 'get_keystone_client') - def test_init_with_token_and_url(self, keystone_client, http_client): + @mock.patch('magnumclient.common.httpclient.SessionClient') + @mock.patch('keystoneauth1.session.Session') + def test_init_with_session(self, mock_session, http_client): + session = mock.Mock() + client.Client(session=session) + mock_session.assert_not_called() + http_client.assert_called_once_with( + interface='public', + region_name=None, + service_name=None, + service_type='container', + session=session) + + @mock.patch('magnumclient.common.httpclient.SessionClient') + @mock.patch('keystoneauth1.token_endpoint.Token') + @mock.patch('keystoneauth1.session.Session') + def test_init_with_token_and_url( + self, mock_session, mock_token, http_client): + mock_auth_plugin = mock.Mock() + mock_token.return_value = mock_auth_plugin + session = mock.Mock() + mock_session.return_value = session client.Client(input_auth_token='mytoken', magnum_url='http://myurl/') - self.assertFalse(keystone_client.called) + mock_session.assert_called_once_with(auth=mock_auth_plugin) http_client.assert_called_once_with( - 'http://myurl/', token='mytoken', auth_ref=None) - - @mock.patch('magnumclient.common.httpclient.HTTPClient') - @mock.patch.object(client.Client, 'get_keystone_client') - def test_init_with_token(self, keystone_client, http_client): - mocked = mock.Mock() - mocked.service_catalog.url_for.return_value = 'http://myurl/' - keystone_client.return_value = mocked + endpoint_override='http://myurl/', + interface='public', + region_name=None, + service_name=None, + service_type='container', + session=session) + @mock.patch('magnumclient.common.httpclient.SessionClient') + @mock.patch('keystoneauth1.loading.get_plugin_loader') + @mock.patch('keystoneauth1.session.Session') + def test_init_with_token( + self, mock_session, mock_loader, http_client): + mock_plugin = mock.Mock() + mock_loader.return_value = mock_plugin client.Client(input_auth_token='mytoken', auth_url='authurl') - keystone_client.assert_called_once_with( - token='mytoken', username=None, api_key=None, - project_name=None, project_id=None, - auth_url='authurl') + mock_loader.assert_called_once_with('token') + mock_plugin.load_from_options.assert_called_once_with( + auth_url='authurl', + project_id=None, + project_name=None, + token='mytoken') http_client.assert_called_once_with( - 'http://myurl/', token='mytoken', auth_ref=None) + interface='public', + region_name=None, + service_name=None, + service_type='container', + session=mock.ANY) - @mock.patch('magnumclient.common.httpclient.HTTPClient') - @mock.patch.object(client.Client, 'get_keystone_client') - def test_init_with_user(self, keystone_client, http_client): - mocked = mock.Mock() - mocked.auth_token = 'mytoken' - mocked.service_catalog.url_for.return_value = 'http://myurl/' - keystone_client.return_value = mocked - - client.Client(username='user', api_key='pass', project_name='prj', - auth_url='authurl') - keystone_client.assert_called_once_with( - username='user', api_key='pass', - project_name='prj', project_id=None, - auth_url='authurl') + @mock.patch('magnumclient.common.httpclient.SessionClient') + @mock.patch('keystoneauth1.loading.get_plugin_loader') + @mock.patch('keystoneauth1.session.Session') + def test_init_with_user( + self, mock_session, mock_loader, http_client): + mock_plugin = mock.Mock() + mock_loader.return_value = mock_plugin + client.Client(username='myuser', auth_url='authurl') + mock_loader.assert_called_once_with('password') + mock_plugin.load_from_options.assert_called_once_with( + auth_url='authurl', + username='myuser', + password=None, + project_id=None, + project_name=None) http_client.assert_called_once_with( - 'http://myurl/', token='mytoken', auth_ref=None) - - @mock.patch.object(client.Client, 'get_keystone_client') - def test_init_unauthorized(self, keystone_client): - mocked = mock.Mock() - mocked.auth_token = None - keystone_client.return_value = mocked + interface='public', + region_name=None, + service_name=None, + service_type='container', + session=mock.ANY) + @mock.patch('magnumclient.common.httpclient.SessionClient') + @mock.patch('keystoneauth1.loading.get_plugin_loader') + @mock.patch('keystoneauth1.session.Session') + def test_init_unauthorized( + self, mock_session, mock_loader, http_client): + mock_plugin = mock.Mock() + mock_loader.return_value = mock_plugin + mock_session_obj = mock.Mock() + mock_session.return_value = mock_session_obj + mock_session_obj.get_endpoint.side_effect = Exception() self.assertRaises( - RuntimeError, client.Client, - username='user', api_key='pass', project_name='prj', - auth_url='authurl') - - def _test_get_keystone_client(self, auth_url, keystone_client): - client.Client.get_keystone_client( - username='user', api_key='pass', project_name='prj', - auth_url=auth_url) - self.assertTrue(keystone_client.called) - - @mock.patch('keystoneclient.v2_0.client.Client') - def test_get_keystone_client_v2(self, keystone_client): - self._test_get_keystone_client( - 'http://authhost/v2.0', keystone_client) - - @mock.patch('keystoneclient.v3.client.Client') - def test_get_keystone_client_v3(self, keystone_client): - self._test_get_keystone_client( - 'http://authhost/v3', keystone_client) - - def test_get_keystone_client_no_url(self): - self.assertRaises(RuntimeError, - self._test_get_keystone_client, - None, None) + RuntimeError, + client.Client, username='myuser', auth_url='authurl') + mock_loader.assert_called_once_with('password') + mock_plugin.load_from_options.assert_called_once_with( + auth_url='authurl', + username='myuser', + password=None, + project_id=None, + project_name=None) + http_client.assert_not_called() diff --git a/magnumclient/v1/client.py b/magnumclient/v1/client.py index 49ef4c18..b96b1504 100644 --- a/magnumclient/v1/client.py +++ b/magnumclient/v1/client.py @@ -13,8 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from keystoneclient.v2_0 import client as keystone_client_v2 -from keystoneclient.v3 import client as keystone_client_v3 +from keystoneauth1 import loading +from keystoneauth1 import session as ksa_session from magnumclient.common import httpclient from magnumclient.v1 import baymodels @@ -31,48 +31,74 @@ from magnumclient.v1 import services class Client(object): def __init__(self, username=None, api_key=None, project_id=None, project_name=None, auth_url=None, magnum_url=None, - endpoint_type='publicURL', service_type='container', - region_name=None, input_auth_token=None): + endpoint_type=None, service_type='container', + region_name=None, input_auth_token=None, + session=None, password=None, auth_type='password', + interface='public', service_name=None): - keystone = None - if not input_auth_token: - keystone = self.get_keystone_client(username=username, - api_key=api_key, - auth_url=auth_url, - project_id=project_id, - project_name=project_name) - input_auth_token = keystone.auth_token - if not input_auth_token: - raise RuntimeError("Not Authorized") + # We have to keep the api_key are for backwards compat, but let's + # remove it from the rest of our code since it's not a keystone + # concept + if not password: + password = api_key + # Backwards compat for people assing in endpoint_type + if endpoint_type: + interface = endpoint_type - if not magnum_url: - keystone = keystone or self.get_keystone_client( - username=username, - api_key=api_key, - auth_url=auth_url, + if magnum_url and input_auth_token: + auth_type = 'admin_token' + session = None + loader_kwargs = dict( token=input_auth_token, + endpoint=magnum_url) + elif input_auth_token and not session: + auth_type = 'token' + loader_kwargs = dict( + token=input_auth_token, + auth_url=auth_url, + project_id=project_id, + project_name=project_name) + else: + loader_kwargs = dict( + username=username, + password=password, + auth_url=auth_url, project_id=project_id, project_name=project_name) - magnum_url = keystone.service_catalog.url_for( - service_type=service_type, - endpoint_type=endpoint_type, - region_name=region_name) - http_cli_kwargs = { - 'token': input_auth_token, - # TODO(yuanying): - use insecure - # 'insecure': kwargs.get('insecure'), - # TODO(yuanying): - use timeout - # 'timeout': kwargs.get('timeout'), - # TODO(yuanying): - use ca_file - # 'ca_file': kwargs.get('ca_file'), - # TODO(yuanying): - use cert_file - # 'cert_file': kwargs.get('cert_file'), - # TODO(yuanying): - use key_file - # 'key_file': kwargs.get('key_file'), - 'auth_ref': None, - } - self.http_client = httpclient.HTTPClient(magnum_url, **http_cli_kwargs) + # Backwards compatability for people not passing in Session + if session is None: + loader = loading.get_plugin_loader(auth_type) + + # This only supports keystone v2 password auth - but we can + # support other auth by passing in a Session, which is the + # right thing to do anyway + auth_plugin = loader.load_from_options(**loader_kwargs) + session = ksa_session.Session(auth=auth_plugin) + + client_kwargs = {} + if magnum_url: + client_kwargs['endpoint_override'] = magnum_url + + if not magnum_url: + try: + # Trigger an auth error so that we can throw the exception + # we always have + session.get_endpoint( + service_type=service_type, + service_name=service_name, + interface=interface, + region_name=region_name) + except Exception: + raise RuntimeError("Not Authorized") + + self.http_client = httpclient.SessionClient( + service_type=service_type, + service_name=service_name, + interface=interface, + region_name=region_name, + session=session, + **client_kwargs) self.bays = bays.BayManager(self.http_client) self.certificates = certificates.CertificateManager(self.http_client) self.baymodels = baymodels.BayModelManager(self.http_client) @@ -82,23 +108,3 @@ class Client(object): self.rcs = rcs.ReplicationControllerManager(self.http_client) self.services = services.ServiceManager(self.http_client) self.mservices = mservices.MServiceManager(self.http_client) - - @staticmethod - def get_keystone_client(username=None, api_key=None, auth_url=None, - token=None, project_id=None, project_name=None): - if not auth_url: - raise RuntimeError("No auth url specified") - imported_client = (keystone_client_v2 if "v2.0" in auth_url - else keystone_client_v3) - - client = imported_client.Client( - username=username, - password=api_key, - token=token, - tenant_id=project_id, - tenant_name=project_name, - auth_url=auth_url, - endpoint=auth_url) - client.authenticate() - - return client diff --git a/requirements.txt b/requirements.txt index c85625a4..5344b03e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,16 +1,11 @@ # The order of packages is significant, because pip processes them in the order # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -argparse pbr>=1.6 + Babel>=1.3 -oslo.config>=2.6.0 # Apache-2.0 oslo.i18n>=1.5.0 # Apache-2.0 oslo.serialization>=1.10.0 # Apache-2.0 oslo.utils!=2.6.0,>=2.4.0 # Apache-2.0 -iso8601>=0.1.9 -requests!=2.8.0,>=2.5.2 -python-keystoneclient!=1.8.0,>=1.6.0 -PyYAML>=3.1.0 -stevedore>=1.5.0 # Apache-2.0 -six>=1.9.0 +os-client-config>=1.4.0,!=1.6.2 +PrettyTable>=0.7,<0.8