openstack/glance
Code Issues Proposed changes

disk/container_format required on image activate

Browse Source 
Fixes lp 937216

The initial change[1] to require the container and disk formats be
explicitly set was enforced on the initial image POST, regardless
of whether the image content accompanied that request (either in
as the entity-body, or externally specified via x-image-meta-location).

This turned out to be overly restrictive, as these formats may not
be known to the caller until the image content is supplied via a
subsequent PUT on the new image.

Now we only enforce the strict requirement for these formats to be
non-empty when the image is activated (as opposed to the initial
enqueue).

[1] https://github.com/openstack/glance/commit/62c913c3

Change-Id: I89e068f35fd7da427b547b18cdea4ae84ab3ec87
This commit is contained in:
Eoghan Glynn 2012-02-20 21:49:17 +00:00
parent 62c913c3ad
commit 5f31371330
5 changed files with 129 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
glance/api/v1/images.py
View File

@ -444,8 +444,18 @@ class Controller(controller.BaseController):
image_meta = {}
image_meta['location'] = location
image_meta['status'] = 'active'
return registry.update_image_metadata(req.context, image_id,
image_meta)
try:
return registry.update_image_metadata(req.context,
image_id,
image_meta)
except exception.Invalid, e:
msg = (_("Failed to activate image. Got error: %(e)s")
% locals())
for line in msg.split('\n'):
logger.error(line)
self.notifier.error('image.update', msg)
raise HTTPBadRequest(msg, request=req, content_type="text/plain")
def _kill(self, req, image_id):
"""

14
glance/registry/db/api.py
View File

@ -313,11 +313,21 @@ def validate_image(values):
msg = "Invalid image status '%s' for image." % status
raise exception.Invalid(msg)
if not disk_format or disk_format not in DISK_FORMATS:
def _required_format_absent(format, formats):
activating = status == 'active'
unrecognized = format not in formats
# We don't mind having format = None when we're just registering
# an image, but if the image is being activated, make sure that the
# format is valid. Conversely if the format happens to be set on
# registration, it must be one of the recognized formats.
return ((activating and (not format or unrecognized))
or (not activating and format and unrecognized))
if _required_format_absent(disk_format, DISK_FORMATS):
msg = "Invalid disk format '%s' for image." % disk_format
raise exception.Invalid(msg)
if not container_format or container_format not in CONTAINER_FORMATS:
if _required_format_absent(container_format, CONTAINER_FORMATS):
msg = "Invalid container format '%s' for image." % container_format
raise exception.Invalid(msg)

64
glance/tests/functional/test_api.py
View File

@ -631,8 +631,7 @@ class TestApi(functional.FunctionalTest):
response, content = http.request(path, 'POST',
body=test_data_file.name)
self.assertEqual(response.status, 400)
expected = ("Failed to reserve image. Got error: "
"Data supplied was not valid. Details: 400 Bad Request")
expected = "Content-Type must be application/octet-stream"
self.assertTrue(expected in content,
"Could not find '%s' in '%s'" % (expected, content))
@ -1319,7 +1318,7 @@ class TestApi(functional.FunctionalTest):
expected_exitcode=255,
**self.__dict__.copy())
def _do_test_unset_required_format(self, format):
def _do_test_post_image_content_missing_format(self, format):
"""
We test that missing container/disk format fails with 400 "Bad Request"
@ -1349,9 +1348,60 @@ class TestApi(functional.FunctionalTest):
self.stop_servers()
@skip_if_disabled
def test_unset_container_format(self):
self._do_test_unset_required_format('container_format')
def _do_test_post_image_content_missing_diskformat(self):
self._do_test_post_image_content_missing_format('container_format')
@skip_if_disabled
def test_unset_disk_format(self):
self._do_test_unset_required_format('disk_format')
def _do_test_post_image_content_missing_disk_format(self):
self._do_test_post_image_content_missing_format('disk_format')
def _do_test_put_image_content_missing_format(self, format):
"""
We test that missing container/disk format only fails with
400 "Bad Request" when the image content is PUT (i.e. not
on the original POST of a queued image).
:see https://bugs.launchpad.net/glance/+bug/937216
"""
self.cleanup()
self.start_servers(**self.__dict__.copy())
# POST queued image
path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port)
headers = {
'X-Image-Meta-Name': 'Image1',
'X-Image-Meta-Is-Public': 'True',
}
http = httplib2.Http()
response, content = http.request(path, 'POST', headers=headers)
self.assertEqual(response.status, 201)
data = json.loads(content)
image_id = data['image']['id']
# PUT image content images without given format being specified
path = ("http://%s:%d/v1/images/%s" %
("0.0.0.0", self.api_port, image_id))
headers = minimal_headers('Image1')
del headers['X-Image-Meta-' + format]
with tempfile.NamedTemporaryFile() as test_data_file:
test_data_file.write("XXX")
test_data_file.flush()
http = httplib2.Http()
response, content = http.request(path, 'PUT',
headers=headers,
body=test_data_file.name)
self.assertEqual(response.status, 400)
type = format.replace('_format', '')
expected = "Details: Invalid %s format 'None' for image" % type
self.assertTrue(expected in content,
"Could not find '%s' in '%s'" % (expected, content))
self.stop_servers()
@skip_if_disabled
def _do_test_put_image_content_missing_diskformat(self):
self._do_test_put_image_content_missing_format('container_format')
@skip_if_disabled
def _do_test_put_image_content_missing_disk_format(self):
self._do_test_put_image_content_missing_format('disk_format')

