From 62c913c3ad4db1a03461f51304fac885196df8ca Mon Sep 17 00:00:00 2001 From: Eoghan Glynn Date: Fri, 17 Feb 2012 13:46:49 +0000 Subject: [PATCH] Require container & disk formats on image create Fixes lp 933702 For images created via the glance CLI, the container and disk formats were previously defaulted if not explicitly set. However if created via the python or REST APIs, these attributes were not defaulted if unset. There is no real sensible default for these formats, so now an image create fails with 400 "Bad Request" if the format metadata are missing. Also we ensure unset image metadata are not reported in x-image-meta-* headers in order to disambiguate None and empty string values. Change-Id: I8189383f5f9adf42a8cdac7f8dc7e9327baf46da --- bin/glance | 19 ++-- doc/source/glance.rst | 47 ++++++---- doc/source/glanceapi.rst | 12 +-- glance/common/utils.py | 18 ++-- glance/registry/db/api.py | 4 +- glance/tests/functional/test_api.py | 92 ++++++++++++------- glance/tests/functional/test_bin_glance.py | 38 +++++--- .../test_bin_glance_cache_manage.py | 6 +- .../tests/functional/test_cache_middleware.py | 15 +-- glance/tests/functional/test_misc.py | 11 +-- .../tests/functional/test_private_images.py | 49 +++++----- glance/tests/functional/test_shared_images.py | 8 +- glance/tests/functional/test_ssl.py | 60 +++++------- glance/tests/unit/test_api.py | 30 ++++++ glance/tests/unit/test_misc.py | 29 ++++++ glance/tests/unit/test_wsgi.py | 5 +- glance/tests/utils.py | 18 ++++ 17 files changed, 291 insertions(+), 170 deletions(-) diff --git a/bin/glance b/bin/glance index 0900ae24a8..de5483db35 100755 --- a/bin/glance +++ b/bin/glance @@ -141,7 +141,7 @@ def print_image_formatted(client, image): print "Container format: %s" % image['container_format'] print "Minimum Ram Required (MB): %s" % image['min_ram'] print "Minimum Disk Required (GB): %s" % image['min_disk'] - if image['owner']: + if image.get('owner'): print "Owner: %s" % image['owner'] if len(image['properties']) > 0: for k, v in image['properties'].items(): @@ -173,10 +173,9 @@ is_public Optional. If specified, interpreted as a boolean value protected Optional. If specified, interpreted as a boolean value and enables or disables deletion protection. The default value is False. -disk_format Optional. Possible values are 'vhd','vmdk','raw', 'qcow2', - and 'ami'. Default value is 'raw'. -container_format Optional. Possible values are 'ovf' and 'ami'. - Default value is 'ovf'. +disk_format Required. Possible values are 'vhd','vmdk','raw', 'qcow2', + and 'ami'. +container_format Required. Possible values are 'ovf' and 'ami'. location Optional. When specified, should be a readable location in the form of a URI: $STORE://LOCATION. For example, if the image data is stored in a file on the local @@ -202,7 +201,8 @@ EXAMPLES location=http://images.ubuntu.org/images/lucid-10.04-i686.iso \\ distro="Ubuntu 10.04 LTS" -%(prog)s add name="My Image" distro="Ubuntu 10.04 LTS" < /tmp/myimage.iso""" +%(prog)s add name="My Image" disk_format=raw container_format=ovf \\ + distro="Ubuntu 10.04 LTS" < /tmp/myimage.iso""" c = get_client(options) try: @@ -220,10 +220,13 @@ EXAMPLES fields.pop('is_public', False)), 'protected': utils.bool_from_string( fields.pop('protected', False)), - 'disk_format': fields.pop('disk_format', 'raw'), 'min_disk': fields.pop('min_disk', 0), 'min_ram': fields.pop('min_ram', 0), - 'container_format': fields.pop('container_format', 'ovf')} + } + + for format in ['disk_format', 'container_format']: + if format in fields: + image_meta[format] = fields.pop(format) # Strip any args that are not supported unsupported_fields = ['status', 'size'] diff --git a/doc/source/glance.rst b/doc/source/glance.rst index e105eb48e6..e585f8d416 100644 --- a/doc/source/glance.rst +++ b/doc/source/glance.rst @@ -131,8 +131,8 @@ like so:: name A name for the image. is_public If specified, interpreted as a boolean value and sets or unsets the image's availability to the public. - disk_format Format of the disk image - container_format Format of the container + disk_format Format of the disk image (required) + container_format Format of the container (required) All other field names are considered to be custom properties so be careful to spell field names correctly. :) @@ -186,21 +186,27 @@ that the image should be public -- anyone should be able to fetch it. Here is how we'd upload this image to Glance. Change example IP number to your server IP number.:: - $> glance add name="My Image" is_public=true < /tmp/images/myimage.iso \ - --host=65.114.169.29 + $> glance add name="My Image" is_public=true \ + container_format=ovf disk_format=raw \ + --host=65.114.169.29 < /tmp/images/myimage.iso + +Note that the disk container formats are no longer defaulted and are thus +strictly required. If Glance was able to successfully upload and store your VM image data and metadata attributes, you would see something like this:: - $> glance add name="My Image" is_public=true < /tmp/images/myimage.iso \ - --host=65.114.169.29 + $> glance add name="My Image" is_public=true \ + container_format=ovf disk_format=raw \ + --host=65.114.169.29 < /tmp/images/myimage.iso Added new image with ID: 991baaf9-cc0d-4183-a201-8facdf1a1430 You can use the ``--verbose`` (or ``-v``) command-line option to print some more information about the metadata that was saved with the image:: - $> glance --verbose add name="My Image" is_public=true < \ - /tmp/images/myimage.iso --host=65.114.169.29 + $> glance --verbose add name="My Image" is_public=true \ + container_format=ovf disk_format=raw \ + --host=65.114.169.29 < /tmp/images/myimage.iso Added new image with ID: 541424be-27b1-49d6-a55b-6430b8ae0f5f Returned the following metadata for the new image: container_format => ovf @@ -221,8 +227,9 @@ information about the metadata that was saved with the image:: If you are unsure about what will be added, you can use the ``--dry-run`` command-line option, which will simply show you what *would* have happened:: - $> glance --dry-run add name="Foo" distro="Ubuntu" is_public=True < \ - /tmp/images/myimage.iso --host=65.114.169.29 + $> glance --dry-run add name="Foo" distro="Ubuntu" is_public=True \ + container_format=ovf disk_format=raw \ + --host=65.114.169.29 < /tmp/images/myimage.iso Dry run. We would have done the following: Add new image with metadata: container_format => ovf @@ -243,42 +250,46 @@ Examples of uploading different kinds of images To upload an EC2 tarball VM image:: - $> glance add name="ubuntu-10.10-amd64" is_public=true < \ - /root/maverick-server-uec-amd64.tar.gz + $> glance add name="ubuntu-10.10-amd64" is_public=true \ + container_format=ovf disk_format=raw \ + < /root/maverick-server-uec-amd64.tar.gz To upload an EC2 tarball VM image with an associated property (e.g., distro):: $> glance add name="ubuntu-10.10-amd64" is_public=true \ + container_format=ovf disk_format=raw \ distro="ubuntu 10.10" < /root/maverick-server-uec-amd64.tar.gz To upload an EC2 tarball VM image from a URL:: $> glance add name="uubntu-10.04-amd64" is_public=true \ + container_format=ovf disk_format=raw \ location="http://uec-images.ubuntu.com/lucid/current/\ lucid-server-uec-amd64.tar.gz" To upload a qcow2 image:: $> glance add name="ubuntu-11.04-amd64" is_public=true \ - distro="ubuntu 11.04" disk_format="qcow2" < /data/images/rock_natty.qcow2 + container_format=ovf disk_format=qcow2 \ + distro="ubuntu 11.04" < /data/images/rock_natty.qcow2 To upload a kernel file, ramdisk file and filesystem image file:: - $> glance add --disk-format=aki --container-format=aki \ + $> glance add disk_format=aki container_format=aki \ ./maverick-server-uec-amd64-vmlinuz-virtual \ maverick-server-uec-amd64-vmlinuz-virtual - $> glance add --disk-format=ari --container-format=ari \ + $> glance add disk_format=ari container_format=ari \ ./maverick-server-uec-amd64-loader maverick-server-uec-amd64-loader # Determine what the ids associated with the kernel and ramdisk files $> glance index # Assuming the ids are 7 and 8: - $> glance add --disk-format=ami --container-format=ami --kernel=7 \ + $> glance add disk_format=ami container_format=ami --kernel=7 \ --ramdisk=8 ./maverick-server-uec-amd64.img maverick-server-uec-amd64.img To upload a raw image file:: - $> glance add --disk_format=raw ./maverick-server-uec-amd64.img \ - maverick-server-uec-amd64.img_v2 + $> glance add disk_format=raw container_format=ovf \ + ./maverick-server-uec-amd64.img maverick-server-uec-amd64.img_v2 Register a virtual machine image in another location diff --git a/doc/source/glanceapi.rst b/doc/source/glanceapi.rst index 40dc1b943b..0afc2b5e41 100644 --- a/doc/source/glanceapi.rst +++ b/doc/source/glanceapi.rst @@ -181,8 +181,8 @@ following shows an example of the HTTP headers returned from the above x-image-meta-uri http://glance.example.com/images/71c675ab-d94f-49cd-a114-e12490b328d9 x-image-meta-name Ubuntu 10.04 Plain 5GB - x-image-meta-disk-format vhd - x-image-meta-container-format ovf + x-image-meta-disk_format vhd + x-image-meta-container_format ovf x-image-meta-size 5368709120 x-image-meta-checksum c2e5db72bd7fd153f53ede5da5a06de3 x-image-meta-created_at 2010-02-03 09:34:01 @@ -244,8 +244,8 @@ returned from the above ``GET`` request:: x-image-meta-uri http://glance.example.com/images/71c675ab-d94f-49cd-a114-e12490b328d9 x-image-meta-name Ubuntu 10.04 Plain 5GB - x-image-meta-disk-format vhd - x-image-meta-container-format ovf + x-image-meta-disk_format vhd + x-image-meta-container_format ovf x-image-meta-size 5368709120 x-image-meta-checksum c2e5db72bd7fd153f53ede5da5a06de3 x-image-meta-created_at 2010-02-03 09:34:01 @@ -353,14 +353,14 @@ The list of metadata headers that Glance accepts are listed below. store that is marked default. See the configuration option ``default_store`` for more information. -* ``x-image-meta-disk-format`` +* ``x-image-meta-disk_format`` This header is optional. Valid values are one of ``aki``, ``ari``, ``ami``, ``raw``, ``iso``, ``vhd``, ``vdi``, ``qcow2``, or ``vmdk``. For more information, see :doc:`About Disk and Container Formats ` -* ``x-image-meta-container-format`` +* ``x-image-meta-container_format`` This header is optional. Valid values are one of ``aki``, ``ari``, ``ami``, ``bare``, or ``ovf``. diff --git a/glance/common/utils.py b/glance/common/utils.py index 4af74e4b6b..01ebd65914 100644 --- a/glance/common/utils.py +++ b/glance/common/utils.py @@ -77,16 +77,14 @@ def image_meta_to_http_headers(image_meta): """ headers = {} for k, v in image_meta.items(): - if v is None: - v = '' - if k == 'properties': - for pk, pv in v.items(): - if pv is None: - pv = '' - headers["x-image-meta-property-%s" - % pk.lower()] = unicode(pv) - else: - headers["x-image-meta-%s" % k.lower()] = unicode(v) + if v is not None: + if k == 'properties': + for pk, pv in v.items(): + if pv is not None: + headers["x-image-meta-property-%s" + % pk.lower()] = unicode(pv) + else: + headers["x-image-meta-%s" % k.lower()] = unicode(v) return headers diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py index 78eef7cb5f..a52c040402 100644 --- a/glance/registry/db/api.py +++ b/glance/registry/db/api.py @@ -313,11 +313,11 @@ def validate_image(values): msg = "Invalid image status '%s' for image." % status raise exception.Invalid(msg) - if disk_format and disk_format not in DISK_FORMATS: + if not disk_format or disk_format not in DISK_FORMATS: msg = "Invalid disk format '%s' for image." % disk_format raise exception.Invalid(msg) - if container_format and container_format not in CONTAINER_FORMATS: + if not container_format or container_format not in CONTAINER_FORMATS: msg = "Invalid container format '%s' for image." % container_format raise exception.Invalid(msg) diff --git a/glance/tests/functional/test_api.py b/glance/tests/functional/test_api.py index 1fa6f289c1..75c67e8505 100644 --- a/glance/tests/functional/test_api.py +++ b/glance/tests/functional/test_api.py @@ -25,7 +25,7 @@ from glance.common import utils from glance.tests import functional -from glance.tests.utils import execute, skip_if_disabled +from glance.tests.utils import execute, skip_if_disabled, minimal_headers FIVE_KB = 5 * 1024 FIVE_GB = 5 * 1024 * 1024 * 1024 @@ -86,9 +86,7 @@ def test_get_head_simple_post(self): # 2. POST /images with public image named Image1 # attribute and no custom properties. Verify a 200 OK is returned image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image1') path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers, @@ -124,8 +122,8 @@ def test_get_head_simple_post(self): 'x-image-meta-name': 'Image1', 'x-image-meta-is_public': 'True', 'x-image-meta-status': 'active', - 'x-image-meta-disk_format': '', - 'x-image-meta-container_format': '', + 'x-image-meta-disk_format': 'raw', + 'x-image-meta-container_format': 'ovf', 'x-image-meta-size': str(FIVE_KB)} expected_std_headers = { @@ -157,8 +155,8 @@ def test_get_head_simple_post(self): self.assertEqual(response.status, 200) expected_result = {"images": [ - {"container_format": None, - "disk_format": None, + {"container_format": "ovf", + "disk_format": "raw", "id": image_id, "name": "Image1", "checksum": "c2e5db72bd7fd153f53ede5da5a06de3", @@ -176,8 +174,8 @@ def test_get_head_simple_post(self): "status": "active", "name": "Image1", "deleted": False, - "container_format": None, - "disk_format": None, + "container_format": "ovf", + "disk_format": "raw", "id": image_id, "is_public": True, "deleted_at": None, @@ -217,8 +215,8 @@ def test_get_head_simple_post(self): "status": "active", "name": "Image1", "deleted": False, - "container_format": None, - "disk_format": None, + "container_format": "ovf", + "disk_format": "raw", "id": image_id, "is_public": True, "deleted_at": None, @@ -314,9 +312,7 @@ def test_queued_process_flow(self): # 1. POST /images with public image named Image1 # with no location or image data - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image1') path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers) @@ -324,8 +320,8 @@ def test_queued_process_flow(self): data = json.loads(content) self.assertEqual(data['image']['checksum'], None) self.assertEqual(data['image']['size'], 0) - self.assertEqual(data['image']['container_format'], None) - self.assertEqual(data['image']['disk_format'], None) + self.assertEqual(data['image']['container_format'], 'ovf') + self.assertEqual(data['image']['disk_format'], 'raw') self.assertEqual(data['image']['name'], "Image1") self.assertEqual(data['image']['is_public'], True) @@ -341,8 +337,8 @@ def test_queued_process_flow(self): self.assertEqual(data['images'][0]['id'], image_id) self.assertEqual(data['images'][0]['checksum'], None) self.assertEqual(data['images'][0]['size'], 0) - self.assertEqual(data['images'][0]['container_format'], None) - self.assertEqual(data['images'][0]['disk_format'], None) + self.assertEqual(data['images'][0]['container_format'], 'ovf') + self.assertEqual(data['images'][0]['disk_format'], 'raw') self.assertEqual(data['images'][0]['name'], "Image1") # 3. HEAD /images @@ -394,8 +390,8 @@ def test_queued_process_flow(self): hashlib.md5(image_data).hexdigest()) self.assertEqual(data['images'][0]['id'], image_id) self.assertEqual(data['images'][0]['size'], FIVE_KB) - self.assertEqual(data['images'][0]['container_format'], None) - self.assertEqual(data['images'][0]['disk_format'], None) + self.assertEqual(data['images'][0]['container_format'], 'ovf') + self.assertEqual(data['images'][0]['disk_format'], 'raw') self.assertEqual(data['images'][0]['name'], "Image1") # DELETE image @@ -592,6 +588,8 @@ def test_size_greater_2G_mysql(self): 'X-Image-Meta-Location': 'http://example.com/fakeimage', 'X-Image-Meta-Size': str(FIVE_GB), 'X-Image-Meta-Name': 'Image1', + 'X-Image-Meta-disk_format': 'raw', + 'X-image-Meta-container_format': 'ovf', 'X-Image-Meta-Is-Public': 'True'} path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() @@ -633,7 +631,8 @@ def test_traceback_not_consumed(self): response, content = http.request(path, 'POST', body=test_data_file.name) self.assertEqual(response.status, 400) - expected = "Content-Type must be application/octet-stream" + expected = ("Failed to reserve image. Got error: " + "Data supplied was not valid. Details: 400 Bad Request") self.assertTrue(expected in content, "Could not find '%s' in '%s'" % (expected, content)) @@ -1014,27 +1013,21 @@ def test_limited_images(self): image_ids = [] # 1. POST /images with three public images with various attributes - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image1') path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers) self.assertEqual(response.status, 201) image_ids.append(json.loads(content)['image']['id']) - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image2', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image2') path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers) self.assertEqual(response.status, 201) image_ids.append(json.loads(content)['image']['id']) - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image3', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image3') path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers) @@ -1325,3 +1318,40 @@ def test_unsupported_default_store(self): expect_launch=False, expected_exitcode=255, **self.__dict__.copy()) + + def _do_test_unset_required_format(self, format): + """ + We test that missing container/disk format fails with 400 "Bad Request" + + :see https://bugs.launchpad.net/glance/+bug/933702 + """ + self.cleanup() + self.start_servers(**self.__dict__.copy()) + + path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) + + # POST /images without given format being specified + 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, 'POST', + 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 test_unset_container_format(self): + self._do_test_unset_required_format('container_format') + + @skip_if_disabled + def test_unset_disk_format(self): + self._do_test_unset_required_format('disk_format') diff --git a/glance/tests/functional/test_bin_glance.py b/glance/tests/functional/test_bin_glance.py index 6776ca2d66..8a92dac24c 100644 --- a/glance/tests/functional/test_bin_glance.py +++ b/glance/tests/functional/test_bin_glance.py @@ -23,7 +23,7 @@ from glance.common import utils from glance.tests import functional -from glance.tests.utils import execute +from glance.tests.utils import execute, minimal_add_command class TestBinGlance(functional.FunctionalTest): @@ -54,8 +54,9 @@ def test_add_with_location(self): self.assertEqual('', out.strip()) # 1. Add public image - cmd = "bin/glance --port=%d add is_public=True"\ - " name=MyImage location=http://example.com" % api_port + cmd = minimal_add_command(api_port, + 'MyImage', + 'location=http://example.com') exitcode, out, err = execute(cmd) self.assertEqual(0, exitcode) @@ -99,8 +100,10 @@ def test_add_with_location_and_stdin(self): image_file.write("XXX") image_file.flush() file_name = image_file.name - cmd = "bin/glance --port=%d add is_public=True name=MyImage " \ - "location=http://example.com < %s" % (api_port, file_name) + cmd = minimal_add_command(api_port, + 'MyImage', + 'location=http://example.com < %s' % + file_name) exitcode, out, err = execute(cmd) self.assertEqual(0, exitcode) @@ -153,8 +156,9 @@ def test_add_list_delete_list(self): image_file.write("XXX") image_file.flush() image_file_name = image_file.name - cmd = "bin/glance --port=%d add is_public=True"\ - " name=MyImage < %s" % (api_port, image_file_name) + cmd = minimal_add_command(api_port, + 'MyImage', + '< %s' % image_file_name) exitcode, out, err = execute(cmd) @@ -227,7 +231,10 @@ def test_add_list_update_list(self): self.assertEqual('', out.strip()) # 1. Add public image - cmd = "bin/glance --port=%d add name=MyImage" % api_port + cmd = minimal_add_command(api_port, + 'MyImage', + 'location=http://example.com', + public=False) exitcode, out, err = execute(cmd) @@ -355,8 +362,9 @@ def test_add_clear(self): # 1. Add some images for i in range(1, 5): - cmd = "bin/glance --port=%d add is_public=True name=MyName " \ - " foo=bar" % api_port + cmd = minimal_add_command(api_port, + 'MyImage', + 'foo=bar') exitcode, out, err = execute(cmd) self.assertEqual(0, exitcode) @@ -705,7 +713,11 @@ def test_show_image_format(self): image_file.flush() image_file_name = image_file.name cmd = "bin/glance --port=%d add is_public=True"\ + " disk_format=raw container_format=ovf " \ " name=MyImage < %s" % (api_port, image_file_name) + cmd = minimal_add_command(api_port, + 'MyImage', + '< %s' % image_file_name) exitcode, out, err = execute(cmd) @@ -771,8 +783,10 @@ def test_protected_image(self): self.assertEqual('', out.strip()) # 1. Add public image - cmd = "echo testdata | bin/glance --port=%d add is_public=True"\ - " protected=True name=MyImage" % api_port + cmd = ("echo testdata | " + + minimal_add_command(api_port, + 'MyImage', + 'protected=True')) exitcode, out, err = execute(cmd) diff --git a/glance/tests/functional/test_bin_glance_cache_manage.py b/glance/tests/functional/test_bin_glance_cache_manage.py index eb3c3a138b..0761b4a990 100644 --- a/glance/tests/functional/test_bin_glance_cache_manage.py +++ b/glance/tests/functional/test_bin_glance_cache_manage.py @@ -27,7 +27,7 @@ from glance.common import utils from glance.tests import functional -from glance.tests.utils import execute +from glance.tests.utils import execute, minimal_headers FIVE_KB = 5 * 1024 @@ -54,9 +54,7 @@ def add_image(self, name): image identifier. """ image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': name, - 'X-Image-Meta-Is-Public': 'true'} + headers = minimal_headers(name) path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers, diff --git a/glance/tests/functional/test_cache_middleware.py b/glance/tests/functional/test_cache_middleware.py index 19f38077cf..c3c9ab8211 100644 --- a/glance/tests/functional/test_cache_middleware.py +++ b/glance/tests/functional/test_cache_middleware.py @@ -35,7 +35,9 @@ from glance.tests import functional from glance.tests.utils import (skip_if_disabled, execute, - xattr_writes_supported) + xattr_writes_supported, + minimal_headers, + ) FIVE_KB = 5 * 1024 @@ -90,9 +92,7 @@ def test_cache_middleware_transparent(self): # Add an image and verify a 200 OK is returned image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image1') path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers, @@ -177,6 +177,8 @@ def serve_requests(httpd): # Add a remote image and verify a 201 Created is returned remote_uri = 'http://%s:%d/images/2' % (remote_ip, remote_port) headers = {'X-Image-Meta-Name': 'Image2', + 'X-Image-Meta-disk_format': 'raw', + 'X-Image-Meta-container_format': 'ovf', 'X-Image-Meta-Is-Public': 'True', 'X-Image-Meta-Location': remote_uri} path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) @@ -226,9 +228,8 @@ def add_image(self, name): identifier """ image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': '%s' % name, - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('%s' % name) + path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers, diff --git a/glance/tests/functional/test_misc.py b/glance/tests/functional/test_misc.py index 4894458d26..3bd9270788 100644 --- a/glance/tests/functional/test_misc.py +++ b/glance/tests/functional/test_misc.py @@ -22,7 +22,7 @@ import tempfile from glance.tests import functional -from glance.tests.utils import execute +from glance.tests.utils import execute, minimal_headers, minimal_add_command FIVE_KB = 5 * 1024 FIVE_GB = 5 * 1024 * 1024 * 1024 @@ -44,9 +44,7 @@ def test_api_response_when_image_deleted_from_filesystem(self): # 1. POST /images with public image named Image1 # attribute and no custom properties. Verify a 200 OK is returned image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image1') path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers, @@ -132,8 +130,9 @@ def test_api_treats_size_as_a_normal_property(self): image_file.write("XXX") image_file.flush() image_file_name = image_file.name - cmd = "bin/glance --port=%d add is_public=True name=MyImage "\ - "size=12345 < %s" % (self.api_port, image_file_name) + cmd = minimal_add_command(self.api_port, + 'MyImage', + 'size=12345 < %s' % image_file_name) exitcode, out, err = execute(cmd) diff --git a/glance/tests/functional/test_private_images.py b/glance/tests/functional/test_private_images.py index 113e43dd6e..a2b8783261 100644 --- a/glance/tests/functional/test_private_images.py +++ b/glance/tests/functional/test_private_images.py @@ -23,7 +23,11 @@ from glance.tests import functional from glance.tests.functional import keystone_utils -from glance.tests.utils import execute, skip_if_disabled +from glance.tests.utils import (execute, + skip_if_disabled, + minimal_headers, + minimal_add_command, + ) FIVE_KB = 5 * 1024 FIVE_GB = 5 * 1024 * 1024 * 1024 @@ -46,9 +50,8 @@ def test_private_images_notadmin(self): # First, we need to push an image up image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Auth-Token': keystone_utils.pattieblack_token, - 'X-Image-Meta-Name': 'Image1'} + headers = minimal_headers('Image1', public=False) + headers['X-Auth-Token'] = keystone_utils.pattieblack_token path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers, @@ -298,9 +301,8 @@ def test_private_images_admin(self): # Need to push an image up image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Auth-Token': keystone_utils.pattieblack_token, - 'X-Image-Meta-Name': 'Image1'} + headers = minimal_headers('Image1', public=False) + headers['X-Auth-Token'] = keystone_utils.pattieblack_token path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers, @@ -478,7 +480,7 @@ def test_private_images_admin(self): self.assertEqual(response.status, 200) self.assertEqual(response['x-image-meta-name'], "Image1") self.assertEqual(response['x-image-meta-is_public'], "False") - self.assertEqual(response['x-image-meta-owner'], '') + self.assertFalse('x-image-meta-owner' in response) # And of course the image itself headers = {'X-Auth-Token': keystone_utils.pattieblack_token} @@ -490,7 +492,7 @@ def test_private_images_admin(self): self.assertEqual(content, "*" * FIVE_KB) self.assertEqual(response['x-image-meta-name'], "Image1") self.assertEqual(response['x-image-meta-is_public'], "False") - self.assertEqual(response['x-image-meta-owner'], '') + self.assertFalse('x-image-meta-owner' in response) # Pattieblack can't change is-public, though headers = {'X-Auth-Token': keystone_utils.pattieblack_token, @@ -531,8 +533,7 @@ def test_private_images_anon(self): # Make sure anonymous user can't push up an image image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1'} + headers = minimal_headers('Image1', public=False) path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers, @@ -541,9 +542,8 @@ def test_private_images_anon(self): # Now push up an image for anonymous user to try to access image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Auth-Token': keystone_utils.pattieblack_token, - 'X-Image-Meta-Name': 'Image1'} + headers = minimal_headers('Image1', public=False) + headers['X-Auth-Token'] = keystone_utils.pattieblack_token path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers, @@ -644,7 +644,7 @@ def test_private_images_anon(self): self.assertEqual(response.status, 200) self.assertEqual(response['x-image-meta-name'], "Image1") self.assertEqual(response['x-image-meta-is_public'], "False") - self.assertEqual(response['x-image-meta-owner'], '') + self.assertFalse('x-image-meta-owner' in response) # And even the image itself... path = "http://%s:%d/v1/images/%s" % ("0.0.0.0", self.api_port, @@ -655,7 +655,7 @@ def test_private_images_anon(self): self.assertEqual(content, "*" * FIVE_KB) self.assertEqual(response['x-image-meta-name'], "Image1") self.assertEqual(response['x-image-meta-is_public'], "False") - self.assertEqual(response['x-image-meta-owner'], '') + self.assertFalse('x-image-meta-owner' in response) # Anonymous still shouldn't be able to make the image # public... @@ -835,7 +835,7 @@ def _do_test_glance_cli(self, cmd): self.assertEqual(response.status, 200) self.assertEqual(response['x-image-meta-name'], "MyImage") self.assertEqual(response['x-image-meta-is_public'], "True") - self.assertEqual(response['x-image-meta-owner'], '') + self.assertFalse('x-image-meta-owner' in response) self.stop_servers() @@ -850,8 +850,8 @@ def test_glance_cli_noauth_strategy(self): """ Test the CLI with the noauth strategy defaulted to. """ - cmd = ("bin/glance --port=%d --auth_token=%s add name=MyImage" % - (self.api_port, keystone_utils.pattieblack_token)) + suffix = '--auth_token=%s' % keystone_utils.pattieblack_token + cmd = minimal_add_command(self.api_port, 'MyImage', suffix, False) self._do_test_glance_cli(cmd) @skip_if_disabled @@ -860,11 +860,12 @@ def test_glance_cli_keystone_strategy_switches(self): Test the CLI with the keystone (v1) strategy enabled via command line switches. """ - substitutions = (self.api_port, self.auth_port, 'keystone', + substitutions = (self.auth_port, 'keystone', 'pattieblack', 'secrete') - cmd = ("bin/glance --port=%d --auth_url=http://localhost:%d/v1.0 " - "--auth_strategy=%s --username=%s --password=%s " - " add name=MyImage" % substitutions) + suffix = ("--auth_url=http://localhost:%d/v1.0 " + "--auth_strategy=%s --username=%s --password=%s " + % substitutions) + cmd = minimal_add_command(self.api_port, 'MyImage', suffix, False) self._do_test_glance_cli(cmd) @skip_if_disabled @@ -877,7 +878,7 @@ def test_glance_cli_keystone_strategy_environment(self): os.environ['OS_AUTH_STRATEGY'] = 'keystone' os.environ['OS_AUTH_USER'] = 'pattieblack' os.environ['OS_AUTH_KEY'] = 'secrete' - cmd = "bin/glance --port=%d add name=MyImage" % self.api_port + cmd = minimal_add_command(self.api_port, 'MyImage', public=False) self._do_test_glance_cli(cmd) @skip_if_disabled diff --git a/glance/tests/functional/test_shared_images.py b/glance/tests/functional/test_shared_images.py index 5a39d22365..47982cb2c4 100644 --- a/glance/tests/functional/test_shared_images.py +++ b/glance/tests/functional/test_shared_images.py @@ -22,7 +22,7 @@ from glance.tests import functional from glance.tests.functional import keystone_utils -from glance.tests.utils import execute, skip_if_disabled +from glance.tests.utils import execute, skip_if_disabled, minimal_headers FIVE_KB = 5 * 1024 FIVE_GB = 5 * 1024 * 1024 * 1024 @@ -32,8 +32,7 @@ class TestSharedImagesApi(keystone_utils.KeystoneTests): def _push_image(self): # First, we need to push an image up image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1'} + headers = minimal_headers('Image1', public=False) path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) response, content = self._request(path, 'POST', keystone_utils.pattieblack_token, @@ -378,8 +377,7 @@ class TestSharedImagesCli(keystone_utils.KeystoneTests): def _push_image(self, name=1): # First, we need to push an image up image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': str(name)} + headers = minimal_headers(str(name), public=False) path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) response, content = self._request(path, 'POST', keystone_utils.pattieblack_token, diff --git a/glance/tests/functional/test_ssl.py b/glance/tests/functional/test_ssl.py index 4c94d1cf9f..be9ef4c21f 100644 --- a/glance/tests/functional/test_ssl.py +++ b/glance/tests/functional/test_ssl.py @@ -45,7 +45,7 @@ from glance.common import utils from glance.store.location import get_location_from_uri from glance.tests import functional -from glance.tests.utils import execute, skip_if_disabled +from glance.tests.utils import execute, skip_if_disabled, minimal_headers FIVE_KB = 5 * 1024 FIVE_GB = 5 * 1024 * 1024 * 1024 @@ -144,9 +144,7 @@ def test_get_head_simple_post(self): # 2. POST /images with public image named Image1 # attribute and no custom properties. Verify a 200 OK is returned image_data = "*" * FIVE_KB - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image1') path = "https://%s:%d/v1/images" % ("0.0.0.0", self.api_port) https = httplib2.Http(disable_ssl_certificate_validation=True) response, content = https.request(path, 'POST', headers=headers, @@ -183,8 +181,8 @@ def test_get_head_simple_post(self): 'x-image-meta-name': 'Image1', 'x-image-meta-is_public': 'True', 'x-image-meta-status': 'active', - 'x-image-meta-disk_format': '', - 'x-image-meta-container_format': '', + 'x-image-meta-disk_format': 'raw', + 'x-image-meta-container_format': 'ovf', 'x-image-meta-size': str(FIVE_KB)} expected_std_headers = { @@ -216,8 +214,8 @@ def test_get_head_simple_post(self): self.assertEqual(response.status, 200) expected_result = {"images": [ - {"container_format": None, - "disk_format": None, + {"container_format": "ovf", + "disk_format": "raw", "id": image_id, "name": "Image1", "checksum": "c2e5db72bd7fd153f53ede5da5a06de3", @@ -235,8 +233,8 @@ def test_get_head_simple_post(self): "status": "active", "name": "Image1", "deleted": False, - "container_format": None, - "disk_format": None, + "container_format": "ovf", + "disk_format": "raw", "id": image_id, "is_public": True, "deleted_at": None, @@ -276,8 +274,8 @@ def test_get_head_simple_post(self): "status": "active", "name": "Image1", "deleted": False, - "container_format": None, - "disk_format": None, + "container_format": "ovf", + "disk_format": "raw", "id": image_id, "is_public": True, "deleted_at": None, @@ -366,9 +364,7 @@ def test_queued_process_flow(self): # 1. POST /images with public image named Image1 # with no location or image data - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image1') path = "https://%s:%d/v1/images" % ("0.0.0.0", self.api_port) https = httplib2.Http(disable_ssl_certificate_validation=True) response, content = https.request(path, 'POST', headers=headers) @@ -376,8 +372,8 @@ def test_queued_process_flow(self): data = json.loads(content) self.assertEqual(data['image']['checksum'], None) self.assertEqual(data['image']['size'], 0) - self.assertEqual(data['image']['container_format'], None) - self.assertEqual(data['image']['disk_format'], None) + self.assertEqual(data['image']['container_format'], 'ovf') + self.assertEqual(data['image']['disk_format'], 'raw') self.assertEqual(data['image']['name'], "Image1") self.assertEqual(data['image']['is_public'], True) @@ -393,8 +389,8 @@ def test_queued_process_flow(self): self.assertEqual(data['images'][0]['id'], image_id) self.assertEqual(data['images'][0]['checksum'], None) self.assertEqual(data['images'][0]['size'], 0) - self.assertEqual(data['images'][0]['container_format'], None) - self.assertEqual(data['images'][0]['disk_format'], None) + self.assertEqual(data['images'][0]['container_format'], 'ovf') + self.assertEqual(data['images'][0]['disk_format'], 'raw') self.assertEqual(data['images'][0]['name'], "Image1") # 3. HEAD /images @@ -446,8 +442,8 @@ def test_queued_process_flow(self): hashlib.md5(image_data).hexdigest()) self.assertEqual(data['images'][0]['id'], image_id) self.assertEqual(data['images'][0]['size'], FIVE_KB) - self.assertEqual(data['images'][0]['container_format'], None) - self.assertEqual(data['images'][0]['disk_format'], None) + self.assertEqual(data['images'][0]['container_format'], 'ovf') + self.assertEqual(data['images'][0]['disk_format'], 'raw') self.assertEqual(data['images'][0]['name'], "Image1") self.stop_servers() @@ -633,11 +629,9 @@ def test_size_greater_2G_mysql(self): # X-Image-Meta-Location attribute to make Glance forego # "adding" the image data. # Verify a 201 OK is returned - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Location': 'https://example.com/fakeimage', - 'X-Image-Meta-Size': str(FIVE_GB), - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image1') + headers['X-Image-Meta-Location'] = 'https://example.com/fakeimage' + headers['X-Image-Meta-Size'] = str(FIVE_GB) path = "https://%s:%d/v1/images" % ("0.0.0.0", self.api_port) https = httplib2.Http(disable_ssl_certificate_validation=True) response, content = https.request(path, 'POST', headers=headers) @@ -678,7 +672,7 @@ def test_traceback_not_consumed(self): response, content = https.request(path, 'POST', body=test_data_file.name) self.assertEqual(response.status, 400) - expected = "Content-Type must be application/octet-stream" + expected = "Data supplied was not valid. Details: 400 Bad Request" self.assertTrue(expected in content, "Could not find '%s' in '%s'" % (expected, content)) @@ -956,9 +950,7 @@ def test_limited_images(self): self.assertEqual(content, '{"images": []}') # 1. POST /images with three public images with various attributes - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image1') path = "https://%s:%d/v1/images" % ("0.0.0.0", self.api_port) https = httplib2.Http(disable_ssl_certificate_validation=True) response, content = https.request(path, 'POST', headers=headers) @@ -967,9 +959,7 @@ def test_limited_images(self): image_ids = [data['image']['id']] - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image2', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image2') path = "https://%s:%d/v1/images" % ("0.0.0.0", self.api_port) https = httplib2.Http(disable_ssl_certificate_validation=True) response, content = https.request(path, 'POST', headers=headers) @@ -978,9 +968,7 @@ def test_limited_images(self): image_ids.append(data['image']['id']) - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Name': 'Image3', - 'X-Image-Meta-Is-Public': 'True'} + headers = minimal_headers('Image3') path = "https://%s:%d/v1/images" % ("0.0.0.0", self.api_port) https = httplib2.Http(disable_ssl_certificate_validation=True) response, content = https.request(path, 'POST', headers=headers) diff --git a/glance/tests/unit/test_api.py b/glance/tests/unit/test_api.py index 036d4a1156..7e00955011 100644 --- a/glance/tests/unit/test_api.py +++ b/glance/tests/unit/test_api.py @@ -2194,6 +2194,32 @@ def test_add_image_unauthorized(self): res = req.get_response(self.api) self.assertEquals(res.status_int, 401) + def _do_test_add_image_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', + '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.BAD_REQUEST) + + def test_add_image_missing_disk_format(self): + """Tests creation of an image with missing disk format""" + self._do_test_add_image_missing_format('disk_format') + + def test_add_image_missing_container_type(self): + """Tests creation of an image with missing container format""" + self._do_test_add_image_missing_format('container_format') + def test_register_and_upload(self): """ Test that the process of registering an image with @@ -2714,6 +2740,8 @@ def test_delete_queued_image(self): # We will stop the process after the reservation stage, then # try to delete the image. 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'} req = webob.Request.blank("/images") @@ -2735,6 +2763,8 @@ def test_delete_queued_image(self): def test_delete_protected_image(self): fixture_headers = {'x-image-meta-store': 'file', 'x-image-meta-name': 'fake image #3', + 'x-image-meta-disk-format': 'vhd', + 'x-image-meta-container-format': 'ovf', 'x-image-meta-protected': 'True'} req = webob.Request.blank("/images") diff --git a/glance/tests/unit/test_misc.py b/glance/tests/unit/test_misc.py index f94d0b6ad7..c151a456f4 100644 --- a/glance/tests/unit/test_misc.py +++ b/glance/tests/unit/test_misc.py @@ -145,3 +145,32 @@ def test_encryption(self): self.assertTrue(ciphertext != plaintext) text = crypt.urlsafe_decrypt(key, ciphertext) self.assertTrue(plaintext == text) + + def test_empty_metadata_headers(self): + """Ensure unset metadata is not encoded in HTTP headers""" + + metadata = { + 'foo': 'bar', + 'snafu': None, + 'bells': 'whistles', + 'unset': None, + 'empty': '', + 'properties': { + 'distro': '', + 'arch': None, + 'user': 'nobody', + }, + } + + headers = utils.image_meta_to_http_headers(metadata) + + self.assertFalse('x-image-meta-snafu' in headers) + self.assertFalse('x-image-meta-uset' in headers) + self.assertFalse('x-image-meta-snafu' in headers) + self.assertFalse('x-image-meta-property-arch' in headers) + + self.assertEquals(headers.get('x-image-meta-foo'), 'bar') + self.assertEquals(headers.get('x-image-meta-bells'), 'whistles') + self.assertEquals(headers.get('x-image-meta-empty'), '') + self.assertEquals(headers.get('x-image-meta-property-distro'), '') + self.assertEquals(headers.get('x-image-meta-property-user'), 'nobody') diff --git a/glance/tests/unit/test_wsgi.py b/glance/tests/unit/test_wsgi.py index be5c3d8e86..e7e2b6c8d8 100644 --- a/glance/tests/unit/test_wsgi.py +++ b/glance/tests/unit/test_wsgi.py @@ -230,7 +230,10 @@ class FakeResponse(): response.headers = headers result = utils.get_image_meta_from_headers(response) for k, v in fixture.iteritems(): - self.assertEqual(v, result[k]) + if v is not None: + self.assertEqual(v, result[k]) + else: + self.assertFalse(k in result) def test_boolean_header_values(self): """ diff --git a/glance/tests/utils.py b/glance/tests/utils.py index 3b0960ef5e..2fb05a3153 100644 --- a/glance/tests/utils.py +++ b/glance/tests/utils.py @@ -310,3 +310,21 @@ def set_xattr(path, key, value): os.unlink(fake_filepath) return result + + +def minimal_headers(name, public=True): + headers = {'Content-Type': 'application/octet-stream', + 'X-Image-Meta-Name': name, + 'X-Image-Meta-disk_format': 'raw', + 'X-Image-Meta-container_format': 'ovf', + } + if public: + headers['X-Image-Meta-Is-Public'] = 'True' + return headers + + +def minimal_add_command(port, name, suffix='', public=True): + visibility = 'is_public=True' if public else '' + return ("bin/glance --port=%d add %s" + " disk_format=raw container_format=ovf" + " name=%s %s" % (port, visibility, name, suffix))