Merge "Validate outermost request body element name consistently"

cinder/api/contrib/backups.py

@@ -240,13 +240,12 @@ class BackupsController(wsgi.Controller):
     def create(self, req, body):
         """Create a new backup."""
         LOG.debug('Creating new backup %s', body)
-        if not self.is_valid_body(body, 'backup'):
+        self.assert_valid_body(body, 'backup')
-            raise exc.HTTPBadRequest()
 
         context = req.environ['cinder.context']
+        backup = body['backup']
 
         try:
-            backup = body['backup']
             volume_id = backup['volume_id']
         except KeyError:
             msg = _("Incorrect request body format")
@@ -282,9 +281,7 @@ class BackupsController(wsgi.Controller):
         """Restore an existing backup to a volume."""
         LOG.debug('Restoring backup %(backup_id)s (%(body)s)',
                   {'backup_id': id, 'body': body})
-        if not self.is_valid_body(body, 'restore'):
+        self.assert_valid_body(body, 'restore')
-            msg = _("Incorrect request body format")
-            raise exc.HTTPBadRequest(explanation=msg)
 
         context = req.environ['cinder.context']
         restore = body['restore']
@@ -344,9 +341,7 @@ class BackupsController(wsgi.Controller):
     def import_record(self, req, body):
         """Import a backup."""
         LOG.debug('Importing record from %s.', body)
-        if not self.is_valid_body(body, 'backup-record'):
+        self.assert_valid_body(body, 'backup-record')
-            msg = _("Incorrect request body format.")
-            raise exc.HTTPBadRequest(explanation=msg)
         context = req.environ['cinder.context']
         import_data = body['backup-record']
         # Verify that body elements are provided

cinder/api/contrib/cgsnapshots.py

@@ -155,16 +155,10 @@ class CgsnapshotsController(wsgi.Controller):
     def create(self, req, body):
         """Create a new cgsnapshot."""
         LOG.debug('Creating new cgsnapshot %s', body)
-        if not self.is_valid_body(body, 'cgsnapshot'):
+        self.assert_valid_body(body, 'cgsnapshot')
-            raise exc.HTTPBadRequest()
 
         context = req.environ['cinder.context']
-
-        try:
         cgsnapshot = body['cgsnapshot']
-        except KeyError:
-            msg = _("Incorrect request body format")
-            raise exc.HTTPBadRequest(explanation=msg)
 
         try:
             group_id = cgsnapshot['consistencygroup_id']

cinder/api/contrib/consistencygroups.py

@@ -211,16 +211,10 @@ class ConsistencyGroupsController(wsgi.Controller):
     def create(self, req, body):
         """Create a new consistency group."""
         LOG.debug('Creating new consistency group %s', body)
-        if not self.is_valid_body(body, 'consistencygroup'):
+        self.assert_valid_body(body, 'consistencygroup')
-            raise exc.HTTPBadRequest()
 
         context = req.environ['cinder.context']
-
-        try:
         consistencygroup = body['consistencygroup']
-        except KeyError:
-            msg = _("Incorrect request body format")
-            raise exc.HTTPBadRequest(explanation=msg)
         name = consistencygroup.get('name', None)
         description = consistencygroup.get('description', None)
         volume_types = consistencygroup.get('volume_types', None)
