From 54a52ae7169adf4bb694c9ddc95c0d5068e76d5a Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Mon, 19 Jan 2015 16:44:12 +1000 Subject: [PATCH] Use the ksc Adapter instead of custom HTTPClient Keystoneclient provides the adapter to make it easier for projects to work with sessions. It allows more flexibility in querying the service catalog, and additional http options in a consistent way across projects. Change-Id: Ief96aec1057847ac0fde14f4a98b3d7b146415a4 --- barbicanclient/base.py | 2 +- barbicanclient/client.py | 97 +++++++++++------------------ barbicanclient/containers.py | 16 ++--- barbicanclient/orders.py | 11 ++-- barbicanclient/secrets.py | 13 ++-- barbicanclient/tests/test_client.py | 59 +++++++----------- 6 files changed, 78 insertions(+), 120 deletions(-) diff --git a/barbicanclient/base.py b/barbicanclient/base.py index c3350937..ec3f4aad 100644 --- a/barbicanclient/base.py +++ b/barbicanclient/base.py @@ -52,6 +52,6 @@ class BaseEntityManager(object): """ href = '{0}/{1}'.format(self._api._base_url, self._entity) params = {'limit': 0, 'offset': 0} - resp = self._api._get(href, params) + resp = self._api.get(href, params=params) return resp['total'] diff --git a/barbicanclient/client.py b/barbicanclient/client.py index 2af28101..e1571e41 100644 --- a/barbicanclient/client.py +++ b/barbicanclient/client.py @@ -16,6 +16,7 @@ import json import logging import os +from keystoneclient import adapter from keystoneclient.auth.base import BaseAuthPlugin from keystoneclient import session as ks_session @@ -57,13 +58,23 @@ class HTTPAuthError(HTTPError): pass -class _HTTPClient(object): +class _HTTPClient(adapter.Adapter): - def __init__(self, session, endpoint=None, project_id=None, - verify=True, service_type=_DEFAULT_SERVICE_TYPE, - service_name=None, interface=_DEFAULT_SERVICE_INTERFACE, - region_name=None): - self._session = session + def __init__(self, session, project_id=None, **kwargs): + kwargs.setdefault('interface', _DEFAULT_SERVICE_INTERFACE) + kwargs.setdefault('service_type', _DEFAULT_SERVICE_TYPE) + + self._base_url = None + + try: + endpoint = kwargs.pop('endpoint') + except KeyError: + pass + else: + self._base_url = '{0}/{1}'.format(endpoint, _DEFAULT_API_VERSION) + kwargs.setdefault('endpoint_override', self._base_url) + + super(_HTTPClient, self).__init__(session, **kwargs) if project_id is None: self._default_headers = dict() @@ -71,67 +82,31 @@ class _HTTPClient(object): # If provided we'll include the project ID in all requests. self._default_headers = {'X-Project-Id': project_id} - if not endpoint: - endpoint = session.get_endpoint(service_type=service_type, - service_name=service_name, - interface=interface, - region_name=region_name) - - if endpoint.endswith('/'): - endpoint = endpoint[:-1] - - self._barbican_endpoint = endpoint - self._base_url = '{0}/{1}'.format(endpoint, _DEFAULT_API_VERSION) - - def _get(self, href, params=None): - headers = {'Accept': 'application/json'} + def request(self, *args, **kwargs): + headers = kwargs.setdefault('headers', {}) headers.update(self._default_headers) - resp = self._session.get(href, params=params, headers=headers) + resp = super(_HTTPClient, self).request(*args, **kwargs) + # NOTE(jamielennox): _check_status_code is being completely ignored as + # errors are being raised from session.request. This behaviour is + # enforced by tests. Pass raise_exc=False to request() to make this + # work again. self._check_status_code(resp) - return resp.json() + return resp - def _get_raw(self, href, headers): - headers.update(self._default_headers) - resp = self._session.get(href, headers=headers) - self._check_status_code(resp) - return resp.content + def get(self, *args, **kwargs): + headers = kwargs.setdefault('headers', {}) + headers.setdefault('Accept', 'application/json') - def _delete(self, href, json=None): - headers = dict() - headers.update(self._default_headers) - resp = self._session.delete(href, headers=headers, json=json) - self._check_status_code(resp) + return super(_HTTPClient, self).get(*args, **kwargs).json() - def _deserialization_helper(self, obj): - """ - Help deserialization of objects which may require special processing - (for example datetime objects). If your object gives you - json.dumps errors when you attempt to deserialize then this - function is the place where you will handle that special case. + def post(self, path, *args, **kwargs): + if not path[-1] == '/': + path += '/' - :param obj: an object that may or may not require special processing - :return: the stringified object (if it required special processing) or - the object itself. - """ - # by default, return the object itself - return_str = obj + return super(_HTTPClient, self).post(path, *args, **kwargs).json() - # special case for objects that contain isoformat method (ie datetime) - if hasattr(obj, 'isoformat'): - return_str = obj.isoformat() - - return return_str - - def _post(self, path, data): - url = '{0}/{1}/'.format(self._base_url, path) - headers = {'Content-Type': 'application/json'} - headers.update(self._default_headers) - resp = self._session.post( - url, - data=json.dumps(data, default=self._deserialization_helper), - headers=headers) - self._check_status_code(resp) - return resp.json() + def _get_raw(self, path, *args, **kwargs): + return self.request(path, 'GET', *args, **kwargs).content def _check_status_code(self, resp): status = resp.status_code @@ -200,7 +175,7 @@ class Client(object): if not session: session = ks_session.Session(verify=kwargs.pop('verify', True)) - if session.auth is None: + if session.auth is None and kwargs.get('auth') is None: if kwargs.get('endpoint') is None: raise ValueError('Barbican endpoint url must be provided when ' 'not using auth in the Keystone Session.') diff --git a/barbicanclient/containers.py b/barbicanclient/containers.py index 849394e2..ecb2dba5 100644 --- a/barbicanclient/containers.py +++ b/barbicanclient/containers.py @@ -192,7 +192,7 @@ class Container(ContainerFormatter): LOG.debug("Request body: {0}".format(container_dict)) # Save, store container_ref and return - response = self._api._post(self._entity, container_dict) + response = self._api.post(self._entity, json=container_dict) if response: self._container_ref = response['container_ref'] return self.container_ref @@ -200,7 +200,7 @@ class Container(ContainerFormatter): def delete(self): """Delete container from Barbican""" if self._container_ref: - self._api._delete(self._container_ref) + self._api.delete(self._container_ref) self._container_ref = None self._status = None self._created = None @@ -225,7 +225,7 @@ class Container(ContainerFormatter): .format(self._container_ref)) base.validate_ref(self._container_ref, 'Container') try: - response = self._api._get(self._container_ref) + response = self._api.get(self._container_ref) except AttributeError: raise LookupError('Container {0} could not be found.' .format(self._container_ref)) @@ -527,7 +527,7 @@ class ContainerManager(base.BaseEntityManager): .format(container_ref)) base.validate_ref(container_ref, 'Container') try: - response = self._api._get(container_ref) + response = self._api.get(container_ref) except AttributeError: raise LookupError('Container {0} could not be found.' .format(container_ref)) @@ -677,7 +677,7 @@ class ContainerManager(base.BaseEntityManager): """ if not container_ref: raise ValueError('container_ref is required.') - self._api._delete(container_ref) + self._api.delete(container_ref) def list(self, limit=10, offset=0, name=None, type=None): """ @@ -699,7 +699,7 @@ class ContainerManager(base.BaseEntityManager): if type: params['type'] = type - response = self._api._get(href, params) + response = self._api.get(href, params=params) return [self._generate_typed_container(container) for container in response.get('containers', [])] @@ -721,7 +721,7 @@ class ContainerManager(base.BaseEntityManager): consumer_dict['name'] = name consumer_dict['URL'] = url - response = self._api._post(href, consumer_dict) + response = self._api.post(href, json=consumer_dict) return self._generate_typed_container(response) def remove_consumer(self, container_ref, name, url): @@ -742,4 +742,4 @@ class ContainerManager(base.BaseEntityManager): 'URL': url } - self._api._delete(href, json=consumer_dict) + self._api.delete(href, json=consumer_dict) diff --git a/barbicanclient/orders.py b/barbicanclient/orders.py index 8cfde36c..4cf974b2 100644 --- a/barbicanclient/orders.py +++ b/barbicanclient/orders.py @@ -193,7 +193,7 @@ class Order(object): """ order_dict = {'type': self._type, 'meta': self._meta} LOG.debug("Request body: {0}".format(order_dict)) - response = self._api._post(self._entity, order_dict) + response = self._api.post(self._entity, json=order_dict) if response: self._order_ref = response.get('order_ref') return self._order_ref @@ -203,7 +203,7 @@ class Order(object): Deletes the Order from Barbican """ if self._order_ref: - self._api._delete(self._order_ref) + self._api.delete(self._order_ref) self._order_ref = None else: raise LookupError("Order is not yet stored.") @@ -315,7 +315,7 @@ class OrderManager(base.BaseEntityManager): LOG.debug("Getting order - Order href: {0}".format(order_ref)) base.validate_ref(order_ref, 'Order') try: - response = self._api._get(order_ref) + response = self._api.get(order_ref) except AttributeError: raise LookupError( 'Order {0} could not be found.'.format(order_ref) @@ -395,7 +395,7 @@ class OrderManager(base.BaseEntityManager): """ if not order_ref: raise ValueError('order_ref is required.') - self._api._delete(order_ref) + self._api.delete(order_ref) def list(self, limit=10, offset=0): """ @@ -409,9 +409,8 @@ class OrderManager(base.BaseEntityManager): """ LOG.debug('Listing orders - offset {0} limit {1}'.format(offset, limit)) - href = '{0}/{1}'.format(self._api._base_url, self._entity) params = {'limit': limit, 'offset': offset} - response = self._api._get(href, params) + response = self._api.get(self._entity, params=params) return [ self._create_typed_order(o) for o in response.get('orders', []) diff --git a/barbicanclient/secrets.py b/barbicanclient/secrets.py index 9db80233..00781ab0 100644 --- a/barbicanclient/secrets.py +++ b/barbicanclient/secrets.py @@ -241,7 +241,7 @@ class Secret(SecretFormatter): payload_url = self._secret_ref + '/payload' else: payload_url = self._secret_ref + 'payload' - payload = self._api._get_raw(payload_url, headers) + payload = self._api._get_raw(payload_url, headers=headers) if self.payload_content_type == u'text/plain': self._payload = payload.decode('UTF-8') else: @@ -301,7 +301,7 @@ class Secret(SecretFormatter): LOG.debug("Request body: {0}".format(secret_dict)) # Save, store secret_ref and return - response = self._api._post(self._entity, secret_dict) + response = self._api.post(self._entity, json=secret_dict) if response: self._secret_ref = response.get('secret_ref') return self.secret_ref @@ -311,7 +311,7 @@ class Secret(SecretFormatter): Deletes the Secret from Barbican """ if self._secret_ref: - self._api._delete(self._secret_ref) + self._api.delete(self._secret_ref) self._secret_ref = None else: raise LookupError("Secret is not yet stored.") @@ -353,7 +353,7 @@ class Secret(SecretFormatter): def _fill_lazy_properties(self): if self._secret_ref and not self._name: - result = self._api._get(self._secret_ref) + result = self._api.get(self._secret_ref) self._fill_from_data( name=result.get('name'), expiration=result.get('expiration'), @@ -440,7 +440,7 @@ class SecretManager(base.BaseEntityManager): base.validate_ref(secret_ref, 'Secret') if not secret_ref: raise ValueError('secret_ref is required.') - self._api._delete(secret_ref) + self._api.delete(secret_ref) def list(self, limit=10, offset=0, name=None, algorithm=None, mode=None, bits=0): @@ -462,7 +462,6 @@ class SecretManager(base.BaseEntityManager): """ LOG.debug('Listing secrets - offset {0} limit {1}'.format(offset, limit)) - href = '{0}/{1}'.format(self._api._base_url, self._entity) params = {'limit': limit, 'offset': offset} if name: params['name'] = name @@ -473,7 +472,7 @@ class SecretManager(base.BaseEntityManager): if bits > 0: params['bits'] = bits - response = self._api._get(href, params) + response = self._api.get(self._entity, params=params) return [ Secret(api=self._api, **s) diff --git a/barbicanclient/tests/test_client.py b/barbicanclient/tests/test_client.py index b0269b23..4071d160 100644 --- a/barbicanclient/tests/test_client.py +++ b/barbicanclient/tests/test_client.py @@ -36,12 +36,6 @@ class TestClient(testtools.TestCase): class WhenTestingClientInit(TestClient): - def test_can_be_used_without_a_session(self): - c = client._HTTPClient(session=self.session, - endpoint=self.endpoint, - project_id=self.project_id) - self.assertIsNotNone(c._session) - def test_api_version_is_appended_to_endpoint(self): c = client._HTTPClient(session=self.session, endpoint=self.endpoint, @@ -49,7 +43,7 @@ class WhenTestingClientInit(TestClient): self.assertEqual(c._base_url, 'http://localhost:9311/v1') def test_default_headers_are_empty(self): - c = client._HTTPClient(self.session, self.endpoint) + c = client._HTTPClient(session=self.session, endpoint=self.endpoint) self.assertIsInstance(c._default_headers, dict) self.assertFalse(bool(c._default_headers)) @@ -68,12 +62,6 @@ class WhenTestingClientInit(TestClient): self.assertRaises(ValueError, client.Client, **{"endpoint": self.endpoint}) - def test_client_strips_trailing_slash_from_endpoint(self): - c = client._HTTPClient(session=self.session, - endpoint=self.endpoint + '/', - project_id=self.project_id) - self.assertEqual(c._barbican_endpoint, self.endpoint) - def test_base_url_starts_with_endpoint_url(self): c = client._HTTPClient(session=self.session, endpoint=self.endpoint, @@ -86,39 +74,35 @@ class WhenTestingClientInit(TestClient): project_id=self.project_id) self.assertTrue(c._base_url.endswith(client._DEFAULT_API_VERSION)) - def test_gets_endpoint_from_keystone_session(self): - c = client._HTTPClient(session=self.session, - endpoint=self.endpoint) - self.assertEqual(c._barbican_endpoint, self.endpoint) - class WhenTestingClientPost(TestClient): def setUp(self): super(WhenTestingClientPost, self).setUp() - self.httpclient = client._HTTPClient(self.session, self.endpoint) + self.httpclient = client._HTTPClient(session=self.session, + endpoint=self.endpoint) self.href = self.endpoint + '/v1/secrets/' self.post_mock = self.responses.post(self.href, json={}) def test_post_normalizes_url_with_traling_slash(self): - self.httpclient._post(path='secrets', data={'test_data': 'test'}) + self.httpclient.post(path='secrets', json={'test_data': 'test'}) self.assertTrue(self.post_mock.last_request.url.endswith('/')) def test_post_includes_content_type_header_of_application_json(self): - self.httpclient._post(path='secrets', data={'test_data': 'test'}) + self.httpclient.post(path='secrets', json={'test_data': 'test'}) self.assertEqual('application/json', self.post_mock.last_request.headers['Content-Type']) def test_post_includes_default_headers(self): self.httpclient._default_headers = {'Test-Default-Header': 'test'} - self.httpclient._post(path='secrets', data={'test_data': 'test'}) + self.httpclient.post(path='secrets', json={'test_data': 'test'}) self.assertEqual( 'test', self.post_mock.last_request.headers['Test-Default-Header']) def test_post_checks_status_code(self): self.httpclient._check_status_code = mock.MagicMock() - self.httpclient._post(path='secrets', data={'test_data': 'test'}) + self.httpclient.post(path='secrets', json={'test_data': 'test'}) self.httpclient._check_status_code.assert_called() @@ -126,56 +110,57 @@ class WhenTestingClientGet(TestClient): def setUp(self): super(WhenTestingClientGet, self).setUp() - self.httpclient = client._HTTPClient(self.session, self.endpoint) + self.httpclient = client._HTTPClient(session=self.session, + endpoint=self.endpoint) self.headers = dict() self.href = 'http://test_href/' self.get_mock = self.responses.get(self.href, json={}) def test_get_uses_href_as_is(self): - self.httpclient._get(self.href) + self.httpclient.get(self.href) self.assertEqual(self.get_mock.last_request.url, self.href) def test_get_passes_params(self): params = {'test': 'test1'} - self.httpclient._get(self.href, params=params) + self.httpclient.get(self.href, params=params) self.assertEqual(self.get_mock.last_request.url.split('?')[0], self.href) self.assertEqual(['test1'], self.get_mock.last_request.qs['test']) def test_get_includes_accept_header_of_application_json(self): - self.httpclient._get(self.href) + self.httpclient.get(self.href) self.assertEqual('application/json', self.get_mock.last_request.headers['Accept']) def test_get_includes_default_headers(self): self.httpclient._default_headers = {'Test-Default-Header': 'test'} - self.httpclient._get(self.href) + self.httpclient.get(self.href) self.assertEqual( 'test', self.get_mock.last_request.headers['Test-Default-Header']) def test_get_checks_status_code(self): self.httpclient._check_status_code = mock.MagicMock() - self.httpclient._get(self.href) + self.httpclient.get(self.href) self.httpclient._check_status_code.assert_called() def test_get_raw_uses_href_as_is(self): - self.httpclient._get_raw(self.href, self.headers) + self.httpclient._get_raw(self.href, headers=self.headers) self.assertEqual(self.get_mock.last_request.url, self.href) def test_get_raw_passes_headers(self): - self.httpclient._get_raw(self.href, {'test': 'test'}) + self.httpclient._get_raw(self.href, headers={'test': 'test'}) self.assertEqual('test', self.get_mock.last_request.headers['test']) def test_get_raw_includes_default_headers(self): self.httpclient._default_headers = {'Test-Default-Header': 'test'} - self.httpclient._get_raw(self.href, self.headers) + self.httpclient._get_raw(self.href, headers=self.headers) self.assertIn('Test-Default-Header', self.get_mock.last_request.headers) def test_get_raw_checks_status_code(self): self.httpclient._check_status_code = mock.MagicMock() - self.httpclient._get_raw(self.href, self.headers) + self.httpclient._get_raw(self.href, headers=self.headers) self.httpclient._check_status_code.assert_called() @@ -189,24 +174,24 @@ class WhenTestingClientDelete(TestClient): self.del_mock = self.responses.delete(self.href, status_code=204) def test_delete_uses_href_as_is(self): - self.httpclient._delete(self.href) + self.httpclient.delete(self.href) self.assertTrue(self.del_mock.called) def test_delete_passes_json(self): json = {"test": "test"} - self.httpclient._delete(self.href, json=json) + self.httpclient.delete(self.href, json=json) self.assertEqual(self.del_mock.last_request.text, '{"test": "test"}') def test_delete_includes_default_headers(self): self.httpclient._default_headers = {'Test-Default-Header': 'test'} - self.httpclient._delete(self.href) + self.httpclient.delete(self.href) self.assertEqual( 'test', self.del_mock.last_request.headers['Test-Default-Header']) def test_delete_checks_status_code(self): self.httpclient._check_status_code = mock.MagicMock() - self.httpclient._delete(self.href) + self.httpclient.delete(self.href) self.httpclient._check_status_code.assert_called()