From dbdc35bb1ae056444c8deb70bf9ad7fe85c07d72 Mon Sep 17 00:00:00 2001 From: Dharini Chandrasekar Date: Wed, 15 Feb 2017 23:34:31 +0000 Subject: [PATCH] Do not serve partial img download reqs from cache Currently, when a partial download request for an image is received via v2 api with caching enabled, * For a new, uncached image: An attempt to cache (tee'ing) the partially downloaded image is made which is not desirable. While attempting to do this, the 206 gets converted to 500 due to a checksum mismatch of the partial image getting cached vs the entire image. * For an already cached image: Cache middleware layer serves the request but sends the entire image (200 instead of 206) This patch prevents caching of any partial images that are being downloaded using v2 api and ensures that any partial download requests are not served from cache. Change-Id: I3586c4eda0c36392d3436980f766ed1dd910b47c Closes-Bug: 1664709 --- glance/api/middleware/cache.py | 13 ++ .../tests/functional/test_cache_middleware.py | 178 ++++++++++++++++++ 2 files changed, 191 insertions(+) diff --git a/glance/api/middleware/cache.py b/glance/api/middleware/cache.py index 19289824..ae752358 100644 --- a/glance/api/middleware/cache.py +++ b/glance/api/middleware/cache.py @@ -149,8 +149,17 @@ class CacheFilter(wsgi.Middleware): self._stash_request_info(request, image_id, method, version) + # Partial image download requests shall not be served from cache + # Bug: 1664709 + # TODO(dharinic): If an image is already cached, add support to serve + # only the requested bytes (partial image download) from the cache. + if (request.headers.get('Content-Range') or + request.headers.get('Range')): + return None + if request.method != 'GET' or not self.cache.is_cached(image_id): return None + method = getattr(self, '_get_%s_image_metadata' % version) image_metadata = method(request, image_id) @@ -255,6 +264,10 @@ class CacheFilter(wsgi.Middleware): if not 200 <= status_code < 300: return resp + # Note(dharinic): Bug: 1664709: Do not cache partial images. + if status_code == http.PARTIAL_CONTENT: + return resp + try: (image_id, method, version) = self._fetch_request_info( resp.request) diff --git a/glance/tests/functional/test_cache_middleware.py b/glance/tests/functional/test_cache_middleware.py index 8bca4a0f..1bc86f8b 100644 --- a/glance/tests/functional/test_cache_middleware.py +++ b/glance/tests/functional/test_cache_middleware.py @@ -25,6 +25,7 @@ import os import shutil import sys import time +import uuid import httplib2 from oslo_serialization import jsonutils @@ -163,6 +164,7 @@ class BaseCacheMiddlewareTest(object): # Verify image now in cache image_cached_path = os.path.join(self.api_server.image_cache_dir, image_id) + self.assertTrue(os.path.exists(image_cached_path)) # Now, we delete the image from the server and verify that # the image cache no longer contains the deleted image @@ -176,6 +178,182 @@ class BaseCacheMiddlewareTest(object): self.stop_servers() + @skip_if_disabled + def test_partially_downloaded_images_are_not_cached_v2_api(self): + """ + Verify that we do not cache images that were downloaded partially + using v2 images API. + """ + self.cleanup() + self.start_servers(**self.__dict__.copy()) + + # Add an image and verify success + path = "http://%s:%d/v2/images" % ("0.0.0.0", self.api_port) + http = httplib2.Http() + headers = {'content-type': 'application/json'} + image_entity = { + 'name': 'Image1', + 'visibility': 'public', + 'container_format': 'bare', + 'disk_format': 'raw', + } + response, content = http.request(path, 'POST', + headers=headers, + body=jsonutils.dumps(image_entity)) + self.assertEqual(http_client.CREATED, response.status) + data = jsonutils.loads(content) + image_id = data['id'] + + path = "http://%s:%d/v2/images/%s/file" % ("0.0.0.0", self.api_port, + image_id) + headers = {'content-type': 'application/octet-stream'} + image_data = b'ABCDEFGHIJKLMNOPQRSTUVWXYZ' + response, content = http.request(path, 'PUT', + headers=headers, + body=image_data) + self.assertEqual(http_client.NO_CONTENT, response.status) + + # Verify that this image is not in cache + image_cached_path = os.path.join(self.api_server.image_cache_dir, + image_id) + self.assertFalse(os.path.exists(image_cached_path)) + + # partially download this image and verify status 206 + http = httplib2.Http() + # range download request + range_ = 'bytes=3-5' + headers = { + 'X-Identity-Status': 'Confirmed', + 'X-Auth-Token': '932c5c84-02ac-4fe5-a9ba-620af0e2bb96', + 'X-User-Id': 'f9a41d13-0c13-47e9-bee2-ce4e8bfe958e', + 'X-Tenant-Id': str(uuid.uuid4()), + 'X-Roles': 'member', + 'Range': range_ + } + response, content = http.request(path, 'GET', headers=headers) + self.assertEqual(http_client.PARTIAL_CONTENT, response.status) + self.assertEqual(b'DEF', content) + + # content-range download request + # NOTE(dharinic): Glance incorrectly supports Content-Range for partial + # image downloads in requests. This test is included to ensure that + # we prevent regression. + content_range = 'bytes 3-5/*' + headers = { + 'X-Identity-Status': 'Confirmed', + 'X-Auth-Token': '932c5c84-02ac-4fe5-a9ba-620af0e2bb96', + 'X-User-Id': 'f9a41d13-0c13-47e9-bee2-ce4e8bfe958e', + 'X-Tenant-Id': str(uuid.uuid4()), + 'X-Roles': 'member', + 'Content-Range': content_range + } + response, content = http.request(path, 'GET', headers=headers) + self.assertEqual(http_client.PARTIAL_CONTENT, response.status) + self.assertEqual(b'DEF', content) + + # verify that we do not cache the partial image + image_cached_path = os.path.join(self.api_server.image_cache_dir, + image_id) + self.assertFalse(os.path.exists(image_cached_path)) + + self.stop_servers() + + @skip_if_disabled + def test_partial_download_of_cached_images_v2_api(self): + """ + Verify that partial download requests for a fully cached image + succeeds; we do not serve it from cache. + """ + self.cleanup() + self.start_servers(**self.__dict__.copy()) + + # Add an image and verify success + path = "http://%s:%d/v2/images" % ("0.0.0.0", self.api_port) + http = httplib2.Http() + headers = {'content-type': 'application/json'} + image_entity = { + 'name': 'Image1', + 'visibility': 'public', + 'container_format': 'bare', + 'disk_format': 'raw', + } + response, content = http.request(path, 'POST', + headers=headers, + body=jsonutils.dumps(image_entity)) + self.assertEqual(http_client.CREATED, response.status) + data = jsonutils.loads(content) + image_id = data['id'] + + path = "http://%s:%d/v2/images/%s/file" % ("0.0.0.0", self.api_port, + image_id) + headers = {'content-type': 'application/octet-stream'} + image_data = b'ABCDEFGHIJKLMNOPQRSTUVWXYZ' + response, content = http.request(path, 'PUT', + headers=headers, + body=image_data) + self.assertEqual(http_client.NO_CONTENT, response.status) + + # Verify that this image is not in cache + image_cached_path = os.path.join(self.api_server.image_cache_dir, + image_id) + self.assertFalse(os.path.exists(image_cached_path)) + + # Download the entire image + http = httplib2.Http() + response, content = http.request(path, 'GET') + self.assertEqual(http_client.OK, response.status) + self.assertEqual(b'ABCDEFGHIJKLMNOPQRSTUVWXYZ', content) + + # Verify that the image is now in cache + image_cached_path = os.path.join(self.api_server.image_cache_dir, + image_id) + self.assertTrue(os.path.exists(image_cached_path)) + # Modify the data in cache so we can verify the partially downloaded + # content was not from cache indeed. + with open(image_cached_path, 'w') as cache_file: + cache_file.write('0123456789') + + # Partially attempt a download of this image and verify that is not + # from cache + # range download request + range_ = 'bytes=3-5' + headers = { + 'X-Identity-Status': 'Confirmed', + 'X-Auth-Token': '932c5c84-02ac-4fe5-a9ba-620af0e2bb96', + 'X-User-Id': 'f9a41d13-0c13-47e9-bee2-ce4e8bfe958e', + 'X-Tenant-Id': str(uuid.uuid4()), + 'X-Roles': 'member', + 'Range': range_, + 'content-type': 'application/json' + } + response, content = http.request(path, 'GET', headers=headers) + self.assertEqual(http_client.PARTIAL_CONTENT, response.status) + self.assertEqual(b'DEF', content) + self.assertNotEqual(b'345', content) + self.assertNotEqual(image_data, content) + + # content-range download request + # NOTE(dharinic): Glance incorrectly supports Content-Range for partial + # image downloads in requests. This test is included to ensure that + # we prevent regression. + content_range = 'bytes 3-5/*' + headers = { + 'X-Identity-Status': 'Confirmed', + 'X-Auth-Token': '932c5c84-02ac-4fe5-a9ba-620af0e2bb96', + 'X-User-Id': 'f9a41d13-0c13-47e9-bee2-ce4e8bfe958e', + 'X-Tenant-Id': str(uuid.uuid4()), + 'X-Roles': 'member', + 'Content-Range': content_range, + 'content-type': 'application/json' + } + response, content = http.request(path, 'GET', headers=headers) + self.assertEqual(http_client.PARTIAL_CONTENT, response.status) + self.assertEqual(b'DEF', content) + self.assertNotEqual(b'345', content) + self.assertNotEqual(image_data, content) + + self.stop_servers() + @skip_if_disabled def test_cache_remote_image(self): """