Use HTTPClient from common apiclient code

Oslo common code contains HTTPClient class which handles sending HTTP
requests to OpenStack servers. It also can handle authentication,
logging and so on. So we can use features from common apiclient instead
of tuskarclient implementation.

Inherited tuskar's HTTPClient from common HTTPClient, removed
duplicated methods, modified _http_request() method to use request()
method from common HTTPClient.
Removed VerifiedHTTPSConnection class - we should use a similar
functionality from common HTTPClient.

Changed an exception processing - `requests` module (which is used in
common code) keeps status in response `status_code` attribute instead of
`status`. So we should use `response.status_code` in from_response()
function from tuskarclient.exc module to get response status.

Added missed requirement - stevedore>=0.12. It's used in apiclient.auth,
module (which is used in tuskarclient.common.http).

Removed unused code from test_http module.

Change-Id: Ib731187661a409a036931aa213278ca9b03a3519

requirements.txt

@@ -8,3 +8,4 @@ python-keystoneclient>=0.2,<0.4
 requests>=0.8
 simplejson
 six>=1.4.1
+stevedore>=0.12

tuskarclient/common/base.py

@@ -70,7 +70,7 @@ class Manager(object):
         return self._path(id)
 
     def _create(self, url, body):
-        resp, body = self.api.json_request('POST', url, body=body)
+        resp, body = self.api.json_request('POST', url, data=body)
         if body:
             return self.resource_class(self, body)
 
@@ -100,7 +100,7 @@ class Manager(object):
         return [obj_class(self, res, loaded=True) for res in data if res]
 
     def _update(self, url, body, response_key=None):
-        resp, body = self.api.json_request('PUT', url, body=body)
+        resp, body = self.api.json_request('PUT', url, data=body)
         # PUT requests may not return a body
         if body:
             return self.resource_class(self, body)

tuskarclient/common/http.py

@@ -13,33 +13,19 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import copy
 import logging
-import os
-import socket
 
-from six.moves import http_client as httplib
-from six.moves.urllib import parse as urlparse
 from six import StringIO
 
-try:
-    import ssl
-except ImportError:
-    #TODO(bcwaldon): Handle this failure more gracefully
-    pass
-
 try:
     import json
 except ImportError:
     import simplejson as json
 
-# Python 2.5 compat fix
+from tuskarclient import client as tuskarclient
-if not hasattr(urlparse, 'parse_qsl'):
-    import cgi
-    urlparse.parse_qsl = cgi.parse_qsl
-
 from tuskarclient import exc as tuskar_exc
-from tuskarclient.openstack.common.apiclient import exceptions as exc
+from tuskarclient.openstack.common.apiclient import auth
+from tuskarclient.openstack.common.apiclient import client
 
 
 LOG = logging.getLogger(__name__)
@@ -47,142 +33,49 @@ USER_AGENT = 'python-tuskarclient'
 CHUNKSIZE = 1024 * 64  # 64kB
 
 
-class HTTPClient(object):
+class TuskarAuthPlugin(auth.BaseAuthPlugin):
+
+    def _do_authenticate(self, http_client):
+        self.ksclient = tuskarclient._get_ksclient(**http_client.kwargs)
+
+    def token_and_endpoint(self, endpoint_type, service_type):
+        token = self.ksclient.auth_token
+        endpoint = tuskarclient._get_endpoint(self.ksclient,
+                                              endpoint_type=endpoint_type,
+                                              service_type=service_type)
+        return token, endpoint
+
+
+class HTTPClient(client.HTTPClient):
 
     def __init__(self, endpoint, **kwargs):
+        self.kwargs = kwargs
         self.endpoint = endpoint
