openstack
/
cinder
Code Issues Proposed changes

Merge "Validate name and description string"

This commit is contained in:
Jenkins 2015-08-14 22:11:49 +00:00 committed by Gerrit Code Review
parent b5efaeb475 a1bb185a1f
commit edd25e4688
8 changed files with 150 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cinder/api/contrib/backups.py
View File

@ -252,6 +252,7 @@ class BackupsController(wsgi.Controller):
msg = _("Incorrect request body format")
raise exc.HTTPBadRequest(explanation=msg)
container = backup.get('container', None)
self.validate_name_and_description(backup)
name = backup.get('name', None)
description = backup.get('description', None)
incremental = backup.get('incremental', False)

20
cinder/api/openstack/wsgi.py
View File

@ -1220,6 +1220,26 @@ class Controller(object):
explanation=_("Missing required element '%s' in "
"request body.") % entity_name)
@staticmethod
def validate_name_and_description(body):
name = body.get('name')
if name is not None:
if isinstance(name, six.string_types):
body['name'] = name.strip()
try:
utils.check_string_length(body['name'], 'Name',
min_length=1, max_length=255)
except exception.InvalidInput as error:
raise webob.exc.HTTPBadRequest(explanation=error.msg)
description = body.get('description')
if description is not None:
try:
utils.check_string_length(description, 'Description',
min_length=0, max_length=255)
except exception.InvalidInput as error:
raise webob.exc.HTTPBadRequest(explanation=error.msg)
class Fault(webob.exc.HTTPException):
"""Wrap webob.exc.HTTPException to provide API friendly response."""

11
cinder/api/v2/snapshots.py
View File

@ -185,11 +185,11 @@ class SnapshotsController(wsgi.Controller):
force = snapshot.get('force', False)
msg = _LI("Create snapshot from volume %s")
LOG.info(msg, volume_id, context=context)
self.validate_name_and_description(snapshot)
# NOTE(thingee): v2 API allows name instead of display_name
if 'name' in snapshot:
snapshot['display_name'] = snapshot.get('name')
del snapshot['name']
snapshot['display_name'] = snapshot.pop('name')
try:
force = strutils.bool_from_string(force, strict=True)
@ -240,17 +240,16 @@ class SnapshotsController(wsgi.Controller):
'display_name',
'display_description',
)
self.validate_name_and_description(snapshot)
# NOTE(thingee): v2 API allows name instead of display_name
if 'name' in snapshot:
snapshot['display_name'] = snapshot['name']
del snapshot['name']
snapshot['display_name'] = snapshot.pop('name')
# NOTE(thingee): v2 API allows description instead of
# display_description
if 'description' in snapshot:
snapshot['display_description'] = snapshot['description']
del snapshot['description']
snapshot['display_description'] = snapshot.pop('description')
for key in valid_update_keys:
if key in snapshot:

24
cinder/api/v2/volumes.py
View File

@ -326,17 +326,16 @@ class VolumeController(wsgi.Controller):
volume = body['volume']
kwargs = {}
self.validate_name_and_description(volume)
# NOTE(thingee): v2 API allows name instead of display_name
if volume.get('name'):
volume['display_name'] = volume.get('name')
del volume['name']
if 'name' in volume:
volume['display_name'] = volume.pop('name')
# NOTE(thingee): v2 API allows description instead of
# display_description
if volume.get('description'):
volume['display_description'] = volume.get('description')
del volume['description']
if 'description' in volume:
volume['display_description'] = volume.pop('description')
if 'image_id' in volume:
volume['imageRef'] = volume.get('image_id')
@ -471,15 +470,16 @@ class VolumeController(wsgi.Controller):
if key in volume:
update_dict[key] = volume[key]
# NOTE(thingee): v2 API allows name instead of display_name
if 'name' in update_dict:
update_dict['display_name'] = update_dict['name']
del update_dict['name']
self.validate_name_and_description(update_dict)
# NOTE(thingee): v2 API allows name instead of display_name
if 'name' in update_dict:
update_dict['display_name'] = update_dict.pop('name')
# NOTE(thingee): v2 API allows description instead of
# display_description
if 'description' in update_dict:
update_dict['display_description'] = update_dict['description']
del update_dict['description']
update_dict['display_description'] = update_dict.pop('description')
try:
volume = self.volume_api.get(context, id, viewable_admin_meta=True)

