diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index c005546548..b0fb908165 100644 --- a/glance/api/v2/images.py +++ b/glance/api/v2/images.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import hashlib import re import glance_store @@ -45,6 +46,7 @@ CONF.import_opt('disk_formats', 'glance.common.config', group='image_format') CONF.import_opt('container_formats', 'glance.common.config', group='image_format') CONF.import_opt('show_multiple_locations', 'glance.common.config') +CONF.import_opt('hashing_algorithm', 'glance.common.config') class ImagesController(object): @@ -364,6 +366,76 @@ class ImagesController(object): except exception.NotAuthenticated as e: raise webob.exc.HTTPUnauthorized(explanation=e.msg) + def _validate_validation_data(self, image, locations): + val_data = {} + for loc in locations: + if 'validation_data' not in loc: + continue + for k, v in loc['validation_data'].items(): + if val_data.get(k, v) != v: + msg = _("Conflicting values for %s") % k + raise webob.exc.HTTPConflict(explanation=msg) + val_data[k] = v + + # NOTE(imacdonn): values may be provided for items which are + # already set, so long as the values exactly match. In this + # case, nothing actually needs to be updated, but we should + # reject the request if there's an apparent attempt to supply + # a different value. + new_val_data = {} + for k, v in val_data.items(): + current = getattr(image, k) + if v == current: + continue + if current: + msg = _("%s is already set with a different value") % k + raise webob.exc.HTTPConflict(explanation=msg) + new_val_data[k] = v + + if not new_val_data: + return {} + + if image.status != 'queued': + msg = _("New value(s) for %s may only be provided when image " + "status is 'queued'") % ', '.join(new_val_data.keys()) + raise webob.exc.HTTPConflict(explanation=msg) + + if 'checksum' in new_val_data: + try: + checksum_bytes = bytearray.fromhex(new_val_data['checksum']) + except ValueError: + msg = (_("checksum (%s) is not a valid hexadecimal value") % + new_val_data['checksum']) + raise webob.exc.HTTPConflict(explanation=msg) + if len(checksum_bytes) != 16: + msg = (_("checksum (%s) is not the correct size for md5 " + "(should be 16 bytes)") % + new_val_data['checksum']) + raise webob.exc.HTTPConflict(explanation=msg) + + hash_algo = new_val_data.get('os_hash_algo') + if hash_algo != CONF['hashing_algorithm']: + msg = (_("os_hash_algo must be %(want)s, not %(got)s") % + {'want': CONF['hashing_algorithm'], 'got': hash_algo}) + raise webob.exc.HTTPConflict(explanation=msg) + + try: + hash_bytes = bytearray.fromhex(new_val_data['os_hash_value']) + except ValueError: + msg = (_("os_hash_value (%s) is not a valid hexadecimal value") % + new_val_data['os_hash_value']) + raise webob.exc.HTTPConflict(explanation=msg) + want_size = hashlib.new(hash_algo).digest_size + if len(hash_bytes) != want_size: + msg = (_("os_hash_value (%(value)s) is not the correct size for " + "%(algo)s (should be %(want)d bytes)") % + {'value': new_val_data['os_hash_value'], + 'algo': hash_algo, + 'want': want_size}) + raise webob.exc.HTTPConflict(explanation=msg) + + return new_val_data + def _get_locations_op_pos(self, path_pos, max_pos, allow_max): if path_pos is None or max_pos is None: return None @@ -387,11 +459,15 @@ class ImagesController(object): "%s.") % image.status raise webob.exc.HTTPConflict(explanation=msg) + val_data = self._validate_validation_data(image, value) + try: # NOTE(flwang): _locations_proxy's setattr method will check if # the update is acceptable. image.locations = value if image.status == 'queued': + for k, v in val_data.items(): + setattr(image, k, v) image.status = 'active' except (exception.BadStoreUri, exception.DuplicateLocation) as e: raise webob.exc.HTTPBadRequest(explanation=e.msg) @@ -410,6 +486,8 @@ class ImagesController(object): "%s.") % image.status raise webob.exc.HTTPConflict(explanation=msg) + val_data = self._validate_validation_data(image, [value]) + pos = self._get_locations_op_pos(path_pos, len(image.locations), True) if pos is None: @@ -418,6 +496,8 @@ class ImagesController(object): try: image.locations.insert(pos, value) if image.status == 'queued': + for k, v in val_data.items(): + setattr(image, k, v) image.status = 'active' except (exception.BadStoreUri, exception.DuplicateLocation) as e: raise webob.exc.HTTPBadRequest(explanation=e.msg) @@ -1164,6 +1244,36 @@ def get_base_properties(): 'metadata': { 'type': 'object', }, + 'validation_data': { + 'description': _( + 'Values to be used to populate the corresponding ' + 'image properties. If the image status is not ' + '\'queued\', values must exactly match those ' + 'already contained in the image properties.' + ), + 'type': 'object', + 'writeOnly': True, + 'additionalProperties': False, + 'properties': { + 'checksum': { + 'type': 'string', + 'minLength': 32, + 'maxLength': 32, + }, + 'os_hash_algo': { + 'type': 'string', + 'maxLength': 64, + }, + 'os_hash_value': { + 'type': 'string', + 'maxLength': 128, + }, + }, + 'required': [ + 'os_hash_algo', + 'os_hash_value', + ], + }, }, 'required': ['url', 'metadata'], }, diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index 30379d4205..45b7951e34 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/glance/tests/unit/v2/test_images_resource.py @@ -1725,33 +1725,131 @@ class TestImagesController(base.IsolatedUnitTest): @mock.patch.object(glance.location.ImageRepoProxy, '_set_acls') @mock.patch.object(store, 'get_size_from_uri_and_backend') @mock.patch.object(store, 'get_size_from_backend') - def test_replace_location_on_queued(self, - mock_get_size, - mock_get_size_uri, - mock_set_acls, - mock_check_loc, - mock_calc): + def test_replace_locations_on_queued(self, + mock_get_size, + mock_get_size_uri, + mock_set_acls, + mock_check_loc, + mock_calc): mock_calc.return_value = 1 mock_get_size.return_value = 1 mock_get_size_uri.return_value = 1 self.config(show_multiple_locations=True) + image_id = str(uuid.uuid4()) self.images = [ - _db_fixture('1', owner=TENANT1, checksum=CHKSUM, + _db_fixture(image_id, owner=TENANT1, name='1', disk_format='raw', container_format='bare', - status='queued'), + status='queued', + checksum=None, + os_hash_algo=None, + os_hash_value=None), ] self.db.image_create(None, self.images[0]) request = unit_test_utils.get_fake_request() - new_location = {'url': '%s/fake_location_1' % BASE_URI, 'metadata': {}} + new_location1 = {'url': '%s/fake_location_1' % BASE_URI, + 'metadata': {}, + 'validation_data': {'checksum': CHKSUM, + 'os_hash_algo': 'sha512', + 'os_hash_value': MULTIHASH1}} + new_location2 = {'url': '%s/fake_location_2' % BASE_URI, + 'metadata': {}, + 'validation_data': {'checksum': CHKSUM, + 'os_hash_algo': 'sha512', + 'os_hash_value': MULTIHASH1}} + changes = [{'op': 'replace', 'path': ['locations'], + 'value': [new_location1, new_location2]}] + output = self.controller.update(request, image_id, changes) + self.assertEqual(image_id, output.image_id) + self.assertEqual(2, len(output.locations)) + self.assertEqual(new_location1['url'], output.locations[0]['url']) + self.assertEqual(new_location2['url'], output.locations[1]['url']) + self.assertEqual('active', output.status) + self.assertEqual(CHKSUM, output.checksum) + self.assertEqual('sha512', output.os_hash_algo) + self.assertEqual(MULTIHASH1, output.os_hash_value) + + @mock.patch.object(glance.quota, '_calc_required_size') + @mock.patch.object(glance.location, '_check_image_location') + @mock.patch.object(glance.location.ImageRepoProxy, '_set_acls') + @mock.patch.object(store, 'get_size_from_uri_and_backend') + @mock.patch.object(store, 'get_size_from_backend') + def test_add_location_new_validation_data_on_active(self, + mock_get_size, + mock_get_size_uri, + mock_set_acls, + mock_check_loc, + mock_calc): + mock_calc.return_value = 1 + mock_get_size.return_value = 1 + mock_get_size_uri.return_value = 1 + self.config(show_multiple_locations=True) + image_id = str(uuid.uuid4()) + self.images = [ + _db_fixture(image_id, owner=TENANT1, + name='1', + disk_format='raw', + container_format='bare', + status='active', + checksum=None, + os_hash_algo=None, + os_hash_value=None), + ] + self.db.image_create(None, self.images[0]) + request = unit_test_utils.get_fake_request() + new_location = {'url': '%s/fake_location_1' % BASE_URI, + 'metadata': {}, + 'validation_data': {'checksum': CHKSUM, + 'os_hash_algo': 'sha512', + 'os_hash_value': MULTIHASH1}} + changes = [{'op': 'add', 'path': ['locations', '-'], + 'value': new_location}] + self.assertRaisesRegexp(webob.exc.HTTPConflict, + "may only be provided when image status " + "is 'queued'", + self.controller.update, + request, image_id, changes) + + @mock.patch.object(glance.quota, '_calc_required_size') + @mock.patch.object(glance.location, '_check_image_location') + @mock.patch.object(glance.location.ImageRepoProxy, '_set_acls') + @mock.patch.object(store, 'get_size_from_uri_and_backend') + @mock.patch.object(store, 'get_size_from_backend') + def test_replace_locations_different_validation_data(self, + mock_get_size, + mock_get_size_uri, + mock_set_acls, + mock_check_loc, + mock_calc): + mock_calc.return_value = 1 + mock_get_size.return_value = 1 + mock_get_size_uri.return_value = 1 + self.config(show_multiple_locations=True) + image_id = str(uuid.uuid4()) + self.images = [ + _db_fixture(image_id, owner=TENANT1, + name='1', + disk_format='raw', + container_format='bare', + status='active', + checksum=CHKSUM, + os_hash_algo='sha512', + os_hash_value=MULTIHASH1), + ] + self.db.image_create(None, self.images[0]) + request = unit_test_utils.get_fake_request() + new_location = {'url': '%s/fake_location_1' % BASE_URI, + 'metadata': {}, + 'validation_data': {'checksum': CHKSUM1, + 'os_hash_algo': 'sha512', + 'os_hash_value': MULTIHASH2}} changes = [{'op': 'replace', 'path': ['locations'], 'value': [new_location]}] - output = self.controller.update(request, '1', changes) - self.assertEqual('1', output.image_id) - self.assertEqual(1, len(output.locations)) - self.assertEqual(new_location, output.locations[0]) - self.assertEqual('active', output.status) + self.assertRaisesRegexp(webob.exc.HTTPConflict, + "already set with a different value", + self.controller.update, + request, image_id, changes) @mock.patch.object(glance.quota, '_calc_required_size') @mock.patch.object(glance.location, '_check_image_location') @@ -1768,8 +1866,9 @@ class TestImagesController(base.IsolatedUnitTest): mock_get_size.return_value = 1 mock_get_size_uri.return_value = 1 self.config(show_multiple_locations=True) + image_id = str(uuid.uuid4()) self.images = [ - _db_fixture('1', owner=TENANT1, checksum=CHKSUM, + _db_fixture(image_id, owner=TENANT1, checksum=CHKSUM, name='1', disk_format='raw', container_format='bare', @@ -1777,15 +1876,189 @@ class TestImagesController(base.IsolatedUnitTest): ] self.db.image_create(None, self.images[0]) request = unit_test_utils.get_fake_request() - new_location = {'url': '%s/fake_location_1' % BASE_URI, 'metadata': {}} + new_location = {'url': '%s/fake_location_1' % BASE_URI, + 'metadata': {}} changes = [{'op': 'add', 'path': ['locations', '-'], 'value': new_location}] - output = self.controller.update(request, '1', changes) - self.assertEqual('1', output.image_id) + output = self.controller.update(request, image_id, changes) + self.assertEqual(image_id, output.image_id) self.assertEqual(1, len(output.locations)) self.assertEqual(new_location, output.locations[0]) self.assertEqual('active', output.status) + @mock.patch.object(glance.quota, '_calc_required_size') + @mock.patch.object(glance.location, '_check_image_location') + @mock.patch.object(glance.location.ImageRepoProxy, '_set_acls') + @mock.patch.object(store, 'get_size_from_uri_and_backend') + @mock.patch.object(store, 'get_size_from_backend') + def test_add_location_invalid_validation_data(self, + mock_get_size, + mock_get_size_uri, + mock_set_acls, + mock_check_loc, + mock_calc): + mock_calc.return_value = 1 + mock_get_size.return_value = 1 + mock_get_size_uri.return_value = 1 + self.config(show_multiple_locations=True) + image_id = str(uuid.uuid4()) + self.images = [ + _db_fixture(image_id, owner=TENANT1, + checksum=None, + os_hash_algo=None, + os_hash_value=None, + name='1', + disk_format='raw', + container_format='bare', + status='queued'), + ] + self.db.image_create(None, self.images[0]) + request = unit_test_utils.get_fake_request() + + location = { + 'url': '%s/fake_location_1' % BASE_URI, + 'metadata': {}, + 'validation_data': {} + } + changes = [{'op': 'add', 'path': ['locations', '-'], + 'value': location}] + + changes[0]['value']['validation_data'] = { + 'checksum': 'something the same length as md5', + 'os_hash_algo': 'sha512', + 'os_hash_value': MULTIHASH1, + } + self.assertRaisesRegexp(webob.exc.HTTPConflict, + 'checksum .* is not a valid hexadecimal value', + self.controller.update, + request, image_id, changes) + + changes[0]['value']['validation_data'] = { + 'checksum': '0123456789abcdef', + 'os_hash_algo': 'sha512', + 'os_hash_value': MULTIHASH1, + } + self.assertRaisesRegexp(webob.exc.HTTPConflict, + 'checksum .* is not the correct size', + self.controller.update, + request, image_id, changes) + + changes[0]['value']['validation_data'] = { + 'checksum': CHKSUM, + 'os_hash_algo': 'sha256', + 'os_hash_value': MULTIHASH1, + } + self.assertRaisesRegexp(webob.exc.HTTPConflict, + 'os_hash_algo must be sha512', + self.controller.update, + request, image_id, changes) + + changes[0]['value']['validation_data'] = { + 'checksum': CHKSUM, + 'os_hash_algo': 'sha512', + 'os_hash_value': 'not a hex value', + } + self.assertRaisesRegexp(webob.exc.HTTPConflict, + 'os_hash_value .* is not a valid hexadecimal ' + 'value', + self.controller.update, + request, image_id, changes) + + changes[0]['value']['validation_data'] = { + 'checksum': CHKSUM, + 'os_hash_algo': 'sha512', + 'os_hash_value': '0123456789abcdef', + } + self.assertRaisesRegexp(webob.exc.HTTPConflict, + 'os_hash_value .* is not the correct size ' + 'for sha512', + self.controller.update, + request, image_id, changes) + + @mock.patch.object(glance.quota, '_calc_required_size') + @mock.patch.object(glance.location, '_check_image_location') + @mock.patch.object(glance.location.ImageRepoProxy, '_set_acls') + @mock.patch.object(store, 'get_size_from_uri_and_backend') + @mock.patch.object(store, 'get_size_from_backend') + def test_add_location_same_validation_data(self, + mock_get_size, + mock_get_size_uri, + mock_set_acls, + mock_check_loc, + mock_calc): + mock_calc.return_value = 1 + mock_get_size.return_value = 1 + mock_get_size_uri.return_value = 1 + self.config(show_multiple_locations=True) + image_id = str(uuid.uuid4()) + os_hash_value = '6513f21e44aa3da349f248188a44bc304a3653a04122d8fb45' \ + '35423c8e1d14cd6a153f735bb0982e2161b5b5186106570c17' \ + 'a9e58b64dd39390617cd5a350f78' + self.images = [ + _db_fixture(image_id, owner=TENANT1, + name='1', + disk_format='raw', + container_format='bare', + status='active', + checksum='checksum1', + os_hash_algo='sha512', + os_hash_value=os_hash_value), + ] + self.db.image_create(None, self.images[0]) + request = unit_test_utils.get_fake_request() + new_location = {'url': '%s/fake_location_1' % BASE_URI, + 'metadata': {}, + 'validation_data': {'checksum': 'checksum1', + 'os_hash_algo': 'sha512', + 'os_hash_value': os_hash_value}} + changes = [{'op': 'add', 'path': ['locations', '-'], + 'value': new_location}] + output = self.controller.update(request, image_id, changes) + self.assertEqual(image_id, output.image_id) + self.assertEqual(1, len(output.locations)) + self.assertEqual(new_location, output.locations[0]) + self.assertEqual('active', output.status) + + @mock.patch.object(glance.quota, '_calc_required_size') + @mock.patch.object(glance.location, '_check_image_location') + @mock.patch.object(glance.location.ImageRepoProxy, '_set_acls') + @mock.patch.object(store, 'get_size_from_uri_and_backend') + @mock.patch.object(store, 'get_size_from_backend') + def test_add_location_different_validation_data(self, + mock_get_size, + mock_get_size_uri, + mock_set_acls, + mock_check_loc, + mock_calc): + mock_calc.return_value = 1 + mock_get_size.return_value = 1 + mock_get_size_uri.return_value = 1 + self.config(show_multiple_locations=True) + image_id = str(uuid.uuid4()) + self.images = [ + _db_fixture(image_id, owner=TENANT1, + name='1', + disk_format='raw', + container_format='bare', + status='active', + checksum=CHKSUM, + os_hash_algo='sha512', + os_hash_value=MULTIHASH1), + ] + self.db.image_create(None, self.images[0]) + request = unit_test_utils.get_fake_request() + new_location = {'url': '%s/fake_location_1' % BASE_URI, + 'metadata': {}, + 'validation_data': {'checksum': CHKSUM1, + 'os_hash_algo': 'sha512', + 'os_hash_value': MULTIHASH2}} + changes = [{'op': 'add', 'path': ['locations', '-'], + 'value': new_location}] + self.assertRaisesRegexp(webob.exc.HTTPConflict, + "already set with a different value", + self.controller.update, + request, image_id, changes) + def _test_update_locations_status(self, image_status, update): self.config(show_multiple_locations=True) self.images = [ @@ -2696,6 +2969,44 @@ class TestImagesDeserializer(test_utils.BaseTestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.deserializer.update, request) + def test_update_invalid_validation_data(self): + request = self._get_fake_patch_request() + changes = [{ + 'op': 'add', + 'path': '/locations/0', + 'value': { + 'url': 'http://localhost/fake', + 'metadata': {}, + } + }] + + changes[0]['value']['validation_data'] = { + 'os_hash_algo': 'sha512', + 'os_hash_value': MULTIHASH1, + 'checksum': CHKSUM, + } + request.body = jsonutils.dump_as_bytes(changes) + self.deserializer.update(request) + + changes[0]['value']['validation_data'] = { + 'os_hash_algo': 'sha512', + 'os_hash_value': MULTIHASH1, + 'checksum': CHKSUM, + 'bogus_key': 'bogus_value', + } + request.body = jsonutils.dump_as_bytes(changes) + self.assertRaisesRegexp(webob.exc.HTTPBadRequest, + 'Additional properties are not allowed', + self.deserializer.update, request) + + changes[0]['value']['validation_data'] = { + 'checksum': CHKSUM, + } + request.body = jsonutils.dump_as_bytes(changes) + self.assertRaisesRegexp(webob.exc.HTTPBadRequest, + 'os_hash.* is a required property', + self.deserializer.update, request) + def test_update(self): request = self._get_fake_patch_request() body = [