-        self.auth_token = kwargs.get('token')
+        self.auth_plugin = TuskarAuthPlugin()
-        self.connection_params = self.get_connection_params(endpoint, **kwargs)
+        verify = not kwargs.get('insecure', False)
-
+        cert = kwargs.get('ca_file')
-    @staticmethod
+        timeout = kwargs.get('timeout')
-    def get_connection_params(endpoint, **kwargs):
+        super(HTTPClient, self).__init__(self.auth_plugin, verify=verify,
-        parts = urlparse.urlparse(endpoint)
+                                         cert=cert, timeout=timeout)
-
-        _args = (parts.hostname, parts.port, parts.path)
-        _kwargs = {'timeout': (float(kwargs.get('timeout'))
-                               if kwargs.get('timeout') else 600)}
-
-        if parts.scheme == 'https':
-            _class = VerifiedHTTPSConnection
-            _kwargs['ca_file'] = kwargs.get('ca_file', None)
-            _kwargs['cert_file'] = kwargs.get('cert_file', None)
-            _kwargs['key_file'] = kwargs.get('key_file', None)
-            _kwargs['insecure'] = kwargs.get('insecure', False)
-        elif parts.scheme == 'http':
-            _class = httplib.HTTPConnection
-        else:
-            msg = 'Unsupported scheme: %s' % parts.scheme
-            raise tuskar_exc.InvalidEndpoint(msg)
-
-        return (_class, _args, _kwargs)
-
-    def get_connection(self):
-        _class = self.connection_params[0]
-        try:
-            return _class(*self.connection_params[1][0:2],
-                          **self.connection_params[2])
-        except httplib.InvalidURL:
-            raise tuskar_exc.InvalidEndpoint()
-
-    def log_curl_request(self, method, url, kwargs):
-        curl = ['curl -i -X %s' % method]
-
-        for (key, value) in kwargs['headers'].items():
-            header = '-H \'%s: %s\'' % (key, value)
-            curl.append(header)
-
-        conn_params_fmt = [
-            ('key_file', '--key %s'),
-            ('cert_file', '--cert %s'),
-            ('ca_file', '--cacert %s'),
-        ]
-        for (key, fmt) in conn_params_fmt:
-            value = self.connection_params[2].get(key)
-            if value:
-                curl.append(fmt % value)
-
-        if self.connection_params[2].get('insecure'):
-            curl.append('-k')
-
-        if 'body' in kwargs:
-            curl.append('-d \'%s\'' % kwargs['body'])
-
-        curl.append('%s%s' % (self.endpoint, url))
-        LOG.debug(' '.join(curl))
-
-    @staticmethod
-    def log_http_response(resp, body=None):
-        status = (resp.version / 10.0, resp.status, resp.reason)
-        dump = ['\nHTTP/%.1f %s %s' % status]
-        dump.extend(['%s: %s' % (k, v) for k, v in resp.getheaders()])
-        dump.append('')
-        if body:
-            dump.extend([body, ''])
-        LOG.debug('\n'.join(dump))
-
-    def _make_connection_url(self, url):
-        # if we got absolute http path, we should do nothing with it
-        if url.startswith('http://') or url.startswith('https://'):
-            return url
-
-        (_class, _args, _kwargs) = self.connection_params
-        base_url = _args[2]
-        return '%s/%s' % (base_url.rstrip('/'), url.lstrip('/'))
 
     def _http_request(self, url, method, **kwargs):
-        """Send an http request with the specified characteristics.
+        """Send an http request with the specified characteristics."""
-
+        url = client.HTTPClient.concat_url(self.endpoint, url)
-        Wrapper around http_client.HTTP(S)Connection.request to handle tasks
+        resp = self.request(method, url, **kwargs)
-        such as setting headers and error handling.
+        self._http_log_resp(resp)
-        """
+        if resp.status_code == 300:
-        # Copy the kwargs so we can reuse the original in case of redirects
-        kwargs['headers'] = copy.deepcopy(kwargs.get('headers', {}))
-        kwargs['headers'].setdefault('User-Agent', USER_AGENT)
-        if self.auth_token:
-            kwargs['headers'].setdefault('X-Auth-Token', self.auth_token)
-
-        self.log_curl_request(method, url, kwargs)
-        conn = self.get_connection()
-
-        try:
-            conn_url = self._make_connection_url(url)
-            conn.request(method, conn_url, **kwargs)
-            resp = conn.getresponse()
-        except socket.gaierror as e:
-            raise tuskar_exc.InvalidEndpoint(
-                message="Error finding address for %(url)s: %(e)s" % {
-                        'url': url, 'e': e})
-        except (socket.error, socket.timeout) as e:
-            raise tuskar_exc.CommunicationError(
-                message="Error communicating with %(endpoint)s %(e)s" % {
-                    'endpoint': self.endpoint, 'e': e})
-
-        body_iter = ResponseBodyIterator(resp)
-
-        # Read body into string if it isn't obviously image data
-        if resp.getheader('content-type', None) != 'application/octet-stream':
-            body_str = ''.join([chunk for chunk in body_iter])
-            self.log_http_response(resp, body_str)
-            body_iter = StringIO(body_str)
-        else:
-            self.log_http_response(resp)
-
-        if 400 <= resp.status < 600:
-            LOG.warn("Request returned failure status.")
-            # NOTE(viktors): from_response() method checks for `status_code`
-            # attribute, instead of `status`, so we should add it to response
-            resp.status_code = resp.status
-            raise exc.from_response(resp, method, conn_url)
-        elif resp.status in (301, 302, 305):
-            # Redirected. Reissue the request to the new location.
-            new_location = resp.getheader('location')
-            return self._http_request(new_location, method, **kwargs)
-        elif resp.status == 300:
             # TODO(viktors): we should use exception for status 300 from common
             #                code, when we will have required exception in Oslo
             #                See patch https://review.openstack.org/#/c/63111/
             raise tuskar_exc.from_response(resp)
 
