Embed validation data in locations

Allow embedding of checksum, os_hash_algo and os_hash_value in a
special writeOnly json-patch object when updating locations for an image.

Depends-On: https://review.openstack.org/597648
Change-Id: I1308fb1dec21002a9777bd0c77e9c02e59527551

2 files changed, 439 insertions, 18 deletions

diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py
index c00554654..b0fb90816 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 30379d420..45b7951e3 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_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])
+                    '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]}]
+        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,47 @@ 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',
+                        status='queued'),
+        ]
+        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': {}}
+        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_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',
@@ -1777,15 +1914,151 @@ 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': {}}
+
+        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, '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_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 = [