openstack/glance
Code Issues Proposed changes

Merge "Embed validation data in locations"

Browse Source
This commit is contained in:
Zuul 2018-11-12 06:50:23 +00:00 committed by Gerrit Code Review
commit d2e62d4397
2 changed files with 439 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

110
glance/api/v2/images.py
View File

@ -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'],
},

347
glance/tests/unit/v2/test_images_resource.py
View File

@ -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 = [