2
glance/tests/functional/test_ssl.py
View File

@ -672,7 +672,7 @@ class TestSSL(functional.FunctionalTest):
response, content = https.request(path, 'POST',
body=test_data_file.name)
self.assertEqual(response.status, 400)
expected = "Data supplied was not valid. Details: 400 Bad Request"
expected = "Content-Type must be application/octet-stream"
self.assertTrue(expected in content,
"Could not find '%s' in '%s'" % (expected, content))

52
glance/tests/unit/test_api.py
View File

@ -2194,7 +2194,7 @@ class TestGlanceAPI(base.IsolatedUnitTest):
res = req.get_response(self.api)
self.assertEquals(res.status_int, 401)
def _do_test_add_image_missing_format(self, missing):
def _do_test_post_image_content_missing_format(self, missing):
"""Tests creation of an image with missing format"""
fixture_headers = {'x-image-meta-store': 'file',
'x-image-meta-disk-format': 'vhd',
@ -2209,16 +2209,58 @@ class TestGlanceAPI(base.IsolatedUnitTest):
req.method = 'POST'
for k, v in fixture_headers.iteritems():
req.headers[k] = v
req.headers['Content-Type'] = 'application/octet-stream'
req.body = "chunk00000remainder"
res = req.get_response(self.api)
self.assertEquals(res.status_int, httplib.BAD_REQUEST)
def test_add_image_missing_disk_format(self):
def test_post_image_content_missing_disk_format(self):
"""Tests creation of an image with missing disk format"""
self._do_test_add_image_missing_format('disk_format')
self._do_test_post_image_content_missing_format('disk_format')
def test_add_image_missing_container_type(self):
def test_post_image_content_missing_container_type(self):
"""Tests creation of an image with missing container format"""
self._do_test_add_image_missing_format('container_format')
self._do_test_post_image_content_missing_format('container_format')
def _do_test_put_image_content_missing_format(self, missing):
"""Tests delayed activation of an image with missing format"""
fixture_headers = {'x-image-meta-store': 'file',
'x-image-meta-disk-format': 'vhd',
'x-image-meta-container-format': 'ovf',
'x-image-meta-name': 'fake image #3'}
header = 'x-image-meta-' + missing.replace('_', '-')
del fixture_headers[header]
req = webob.Request.blank("/images")
req.method = 'POST'
for k, v in fixture_headers.iteritems():
req.headers[k] = v
res = req.get_response(self.api)
self.assertEquals(res.status_int, httplib.CREATED)
res_body = json.loads(res.body)['image']
self.assertEquals('queued', res_body['status'])
image_id = res_body['id']
req = webob.Request.blank("/images/%s" % image_id)
req.method = 'PUT'
for k, v in fixture_headers.iteritems():
req.headers[k] = v
req.headers['Content-Type'] = 'application/octet-stream'
req.body = "chunk00000remainder"
res = req.get_response(self.api)
self.assertEquals(res.status_int, httplib.BAD_REQUEST)
def test_put_image_content_missing_disk_format(self):
"""Tests delayed activation of image with missing disk format"""
self._do_test_put_image_content_missing_format('disk_format')
def test_put_image_content_missing_container_type(self):
"""Tests delayed activation of image with missing container format"""
self._do_test_put_image_content_missing_format('container_format')
def test_register_and_upload(self):
"""