From 1eb7fd233d29d2eb6c7c3c50b306c1a8602eb01f Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Tue, 16 Dec 2014 09:33:34 -0500 Subject: [PATCH] Switch to oslo.vmware API for reading and writing files This started off as an exercise to remove httplib.HTTPSConnection to prevent MiM attacks but morphed into a full fledged replacement of code in read_write_util to use functionality from oslo.vmware. As a side-effect, we remove the use of the HTTPSConnection and are switching over to requests library as well. Closes-Bug: #1374000 Change-Id: I917c34042c501af03725b0504542e00e7d80e511 --- nova/tests/unit/virt/vmwareapi/test_images.py | 6 +- .../virt/vmwareapi/test_read_write_util.py | 36 ++-- nova/virt/vmwareapi/images.py | 7 +- nova/virt/vmwareapi/read_write_util.py | 159 ++---------------- 4 files changed, 34 insertions(+), 174 deletions(-) diff --git a/nova/tests/unit/virt/vmwareapi/test_images.py b/nova/tests/unit/virt/vmwareapi/test_images.py index de72be62c496..1741cbc84193 100644 --- a/nova/tests/unit/virt/vmwareapi/test_images.py +++ b/nova/tests/unit/virt/vmwareapi/test_images.py @@ -19,13 +19,13 @@ import contextlib import mock from oslo.utils import units +from oslo.vmware import rw_handles from nova import exception from nova import test import nova.tests.unit.image.fake from nova.virt.vmwareapi import constants from nova.virt.vmwareapi import images -from nova.virt.vmwareapi import read_write_util class VMwareImagesTestCase(test.NoDBTestCase): @@ -61,9 +61,9 @@ class VMwareImagesTestCase(test.NoDBTestCase): return write_file_handle with contextlib.nested( - mock.patch.object(read_write_util, 'GlanceFileRead', + mock.patch.object(rw_handles, 'ImageReadHandle', side_effect=fake_read_handle), - mock.patch.object(read_write_util, 'VMwareHTTPWriteFile', + mock.patch.object(rw_handles, 'FileWriteHandle', side_effect=fake_write_handle), mock.patch.object(images, 'start_transfer'), mock.patch.object(images.IMAGE_API, 'get', diff --git a/nova/tests/unit/virt/vmwareapi/test_read_write_util.py b/nova/tests/unit/virt/vmwareapi/test_read_write_util.py index 1c5cd641a7da..dfc26ed63f07 100644 --- a/nova/tests/unit/virt/vmwareapi/test_read_write_util.py +++ b/nova/tests/unit/virt/vmwareapi/test_read_write_util.py @@ -13,11 +13,9 @@ # License for the specific language governing permissions and limitations # under the License. -import httplib -import urllib2 - import mock from oslo.config import cfg +import requests from nova import test from nova.virt.vmwareapi import read_write_util @@ -26,25 +24,9 @@ CONF = cfg.CONF class ReadWriteUtilTestCase(test.NoDBTestCase): - def test_ipv6_host(self): - ipv6_host = 'fd8c:215d:178e:c51e:200:c9ff:fed1:584c' - self.mox.StubOutWithMock(httplib.HTTPConnection, 'endheaders') - httplib.HTTPConnection.endheaders() - self.mox.ReplayAll() - file = read_write_util.VMwareHTTPWriteFile(ipv6_host, - 443, - 'fake_dc', - 'fake_ds', - dict(), - '/tmp/fake.txt', - 0) - self.assertEqual(ipv6_host, file.conn.host) - self.assertEqual(443, file.conn.port) - @mock.patch.object(urllib2, 'Request', - return_value='fake_request') - @mock.patch.object(urllib2, 'urlopen') - def test_ipv6_host_read(self, mock_open, mock_request): + @mock.patch.object(requests.api, 'request') + def test_ipv6_host_read(self, mock_request): ipv6_host = 'fd8c:215d:178e:c51e:200:c9ff:fed1:584c' port = 7443 folder = 'tmp/fake.txt' @@ -56,8 +38,10 @@ class ReadWriteUtilTestCase(test.NoDBTestCase): folder) base_url = 'https://[%s]:%s/folder/%s' % (ipv6_host, port, folder) base_url += '?dsName=fake_ds&dcPath=fake_dc' - headers = {'Cookie': '', 'User-Agent': 'OpenStack-ESX-Adapter'} - mock_request.assert_called_with(base_url, - None, - headers) - mock_open.assert_called_with('fake_request') + headers = {'User-Agent': 'OpenStack-ESX-Adapter'} + mock_request.assert_called_once_with('get', + base_url, + headers=headers, + allow_redirects=True, + stream=True, + verify=False) diff --git a/nova/virt/vmwareapi/images.py b/nova/virt/vmwareapi/images.py index e6f720ae6f05..e8a670e46659 100644 --- a/nova/virt/vmwareapi/images.py +++ b/nova/virt/vmwareapi/images.py @@ -30,7 +30,6 @@ from nova import image from nova.openstack.common import log as logging from nova.virt.vmwareapi import constants from nova.virt.vmwareapi import io_util -from nova.virt.vmwareapi import read_write_util # NOTE(mdbooth): We use use_linked_clone below, but don't have to import it # because nova.virt.vmwareapi.driver is imported first. In fact, it is not @@ -203,7 +202,7 @@ def upload_iso_to_datastore(iso_path, instance, **kwargs): LOG.debug("Uploading iso %s to datastore", iso_path, instance=instance) with open(iso_path, 'r') as iso_file: - write_file_handle = read_write_util.VMwareHTTPWriteFile( + write_file_handle = rw_handles.FileWriteHandle( kwargs.get("host"), kwargs.get("port"), kwargs.get("data_center_name"), @@ -238,8 +237,8 @@ def fetch_image(context, instance, host, port, dc_name, ds_name, file_path, metadata = IMAGE_API.get(context, image_ref) file_size = int(metadata['size']) read_iter = IMAGE_API.download(context, image_ref) - read_file_handle = read_write_util.GlanceFileRead(read_iter) - write_file_handle = read_write_util.VMwareHTTPWriteFile( + read_file_handle = rw_handles.ImageReadHandle(read_iter) + write_file_handle = rw_handles.FileWriteHandle( host, port, dc_name, ds_name, cookies, file_path, file_size) start_transfer(context, read_file_handle, file_size, write_file_handle=write_file_handle) diff --git a/nova/virt/vmwareapi/read_write_util.py b/nova/virt/vmwareapi/read_write_util.py index 876a3e72d4a0..e325f23ad38c 100644 --- a/nova/virt/vmwareapi/read_write_util.py +++ b/nova/virt/vmwareapi/read_write_util.py @@ -20,159 +20,36 @@ Collection of classes to handle image upload/download to/from Image service """ -import httplib import urllib -import urllib2 from oslo.utils import netutils -import six.moves.urllib.parse as urlparse - -from nova.openstack.common import log as logging - -LOG = logging.getLogger(__name__) - -USER_AGENT = "OpenStack-ESX-Adapter" - -READ_CHUNKSIZE = 65536 +from oslo.vmware import rw_handles -class GlanceFileRead(object): - """Glance file read handler class.""" - - def __init__(self, glance_read_iter): - self.glance_read_iter = glance_read_iter - self.iter = self.get_next() - - def read(self, chunk_size): - """Read an item from the queue. - - The chunk size is ignored for the Client ImageBodyIterator - uses its own CHUNKSIZE. - """ - try: - return self.iter.next() - except StopIteration: - return "" - - def get_next(self): - """Get the next item from the image iterator.""" - for data in self.glance_read_iter: - yield data - - def close(self): - """A dummy close just to maintain consistency.""" - pass - - -class VMwareHTTPFile(object): - """Base class for HTTP file.""" - - def __init__(self, file_handle): - self.eof = False - self.file_handle = file_handle - - def set_eof(self, eof): - """Set the end of file marker.""" - self.eof = eof - - def get_eof(self): - """Check if the end of file has been reached.""" - return self.eof - - def close(self): - """Close the file handle.""" - try: - self.file_handle.close() - except Exception as exc: - LOG.exception(exc) - - def _build_vim_cookie_headers(self, vim_cookies): - """Build ESX host session cookie headers.""" - cookie_header = "" - for vim_cookie in vim_cookies: - cookie_header = vim_cookie.name + "=" + vim_cookie.value - break - return cookie_header - - def write(self, data): - """Write data to the file.""" - raise NotImplementedError() - - def read(self, chunk_size): - """Read a chunk of data.""" - raise NotImplementedError() - - def get_size(self): - """Get size of the file to be read.""" - raise NotImplementedError() - - def _get_base_url(self, scheme, host, port, file_path): - if netutils.is_valid_ipv6(host): - base_url = ("%s://[%s]:%s/folder/%s" - % (scheme, host, port, urllib.pathname2url(file_path))) - else: - base_url = ("%s://%s:%s/folder/%s" - % (scheme, host, port, urllib.pathname2url(file_path))) - return base_url - - -class VMwareHTTPWriteFile(VMwareHTTPFile): - """VMware file write handler class.""" - - def __init__(self, host, port, data_center_name, datastore_name, cookies, - file_path, file_size, scheme="https"): - base_url = self._get_base_url(scheme, host, port, file_path) - param_list = {"dcPath": data_center_name, "dsName": datastore_name} - base_url = base_url + "?" + urllib.urlencode(param_list) - _urlparse = urlparse.urlparse(base_url) - scheme, netloc, path, params, query, fragment = _urlparse - if scheme == "http": - conn = httplib.HTTPConnection(netloc) - elif scheme == "https": - conn = httplib.HTTPSConnection(netloc) - conn.putrequest("PUT", path + "?" + query) - conn.putheader("User-Agent", USER_AGENT) - conn.putheader("Content-Length", file_size) - conn.putheader("Cookie", self._build_vim_cookie_headers(cookies)) - conn.endheaders() - self.conn = conn - VMwareHTTPFile.__init__(self, conn) - - def write(self, data): - """Write to the file.""" - self.file_handle.send(data) - - def close(self): - """Get the response and close the connection.""" - try: - self.conn.getresponse() - except Exception: - LOG.debug("Exception during HTTP connection close in " - "VMwareHTTPWrite", exc_info=True) - super(VMwareHTTPWriteFile, self).close() - - -class VMwareHTTPReadFile(VMwareHTTPFile): +class VMwareHTTPReadFile(rw_handles.FileHandle): """VMware file read handler class.""" def __init__(self, host, port, data_center_name, datastore_name, cookies, file_path, scheme="https"): - base_url = self._get_base_url(scheme, host, port, file_path) + self._base_url = self._get_base_url(scheme, host, port, file_path) param_list = {"dcPath": data_center_name, "dsName": datastore_name} - base_url = base_url + "?" + urllib.urlencode(param_list) - headers = {'User-Agent': USER_AGENT, - 'Cookie': self._build_vim_cookie_headers(cookies)} - request = urllib2.Request(base_url, None, headers) - conn = urllib2.urlopen(request) - VMwareHTTPFile.__init__(self, conn) + self._base_url = self._base_url + "?" + urllib.urlencode(param_list) + self._conn = self._create_read_connection(self._base_url, + cookies=cookies) + rw_handles.FileHandle.__init__(self, self._conn) def read(self, chunk_size): - """Read a chunk of data.""" - # We are ignoring the chunk size passed for we want the pipe to hold - # data items of the chunk-size that Glance Client uses for read - # while writing. - return self.file_handle.read(READ_CHUNKSIZE) + return self._file_handle.read(rw_handles.READ_CHUNKSIZE) + + def _get_base_url(self, scheme, host, port, file_path): + if netutils.is_valid_ipv6(host): + base_url = "%s://[%s]:%s/folder/%s" % (scheme, host, port, + urllib.pathname2url(file_path)) + else: + base_url = "%s://%s:%s/folder/%s" % (scheme, host, port, + urllib.pathname2url(file_path)) + return base_url def get_size(self): """Get size of the file to be read.""" - return self.file_handle.headers.get("Content-Length", -1) + return self._file_handle.headers.get("Content-Length", -1)