24
cinder/tests/unit/api/contrib/test_backups.py
View File

@ -378,7 +378,10 @@ class BackupsAPITestCase(test.TestCase):
db.backup_destroy(context.get_admin_context(), backup_id1)
@mock.patch('cinder.db.service_get_all_by_topic')
def test_create_backup_json(self, _mock_service_get_all_by_topic):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_create_backup_json(self, mock_validate,
_mock_service_get_all_by_topic):
_mock_service_get_all_by_topic.return_value = [
{'availability_zone': "fake_az", 'host': 'test_host',
'disabled': 0, 'updated_at': timeutils.utcnow()}]
@ -403,6 +406,7 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(202, res.status_int)
self.assertIn('id', res_dict['backup'])
self.assertTrue(_mock_service_get_all_by_topic.called)
self.assertTrue(mock_validate.called)
db.volume_destroy(context.get_admin_context(), volume_id)
@ -470,7 +474,10 @@ class BackupsAPITestCase(test.TestCase):
db.volume_destroy(context.get_admin_context(), volume_id)
@mock.patch('cinder.db.service_get_all_by_topic')
def test_create_backup_snapshot_json(self, _mock_service_get_all_by_topic):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_create_backup_snapshot_json(self, mock_validate,
_mock_service_get_all_by_topic):
_mock_service_get_all_by_topic.return_value = [
{'availability_zone': "fake_az", 'host': 'test_host',
'disabled': 0, 'updated_at': timeutils.utcnow()}]
@ -495,11 +502,15 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(202, res.status_int)
self.assertIn('id', res_dict['backup'])
self.assertTrue(_mock_service_get_all_by_topic.called)
self.assertTrue(mock_validate.called)
db.volume_destroy(context.get_admin_context(), volume_id)
@mock.patch('cinder.db.service_get_all_by_topic')
def test_create_backup_xml(self, _mock_service_get_all_by_topic):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_create_backup_xml(self, mock_validate,
_mock_service_get_all_by_topic):
_mock_service_get_all_by_topic.return_value = [
{'availability_zone': "fake_az", 'host': 'test_host',
'disabled': 0, 'updated_at': timeutils.utcnow()}]
@ -520,11 +531,15 @@ class BackupsAPITestCase(test.TestCase):
backup = dom.getElementsByTagName('backup')
self.assertTrue(backup.item(0).hasAttribute('id'))
self.assertTrue(_mock_service_get_all_by_topic.called)
self.assertTrue(mock_validate.called)
db.volume_destroy(context.get_admin_context(), volume_id)
@mock.patch('cinder.db.service_get_all_by_topic')
def test_create_backup_delta(self, _mock_service_get_all_by_topic):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_create_backup_delta(self, mock_validate,
_mock_service_get_all_by_topic):
_mock_service_get_all_by_topic.return_value = [
{'availability_zone': "fake_az", 'host': 'test_host',
'disabled': 0, 'updated_at': timeutils.utcnow()}]
@ -550,6 +565,7 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(202, res.status_int)
self.assertIn('id', res_dict['backup'])
self.assertTrue(_mock_service_get_all_by_topic.called)
self.assertTrue(mock_validate.called)
db.backup_destroy(context.get_admin_context(), backup_id)
db.volume_destroy(context.get_admin_context(), volume_id)

41
cinder/tests/unit/api/openstack/test_wsgi.py
View File

