From 1263bd7c3a8ccded3cef7c799a2f8c744fb79aa2 Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Wed, 11 Dec 2013 07:38:46 +1000 Subject: [PATCH] Provide a conversion function for creating session Session.construct will create a session based upon the kwargs that used to be passed to a client __init__ function. This will allow clients an easy path to providing compatibility with deprecated arguments. Make use of the function throughout discovery. Discovery was initially released prior to the session object being completed and was therefore handled with the same arguments as a client. Instead we should use a session object so use the conversion function to convert those kwargs into a session object if one is not provided. Change-Id: I8dc1e0810ea6ebc6ea648ec37d7881825c566676 --- keystoneclient/client.py | 12 +++-- keystoneclient/discover.py | 67 ++++++++++++++++++++-------- keystoneclient/httpclient.py | 32 ++++--------- keystoneclient/session.py | 37 +++++++++++++++ keystoneclient/tests/test_session.py | 30 +++++++++++++ 5 files changed, 132 insertions(+), 46 deletions(-) diff --git a/keystoneclient/client.py b/keystoneclient/client.py index 832e3fa7b..cb9b76c51 100644 --- a/keystoneclient/client.py +++ b/keystoneclient/client.py @@ -14,13 +14,14 @@ from keystoneclient import discover from keystoneclient import httpclient +from keystoneclient import session as client_session # Using client.HTTPClient is deprecated. Use httpclient.HTTPClient instead. HTTPClient = httpclient.HTTPClient -def Client(version=None, unstable=False, **kwargs): +def Client(version=None, unstable=False, session=None, **kwargs): """Factory function to create a new identity service client. :param tuple version: The required version of the identity API. If @@ -29,6 +30,9 @@ def Client(version=None, unstable=False, **kwargs): at least the specified minor version. For example to specify the 3.1 API use (3, 1). :param bool unstable: Accept endpoints not marked as 'stable'. (optional) + :param Session session: A session object to be used for communication. If + one is not provided it will be constructed from the + provided kwargs. (optional) :param kwargs: Additional arguments are passed through to the client that is being created. :returns: New keystone client object @@ -37,6 +41,8 @@ def Client(version=None, unstable=False, **kwargs): :raises: DiscoveryFailure if the server's response is invalid :raises: VersionNotAvailable if a suitable client cannot be found. """ + if not session: + session = client_session.Session.construct(kwargs) - return discover.Discover(**kwargs).create_client(version=version, - unstable=unstable) + d = discover.Discover(session=session, **kwargs) + return d.create_client(version=version, unstable=unstable) diff --git a/keystoneclient/discover.py b/keystoneclient/discover.py index e79432112..3cbb5e0e6 100644 --- a/keystoneclient/discover.py +++ b/keystoneclient/discover.py @@ -17,7 +17,7 @@ import logging import six from keystoneclient import exceptions -from keystoneclient import httpclient +from keystoneclient import session as client_session from keystoneclient.v2_0 import client as v2_client from keystoneclient.v3 import client as v3_client @@ -81,7 +81,7 @@ class _KeystoneVersion(object): def __eq__(self, other): return self.version == other.version and self.status == other.status - def __call__(self, **kwargs): + def create_client(self, **kwargs): if kwargs: client_kwargs = self.client_kwargs.copy() client_kwargs.update(kwargs) @@ -89,6 +89,9 @@ class _KeystoneVersion(object): client_kwargs = self.client_kwargs return self.client_class(**client_kwargs) + def __call__(self, **kwargs): + return self.create_client(**kwargs) + @property def _str_ver(self): ver = ".".join([str(v) for v in self.version]) @@ -132,28 +135,35 @@ def _normalize_version_number(version): raise TypeError("Invalid version specified: %s" % version) -def available_versions(url, **kwargs): +def available_versions(url, session=None, **kwargs): headers = {'Accept': 'application/json'} - client = httpclient.HTTPClient(**kwargs) - resp, body_resp = client.request(url, 'GET', headers=headers) + if not session: + session = client_session.Session.construct(kwargs) - # In the event of querying a root URL we will get back a list of - # available versions. - try: - return body_resp['versions']['values'] - except (KeyError, TypeError): - pass + resp = session.get(url, headers=headers) - # Otherwise if we query an endpoint like /v2.0 then we will get back - # just the one available version. try: - return [body_resp['version']] - except (KeyError, TypeError): + body_resp = resp.json() + except ValueError: pass + else: + # In the event of querying a root URL we will get back a list of + # available versions. + try: + return body_resp['versions']['values'] + except (KeyError, TypeError): + pass + + # Otherwise if we query an endpoint like /v2.0 then we will get back + # just the one available version. + try: + return [body_resp['version']] + except KeyError: + pass raise exceptions.DiscoveryFailure("Invalid Response - Bad version" - " data returned: %s" % body_resp) + " data returned: %s" % resp.text) class Discover(object): @@ -164,7 +174,7 @@ class Discover(object): operates upon the data that was retrieved. """ - def __init__(self, **kwargs): + def __init__(self, session=None, **kwargs): """Construct a new discovery object. The connection parameters associated with this method are the same @@ -178,6 +188,9 @@ class Discover(object): The initialization process also queries the server. + :param Session session: A session object that will be used for + communication. Clients will also be constructed + with this session. :param string auth_url: Identity service endpoint for authorization. (optional) :param string endpoint: A user-supplied endpoint URL for the identity @@ -185,26 +198,42 @@ class Discover(object): :param string original_ip: The original IP of the requesting user which will be sent to identity service in a 'Forwarded' header. (optional) + DEPRECATED: use the session object. This is + ignored if a session is provided. :param boolean debug: Enables debug logging of all request and responses to the identity service. default False (optional) + DEPRECATED: use the session object. This is + ignored if a session is provided. :param string cacert: Path to the Privacy Enhanced Mail (PEM) file which contains the trusted authority X.509 certificates needed to established SSL connection with the identity service. (optional) + DEPRECATED: use the session object. This is + ignored if a session is provided. :param string key: Path to the Privacy Enhanced Mail (PEM) file which contains the unencrypted client private key needed to established two-way SSL connection with the identity service. (optional) + DEPRECATED: use the session object. This is + ignored if a session is provided. :param string cert: Path to the Privacy Enhanced Mail (PEM) file which contains the corresponding X.509 client certificate needed to established two-way SSL connection with the identity service. (optional) + DEPRECATED: use the session object. This is + ignored if a session is provided. :param boolean insecure: Does not perform X.509 certificate validation when establishing SSL connection with identity service. default: False (optional) + DEPRECATED: use the session object. This is + ignored if a session is provided. """ + if not session: + session = client_session.Session.construct(kwargs) + kwargs['session'] = session + url = kwargs.get('endpoint') or kwargs.get('auth_url') if not url: raise exceptions.DiscoveryFailure('Not enough information to ' @@ -212,7 +241,7 @@ class Discover(object): 'auth_url or endpoint') self._client_kwargs = kwargs - self._available_versions = available_versions(url, **kwargs) + self._available_versions = available_versions(url, session=session) def _get_client_constructor_kwargs(self, kwargs_dict={}, **kwargs): client_kwargs = self._client_kwargs.copy() @@ -420,4 +449,4 @@ class Discover(object): raise exceptions.VersionNotAvailable(msg) - return chosen() + return chosen.create_client() diff --git a/keystoneclient/httpclient.py b/keystoneclient/httpclient.py index da3354f25..87b40aefe 100644 --- a/keystoneclient/httpclient.py +++ b/keystoneclient/httpclient.py @@ -55,14 +55,13 @@ request = client_session.request class HTTPClient(object): def __init__(self, username=None, tenant_id=None, tenant_name=None, - password=None, auth_url=None, region_name=None, timeout=None, - endpoint=None, token=None, cacert=None, key=None, - cert=None, insecure=False, original_ip=None, debug=False, - auth_ref=None, use_keyring=False, force_new_token=False, - stale_duration=None, user_id=None, user_domain_id=None, - user_domain_name=None, domain_id=None, domain_name=None, - project_id=None, project_name=None, project_domain_id=None, - project_domain_name=None, trust_id=None, session=None): + password=None, auth_url=None, region_name=None, endpoint=None, + token=None, debug=False, auth_ref=None, use_keyring=False, + force_new_token=False, stale_duration=None, user_id=None, + user_domain_id=None, user_domain_name=None, domain_id=None, + domain_name=None, project_id=None, project_name=None, + project_domain_id=None, project_domain_name=None, + trust_id=None, session=None, **kwargs): """Construct a new http client :param string user_id: User ID for authentication. (optional) @@ -223,22 +222,7 @@ class HTTPClient(object): self._auth_token = None if not session: - verify = cacert or True - if insecure: - verify = False - - session_cert = None - if cert and key: - session_cert = (cert, key) - elif cert: - _logger.warn("Client cert was provided without corresponding " - "key. Ignoring.") - - timeout = float(timeout) if timeout is not None else None - session = client_session.Session(verify=verify, - cert=session_cert, - original_ip=original_ip, - timeout=timeout) + session = client_session.Session.construct(kwargs) self.session = session self.domain = '' diff --git a/keystoneclient/session.py b/keystoneclient/session.py index aa878d024..368a7603f 100644 --- a/keystoneclient/session.py +++ b/keystoneclient/session.py @@ -249,3 +249,40 @@ class Session(object): def patch(self, url, **kwargs): return self.request(url, 'PATCH', **kwargs) + + @classmethod + def construct(cls, kwargs): + """Handles constructing a session from the older HTTPClient args as + well as the new request style arguments. + + *DEPRECATED*: This function is purely for bridging the gap between + older client arguments and the session arguments that they relate to. + It is not intended to be used as a generic Session Factory. + + This function purposefully modifies the input kwargs dictionary so that + the remaining kwargs dict can be reused and passed on to other + functionswithout session arguments. + + """ + verify = kwargs.pop('verify', None) + cacert = kwargs.pop('cacert', None) + cert = kwargs.pop('cert', None) + key = kwargs.pop('key', None) + insecure = kwargs.pop('insecure', False) + + if verify is None: + if insecure: + verify = False + else: + verify = cacert or True + + if cert and key: + # passing cert and key together is deprecated in favour of the + # requests lib form of having the cert and key as a tuple + cert = (cert, key) + + return cls(verify=verify, cert=cert, + timeout=kwargs.pop('timeout', None), + session=kwargs.pop('session', None), + original_ip=kwargs.pop('original_ip', None), + user_agent=kwargs.pop('user_agent', None)) diff --git a/keystoneclient/tests/test_session.py b/keystoneclient/tests/test_session.py index 0edaac48c..e46603f08 100644 --- a/keystoneclient/tests/test_session.py +++ b/keystoneclient/tests/test_session.py @@ -222,3 +222,33 @@ class RedirectTests(utils.TestCase): for r, s in zip(req_resp.history, ses_resp.history): self.assertEqual(r.url, s.url) self.assertEqual(r.status_code, s.status_code) + + +class ConstructSessionFromArgsTests(utils.TestCase): + + KEY = 'keyfile' + CERT = 'certfile' + CACERT = 'cacert-path' + + def _s(self, k=None, **kwargs): + k = k or kwargs + return client_session.Session.construct(k) + + def test_verify(self): + self.assertFalse(self._s(insecure=True).verify) + self.assertTrue(self._s(verify=True, insecure=True).verify) + self.assertFalse(self._s(verify=False, insecure=True).verify) + self.assertEqual(self._s(cacert=self.CACERT).verify, self.CACERT) + + def test_cert(self): + tup = (self.CERT, self.KEY) + self.assertEqual(self._s(cert=tup).cert, tup) + self.assertEqual(self._s(cert=self.CERT, key=self.KEY).cert, tup) + self.assertIsNone(self._s(key=self.KEY).cert) + + def test_pass_through(self): + value = 42 # only a number because timeout needs to be + for key in ['timeout', 'session', 'original_ip', 'user_agent']: + args = {key: value} + self.assertEqual(getattr(self._s(args), key), value) + self.assertNotIn(key, args)