From ae542f685466dc65967c6d74d38d8935685256f5 Mon Sep 17 00:00:00 2001 From: James Page Date: Mon, 20 Aug 2018 15:22:10 +0100 Subject: [PATCH] metadata: use requests for comms with nova api httplib2 makes use of the ssl module provided by Python; under Python 2, the ssl module does not support IP addresses as subject alternate names (SAN's) which although an optional part of the associated RFC, is awkward to work with in environments where certificate management approaches rely on use of IP addresses in SAN's. The requests module is more than happy to deal with this scenario; switch to requests in preference of httplib2 for metadata proxy calls. httplib2 is retained as its used elsewhere in the codebase. Closes-Bug: 1790598 Change-Id: Ife4adf09ddbf7116da2f8596c80aed53fb6790df (cherry picked from commit 7e0dd2f18d4919964655cfce7a282d1c5c131fc4) --- neutron/agent/metadata/agent.py | 44 +++++++++++-------- .../tests/unit/agent/metadata/test_agent.py | 43 +++++++----------- requirements.txt | 1 + 3 files changed, 43 insertions(+), 45 deletions(-) diff --git a/neutron/agent/metadata/agent.py b/neutron/agent/metadata/agent.py index 8607d627e78..57540e92d12 100644 --- a/neutron/agent/metadata/agent.py +++ b/neutron/agent/metadata/agent.py @@ -15,7 +15,6 @@ import hashlib import hmac -import httplib2 from neutron_lib import constants from neutron_lib import context from oslo_config import cfg @@ -23,6 +22,7 @@ from oslo_log import log as logging import oslo_messaging from oslo_service import loopingcall from oslo_utils import encodeutils +import requests import six import six.moves.urllib.parse as urlparse import webob @@ -189,35 +189,41 @@ class MetadataProxyHandler(object): req.query_string, '')) - h = httplib2.Http( - ca_certs=self.conf.auth_ca_cert, - disable_ssl_certificate_validation=self.conf.nova_metadata_insecure - ) - if self.conf.nova_client_cert and self.conf.nova_client_priv_key: - h.add_certificate(self.conf.nova_client_priv_key, - self.conf.nova_client_cert, - nova_host_port) - resp, content = h.request(url, method=req.method, headers=headers, - body=req.body) + disable_ssl_certificate_validation = self.conf.nova_metadata_insecure + if self.conf.auth_ca_cert and not disable_ssl_certificate_validation: + verify_cert = self.conf.auth_ca_cert + else: + verify_cert = not disable_ssl_certificate_validation - if resp.status == 200: - req.response.content_type = resp['content-type'] - req.response.body = content + client_cert = None + if self.conf.nova_client_cert and self.conf.nova_client_priv_key: + client_cert = (self.conf.nova_client_cert, + self.conf.nova_client_priv_key) + + resp = requests.request(method=req.method, url=url, + headers=headers, + data=req.body, + cert=client_cert, + verify=verify_cert) + + if resp.status_code == 200: + req.response.content_type = resp.headers['content-type'] + req.response.body = resp.content LOG.debug(str(resp)) return req.response - elif resp.status == 403: + elif resp.status_code == 403: LOG.warning( 'The remote metadata server responded with Forbidden. This ' 'response usually occurs when shared secrets do not match.' ) return webob.exc.HTTPForbidden() - elif resp.status == 400: + elif resp.status_code == 400: return webob.exc.HTTPBadRequest() - elif resp.status == 404: + elif resp.status_code == 404: return webob.exc.HTTPNotFound() - elif resp.status == 409: + elif resp.status_code == 409: return webob.exc.HTTPConflict() - elif resp.status == 500: + elif resp.status_code == 500: msg = _( 'Remote metadata server experienced an internal server error.' ) diff --git a/neutron/tests/unit/agent/metadata/test_agent.py b/neutron/tests/unit/agent/metadata/test_agent.py index 8d9cd49938a..09c6c7fa2c1 100644 --- a/neutron/tests/unit/agent/metadata/test_agent.py +++ b/neutron/tests/unit/agent/metadata/test_agent.py @@ -339,37 +339,28 @@ class _TestMetadataProxyHandlerCacheMixin(object): req = mock.Mock(path_info='/the_path', query_string='', headers=hdrs, method=method, body=body) - resp = mock.MagicMock(status=response_code) + resp = mock.MagicMock(status_code=response_code) + resp.content = 'content' req.response = resp with mock.patch.object(self.handler, '_sign_instance_id') as sign: sign.return_value = 'signed' - with mock.patch('httplib2.Http') as mock_http: - resp.__getitem__.return_value = "text/plain" - mock_http.return_value.request.return_value = (resp, 'content') - + with mock.patch('requests.request') as mock_request: + resp.headers = {'content-type': 'text/plain'} + mock_request.return_value = resp retval = self.handler._proxy_request('the_id', 'tenant_id', req) - mock_http.assert_called_once_with( - ca_certs=None, disable_ssl_certificate_validation=True) - mock_http.assert_has_calls([ - mock.call().add_certificate( - self.fake_conf.nova_client_priv_key, - self.fake_conf.nova_client_cert, - "%s:%s" % (self.fake_conf.nova_metadata_host, - self.fake_conf.nova_metadata_port) - ), - mock.call().request( - 'http://9.9.9.9:8775/the_path', - method=method, - headers={ - 'X-Forwarded-For': '8.8.8.8', - 'X-Instance-ID-Signature': 'signed', - 'X-Instance-ID': 'the_id', - 'X-Tenant-ID': 'tenant_id' - }, - body=body - )] - ) + mock_request.assert_called_once_with( + method=method, url='http://9.9.9.9:8775/the_path', + headers={ + 'X-Forwarded-For': '8.8.8.8', + 'X-Instance-ID-Signature': 'signed', + 'X-Instance-ID': 'the_id', + 'X-Tenant-ID': 'tenant_id' + }, + data=body, + cert=(self.fake_conf.nova_client_cert, + self.fake_conf.nova_client_priv_key), + verify=False) return retval diff --git a/requirements.txt b/requirements.txt index b2cf2d0158a..55210099fa6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,6 +10,7 @@ debtcollector>=1.2.0 # Apache-2.0 eventlet!=0.18.3,!=0.20.1,<0.21.0,>=0.18.2 # MIT pecan!=1.0.2,!=1.0.3,!=1.0.4,!=1.2,>=1.0.0 # BSD httplib2>=0.9.1 # MIT +requests!=2.20.0,>=2.14.2 # Apache-2.0 Jinja2!=2.9.0,!=2.9.1,!=2.9.2,!=2.9.3,!=2.9.4,>=2.8 # BSD License (3 clause) keystonemiddleware>=4.17.0 # Apache-2.0 netaddr>=0.7.18 # BSD