From 70f2e7113ebd46bac9e778ba1318d30a542852bd Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Tue, 15 Apr 2014 13:46:12 -0700 Subject: [PATCH] Use requests module for HTTP/HTTPS This change introduces the use of requests in lieu of httplib2 to ensure proper handling of SSL termination. Implements: blueprint tls-verify Change-Id: If182f2addf26421873b8c3d2b60f8cba9b7a9450 --- neutronclient/client.py | 63 ++++++++++++++------------ neutronclient/common/utils.py | 5 +- neutronclient/tests/unit/test_auth.py | 44 ++++++++---------- neutronclient/tests/unit/test_cli20.py | 7 +-- neutronclient/tests/unit/test_http.py | 11 ++--- neutronclient/tests/unit/test_ssl.py | 8 ++-- neutronclient/v2_0/client.py | 14 +++--- requirements.txt | 1 + 8 files changed, 79 insertions(+), 74 deletions(-) diff --git a/neutronclient/client.py b/neutronclient/client.py index 82bd410de..c79cd0beb 100644 --- a/neutronclient/client.py +++ b/neutronclient/client.py @@ -21,7 +21,7 @@ except ImportError: import logging import os -import httplib2 +import requests from neutronclient.common import exceptions from neutronclient.common import utils @@ -29,15 +29,15 @@ from neutronclient.openstack.common.gettextutils import _ _logger = logging.getLogger(__name__) -# httplib2 retries requests on socket.timeout which -# is not idempotent and can lead to orhan objects. -# See: https://code.google.com/p/httplib2/issues/detail?id=124 -httplib2.RETRIES = 1 - if os.environ.get('NEUTRONCLIENT_DEBUG'): ch = logging.StreamHandler() _logger.setLevel(logging.DEBUG) _logger.addHandler(ch) + _requests_log_level = logging.DEBUG +else: + _requests_log_level = logging.WARNING + +logging.getLogger("requests").setLevel(_requests_log_level) class ServiceCatalog(object): @@ -89,7 +89,7 @@ class ServiceCatalog(object): return matching_endpoints[0][endpoint_type] -class HTTPClient(httplib2.Http): +class HTTPClient(object): """Handles the REST calls and responses, include authn.""" USER_AGENT = 'python-neutronclient' @@ -102,7 +102,6 @@ class HTTPClient(httplib2.Http): auth_strategy='keystone', ca_cert=None, log_credentials=False, service_type='network', **kwargs): - super(HTTPClient, self).__init__(timeout=timeout, ca_certs=ca_cert) self.username = username self.tenant_name = tenant_name @@ -112,6 +111,7 @@ class HTTPClient(httplib2.Http): self.service_type = service_type self.endpoint_type = endpoint_type self.region_name = region_name + self.timeout = timeout self.auth_token = token self.auth_tenant_id = None self.auth_user_id = None @@ -119,8 +119,10 @@ class HTTPClient(httplib2.Http): self.endpoint_url = endpoint_url self.auth_strategy = auth_strategy self.log_credentials = log_credentials - # httplib2 overrides - self.disable_ssl_certificate_validation = insecure + if insecure: + self.verify_cert = False + else: + self.verify_cert = ca_cert if ca_cert else True def _cs_request(self, *args, **kwargs): kargs = {} @@ -147,7 +149,7 @@ class HTTPClient(httplib2.Http): utils.http_log_req(_logger, args, log_kargs) try: resp, body = self.request(*args, **kargs) - except httplib2.SSLHandshakeError as e: + except requests.exceptions.SSLError as e: raise exceptions.SslCertificateValidationError(reason=e) except Exception as e: # Wrap the low-level connection error (socket timeout, redirect @@ -155,11 +157,6 @@ class HTTPClient(httplib2.Http): # connection exception (it is excepted in the upper layers of code) _logger.debug("throwing ConnectionFailed : %s", e) raise exceptions.ConnectionFailed(reason=e) - finally: - # Temporary Fix for gate failures. RPC calls and HTTP requests - # seem to be stepping on each other resulting in bogus fd's being - # picked up for making http requests - self.connections.clear() utils.http_log_resp(_logger, resp, body) status_code = self.get_status_code(resp) if status_code == 401: @@ -181,6 +178,22 @@ class HTTPClient(httplib2.Http): elif not self.endpoint_url: self.endpoint_url = self._get_endpoint_url() + def request(self, url, method, **kwargs): + kwargs.setdefault('headers', kwargs.get('headers', {})) + kwargs['headers']['User-Agent'] = self.USER_AGENT + kwargs['headers']['Accept'] = 'application/json' + if 'body' in kwargs: + kwargs['headers']['Content-Type'] = 'application/json' + kwargs['data'] = kwargs['body'] + del kwargs['body'] + resp = requests.request( + method, + url, + verify=self.verify_cert, + **kwargs) + + return resp, resp.text + def do_request(self, url, method, **kwargs): self.authenticate_and_fetch_endpoint_url() # Perform the request once. If we get a 401 back then it @@ -234,16 +247,10 @@ class HTTPClient(httplib2.Http): raise exceptions.NoAuthURLProvided() token_url = self.auth_url + "/tokens" - - # Make sure we follow redirects when trying to reach Keystone - tmp_follow_all_redirects = self.follow_all_redirects - self.follow_all_redirects = True - try: - resp, resp_body = self._cs_request(token_url, "POST", - body=json.dumps(body), - content_type="application/json") - finally: - self.follow_all_redirects = tmp_follow_all_redirects + resp, resp_body = self._cs_request(token_url, "POST", + body=json.dumps(body), + content_type="application/json", + allow_redirects=True) status_code = self.get_status_code(resp) if status_code != 200: raise exceptions.Unauthorized(message=resp_body) @@ -305,10 +312,10 @@ class HTTPClient(httplib2.Http): def get_status_code(self, response): """Returns the integer status code from the response. - Either a Webob.Response (used in testing) or httplib.Response + Either a Webob.Response (used in testing) or requests.Response is returned. """ if hasattr(response, 'status_int'): return response.status_int else: - return response.status + return response.status_code diff --git a/neutronclient/common/utils.py b/neutronclient/common/utils.py index 338839ded..4478e8a24 100644 --- a/neutronclient/common/utils.py +++ b/neutronclient/common/utils.py @@ -178,7 +178,10 @@ def http_log_req(_logger, args, kwargs): def http_log_resp(_logger, resp, body): if not _logger.isEnabledFor(logging.DEBUG): return - _logger.debug(_("RESP:%(resp)s %(body)s\n"), {'resp': resp, 'body': body}) + _logger.debug(_("RESP:%(code)s %(headers)s %(body)s\n"), + {'code': resp.status_code, + 'headers': resp.headers, + 'body': body}) def _safe_encode_without_obj(data): diff --git a/neutronclient/tests/unit/test_auth.py b/neutronclient/tests/unit/test_auth.py index 84867d756..485d51132 100644 --- a/neutronclient/tests/unit/test_auth.py +++ b/neutronclient/tests/unit/test_auth.py @@ -15,11 +15,11 @@ # import copy -import httplib2 import json import uuid import mox +import requests import testtools from neutronclient import client @@ -68,6 +68,13 @@ ENDPOINTS_RESULT = { } +def get_response(status_code, headers=None): + response = mox.Mox().CreateMock(requests.Response) + response.headers = headers or {} + response.status_code = status_code + return response + + class CLITestAuthNoAuth(testtools.TestCase): def setUp(self): @@ -86,8 +93,7 @@ class CLITestAuthNoAuth(testtools.TestCase): def test_get_noauth(self): self.mox.StubOutWithMock(self.client, "request") - res200 = self.mox.CreateMock(httplib2.Response) - res200.status = 200 + res200 = get_response(200) self.client.request( mox.StrContains(ENDPOINT_URL + '/resource'), 'GET', @@ -136,8 +142,7 @@ class CLITestAuthKeystone(testtools.TestCase): def test_get_token(self): self.mox.StubOutWithMock(self.client, "request") - res200 = self.mox.CreateMock(httplib2.Response) - res200.status = 200 + res200 = get_response(200) self.client.request( AUTH_URL + '/tokens', 'POST', @@ -159,10 +164,8 @@ class CLITestAuthKeystone(testtools.TestCase): self.client.auth_token = TOKEN self.client.endpoint_url = ENDPOINT_URL - res200 = self.mox.CreateMock(httplib2.Response) - res200.status = 200 - res401 = self.mox.CreateMock(httplib2.Response) - res401.status = 401 + res200 = get_response(200) + res401 = get_response(401) # If a token is expired, neutron server retruns 401 self.client.request( @@ -187,8 +190,7 @@ class CLITestAuthKeystone(testtools.TestCase): self.client.auth_token = TOKEN self.client.endpoint_url = ENDPOINT_URL - res401 = self.mox.CreateMock(httplib2.Response) - res401.status = 401 + res401 = get_response(401) # If a token is expired, neutron server returns 401 self.client.request( @@ -212,8 +214,7 @@ class CLITestAuthKeystone(testtools.TestCase): self.client.auth_token = TOKEN - res200 = self.mox.CreateMock(httplib2.Response) - res200.status = 200 + res200 = get_response(200) self.client.request( mox.StrContains(AUTH_URL + '/tokens/%s/endpoints' % TOKEN), 'GET', @@ -236,9 +237,7 @@ class CLITestAuthKeystone(testtools.TestCase): self.mox.StubOutWithMock(self.client, "request") self.client.auth_token = TOKEN - - res200 = self.mox.CreateMock(httplib2.Response) - res200.status = 200 + res200 = get_response(200) self.client.request( mox.StrContains(ENDPOINT_OVERRIDE + '/resource'), 'GET', @@ -255,9 +254,7 @@ class CLITestAuthKeystone(testtools.TestCase): self.mox.StubOutWithMock(self.client, "request") self.client.auth_token = TOKEN - - res200 = self.mox.CreateMock(httplib2.Response) - res200.status = 200 + res200 = get_response(200) self.client.request( mox.StrContains(AUTH_URL + '/tokens/%s/endpoints' % TOKEN), 'GET', @@ -274,10 +271,8 @@ class CLITestAuthKeystone(testtools.TestCase): self.client.auth_token = TOKEN - res200 = self.mox.CreateMock(httplib2.Response) - res200.status = 200 - res401 = self.mox.CreateMock(httplib2.Response) - res401.status = 401 + res200 = get_response(200) + res401 = get_response(401) self.client.request( mox.StrContains(AUTH_URL + '/tokens/%s/endpoints' % TOKEN), 'GET', @@ -433,8 +428,7 @@ class CLITestAuthKeystone(testtools.TestCase): self.mox.StubOutWithMock(self.client, "request") self.mox.StubOutWithMock(utils, "http_log_req") - res200 = self.mox.CreateMock(httplib2.Response) - res200.status = 200 + res200 = get_response(200) utils.http_log_req(mox.IgnoreArg(), mox.IgnoreArg(), mox.Func( verify_no_credentials)) diff --git a/neutronclient/tests/unit/test_cli20.py b/neutronclient/tests/unit/test_cli20.py index b9cc2d5ba..51ff0bf00 100644 --- a/neutronclient/tests/unit/test_cli20.py +++ b/neutronclient/tests/unit/test_cli20.py @@ -48,8 +48,9 @@ class FakeStdout: class MyResp(object): - def __init__(self, status, reason=None): - self.status = status + def __init__(self, status_code, headers=None, reason=None): + self.status_code = status_code + self.headers = headers or {} self.reason = reason @@ -533,7 +534,7 @@ class ClientV2TestJson(CLITestV20Base): end_url('/test', query=expect_query, format=self.format), 'PUT', body='', headers=mox.ContainsKeyValue('X-Auth-Token', 'token') - ).AndReturn((MyResp(400, 'An error'), '')) + ).AndReturn((MyResp(400, reason='An error'), '')) self.mox.ReplayAll() error = self.assertRaises(exceptions.NeutronClientException, diff --git a/neutronclient/tests/unit/test_http.py b/neutronclient/tests/unit/test_http.py index 1114f33be..77a3a4a75 100644 --- a/neutronclient/tests/unit/test_http.py +++ b/neutronclient/tests/unit/test_http.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import httplib2 import mox import testtools @@ -33,13 +32,13 @@ class TestHTTPClient(testtools.TestCase): super(TestHTTPClient, self).setUp() self.mox = mox.Mox() - self.mox.StubOutWithMock(httplib2.Http, 'request') + self.mox.StubOutWithMock(HTTPClient, 'request') self.addCleanup(self.mox.UnsetStubs) self.http = HTTPClient(token=AUTH_TOKEN, endpoint_url=END_URL) def test_request_error(self): - httplib2.Http.request( + HTTPClient.request( URL, METHOD, headers=mox.IgnoreArg() ).AndRaise(Exception('error msg')) self.mox.ReplayAll() @@ -54,7 +53,7 @@ class TestHTTPClient(testtools.TestCase): def test_request_success(self): rv_should_be = MyResp(200), 'test content' - httplib2.Http.request( + HTTPClient.request( URL, METHOD, headers=mox.IgnoreArg() ).AndReturn(rv_should_be) self.mox.ReplayAll() @@ -64,7 +63,7 @@ class TestHTTPClient(testtools.TestCase): def test_request_unauthorized(self): rv_should_be = MyResp(401), 'unauthorized message' - httplib2.Http.request( + HTTPClient.request( URL, METHOD, headers=mox.IgnoreArg() ).AndReturn(rv_should_be) self.mox.ReplayAll() @@ -76,7 +75,7 @@ class TestHTTPClient(testtools.TestCase): def test_request_forbidden_is_returned_to_caller(self): rv_should_be = MyResp(403), 'forbidden message' - httplib2.Http.request( + HTTPClient.request( URL, METHOD, headers=mox.IgnoreArg() ).AndReturn(rv_should_be) self.mox.ReplayAll() diff --git a/neutronclient/tests/unit/test_ssl.py b/neutronclient/tests/unit/test_ssl.py index 9f3970dd5..81f831ba7 100644 --- a/neutronclient/tests/unit/test_ssl.py +++ b/neutronclient/tests/unit/test_ssl.py @@ -14,8 +14,8 @@ # under the License. import fixtures -import httplib2 import mox +import requests import testtools from neutronclient.client import HTTPClient @@ -126,10 +126,10 @@ class TestSSL(testtools.TestCase): def test_proper_exception_is_raised_when_cert_validation_fails(self): http = HTTPClient(token=AUTH_TOKEN, endpoint_url=END_URL) - self.mox.StubOutWithMock(httplib2.Http, 'request') - httplib2.Http.request( + self.mox.StubOutWithMock(HTTPClient, 'request') + HTTPClient.request( URL, METHOD, headers=mox.IgnoreArg() - ).AndRaise(httplib2.SSLHandshakeError) + ).AndRaise(requests.exceptions.SSLError) self.mox.ReplayAll() self.assertRaises( diff --git a/neutronclient/v2_0/client.py b/neutronclient/v2_0/client.py index c1d441b6f..cf1a214f6 100644 --- a/neutronclient/v2_0/client.py +++ b/neutronclient/v2_0/client.py @@ -14,11 +14,11 @@ # under the License. # -import httplib import logging import time import urllib +import requests import six.moves.urllib.parse as urlparse from neutronclient import client @@ -1231,10 +1231,10 @@ class Client(object): self.httpclient.content_type = self.content_type() resp, replybody = self.httpclient.do_request(action, method, body=body) status_code = self.get_status_code(resp) - if status_code in (httplib.OK, - httplib.CREATED, - httplib.ACCEPTED, - httplib.NO_CONTENT): + if status_code in (requests.codes.ok, + requests.codes.created, + requests.codes.accepted, + requests.codes.no_content): return self.deserialize(replybody, status_code) else: if not replybody: @@ -1247,13 +1247,13 @@ class Client(object): def get_status_code(self, response): """Returns the integer status code from the response. - Either a Webob.Response (used in testing) or httplib.Response + Either a Webob.Response (used in testing) or requests.Response is returned. """ if hasattr(response, 'status_int'): return response.status_int else: - return response.status + return response.status_code def serialize(self, data): """Serializes a dictionary into either xml or json. diff --git a/requirements.txt b/requirements.txt index 09b4537d2..970993aa0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,6 +4,7 @@ cliff>=1.4.3 httplib2>=0.7.5 iso8601>=0.1.9 netaddr>=0.7.6 +requests>=1.1 simplejson>=2.0.9 six>=1.5.2 Babel>=1.3