@ -982,3 +982,44 @@ class ValidBodyTest(test.TestCase):
wsgi.Resource(controller=None)
body = {'foo': 'bar'}
self.assertFalse(self.controller.is_valid_body(body, 'foo'))
def test_validate_name_and_description_with_name_too_long(self):
body = {'name': 'a' * 256}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.validate_name_and_description,
body)
def test_validate_name_and_description_with_desc_too_long(self):
body = {'description': 'a' * 256}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.validate_name_and_description,
body)
def test_validate_name_and_description_with_name_as_int(self):
body = {'name': 1234}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.validate_name_and_description,
body)
def test_validate_name_and_description_with_desc_as_int(self):
body = {'description': 1234}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.validate_name_and_description,
body)
def test_validate_name_and_description_with_name_zero_length(self):
body = {'name': ""}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.validate_name_and_description,
body)
def test_validate_name_and_description_with_desc_zero_length(self):
body = {'description': ""}
self.controller.validate_name_and_description(body)
self.assertEqual('', body['description'])
def test_validate_name_and_description_with_name_contains_whitespaces(
self):
body = {'name': 'a' * 255 + " "}
self.controller.validate_name_and_description(body)
self.assertEqual('a' * 255, body['name'])

16
cinder/tests/unit/api/v2/test_snapshots.py
View File