@@ -262,16 +256,10 @@ class ConsistencyGroupsController(wsgi.Controller):
         API above.
         """
         LOG.debug('Creating new consistency group %s.', body)
-        if not self.is_valid_body(body, 'consistencygroup-from-src'):
+        self.assert_valid_body(body, 'consistencygroup-from-src')
-            raise exc.HTTPBadRequest()
 
         context = req.environ['cinder.context']
-
-        try:
         consistencygroup = body['consistencygroup-from-src']
-        except KeyError:
-            msg = _("Incorrect request body format.")
-            raise exc.HTTPBadRequest(explanation=msg)
         name = consistencygroup.get('name', None)
         description = consistencygroup.get('description', None)
         cgsnapshot_id = consistencygroup.get('cgsnapshot_id', None)
@@ -322,9 +310,8 @@ class ConsistencyGroupsController(wsgi.Controller):
         if not body:
             msg = _("Missing request body.")
             raise exc.HTTPBadRequest(explanation=msg)
-        if not self.is_valid_body(body, 'consistencygroup'):
+
-            msg = _("Incorrect request body format.")
+        self.assert_valid_body(body, 'consistencygroup')
-            raise exc.HTTPBadRequest(explanation=msg)
         context = req.environ['cinder.context']
 
         consistencygroup = body.get('consistencygroup', None)

cinder/api/contrib/qos_specs_manage.py

@@ -123,8 +123,7 @@ class QoSSpecsController(wsgi.Controller):
         context = req.environ['cinder.context']
         authorize(context)
 
-        if not self.is_valid_body(body, 'qos_specs'):
+        self.assert_valid_body(body, 'qos_specs')
-            raise webob.exc.HTTPBadRequest()
 
         specs = body['qos_specs']
         name = specs.get('name', None)
@@ -166,8 +165,7 @@ class QoSSpecsController(wsgi.Controller):
         context = req.environ['cinder.context']
         authorize(context)
 
-        if not self.is_valid_body(body, 'qos_specs'):
+        self.assert_valid_body(body, 'qos_specs')
-            raise webob.exc.HTTPBadRequest()
         specs = body['qos_specs']
         try:
             qos_specs.update(context, id, specs)

cinder/api/contrib/quotas.py

@@ -107,9 +107,7 @@ class QuotaSetsController(wsgi.Controller):
         context = req.environ['cinder.context']
         authorize_update(context)
         project_id = id
-        if not self.is_valid_body(body, 'quota_set'):
+        self.assert_valid_body(body, 'quota_set')
-            msg = (_("Missing required element quota_set in request body."))
-            raise webob.exc.HTTPBadRequest(explanation=msg)
 
         bad_keys = []
 

cinder/api/contrib/services.py

@@ -161,7 +161,8 @@ class ServiceController(wsgi.Controller):
         try:
             host = body['host']
         except (TypeError, KeyError):
-            raise webob.exc.HTTPBadRequest()
+            msg = _("Missing required element 'host' in request body.")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
 
         ret_val['disabled'] = disabled
         if id == "disable-log-reason" and ext_loaded:

cinder/api/contrib/types_extra_specs.py

@@ -79,8 +79,7 @@ class VolumeTypeExtraSpecsController(wsgi.Controller):
         context = req.environ['cinder.context']
         authorize(context)
 
-        if not self.is_valid_body(body, 'extra_specs'):
+        self.assert_valid_body(body, 'extra_specs')
-            raise webob.exc.HTTPBadRequest()
 
         self._check_type(context, type_id)
         specs = body['extra_specs']

cinder/api/contrib/types_manage.py

@@ -54,8 +54,7 @@ class VolumeTypesManageController(wsgi.Controller):
         context = req.environ['cinder.context']
         authorize(context)
 
-        if not self.is_valid_body(body, 'volume_type'):
+        self.assert_valid_body(body, 'volume_type')
-            raise webob.exc.HTTPBadRequest()
 
         vol_type = body['volume_type']
         name = vol_type.get('name', None)
@@ -103,8 +102,7 @@ class VolumeTypesManageController(wsgi.Controller):
         context = req.environ['cinder.context']
         authorize(context)
 
-        if not self.is_valid_body(body, 'volume_type'):
+        self.assert_valid_body(body, 'volume_type')
-            raise webob.exc.HTTPBadRequest()
 
         vol_type = body['volume_type']
         description = vol_type.get('description')

cinder/api/contrib/volume_manage.py

@@ -96,9 +96,7 @@ class VolumeManageController(wsgi.Controller):
         context = req.environ['cinder.context']
         authorize(context)
 
-        if not self.is_valid_body(body, 'volume'):
+        self.assert_valid_body(body, 'volume')
-            msg = _("Missing required element '%s' in request body") % 'volume'
-            raise exc.HTTPBadRequest(explanation=msg)
 
         volume = body['volume']
 

cinder/api/contrib/volume_transfer.py

@@ -149,13 +149,12 @@ class VolumeTransferController(wsgi.Controller):
     def create(self, req, body):
         """Create a new volume transfer."""
         LOG.debug('Creating new volume transfer %s', body)
-        if not self.is_valid_body(body, 'transfer'):
+        self.assert_valid_body(body, 'transfer')
-            raise exc.HTTPBadRequest()
 
         context = req.environ['cinder.context']
+        transfer = body['transfer']
 
         try:
-            transfer = body['transfer']
             volume_id = transfer['volume_id']
         except KeyError:
             msg = _("Incorrect request body format")
@@ -185,13 +184,12 @@ class VolumeTransferController(wsgi.Controller):
         """Accept a new volume transfer."""
         transfer_id = id
         LOG.debug('Accepting volume transfer %s', transfer_id)
-        if not self.is_valid_body(body, 'accept'):
+        self.assert_valid_body(body, 'accept')
-            raise exc.HTTPBadRequest()
 
         context = req.environ['cinder.context']
+        accept = body['accept']
 
         try:
-            accept = body['accept']
             auth_key = accept['auth_key']
         except KeyError:
             msg = _("Incorrect request body format")

cinder/api/contrib/volume_type_access.py

@@ -106,8 +106,7 @@ class VolumeTypeActionController(wsgi.Controller):
     """The volume type access API controller for the OpenStack API."""
 
     def _check_body(self, body, action_name):
-        if not self.is_valid_body(body, action_name):
+        self.assert_valid_body(body, action_name)
-            raise webob.exc.HTTPBadRequest()
         access = body[action_name]
         project = access.get('project')
         if not uuidutils.is_uuid_like(project):

cinder/api/contrib/volume_type_encryption.py

@@ -112,9 +112,7 @@ class VolumeTypeEncryptionController(wsgi.Controller):
             expl = _('Cannot create encryption specs. Volume type in use.')
             raise webob.exc.HTTPBadRequest(explanation=expl)
 
-        if not self.is_valid_body(body, 'encryption'):
+        self.assert_valid_body(body, 'encryption')
-            expl = _('Create body is not valid.')
-            raise webob.exc.HTTPBadRequest(explanation=expl)
 
         self._check_type(context, type_id)
 
@@ -138,12 +136,8 @@ class VolumeTypeEncryptionController(wsgi.Controller):
         context = req.environ['cinder.context']
         authorize(context)
 
-        if not body:
+        self.assert_valid_body(body, 'encryption')
-            expl = _('Request body empty.')
+
-            raise webob.exc.HTTPBadRequest(explanation=expl)
-        if not self.is_valid_body(body, 'encryption'):
-            expl = _('Update body is not valid. It must contain "encryption."')
-            raise webob.exc.HTTPBadRequest(explanation=expl)
         if len(body) > 1:
             expl = _('Request body contains too many items.')
             raise webob.exc.HTTPBadRequest(explanation=expl)

cinder/api/openstack/wsgi.py

@@ -1204,6 +1204,19 @@ class Controller(object):
 
         return True
 
+    @staticmethod
+    def assert_valid_body(body, entity_name):
+        # NOTE: After v1 api is deprecated need to merge 'is_valid_body' and
+        #       'assert_valid_body' in to one method. Right now it is not
+        #       possible to modify 'is_valid_body' to raise exception because
+        #       in case of V1 api when 'is_valid_body' return False,
+        #       'HTTPUnprocessableEntity' exception is getting raised and in
+        #       V2 api 'HTTPBadRequest' exception is getting raised.
+        if not Controller.is_valid_body(body, entity_name):
+            raise webob.exc.HTTPBadRequest(
+                explanation=_("Missing required element '%s' in "
+                              "request body.") % entity_name)
+
 
 class Fault(webob.exc.HTTPException):
     """Wrap webob.exc.HTTPException to provide API friendly response."""

cinder/api/v2/snapshot_metadata.py

@@ -48,13 +48,9 @@ class Controller(wsgi.Controller):
     @wsgi.serializers(xml=common.MetadataTemplate)
     @wsgi.deserializers(xml=common.MetadataDeserializer)
     def create(self, req, snapshot_id, body):
-        try:
+        self.assert_valid_body(body, 'metadata')
-            metadata = body['metadata']
-        except (KeyError, TypeError):
-            msg = _("Malformed request body")
-            raise exc.HTTPBadRequest(explanation=msg)
-
         context = req.environ['cinder.context']
+        metadata = body['metadata']
 
         new_metadata = self._update_snapshot_metadata(context,
                                                       snapshot_id,
@@ -66,11 +62,8 @@ class Controller(wsgi.Controller):
     @wsgi.serializers(xml=common.MetaItemTemplate)
     @wsgi.deserializers(xml=common.MetaItemDeserializer)
     def update(self, req, snapshot_id, id, body):
-        try:
+        self.assert_valid_body(body, 'meta')
         meta_item = body['meta']
-        except (TypeError, KeyError):
-            expl = _('Malformed request body')
-            raise exc.HTTPBadRequest(explanation=expl)
 
         if id not in meta_item:
             expl = _('Request body and URI mismatch')
@@ -91,13 +84,10 @@ class Controller(wsgi.Controller):
     @wsgi.serializers(xml=common.MetadataTemplate)
     @wsgi.deserializers(xml=common.MetadataDeserializer)
     def update_all(self, req, snapshot_id, body):
-        try:
+        self.assert_valid_body(body, 'metadata')
-            metadata = body['metadata']
-        except (TypeError, KeyError):
-            expl = _('Malformed request body')
-            raise exc.HTTPBadRequest(explanation=expl)
-
         context = req.environ['cinder.context']
+        metadata = body['metadata']
+
         new_metadata = self._update_snapshot_metadata(context,
                                                       snapshot_id,
                                                       metadata,

cinder/api/v2/snapshots.py

@@ -167,10 +167,7 @@ class SnapshotsController(wsgi.Controller):
         kwargs = {}
         context = req.environ['cinder.context']
 
-        if not self.is_valid_body(body, 'snapshot'):
+        self.assert_valid_body(body, 'snapshot')
-            msg = (_("Missing required element '%s' in request body") %
-                   'snapshot')
-            raise exc.HTTPBadRequest(explanation=msg)
 
         snapshot = body['snapshot']
         kwargs['metadata'] = snapshot.get('metadata', None)

cinder/api/v2/volume_metadata.py

@@ -46,13 +46,9 @@ class Controller(wsgi.Controller):
     @wsgi.serializers(xml=common.MetadataTemplate)
     @wsgi.deserializers(xml=common.MetadataDeserializer)
     def create(self, req, volume_id, body):
-        try:
+        self.assert_valid_body(body, 'metadata')
-            metadata = body['metadata']
-        except (KeyError, TypeError):
-            msg = _("Malformed request body")
-            raise webob.exc.HTTPBadRequest(explanation=msg)
-
         context = req.environ['cinder.context']
+        metadata = body['metadata']
 
         new_metadata = self._update_volume_metadata(context,
                                                     volume_id,
@@ -64,11 +60,8 @@ class Controller(wsgi.Controller):
     @wsgi.serializers(xml=common.MetaItemTemplate)
     @wsgi.deserializers(xml=common.MetaItemDeserializer)
     def update(self, req, volume_id, id, body):
-        try:
+        self.assert_valid_body(body, 'meta')
         meta_item = body['meta']
-        except (TypeError, KeyError):
-            expl = _('Malformed request body')
-            raise webob.exc.HTTPBadRequest(explanation=expl)
 
         if id not in meta_item:
             expl = _('Request body and URI mismatch')
@@ -89,13 +82,10 @@ class Controller(wsgi.Controller):
     @wsgi.serializers(xml=common.MetadataTemplate)
     @wsgi.deserializers(xml=common.MetadataDeserializer)
     def update_all(self, req, volume_id, body):
-        try:
+        self.assert_valid_body(body, 'metadata')
         metadata = body['metadata']
-        except (TypeError, KeyError):
-            expl = _('Malformed request body')
-            raise webob.exc.HTTPBadRequest(explanation=expl)
-
         context = req.environ['cinder.context']
+
         new_metadata = self._update_volume_metadata(context,
                                                     volume_id,
                                                     metadata,

cinder/api/v2/volumes.py

@@ -316,9 +316,7 @@ class VolumeController(wsgi.Controller):
     @wsgi.deserializers(xml=CreateDeserializer)
     def create(self, req, body):
         """Creates a new volume."""
-        if not self.is_valid_body(body, 'volume'):
+        self.assert_valid_body(body, 'volume')
-            msg = _("Missing required element '%s' in request body") % 'volume'
-            raise exc.HTTPBadRequest(explanation=msg)
 
         LOG.debug('Create volume request body: %s', body)
         context = req.environ['cinder.context']

cinder/tests/unit/api/contrib/test_backups.py

@@ -536,9 +536,8 @@ class BackupsAPITestCase(test.TestCase):
 
         self.assertEqual(res.status_int, 400)
         self.assertEqual(res_dict['badRequest']['code'], 400)
-        self.assertEqual(res_dict['badRequest']['message'],
+        self.assertEqual("Missing required element 'backup' in request body.",
-                         'The server could not comply with the request since'
+                         res_dict['badRequest']['message'])
-                         ' it is either malformed or otherwise incorrect.')
 
     def test_create_backup_with_body_KeyError(self):
         # omit volume_id from body
@@ -902,8 +901,8 @@ class BackupsAPITestCase(test.TestCase):
 
         self.assertEqual(res.status_int, 400)
         self.assertEqual(res_dict['badRequest']['code'], 400)
-        self.assertEqual(res_dict['badRequest']['message'],
+        self.assertEqual("Missing required element 'restore' in request body.",
-                         'Incorrect request body format')
+                         res_dict['badRequest']['message'])
 
         db.backup_destroy(context.get_admin_context(), backup_id)
 
@@ -923,8 +922,8 @@ class BackupsAPITestCase(test.TestCase):
 
         self.assertEqual(res.status_int, 400)
         self.assertEqual(res_dict['badRequest']['code'], 400)
-        self.assertEqual(res_dict['badRequest']['message'],
+        self.assertEqual("Missing required element 'restore' in request body.",
-                         'Incorrect request body format')
+                         res_dict['badRequest']['message'])
 
     @mock.patch('cinder.volume.API.create')
     def test_restore_backup_volume_id_unspecified(self,
@@ -1502,5 +1501,6 @@ class BackupsAPITestCase(test.TestCase):
         # verify that request is successful
         self.assertEqual(res.status_int, 400)
         self.assertEqual(res_dict['badRequest']['code'], 400)
-        self.assertEqual(res_dict['badRequest']['message'],
+        self.assertEqual("Missing required element 'backup-record' in "
-                         'Incorrect request body format.')
+                         "request body.",
+                         res_dict['badRequest']['message'])

cinder/tests/unit/api/contrib/test_cgsnapshots.py

@@ -383,9 +383,9 @@ class CgsnapshotsAPITestCase(test.TestCase):
 
         self.assertEqual(res.status_int, 400)
         self.assertEqual(res_dict['badRequest']['code'], 400)
-        self.assertEqual(res_dict['badRequest']['message'],
+        self.assertEqual("Missing required element 'cgsnapshot' in "
-                         'The server could not comply with the request since'
+                         "request body.",
-                         ' it is either malformed or otherwise incorrect.')
+                         res_dict['badRequest']['message'])
 
     @mock.patch.object(consistencygroupAPI.API, 'create_cgsnapshot',
                        side_effect=exception.InvalidCgSnapshot(

cinder/tests/unit/api/contrib/test_consistencygroups.py

@@ -340,8 +340,8 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
 
         self.assertEqual(400, res.status_int)
         self.assertEqual(400, res_dict['badRequest']['code'])
-        self.assertEqual('The server could not comply with the request since'
+        self.assertEqual("Missing required element 'consistencygroup' in "
-                         ' it is either malformed or otherwise incorrect.',
+                         "request body.",
                          res_dict['badRequest']['message'])
 
     def test_delete_consistencygroup_available(self):
@@ -767,8 +767,8 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
 
         self.assertEqual(400, res.status_int)
         self.assertEqual(400, res_dict['badRequest']['code'])
-        msg = (_('The server could not comply with the request since '
+        msg = _("Missing required element 'consistencygroup-from-src' in "
-                 'it is either malformed or otherwise incorrect.'))
+                "request body.")
         self.assertEqual(msg, res_dict['badRequest']['message'])
 
     def test_create_consistencygroup_from_src_no_cgsnapshot_id(self):

cinder/tests/unit/api/contrib/test_volume_transfer.py

@@ -313,9 +313,9 @@ class VolumeTransferAPITestCase(test.TestCase):
 
         self.assertEqual(res.status_int, 400)
         self.assertEqual(res_dict['badRequest']['code'], 400)
-        self.assertEqual(res_dict['badRequest']['message'],
+        self.assertEqual("Missing required element 'transfer' in "
-                         'The server could not comply with the request since'
+                         "request body.",
-                         ' it is either malformed or otherwise incorrect.')
+                         res_dict['badRequest']['message'])
 
     def test_create_transfer_with_body_KeyError(self):
         body = {"transfer": {"display_name": "transfer1"}}
@@ -465,9 +465,8 @@ class VolumeTransferAPITestCase(test.TestCase):
 
         self.assertEqual(res.status_int, 400)
         self.assertEqual(res_dict['badRequest']['code'], 400)
-        self.assertEqual(res_dict['badRequest']['message'],
+        self.assertEqual("Missing required element 'accept' in request body.",
-                         'The server could not comply with the request since'
+                         res_dict['badRequest']['message'])
-                         ' it is either malformed or otherwise incorrect.')
 
         db.volume_destroy(context.get_admin_context(), volume_id)
 
@@ -488,9 +487,8 @@ class VolumeTransferAPITestCase(test.TestCase):
 
         self.assertEqual(res.status_int, 400)
         self.assertEqual(res_dict['badRequest']['code'], 400)
-        self.assertEqual(res_dict['badRequest']['message'],
+        self.assertEqual("Missing required element 'accept' in request body.",
-                         'The server could not comply with the request since'
+                         res_dict['badRequest']['message'])
-                         ' it is either malformed or otherwise incorrect.')
 
     def test_accept_transfer_invalid_id_auth_key(self):
         volume_id = self._create_volume()

cinder/tests/unit/api/contrib/test_volume_type_encryption.py

@@ -324,11 +324,13 @@ class VolumeTypeEncryptionTest(test.TestCase):
         db.volume_type_destroy(context.get_admin_context(), volume_type['id'])
 
     def test_create_no_body(self):
-        self._encryption_create_bad_body(body=None)
+        msg = "Missing required element 'encryption' in request body."
+        self._encryption_create_bad_body(body=None, msg=msg)
 
     def test_create_malformed_entity(self):
         body = {'encryption': 'string'}
-        self._encryption_create_bad_body(body=body)
+        msg = "Missing required element 'encryption' in request body."
+        self._encryption_create_bad_body(body=body, msg=msg)
 
     def test_create_negative_key_size(self):
         body = {"encryption": {'cipher': 'cipher',
@@ -570,11 +572,11 @@ class VolumeTypeEncryptionTest(test.TestCase):
 
     def test_update_item_invalid_body(self):
         update_body = {"key_size": "value1"}
-        msg = 'Update body is not valid. It must contain "encryption."'
+        msg = "Missing required element 'encryption' in request body."
         self._encryption_update_bad_body(update_body, msg)
 
     def _encryption_empty_update(self, update_body):
-        msg = 'Request body empty.'
+        msg = "Missing required element 'encryption' in request body."
         self._encryption_update_bad_body(update_body, msg)
 
     def test_update_no_body(self):