+        body_iter = resp.iter_content(CHUNKSIZE)
+
+        # Read body into string if it isn't obviously image data
+        if resp.headers.get('content-type') != 'application/octet-stream':
+            body_str = ''.join([chunk for chunk in body_iter])
+            body_iter = StringIO(body_str)
+
         return resp, body_iter
 
     def json_request(self, method, url, **kwargs):
@@ -190,13 +83,13 @@ class HTTPClient(object):
         kwargs['headers'].setdefault('Content-Type', 'application/json')
         kwargs['headers'].setdefault('Accept', 'application/json')
 
-        if 'body' in kwargs:
+        if 'data' in kwargs:
-            kwargs['body'] = json.dumps(kwargs['body'])
+            kwargs['data'] = json.dumps(kwargs['data'])
 
         resp, body_iter = self._http_request(url, method, **kwargs)
-        content_type = resp.getheader('content-type', None)
+        content_type = resp.headers.get('content-type')
 
-        if resp.status == 204 or resp.status == 205 or content_type is None:
+        if resp.status_code in (204, 205) or content_type is None:
             return resp, list()
 
         if 'application/json' in content_type:
@@ -215,84 +108,3 @@ class HTTPClient(object):
         kwargs['headers'].setdefault('Content-Type',
                                      'application/octet-stream')
         return self._http_request(url, method, **kwargs)
-
-
-class VerifiedHTTPSConnection(httplib.HTTPSConnection):
-    """http_client-compatibile connection using client-side SSL authentication
-
-    :see http://code.activestate.com/recipes/
-            577548-https-httplib-client-connection-with-certificate-v/
-    """
-
-    def __init__(self, host, port, key_file=None, cert_file=None,
-                 ca_file=None, timeout=None, insecure=False):
-        httplib.HTTPSConnection.__init__(self, host, port,
-                                         key_file=key_file,
-                                         cert_file=cert_file)
-        self.key_file = key_file
-        self.cert_file = cert_file
-        if ca_file is not None:
-            self.ca_file = ca_file
-        else:
-            self.ca_file = self.get_system_ca_file()
-        self.timeout = timeout
-        self.insecure = insecure
-
-    def connect(self):
-        """Connect to a host on a given (SSL) port.
-        If ca_file is pointing somewhere, use it to check Server Certificate.
-
-        Redefined/copied and extended from httplib.py:1105 (Python 2.6.x).
-        This is needed to pass cert_reqs=ssl.CERT_REQUIRED as parameter to
-        ssl.wrap_socket(), which forces SSL to check server certificate against
-        our client certificate.
-        """
-        sock = socket.create_connection((self.host, self.port), self.timeout)
-
-        if self._tunnel_host:
-            self.sock = sock
-            self._tunnel()
-
-        if self.insecure is True:
-            kwargs = {'cert_reqs': ssl.CERT_NONE}
-        else:
-            kwargs = {'cert_reqs': ssl.CERT_REQUIRED, 'ca_certs': self.ca_file}
-
-        if self.cert_file:
-            kwargs['certfile'] = self.cert_file
-            if self.key_file:
-                kwargs['keyfile'] = self.key_file
-
-        self.sock = ssl.wrap_socket(sock, **kwargs)
-
-    @staticmethod
-    def get_system_ca_file():
-        """Return path to system default CA file."""
-        # Standard CA file locations for Debian/Ubuntu, RedHat/Fedora,
-        # Suse, FreeBSD/OpenBSD
-        ca_path = ['/etc/ssl/certs/ca-certificates.crt',
-                   '/etc/pki/tls/certs/ca-bundle.crt',
-                   '/etc/ssl/ca-bundle.pem',
-                   '/etc/ssl/cert.pem']
-        for ca in ca_path:
-            if os.path.exists(ca):
-                return ca
-        return None
-
-
-class ResponseBodyIterator(object):
-    """A class that acts as an iterator over an HTTP response."""
-
-    def __init__(self, resp):
-        self.resp = resp
-
-    def __iter__(self):
-        while True:
-            yield self.next()
-
-    def next(self):
-        chunk = self.resp.read(CHUNKSIZE)
-        if chunk:
-            return chunk
-        else:
-            raise StopIteration()

tuskarclient/exc.py

@@ -53,5 +53,5 @@ for obj_name in dir(sys.modules[__name__]):
 
 def from_response(response):
     """Return an instance of an exc.HttpError based on httplib response."""
-    cls = _code_map.get(response.status, exc.HttpError)
+    cls = _code_map.get(response.status_code, exc.HttpError)
     return cls()

tuskarclient/tests/test_http.py

