openstack/nova
Code Issues Proposed changes

Merge "Add basic BDM format validation in the API layer"

Browse Source
This commit is contained in:
Jenkins
2013-07-27 14:57:55 +00:00
committed by Gerrit Code Review
parent 7f45bacc53 e9fd70332e
commit c9c7fff15d
5 changed files with 177 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
nova/api/openstack/compute/servers.py
View File

@@ -602,21 +602,6 @@ class Controller(wsgi.Controller):
def _validate_server_name(self, value):
self._check_string_length(value, 'Server name', max_length=255)
def _validate_block_device(self, bd):
self._check_string_length(bd['device_name'],
'Device name', max_length=255)
if ' ' in bd['device_name']:
msg = _("Device name cannot include spaces.")
raise exc.HTTPBadRequest(explanation=msg)
if 'volume_size' in bd:
try:
bd['volume_size'] = utils.validate_integer(
bd['volume_size'], 'volume_size', min_value=0)
except exception.InvalidInput as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
def _get_injected_files(self, personality):
"""Create a list of injected files from the personality attribute.
@@ -858,10 +843,12 @@ class Controller(wsgi.Controller):
if self.ext_mgr.is_loaded('os-volumes'):
block_device_mapping = server_dict.get('block_device_mapping', [])
for bdm in block_device_mapping:
# Ignore empty volume size
if 'volume_size' in bdm and not bdm['volume_size']:
del bdm['volume_size']
self._validate_block_device(bdm)
try:
block_device.validate_device_name(bdm.get("device_name"))
block_device.validate_and_default_volume_size(bdm)
except exception.InvalidBDMFormat as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
if 'delete_on_termination' in bdm:
bdm['delete_on_termination'] = strutils.bool_from_string(
bdm['delete_on_termination'])
@@ -880,10 +867,20 @@ class Controller(wsgi.Controller):
# Assume legacy format
legacy_bdm = not bool(block_device_mapping_v2)
# This will also do basic validation of block_devices
block_device_mapping_v2 = [
block_device.BlockDeviceDict.from_api(bdm_dict)
for bdm_dict in block_device_mapping_v2]
# NOTE (ndipanov) Don't allow empty device_name values
# for now until we can handle it on the
# compute side.
if any('device_name' not in bdm
for bdm in block_device_mapping_v2):
expl = _('Missing device_name in some block devices.')
raise exc.HTTPBadRequest(explanation=expl)
try:
block_device_mapping_v2 = [
block_device.BlockDeviceDict.from_api(bdm_dict)
for bdm_dict in block_device_mapping_v2]
except exception.InvalidBDMFormat as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
block_device_mapping = (block_device_mapping or
block_device_mapping_v2)

74
nova/block_device.py
View File

@@ -22,6 +22,8 @@ from oslo.config import cfg
from nova import exception
from nova.openstack.common.gettextutils import _
from nova.openstack.common import log as logging
from nova.openstack.common import strutils
from nova import utils
from nova.virt import driver
CONF = cfg.CONF
@@ -73,6 +75,8 @@ class BlockDeviceDict(dict):
_db_only_fields = (bdm_db_only_fields |
bdm_db_inherited_fields)
_required_fields = set(['source_type'])
def __init__(self, bdm_dict=None, do_not_default=None):
super(BlockDeviceDict, self).__init__()
@@ -88,10 +92,39 @@ class BlockDeviceDict(dict):
def _validate(self, bdm_dict):
"""Basic data format validations."""
if (not set(key for key, _ in bdm_dict.iteritems()) <=
dict_fields = set(key for key, _ in bdm_dict.iteritems())
# Check that there are no bogus fields
if not (dict_fields <=
(self._fields | self._db_only_fields)):
raise exception.InvalidBDMFormat()
# TODO(ndipanov): Validate must-have fields!
raise exception.InvalidBDMFormat(
details="Some fields are invalid.")
if bdm_dict.get('no_device'):
return
# Check that all required fields are there
if (self._required_fields and
not ((dict_fields & self._required_fields) ==
self._required_fields)):
raise exception.InvalidBDMFormat(
details="Some required fields are missing")
if 'delete_on_termination' in bdm_dict:
bdm_dict['delete_on_termination'] = strutils.bool_from_string(
bdm_dict['delete_on_termination'])
if bdm_dict.get('device_name') is not None:
validate_device_name(bdm_dict['device_name'])
validate_and_default_volume_size(bdm_dict)
if bdm_dict.get('boot_index'):
try:
bdm_dict['boot_index'] = int(bdm_dict['boot_index'])
except ValueError:
raise exception.InvalidBDMFormat(
details="Boot index is invalid.")
@classmethod
def from_legacy(cls, legacy_bdm):
@@ -134,7 +167,8 @@ class BlockDeviceDict(dict):
pass
else:
raise exception.InvalidBDMFormat()
raise exception.InvalidBDMFormat(
details="Unrecognized legacy format.")
return cls(new_bdm, non_computable_fields)
@@ -150,10 +184,12 @@ class BlockDeviceDict(dict):
device_uuid = api_dict.get('uuid')
if source_type not in ('volume', 'image', 'snapshot', 'blank'):
raise exception.InvalidBDMFormat()
raise exception.InvalidBDMFormat(
details="Invalid source_type field.")
elif source_type != 'blank':
if not device_uuid:
raise exception.InvalidBDMFormat()
raise exception.InvalidBDMFormat(
details="Missing device UUID.")
api_dict[source_type + '_id'] = device_uuid
api_dict.pop('uuid', None)
@@ -257,6 +293,32 @@ def properties_root_device_name(properties):
return root_device_name
def validate_device_name(value):
try:
# NOTE (ndipanov): Do not allow empty device names
# until assigning default values
# is supported by nova.compute
utils.check_string_length(value, 'Device name',
min_length=1, max_length=255)
except exception.InvalidInput as e:
raise exception.InvalidBDMFormat(
details="Device name empty or too long.")
if ' ' in value:
raise exception.InvalidBDMFormat(
details="Device name contains spaces.")
def validate_and_default_volume_size(bdm):
if bdm.get('volume_size'):
try:
bdm['volume_size'] = utils.validate_integer(
bdm['volume_size'], 'volume_size', min_value=0)
except exception.InvalidInput as e:
raise exception.InvalidBDMFormat(
details="Invalid volume_size.")
_ephemeral = re.compile('^ephemeral(\d|[1-9]\d+)$')

3
nova/exception.py
View File

@@ -231,8 +231,7 @@ class InvalidBDMVolume(InvalidBDM):
class InvalidBDMFormat(InvalidBDM):
msg_fmt = _("Block Device Mapping is Invalid: "
"some fields are not recognized, "
"or have invalid values.")
"%(details)s")
class InvalidBDMForLegacy(InvalidBDM):

38
nova/tests/api/openstack/compute/test_servers.py
View File

@@ -2668,8 +2668,10 @@ class ServersControllerCreateTest(test.TestCase):
self.ext_mgr.extensions = {'os-volumes': 'fake',
'os-block-device-mapping-v2-boot': 'fake'}
bdm_v2 = [{'source_type': 'volume',
'device_name': 'fake_dev',
'uuid': 'fake_vol'}]
bdm_v2_expected = [{'source_type': 'volume',
'device_name': 'fake_dev',
'volume_id': 'fake_vol'}]
params = {'block_device_mapping_v2': bdm_v2}
old_create = compute_api.API.create
@@ -2754,6 +2756,42 @@ class ServersControllerCreateTest(test.TestCase):
self.assertRaises(webob.exc.HTTPBadRequest,
self._test_create_extra, params)
def test_create_instance_bdm_v2_missing_device_name(self):
self.ext_mgr.extensions = {'os-volumes': 'fake',
'os-block-device-mapping-v2-boot': 'fake'}
bdm_v2 = [{'source_type': 'volume',
'uuid': 'fake_vol'}]
params = {'block_device_mapping_v2': bdm_v2}
def _validate(*args, **kwargs):
pass
def _validate_bdm(*args, **kwargs):
pass
self.stubs.Set(block_device.BlockDeviceDict,
'_validate', _validate)
self.stubs.Set(compute_api.API, '_validate_bdm',
_validate_bdm)
self.assertRaises(webob.exc.HTTPBadRequest,
self._test_create_extra, params)
def test_create_instance_bdm_v2_validation_error(self):
self.ext_mgr.extensions = {'os-volumes': 'fake',
'os-block-device-mapping-v2-boot': 'fake'}
bdm_v2 = [{'device_name': 'bogus device'}]
params = {'block_device_mapping_v2': bdm_v2}
def _validate(*args, **kwargs):
raise exception.InvalidBDMFormat()
self.stubs.Set(block_device.BlockDeviceDict,
'_validate', _validate)
self.assertRaises(webob.exc.HTTPBadRequest,
self._test_create_extra, params)
def test_create_instance_with_user_data_enabled(self):
self.ext_mgr.extensions = {'os-user-data': 'fake'}
user_data = 'fake'

54
nova/tests/test_block_device.py
View File

@@ -225,10 +225,15 @@ class TestBlockDeviceDict(test.TestCase):
]
def test_init(self):
def fake_validate(obj, dct):
pass
self.stubs.Set(block_device.BlockDeviceDict, '_fields',
set(['field1', 'field2']))
self.stubs.Set(block_device.BlockDeviceDict, '_db_only_fields',
set(['db_field1', 'db_field2']))
self.stubs.Set(block_device.BlockDeviceDict, '_validate',
fake_validate)
# Make sure db fields are not picked up if they are not
# in the original dict
@@ -256,12 +261,53 @@ class TestBlockDeviceDict(test.TestCase):
self.assertFalse('db_field1' in dev_dict)
self.assertFalse('db_field2'in dev_dict)
# Assert basic validation works
# NOTE (ndipanov): Move to separate test once we have
# more complex validations in place
def test_validate(self):
self.assertRaises(exception.InvalidBDMFormat,
block_device.BlockDeviceDict,
{'field1': 'foo', 'bogus_field': 'lame_val'})
{'bogus_field': 'lame_val'})
lame_bdm = dict(self.new_mapping[2])
del lame_bdm['source_type']
self.assertRaises(exception.InvalidBDMFormat,
block_device.BlockDeviceDict,
lame_bdm)
lame_bdm['no_device'] = True
block_device.BlockDeviceDict(lame_bdm)
lame_dev_bdm = dict(self.new_mapping[2])
lame_dev_bdm['device_name'] = "not a valid name"
self.assertRaises(exception.InvalidBDMFormat,
block_device.BlockDeviceDict,
lame_dev_bdm)
lame_dev_bdm['device_name'] = ""
self.assertRaises(exception.InvalidBDMFormat,
block_device.BlockDeviceDict,
lame_dev_bdm)
cool_volume_size_bdm = dict(self.new_mapping[2])
cool_volume_size_bdm['volume_size'] = '42'
cool_volume_size_bdm = block_device.BlockDeviceDict(
cool_volume_size_bdm)
self.assertEquals(cool_volume_size_bdm['volume_size'], 42)
lame_volume_size_bdm = dict(self.new_mapping[2])
lame_volume_size_bdm['volume_size'] = 'some_non_int_string'
self.assertRaises(exception.InvalidBDMFormat,
block_device.BlockDeviceDict,
lame_volume_size_bdm)
truthy_bdm = dict(self.new_mapping[2])
truthy_bdm['delete_on_termination'] = '1'
truthy_bdm = block_device.BlockDeviceDict(truthy_bdm)
self.assertEquals(truthy_bdm['delete_on_termination'], True)
verbose_bdm = dict(self.new_mapping[2])
verbose_bdm['boot_index'] = 'first'
self.assertRaises(exception.InvalidBDMFormat,
block_device.BlockDeviceDict,
verbose_bdm)
def test_from_legacy(self):
for legacy, new in zip(self.legacy_mapping, self.new_mapping):