@ -90,7 +90,9 @@ class SnapshotApiTest(test.TestCase):
self.stubs.Set(db, 'snapshot_get_all',
stubs.stub_snapshot_get_all)
def test_snapshot_create(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_snapshot_create(self, mock_validate):
self.stubs.Set(volume.api.API, "create_snapshot", stub_snapshot_create)
self.stubs.Set(volume.api.API, 'get', stubs.stub_volume_get)
snapshot_name = 'Snapshot Test Name'
@ -107,10 +109,10 @@ class SnapshotApiTest(test.TestCase):
resp_dict = self.controller.create(req, body)
self.assertIn('snapshot', resp_dict)
self.assertEqual(snapshot_name,
resp_dict['snapshot']['name'])
self.assertEqual(snapshot_name, resp_dict['snapshot']['name'])
self.assertEqual(snapshot_description,
resp_dict['snapshot']['description'])
self.assertTrue(mock_validate.called)
def test_snapshot_create_force(self):
self.stubs.Set(volume.api.API, "create_snapshot_force",
@ -166,8 +168,11 @@ class SnapshotApiTest(test.TestCase):
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
@mock.patch('cinder.objects.Volume.get_by_id')
@mock.patch('cinder.objects.Snapshot.get_by_id')
def test_snapshot_update(self, snapshot_get_by_id, volume_get_by_id,
snapshot_metadata_get, update_snapshot):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_snapshot_update(
self, mock_validate, snapshot_get_by_id, volume_get_by_id,
snapshot_metadata_get, update_snapshot):
snapshot = {
'id': UUID,
'volume_id': 1,
@ -202,6 +207,7 @@ class SnapshotApiTest(test.TestCase):
}
}
self.assertEqual(expected, res_dict)
self.assertTrue(mock_validate.called)
self.assertEqual(2, len(self.notifier.notifications))
def test_snapshot_update_missing_body(self):

50
cinder/tests/unit/api/v2/test_volumes.py
View File

@ -61,7 +61,9 @@ class VolumeApiTest(test.TestCase):
stubs.stub_service_get_all_by_topic)
self.maxDiff = None
def test_volume_create(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_create(self, mock_validate):
self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
@ -71,8 +73,11 @@ class VolumeApiTest(test.TestCase):
res_dict = self.controller.create(req, body)
ex = self._expected_vol_from_controller()
self.assertEqual(ex, res_dict)
self.assertTrue(mock_validate.called)
def test_volume_create_with_type(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_create_with_type(self, mock_validate):
vol_type = db.volume_type_create(
context.get_admin_context(),
dict(name=CONF.default_volume_type, extra_specs={})
@ -108,6 +113,7 @@ class VolumeApiTest(test.TestCase):
volume_type={'name': vol_type})])
req = fakes.HTTPRequest.blank('/v2/volumes/detail')
res_dict = self.controller.detail(req)
self.assertTrue(mock_validate.called)
def _vol_in_request_body(self,
size=stubs.DEFAULT_VOL_SIZE,
@ -351,7 +357,9 @@ class VolumeApiTest(test.TestCase):
self.controller.create,
req, body)
def test_volume_create_with_image_ref(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_create_with_image_ref(self, mock_validate):
self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
@ -364,6 +372,7 @@ class VolumeApiTest(test.TestCase):
req = fakes.HTTPRequest.blank('/v2/volumes')
res_dict = self.controller.create(req, body)
self.assertEqual(ex, res_dict)
self.assertTrue(mock_validate.called)
def test_volume_create_with_image_ref_is_integer(self):
self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
@ -401,7 +410,9 @@ class VolumeApiTest(test.TestCase):
req,
body)
def test_volume_create_with_image_id(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_create_with_image_id(self, mock_validate):
self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
@ -414,6 +425,7 @@ class VolumeApiTest(test.TestCase):
req = fakes.HTTPRequest.blank('/v2/volumes')
res_dict = self.controller.create(req, body)
self.assertEqual(ex, res_dict)
self.assertTrue(mock_validate.called)
def test_volume_create_with_image_id_is_integer(self):
self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
@ -451,7 +463,9 @@ class VolumeApiTest(test.TestCase):
req,
body)
def test_volume_create_with_image_name(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_create_with_image_name(self, mock_validate):
self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
self.stubs.Set(fake_image._FakeImageService,
@ -467,6 +481,7 @@ class VolumeApiTest(test.TestCase):
req = fakes.HTTPRequest.blank('/v2/volumes')
res_dict = self.controller.create(req, body)
self.assertEqual(ex, res_dict)
self.assertTrue(mock_validate.called)
def test_volume_create_with_image_name_has_multiple(self):
self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
@ -504,7 +519,9 @@ class VolumeApiTest(test.TestCase):
req,
body)
def test_volume_update(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_update(self, mock_validate):
self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
@ -520,8 +537,11 @@ class VolumeApiTest(test.TestCase):
metadata={'attached_mode': 'rw', 'readonly': 'False'})
self.assertEqual(expected, res_dict)
self.assertEqual(2, len(self.notifier.notifications))
self.assertTrue(mock_validate.called)
def test_volume_update_deprecation(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_update_deprecation(self, mock_validate):
self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
@ -539,8 +559,11 @@ class VolumeApiTest(test.TestCase):
metadata={'attached_mode': 'rw', 'readonly': 'False'})
self.assertEqual(expected, res_dict)
self.assertEqual(2, len(self.notifier.notifications))
self.assertTrue(mock_validate.called)
def test_volume_update_deprecation_key_priority(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_update_deprecation_key_priority(self, mock_validate):
"""Test current update keys have priority over deprecated keys."""
self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
@ -561,8 +584,11 @@ class VolumeApiTest(test.TestCase):
metadata={'attached_mode': 'rw', 'readonly': 'False'})
self.assertEqual(expected, res_dict)
self.assertEqual(2, len(self.notifier.notifications))
self.assertTrue(mock_validate.called)
def test_volume_update_metadata(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_update_metadata(self, mock_validate):
self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
@ -579,8 +605,11 @@ class VolumeApiTest(test.TestCase):
'qos_max_iops': 2000})
self.assertEqual(expected, res_dict)
self.assertEqual(2, len(self.notifier.notifications))
self.assertTrue(mock_validate.called)
def test_volume_update_with_admin_metadata(self):
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_volume_update_with_admin_metadata(self, mock_validate):
self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
volume = stubs.stub_volume("1")
@ -621,6 +650,7 @@ class VolumeApiTest(test.TestCase):
metadata={'key': 'value', 'readonly': 'True'})
self.assertEqual(expected, res_dict)
self.assertEqual(2, len(self.notifier.notifications))
self.assertTrue(mock_validate.called)
def test_update_empty_body(self):
body = {}