From 8409e006c5f362922baae9470f14c12e0443dd70 Mon Sep 17 00:00:00 2001 From: Andrey Kurilin Date: Fri, 17 Jun 2016 14:45:55 +0300 Subject: [PATCH] Create keystone session instance if not present Since all authentication plugins should be stored in Keystone, there is no need to share responsibility for authentication stuff with Keystone team. Let's use convinient and the right way to use keystone - keystone sessions. We have own adapter for it - novaclient.client.Session which can be used in 100% cases of novaclient usage. Let's create session object if it is not transmitted via arguments and get rid of novaclient.client.HTTPClient implementation. NOTE: novaclient.client.HTTPClient and all related code will be removed separately. Co-Authored-By: Clenimar Filemon Change-Id: Ibb99fdafbf02b3e92f1a5d04d168151b3400b901 --- novaclient/client.py | 82 +++++++++---------- novaclient/shell.py | 37 ++------- novaclient/tests/unit/fixture_data/servers.py | 2 +- novaclient/tests/unit/test_client.py | 20 +---- novaclient/tests/unit/v2/test_auth.py | 11 +++ ...tch-to-sessionclient-aa49d16599fea570.yaml | 10 +++ 6 files changed, 67 insertions(+), 95 deletions(-) create mode 100644 releasenotes/notes/switch-to-sessionclient-aa49d16599fea570.yaml diff --git a/novaclient/client.py b/novaclient/client.py index 8a840afcf..7780c9a6b 100644 --- a/novaclient/client.py +++ b/novaclient/client.py @@ -30,7 +30,8 @@ import re import warnings from keystoneauth1 import adapter -from keystoneauth1 import session +from keystoneauth1 import identity +from keystoneauth1 import session as ksession from oslo_utils import importutils from oslo_utils import netutils import pkg_resources @@ -65,7 +66,7 @@ class _ClientConnectionPool(object): def get(self, url): """Store and reuse HTTP adapters per Service URL.""" if url not in self._adapters: - self._adapters[url] = session.TCPKeepAliveAdapter() + self._adapters[url] = ksession.TCPKeepAliveAdapter() return self._adapters[url] @@ -705,6 +706,7 @@ def _construct_http_client(api_version=None, endpoint_type='publicURL', http_log_debug=False, insecure=False, + logger=None, os_cache=False, password=None, project_domain_id=None, @@ -724,49 +726,39 @@ def _construct_http_client(api_version=None, username=None, volume_service_name=None, **kwargs): - # TODO(mordred): If not session, just make a Session, then return - # SessionClient always - if session: - return SessionClient(api_version=api_version, - auth=auth, - endpoint_override=endpoint_override, - interface=endpoint_type, - region_name=region_name, - service_name=service_name, - service_type=service_type, - session=session, - timings=timings, - user_agent=user_agent, - **kwargs) - else: - # FIXME(jamielennox): username and password are now optional. Need - # to test that they were provided in this mode. - logger = kwargs.get('logger') - return HTTPClient(username, - password, - user_id=user_id, - # NOTE(andreykurilin): HTTPClient will be replaced - # fully by SessionClient soon, so there are no - # reasons to spend time renaming projectid variable - # to tenant_name/project_name. - projectid=project_name, - tenant_id=project_id, - auth_url=auth_url, - auth_token=auth_token, - insecure=insecure, - timeout=timeout, - region_name=region_name, - endpoint_type=endpoint_type, - service_type=service_type, - service_name=service_name, - volume_service_name=volume_service_name, - timings=timings, - bypass_url=endpoint_override, - os_cache=os_cache, - http_log_debug=http_log_debug, - cacert=cacert, - api_version=api_version, - logger=logger) + if not session: + if not auth and auth_token: + auth = identity.Token(auth_url=auth_url, + token=auth_token) + elif not auth: + auth = identity.Password(username=username, + user_id=user_id, + password=password, + project_id=project_id, + project_name=project_name, + auth_url=auth_url, + project_domain_id=project_domain_id, + project_domain_name=project_domain_name, + user_domain_id=user_domain_id, + user_domain_name=user_domain_name) + session = ksession.Session(auth=auth, + verify=(cacert or not insecure), + timeout=timeout, + cert=cert, + user_agent=user_agent) + + return SessionClient(api_version=api_version, + auth=auth, + endpoint_override=endpoint_override, + interface=endpoint_type, + logger=logger, + region_name=region_name, + service_name=service_name, + service_type=service_type, + session=session, + timings=timings, + user_agent=user_agent, + **kwargs) def discover_extensions(version, only_contrib=False): diff --git a/novaclient/shell.py b/novaclient/shell.py index 87ca7778b..e6c6bcc01 100644 --- a/novaclient/shell.py +++ b/novaclient/shell.py @@ -674,6 +674,7 @@ class OpenStackComputeShell(object): volume_service_name = args.volume_service_name endpoint_override = args.endpoint_override os_cache = args.os_cache + cert = args.os_cert cacert = args.os_cacert cert = args.os_cert timeout = args.timeout @@ -707,11 +708,6 @@ class OpenStackComputeShell(object): # Expired tokens are handled by client.py:_cs_request must_auth = not (auth_token and endpoint_override) - # Do not use Keystone session for cases with no session support. - use_session = True - if endpoint_override or os_cache or volume_service_name: - use_session = False - # FIXME(usrleon): Here should be restrict for project id same as # for os_username or os_password but for compatibility it is not. if must_auth and not skip_auth: @@ -735,17 +731,12 @@ class OpenStackComputeShell(object): _("You must provide an auth url " "via either --os-auth-url or env[OS_AUTH_URL].")) - if use_session: - # Not using Nova auth plugin, so use keystone - with utils.record_time(self.times, args.timings, - 'auth_url', args.os_auth_url): - keystone_session = ( - loading.load_session_from_argparse_arguments(args)) - keystone_auth = ( - loading.load_auth_from_argparse_arguments(args)) - else: - # set password for auth plugins - os_password = args.os_password + with utils.record_time(self.times, args.timings, + 'auth_url', args.os_auth_url): + keystone_session = ( + loading.load_session_from_argparse_arguments(args)) + keystone_auth = ( + loading.load_auth_from_argparse_arguments(args)) if (not skip_auth and not any([os_project_name, os_project_id])): @@ -870,20 +861,6 @@ class OpenStackComputeShell(object): # the result in our helper. self.cs.client.password = helper.password - try: - # This does a couple of bits which are useful even if we've - # got the token + service URL already. It exits fast in that case. - if not utils.isunauthenticated(args.func): - if not use_session: - # Only call authenticate() if Nova auth plugin is used. - # If keystone is used, authentication is handled as part - # of session. - self.cs.authenticate() - except exc.Unauthorized: - raise exc.CommandError(_("Invalid OpenStack Nova credentials.")) - except exc.AuthorizationFailure: - raise exc.CommandError(_("Unable to authorize user")) - args.func(self.cs, args) if args.timings: diff --git a/novaclient/tests/unit/fixture_data/servers.py b/novaclient/tests/unit/fixture_data/servers.py index 891814b2f..30bdae58e 100644 --- a/novaclient/tests/unit/fixture_data/servers.py +++ b/novaclient/tests/unit/fixture_data/servers.py @@ -329,7 +329,7 @@ class Base(base.Fixture): headers=self.json_headers) def put_server_tag(request, context): - assert request.json() is None + assert request.text is None context.status_code = 201 return None diff --git a/novaclient/tests/unit/test_client.py b/novaclient/tests/unit/test_client.py index 5ff6133ee..8ebdb7f1b 100644 --- a/novaclient/tests/unit/test_client.py +++ b/novaclient/tests/unit/test_client.py @@ -172,25 +172,7 @@ class ClientTest(utils.TestCase): self.assertRaises(novaclient.exceptions.UnsupportedVersion, novaclient.client.get_client_class, '2.latest') - def test_client_with_os_cache_enabled(self): - cs = novaclient.client.Client("2", "user", "password", "project_id", - auth_url="foo/v2", os_cache=True) - self.assertTrue(cs.os_cache) - self.assertTrue(cs.client.os_cache) - - def test_client_with_os_cache_disabled(self): - cs = novaclient.client.Client("2", "user", "password", "project_id", - auth_url="foo/v2", os_cache=False) - self.assertFalse(cs.os_cache) - self.assertFalse(cs.client.os_cache) - - def test_client_set_management_url_v1_1(self): - cs = novaclient.client.Client("2", "user", "password", "project_id", - auth_url="foo/v2") - cs.set_management_url("blabla") - self.assertEqual("blabla", cs.client.management_url) - - def test_client_get_reset_timings_v1_1(self): + def test_client_get_reset_timings_v2(self): cs = novaclient.client.Client("2", "user", "password", "project_id", auth_url="foo/v2") self.assertEqual(0, len(cs.get_timings())) diff --git a/novaclient/tests/unit/v2/test_auth.py b/novaclient/tests/unit/v2/test_auth.py index 512e6743d..f99b233b3 100644 --- a/novaclient/tests/unit/v2/test_auth.py +++ b/novaclient/tests/unit/v2/test_auth.py @@ -29,6 +29,11 @@ def Client(*args, **kwargs): class AuthenticateAgainstKeystoneTests(utils.TestCase): + def setUp(self): + super(AuthenticateAgainstKeystoneTests, self).setUp() + self.skipTest("This TestCase checks deprecated authentication " + "methods, which will be removed in separate patch.") + def get_token(self, **kwargs): resp = fixture.V2Token(**kwargs) resp.set_scope() @@ -315,6 +320,12 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase): class AuthenticationTests(utils.TestCase): + + def setUp(self): + super(AuthenticationTests, self).setUp() + self.skipTest("This TestCase checks deprecated authentication " + "methods, which will be removed in separate patch.") + def test_authenticate_success(self): cs = Client("username", "password", project_name="project_id", auth_url=utils.AUTH_URL) diff --git a/releasenotes/notes/switch-to-sessionclient-aa49d16599fea570.yaml b/releasenotes/notes/switch-to-sessionclient-aa49d16599fea570.yaml new file mode 100644 index 000000000..4a60f7ec4 --- /dev/null +++ b/releasenotes/notes/switch-to-sessionclient-aa49d16599fea570.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + When using novaclient as a library (via novaclient.client.Client + entry-point), an internal implementation of HTTPClient was used if a + session object is not specified. For better user experience we switched + to using SessionClient which uses keystoneauth (Keystone folks maintain + this library) for all auth stuff. + The SessionClient interface is similar to HTTPClient, but there is a + small possibility that you will notice a difference.