@@ -13,44 +13,18 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from tuskarclient.tests import utils as tutils
+import mock
+import requests
 
 from tuskarclient.common import http
 from tuskarclient import exc as tuskar_exc
 from tuskarclient.openstack.common.apiclient import exceptions as exc
+from tuskarclient.tests import utils as tutils
 
-import mock
 
 fixtures = {}
 
 
-class HttpClientUrlGenerationTest(tutils.TestCase):
-
-    def test_url_generation_trailing_slash_in_base(self):
-        client = http.HTTPClient('http://localhost/')
-        url = client._make_connection_url('/v1/resources')
-        print(client.connection_params)
-        self.assertEqual(url, '/v1/resources')
-
-    def test_url_generation_without_trailing_slash_in_base(self):
-        client = http.HTTPClient('http://localhost')
-        url = client._make_connection_url('/v1/resources')
-        print(client.connection_params)
-        self.assertEqual(url, '/v1/resources')
-
-    def test_url_generation_prefix_slash_in_path(self):
-        client = http.HTTPClient('http://localhost/')
-        url = client._make_connection_url('/v1/resources')
-        print(client.connection_params)
-        self.assertEqual(url, '/v1/resources')
-
-    def test_url_generation_without_prefix_slash_in_path(self):
-        client = http.HTTPClient('http://localhost')
-        url = client._make_connection_url('v1/resources')
-        print(client.connection_params)
-        self.assertEqual(url, '/v1/resources')
-
-
 class HttpClientRawRequestTest(tutils.TestCase):
 
     def setUp(self):
@@ -122,7 +96,6 @@ class HttpClientHTTPRequestTest(tutils.TestCase):
     def setUp(self):
         super(HttpClientHTTPRequestTest, self).setUp()
 
-        self.client = http.HTTPClient('http://localhost')
         self.call_args = {
             'provided_method': 'GET',
             'expected_method': 'GET',
@@ -140,31 +113,28 @@ class HttpClientHTTPRequestTest(tutils.TestCase):
         }
 
         self.mock_response = mock.MagicMock()
-        self.mock_response.read = lambda *args: None
+        self.mock_request = mock.MagicMock(return_value=self.mock_response)
 
-        self.mock_response_2 = mock.MagicMock()
+        self.http = mock.MagicMock()
-        self.mock_response_2.read = lambda *args: None
+        self.http.request = self.mock_request
+        requests.Session = mock.MagicMock(return_value=self.http)
 
-        self.mock_request = mock.MagicMock()
+        self.client = http.HTTPClient('http://localhost', http=self.http)
-
-        self.client.get_connection = mock.MagicMock(
-            return_value=self.mock_request,
-        )
 
     def test_raw_request_status_200(self):
-        self.mock_request.getresponse = lambda: self.mock_response
+        self.mock_request = lambda: self.mock_response
-        self.mock_response.status = 200
+        self.mock_response.status_code = 200
 
         args = self.call_args.copy()
         resp, body_iter = self.client._http_request(
             args['provided_url'],
             args['provided_method'],
             **args['provided_args'])
-        self.assertEqual(resp.status, 200)
+        self.assertEqual(resp.status_code, 200)
 
     def test_raw_request_status_300(self):
-        self.mock_request.getresponse = lambda: self.mock_response
+        self.mock_request = lambda: self.mock_response
-        self.mock_response.status = 300
+        self.mock_response.status_code = 300
 
         args = self.call_args.copy()
         self.assertRaises(tuskar_exc.HTTPMultipleChoices,
@@ -172,34 +142,9 @@ class HttpClientHTTPRequestTest(tutils.TestCase):
                           args['provided_url'], args['provided_method'],
                           **args['provided_args'])
 
-    def test_raw_request_status_301(self):
-        new_location = 'http://new_location.com'
-        self.mock_response.getheader.return_value = new_location
-        self.mock_response.status = 301
-        self.mock_response_2.status = 200
-
-        self.mock_request.getresponse.side_effect = [
-            self.mock_response, self.mock_response_2]
-
-        args = self.call_args.copy()
-
-        resp, body_iter = self.client._http_request(
-            args['provided_url'],
-            args['provided_method'],
-            **args['provided_args'])
-
-        self.assertEqual(self.mock_request.getresponse.call_count, 2)
-        self.mock_response.getheader.assert_called_called_with('location')
-        self.mock_request.request.assert_called_with(
-            args['provided_method'],
-            new_location,
-            **args['provided_args']
-        )
-        self.assertEqual(resp.status, 200)
-
     def test_raw_request_status_500(self):
-        self.mock_request.getresponse = lambda: self.mock_response
+        self.mock_request = lambda: self.mock_response
-        self.mock_response.status = 500
+        self.mock_response.status_code = 500
 
         args = self.call_args.copy()
         self.assertRaises(exc.InternalServerError,