diff --git a/bin/glance b/bin/glance index a60ea73a85..36c4ac7b54 100755 --- a/bin/glance +++ b/bin/glance @@ -131,6 +131,7 @@ def print_image_formatted(client, image): image['id']) print "Id: %s" % image['id'] print "Public: " + (image['is_public'] and "Yes" or "No") + print "Protected: " + (image['protected'] and "Yes" or "No") print "Name: %s" % image['name'] print "Status: %s" % image['status'] print "Size: %d" % int(image['size']) @@ -167,6 +168,9 @@ size Optional. Should be size in bytes of the image if is_public Optional. If specified, interpreted as a boolean value and sets or unsets the image's availability to the public. The default value is False. +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'. @@ -212,6 +216,8 @@ EXAMPLES image_meta = {'name': fields.pop('name'), 'is_public': utils.bool_from_string( 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), @@ -291,6 +297,8 @@ name A name for the image. location The location of the image. is_public If specified, interpreted as a boolean value and sets or unsets the image's availability to the public. +protected If specified, interpreted as a boolean value + and enables or disables deletion protection for the image. disk_format Format of the disk image container_format Format of the container @@ -331,6 +339,9 @@ to spell field names correctly. :)""" if 'is_public' in fields: image_meta['is_public'] = utils.bool_from_string( fields.pop('is_public')) + if 'protected' in fields: + image_meta['protected'] = utils.bool_from_string( + fields.pop('protected')) # Add custom attributes, which are all the arguments remaining image_meta['properties'] = fields diff --git a/glance/api/v1/__init__.py b/glance/api/v1/__init__.py index 456c37d443..59fc4ae9bd 100644 --- a/glance/api/v1/__init__.py +++ b/glance/api/v1/__init__.py @@ -17,6 +17,6 @@ SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format', 'min_ram', 'min_disk', 'size_min', 'size_max', - 'is_public', 'changes-since'] + 'is_public', 'changes-since', 'protected'] SUPPORTED_PARAMS = ('limit', 'marker', 'sort_key', 'sort_dir') diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 1290edd6f8..b886220b46 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -567,6 +567,11 @@ class Controller(controller.BaseController): content_type="text/plain") image = self.get_image_meta_or_404(req, id) + if image['protected']: + msg = _("Image is protected") + logger.debug(msg) + raise HTTPForbidden(msg, request=req, + content_type="text/plain") # The image's location field may be None in the case # of a saving or queued image, therefore don't ask a backend diff --git a/glance/common/utils.py b/glance/common/utils.py index a2be1caebc..d9334e146c 100644 --- a/glance/common/utils.py +++ b/glance/common/utils.py @@ -104,10 +104,9 @@ def get_image_meta_from_headers(response): result['properties'] = properties if 'size' in result: result['size'] = int(result['size']) - if 'is_public' in result: - result['is_public'] = bool_from_header_value(result['is_public']) - if 'deleted' in result: - result['deleted'] = bool_from_header_value(result['deleted']) + for key in ('is_public', 'deleted', 'protected'): + if key in result: + result[key] = bool_from_header_value(result[key]) return result diff --git a/glance/registry/api/v1/images.py b/glance/registry/api/v1/images.py index 612038728f..c37aeb1bad 100644 --- a/glance/registry/api/v1/images.py +++ b/glance/registry/api/v1/images.py @@ -38,7 +38,7 @@ DISPLAY_FIELDS_IN_INDEX = ['id', 'name', 'size', SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format', 'min_ram', 'min_disk', 'size_min', 'size_max', - 'changes-since'] + 'changes-since', 'protected'] SUPPORTED_SORT_KEYS = ('name', 'status', 'container_format', 'disk_format', 'size', 'id', 'created_at', 'updated_at') @@ -170,6 +170,14 @@ class Controller(object): except ValueError: raise exc.HTTPBadRequest(_("Unrecognized changes-since value")) + if 'protected' in filters: + value = self._get_bool(filters['protected']) + if value is None: + raise exc.HTTPBadRequest(_("protected must be True, or " + "False")) + + filters['protected'] = value + # only allow admins to filter on 'deleted' if req.context.is_admin: deleted_filter = self._parse_deleted_filter(req) @@ -226,6 +234,15 @@ class Controller(object): raise exc.HTTPBadRequest(explanation=msg) return sort_dir + def _get_bool(self, value): + value = value.lower() + if value == 'true' or value == '1': + return True + elif value == 'false' or value == '0': + return False + + return None + def _get_is_public(self, req): """Parse is_public into something usable.""" is_public = req.str_params.get('is_public', None) @@ -234,16 +251,15 @@ class Controller(object): # NOTE(vish): This preserves the default value of showing only # public images. return True - is_public = is_public.lower() - if is_public == 'none': + elif is_public.lower() == 'none': return None - elif is_public == 'true' or is_public == '1': - return True - elif is_public == 'false' or is_public == '0': - return False - else: - raise exc.HTTPBadRequest(_("is_public must be None, True, " - "or False")) + + value = self._get_bool(is_public) + if value is None: + raise exc.HTTPBadRequest(_("is_public must be None, True, or " + "False")) + + return value def _parse_deleted_filter(self, req): """Parse deleted into something usable.""" diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py index b0653767ca..765d836090 100644 --- a/glance/registry/db/api.py +++ b/glance/registry/db/api.py @@ -49,7 +49,8 @@ BASE_MODEL_ATTRS = set(['id', 'created_at', 'updated_at', 'deleted_at', IMAGE_ATTRS = BASE_MODEL_ATTRS | set(['name', 'status', 'size', 'disk_format', 'container_format', 'min_disk', 'min_ram', 'is_public', - 'location', 'checksum', 'owner']) + 'location', 'checksum', 'owner', + 'protected']) CONTAINER_FORMATS = ['ami', 'ari', 'aki', 'bare', 'ovf'] DISK_FORMATS = ['ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw', 'qcow2', 'vdi', @@ -347,6 +348,7 @@ def _image_update(context, values, image_id, purge_props=False): values['min_disk'] = int(values['min_disk'] or 0) values['is_public'] = bool(values.get('is_public', False)) + values['protected'] = bool(values.get('protected', False)) image_ref = models.Image() # Need to canonicalize ownership diff --git a/glance/registry/db/migrate_repo/versions/013_add_protected.py b/glance/registry/db/migrate_repo/versions/013_add_protected.py new file mode 100644 index 0000000000..02d8ad0d79 --- /dev/null +++ b/glance/registry/db/migrate_repo/versions/013_add_protected.py @@ -0,0 +1,37 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2011 OpenStack LLC. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import MetaData, Table, Column, Boolean + + +meta = MetaData() + +protected = Column('protected', Boolean, default=False) + + +def upgrade(migrate_engine): + meta.bind = migrate_engine + + images = Table('images', meta, autoload=True) + images.create_column(protected) + + +def downgrade(migrate_engine): + meta.bind = migrate_engine + + images = Table('images', meta, autoload=True) + images.drop_column(protected) diff --git a/glance/registry/db/migrate_repo/versions/013_sqlite_downgrade.sql b/glance/registry/db/migrate_repo/versions/013_sqlite_downgrade.sql new file mode 100644 index 0000000000..784a5ac84d --- /dev/null +++ b/glance/registry/db/migrate_repo/versions/013_sqlite_downgrade.sql @@ -0,0 +1,65 @@ +/* + * This is necessary because sqlalchemy has various bugs preventing + * downgrades from working correctly. + */ +BEGIN TRANSACTION; + +CREATE TEMPORARY TABLE images_backup ( + id VARCHAR(36) NOT NULL, + name VARCHAR(255), + size INTEGER, + status VARCHAR(30) NOT NULL, + is_public BOOLEAN NOT NULL, + location TEXT, + created_at DATETIME NOT NULL, + updated_at DATETIME, + deleted_at DATETIME, + deleted BOOLEAN NOT NULL, + disk_format VARCHAR(20), + container_format VARCHAR(20), + checksum VARCHAR(32), + owner VARCHAR(255), + min_disk INTEGER NOT NULL, + min_ram INTEGER NOT NULL, + PRIMARY KEY (id), + CHECK (is_public IN (0, 1)), + CHECK (deleted IN (0, 1)) +); + + +INSERT INTO images_backup +SELECT id, name, size, status, is_public, location, created_at, updated_at, deleted_at, deleted, disk_format, container_format, checksum, owner, min_disk, min_ram +FROM images; + +DROP TABLE images; + +CREATE TABLE images ( + id VARCHAR(36) NOT NULL, + name VARCHAR(255), + size INTEGER, + status VARCHAR(30) NOT NULL, + is_public BOOLEAN NOT NULL, + location TEXT, + created_at DATETIME NOT NULL, + updated_at DATETIME, + deleted_at DATETIME, + deleted BOOLEAN NOT NULL, + disk_format VARCHAR(20), + container_format VARCHAR(20), + checksum VARCHAR(32), + owner VARCHAR(255), + min_disk INTEGER NOT NULL, + min_ram INTEGER NOT NULL, + PRIMARY KEY (id), + CHECK (is_public IN (0, 1)), + CHECK (deleted IN (0, 1)) +); +CREATE INDEX ix_images_is_public ON images (is_public); +CREATE INDEX ix_images_deleted ON images (deleted); + +INSERT INTO images +SELECT id, name, size, status, is_public, location, created_at, updated_at, deleted_at, deleted, disk_format, container_format, checksum, owner, min_disk, min_ram +FROM images_backup; + +DROP TABLE images_backup; +COMMIT; diff --git a/glance/registry/db/models.py b/glance/registry/db/models.py index ddbad219b7..861d189ff7 100644 --- a/glance/registry/db/models.py +++ b/glance/registry/db/models.py @@ -110,6 +110,7 @@ class Image(BASE, ModelBase): min_disk = Column(Integer(), nullable=False, default=0) min_ram = Column(Integer(), nullable=False, default=0) owner = Column(String(255)) + protected = Column(Boolean, nullable=False, default=False) class ImageProperty(BASE, ModelBase): diff --git a/glance/tests/functional/test_api.py b/glance/tests/functional/test_api.py index 01fbb1a59e..08aa265aab 100644 --- a/glance/tests/functional/test_api.py +++ b/glance/tests/functional/test_api.py @@ -667,6 +667,7 @@ class TestApi(functional.FunctionalTest): 'X-Image-Meta-Disk-Format': 'vdi', 'X-Image-Meta-Size': '19', 'X-Image-Meta-Is-Public': 'True', + 'X-Image-Meta-Protected': 'True', 'X-Image-Meta-Property-pants': 'are on'} path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() @@ -684,6 +685,7 @@ class TestApi(functional.FunctionalTest): 'X-Image-Meta-Disk-Format': 'vhd', 'X-Image-Meta-Size': '20', 'X-Image-Meta-Is-Public': 'True', + 'X-Image-Meta-Protected': 'False', 'X-Image-Meta-Property-pants': 'are on'} path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() @@ -701,6 +703,7 @@ class TestApi(functional.FunctionalTest): 'X-Image-Meta-Disk-Format': 'ami', 'X-Image-Meta-Size': '21', 'X-Image-Meta-Is-Public': 'True', + 'X-Image-Meta-Protected': 'False', 'X-Image-Meta-Property-pants': 'are off'} path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port) http = httplib2.Http() @@ -717,7 +720,8 @@ class TestApi(functional.FunctionalTest): 'X-Image-Meta-Container-Format': 'ami', 'X-Image-Meta-Disk-Format': 'ami', 'X-Image-Meta-Size': '22', - 'X-Image-Meta-Is-Public': 'False'} + 'X-Image-Meta-Is-Public': 'False', + 'X-Image-Meta-Protected': '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) @@ -851,7 +855,31 @@ class TestApi(functional.FunctionalTest): for image in data['images']: self.assertNotEqual(image['name'], "My Private Image") - # 12. GET /images with property filter + # 12. Get /images with protected=False filter + # Verify correct images returned with property + params = "protected=False" + path = "http://%s:%d/v1/images?%s" % ( + "0.0.0.0", self.api_port, params) + response, content = http.request(path, 'GET') + self.assertEqual(response.status, 200) + data = json.loads(content) + self.assertEqual(len(data['images']), 2) + for image in data['images']: + self.assertNotEqual(image['name'], "Image1") + + # 13. Get /images with protected=True filter + # Verify correct images returned with property + params = "protected=True" + path = "http://%s:%d/v1/images?%s" % ( + "0.0.0.0", self.api_port, params) + response, content = http.request(path, 'GET') + self.assertEqual(response.status, 200) + data = json.loads(content) + self.assertEqual(len(data['images']), 1) + for image in data['images']: + self.assertEqual(image['name'], "Image1") + + # 14. GET /images with property filter # Verify correct images returned with property params = "property-pants=are%20on" path = "http://%s:%d/v1/images/detail?%s" % ( @@ -863,7 +891,7 @@ class TestApi(functional.FunctionalTest): for image in data['images']: self.assertEqual(image['properties']['pants'], "are on") - # 13. GET /images with property filter and name filter + # 15. GET /images with property filter and name filter # Verify correct images returned with property and name # Make sure you quote the url when using more than one param! params = "name=My%20Image!&property-pants=are%20on" @@ -877,7 +905,7 @@ class TestApi(functional.FunctionalTest): self.assertEqual(image['properties']['pants'], "are on") self.assertEqual(image['name'], "My Image!") - # 14. GET /images with past changes-since filter + # 16. GET /images with past changes-since filter dt1 = datetime.datetime.utcnow() - datetime.timedelta(1) iso1 = utils.isotime(dt1) params = "changes-since=%s" % iso1 @@ -887,7 +915,7 @@ class TestApi(functional.FunctionalTest): data = json.loads(content) self.assertEqual(len(data['images']), 3) - # 15. GET /images with future changes-since filter + # 17. GET /images with future changes-since filter dt2 = datetime.datetime.utcnow() + datetime.timedelta(1) iso2 = utils.isotime(dt2) params = "changes-since=%s" % iso2 @@ -897,14 +925,6 @@ class TestApi(functional.FunctionalTest): data = json.loads(content) self.assertEqual(len(data['images']), 0) - # DELETE images - for image_id in image_ids: - path = "http://%s:%d/v1/images/%s" % ("0.0.0.0", self.api_port, - image_id) - http = httplib2.Http() - response, content = http.request(path, 'DELETE') - self.assertEqual(response.status, 200) - self.stop_servers() @skip_if_disabled diff --git a/glance/tests/functional/test_bin_glance.py b/glance/tests/functional/test_bin_glance.py index 2f714e1270..52c30be5fe 100644 --- a/glance/tests/functional/test_bin_glance.py +++ b/glance/tests/functional/test_bin_glance.py @@ -508,26 +508,26 @@ class TestBinGlance(functional.FunctionalTest): self.assertEqual(0, exitcode) image_lines = out.split("\n")[1:-1] - self.assertEqual(24, len(image_lines)) + self.assertEqual(26, len(image_lines)) self.assertEqual(image_lines[1].split()[1], image_ids[1]) - self.assertEqual(image_lines[13].split()[1], image_ids[0]) + self.assertEqual(image_lines[14].split()[1], image_ids[0]) - # 10. Check min_ram filter + # 12. Check min_ram filter cmd = "min_ram=256" exitcode, out, err = execute("%s %s" % (_details_cmd, cmd)) self.assertEqual(0, exitcode) image_lines = out.split("\n")[2:-1] - self.assertEqual(11, len(image_lines)) + self.assertEqual(12, len(image_lines)) self.assertEqual(image_lines[0].split()[1], image_ids[2]) - # 11. Check min_disk filter + # 13. Check min_disk filter cmd = "min_disk=7" exitcode, out, err = execute("%s %s" % (_details_cmd, cmd)) self.assertEqual(0, exitcode) image_lines = out.split("\n")[2:-1] - self.assertEqual(11, len(image_lines)) + self.assertEqual(12, len(image_lines)) self.assertEqual(image_lines[0].split()[1], image_ids[2]) self.stop_servers() @@ -610,9 +610,9 @@ class TestBinGlance(functional.FunctionalTest): self.assertEqual(0, exitcode) image_lines = out.split("\n")[1:-1] - self.assertEqual(22, len(image_lines)) + self.assertEqual(24, len(image_lines)) self.assertTrue(image_lines[1].split()[1], image_ids[2]) - self.assertTrue(image_lines[12].split()[1], image_ids[1]) + self.assertTrue(image_lines[13].split()[1], image_ids[1]) self.stop_servers() @@ -686,10 +686,10 @@ class TestBinGlance(functional.FunctionalTest): self.assertEqual(0, exitcode) image_lines = out.split("\n")[1:-1] - self.assertEqual(33, len(image_lines)) + self.assertEqual(36, len(image_lines)) self.assertTrue(image_lines[1].split()[1], image_ids[2]) - self.assertTrue(image_lines[12].split()[1], image_ids[1]) - self.assertTrue(image_lines[23].split()[1], image_ids[4]) + self.assertTrue(image_lines[13].split()[1], image_ids[1]) + self.assertTrue(image_lines[25].split()[1], image_ids[4]) self.stop_servers() @@ -744,4 +744,88 @@ class TestBinGlance(functional.FunctionalTest): self.assertEqual(0, exitcode) self.assertEqual('Deleted image %s' % image_id, out.strip()) + def test_protected_image(self): + """ + We test the following: + + 0. Verify no public images in index + 1. Add a public image with a location attr + protected and no image data + 2. Check that image exists in index + 3. Attempt to delete the image + 4. Remove protection from image + 5. Delete the image + 6. Verify no longer in index + """ + self.cleanup() + self.start_servers() + + api_port = self.api_port + registry_port = self.registry_port + + # 0. Verify no public images + cmd = "bin/glance --port=%d index" % api_port + + exitcode, out, err = execute(cmd) + + self.assertEqual(0, exitcode) + 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 + + exitcode, out, err = execute(cmd) + + self.assertEqual(0, exitcode) + self.assertTrue(out.strip().startswith('Added new image with ID:')) + + # 2. Verify image added as public image + cmd = "bin/glance --port=%d index" % api_port + + exitcode, out, err = execute(cmd) + + self.assertEqual(0, exitcode) + lines = out.split("\n")[2:-1] + self.assertEqual(1, len(lines)) + + line = lines[0] + + image_id, name, disk_format, container_format, size = \ + [c.strip() for c in line.split()] + self.assertEqual('MyImage', name) + + # 3. Delete the image + cmd = "bin/glance --port=%d --force delete %s" % (api_port, image_id) + + exitcode, out, err = execute(cmd, raise_error=False) + + self.assertNotEqual(0, exitcode) + self.assertTrue('Image is protected' in err) + + # 4. Remove image protection + cmd = "bin/glance --port=%d --force update %s" \ + " protected=False" % (api_port, image_id) + + exitcode, out, err = execute(cmd) + + self.assertEqual(0, exitcode) + self.assertTrue(out.strip().startswith('Updated image')) + + # 5. Delete the image + cmd = "bin/glance --port=%d --force delete %s" % (api_port, image_id) + + exitcode, out, err = execute(cmd) + + self.assertEqual(0, exitcode) + self.assertTrue(out.strip().startswith('Deleted image')) + + # 6. Verify no public images + cmd = "bin/glance --port=%d index" % api_port + + exitcode, out, err = execute(cmd) + + self.assertEqual(0, exitcode) + self.assertEqual('', out.strip()) + self.stop_servers() diff --git a/glance/tests/unit/test_api.py b/glance/tests/unit/test_api.py index 903058cea5..6717c9f3db 100644 --- a/glance/tests/unit/test_api.py +++ b/glance/tests/unit/test_api.py @@ -2585,6 +2585,27 @@ class TestGlanceAPI(unittest.TestCase): res = req.get_response(self.api) self.assertEquals(res.status_int, 200) + def test_delete_protected_image(self): + fixture_headers = {'x-image-meta-store': 'file', + 'x-image-meta-name': 'fake image #3', + 'x-image-meta-protected': 'True'} + + 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']) + + # Now try to delete the image... + req = webob.Request.blank("/images/%s" % res_body['id']) + req.method = 'DELETE' + res = req.get_response(self.api) + self.assertEquals(res.status_int, httplib.FORBIDDEN) + def test_get_details_invalid_marker(self): """ Tests that the /images/detail registry API returns a 400