Revert "Use HTTPClient from common apiclient code"
This reverts commit e83c56584ea182255af0ae54cf71104a77c6de1b. This code was failing to pass along the correct headers when trying to retrieve the Tuskar endpoint from Keystone. However, it worked upon reverting this commit. The reversion exposed an error in the tests for python 3, which is fixed as described in this abandoned patch: https://review.openstack.org/#/c/65673/ Closes-Bug: #1307945 Change-Id: I17fa9ddd8bd1722f207393bb1249249c81c1342b
This commit is contained in:
parent
3777496afb
commit
55b9990f93
@ -8,4 +8,3 @@ python-keystoneclient>=0.4.1
|
||||
requests>=1.1
|
||||
simplejson>=2.0.9
|
||||
six>=1.4.1
|
||||
stevedore>=0.12
|
||||
|
@ -75,7 +75,7 @@ class Manager(object):
|
||||
return self._path(id)
|
||||
|
||||
def _create(self, url, body):
|
||||
resp, body = self.api.json_request('POST', url, data=body)
|
||||
resp, body = self.api.json_request('POST', url, body=body)
|
||||
if body:
|
||||
return self.resource_class(self, body)
|
||||
|
||||
@ -105,7 +105,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, data=body)
|
||||
resp, body = self.api.json_request('PUT', url, body=body)
|
||||
# PUT requests may not return a body
|
||||
if body:
|
||||
return self.resource_class(self, body)
|
||||
|
@ -13,19 +13,33 @@
|
||||
# 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
|
||||
|
||||
from tuskarclient import client as tuskarclient
|
||||
# Python 2.5 compat fix
|
||||
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 auth
|
||||
from tuskarclient.openstack.common.apiclient import client
|
||||
from tuskarclient.openstack.common.apiclient import exceptions as exc
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
@ -33,49 +47,142 @@ USER_AGENT = 'python-tuskarclient'
|
||||
CHUNKSIZE = 1024 * 64 # 64kB
|
||||
|
||||
|
||||
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):
|
||||
class HTTPClient(object):
|
||||
|
||||
def __init__(self, endpoint, **kwargs):
|
||||
self.kwargs = kwargs
|
||||
self.endpoint = endpoint
|
||||
self.auth_plugin = TuskarAuthPlugin()
|
||||
verify = not kwargs.get('insecure', False)
|
||||
cert = kwargs.get('ca_file')
|
||||
timeout = kwargs.get('timeout')
|
||||
super(HTTPClient, self).__init__(self.auth_plugin, verify=verify,
|
||||
cert=cert, timeout=timeout)
|
||||
self.auth_token = kwargs.get('token')
|
||||
self.connection_params = self.get_connection_params(endpoint, **kwargs)
|
||||
|
||||
@staticmethod
|
||||
def get_connection_params(endpoint, **kwargs):
|
||||
parts = urlparse.urlparse(endpoint)
|
||||
|
||||
_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."""
|
||||
url = client.HTTPClient.concat_url(self.endpoint, url)
|
||||
resp = self.request(method, url, **kwargs)
|
||||
self._http_log_resp(resp)
|
||||
if resp.status_code == 300:
|
||||
"""Send an http request with the specified characteristics.
|
||||
|
||||
Wrapper around http_client.HTTP(S)Connection.request to handle tasks
|
||||
such as setting headers and error handling.
|
||||
"""
|
||||
# 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):
|
||||
@ -83,13 +190,13 @@ class HTTPClient(client.HTTPClient):
|
||||
kwargs['headers'].setdefault('Content-Type', 'application/json')
|
||||
kwargs['headers'].setdefault('Accept', 'application/json')
|
||||
|
||||
if 'data' in kwargs:
|
||||
kwargs['data'] = json.dumps(kwargs['data'])
|
||||
if 'body' in kwargs:
|
||||
kwargs['body'] = json.dumps(kwargs['body'])
|
||||
|
||||
resp, body_iter = self._http_request(url, method, **kwargs)
|
||||
content_type = resp.headers.get('content-type')
|
||||
content_type = resp.getheader('content-type', None)
|
||||
|
||||
if resp.status_code in (204, 205) or content_type is None:
|
||||
if resp.status == 204 or resp.status == 205 or content_type is None:
|
||||
return resp, list()
|
||||
|
||||
if 'application/json' in content_type:
|
||||
@ -108,3 +215,84 @@ class HTTPClient(client.HTTPClient):
|
||||
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()
|
||||
|
@ -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_code, exc.HttpError)
|
||||
cls = _code_map.get(response.status, exc.HttpError)
|
||||
return cls()
|
||||
|
@ -13,18 +13,44 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import mock
|
||||
import requests
|
||||
from tuskarclient.tests import utils as tutils
|
||||
|
||||
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):
|
||||
@ -96,6 +122,7 @@ 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',
|
||||
@ -113,28 +140,33 @@ class HttpClientHTTPRequestTest(tutils.TestCase):
|
||||
}
|
||||
|
||||
self.mock_response = mock.MagicMock()
|
||||
self.mock_request = mock.MagicMock(return_value=self.mock_response)
|
||||
self.mock_response.read = lambda *args: None
|
||||
self.mock_response.version = 42
|
||||
|
||||
self.http = mock.MagicMock()
|
||||
self.http.request = self.mock_request
|
||||
requests.Session = mock.MagicMock(return_value=self.http)
|
||||
self.mock_response_2 = mock.MagicMock()
|
||||
self.mock_response_2.read = lambda *args: None
|
||||
self.mock_response_2.version = 42
|
||||
|
||||
self.client = http.HTTPClient('http://localhost', http=self.http)
|
||||
self.mock_request = mock.MagicMock()
|
||||
|
||||
self.client.get_connection = mock.MagicMock(
|
||||
return_value=self.mock_request,
|
||||
)
|
||||
|
||||
def test_raw_request_status_200(self):
|
||||
self.mock_request = lambda: self.mock_response
|
||||
self.mock_response.status_code = 200
|
||||
self.mock_request.getresponse = lambda: self.mock_response
|
||||
self.mock_response.status = 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_code, 200)
|
||||
self.assertEqual(resp.status, 200)
|
||||
|
||||
def test_raw_request_status_300(self):
|
||||
self.mock_request = lambda: self.mock_response
|
||||
self.mock_response.status_code = 300
|
||||
self.mock_request.getresponse = lambda: self.mock_response
|
||||
self.mock_response.status = 300
|
||||
|
||||
args = self.call_args.copy()
|
||||
self.assertRaises(tuskar_exc.HTTPMultipleChoices,
|
||||
@ -142,9 +174,34 @@ 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 = lambda: self.mock_response
|
||||
self.mock_response.status_code = 500
|
||||
self.mock_request.getresponse = lambda: self.mock_response
|
||||
self.mock_response.status = 500
|
||||
|
||||
args = self.call_args.copy()
|
||||
self.assertRaises(exc.InternalServerError,
|
||||
|
Loading…
x
Reference in New Issue
Block a user