Adds os-volume_attachments 'volume_id' validation

If you POST to os-volume_attachments but give the name of the volume
instead of a UUID, it returns a 404 error; this is invalid HTTP.
404 means that the Request-URI has not been found. In fact, the
Request-URI has been found, but a child attribute has been incorrectly
specified. Some other error could be used.

This fixes the problem by returning 'BadRequest' in place of 'NotFound'.
The 'volumeId' is checked to be a valid UUID string before moving
further.

Fixes bug #1062494

Change-Id: Icc5dbc7ac94051514709997457cafb16e870bea9

2 files changed, 30 insertions, 0 deletions

diff --git a/nova/api/openstack/compute/contrib/volumes.py b/nova/api/openstack/compute/contrib/volumes.py
index abdef3a7d..9564921f4 100644
--- a/nova/api/openstack/compute/contrib/volumes.py
+++ b/nova/api/openstack/compute/contrib/volumes.py
@@ -26,6 +26,7 @@ from nova.api.openstack import xmlutil
 from nova import compute
 from nova import exception
 from nova.openstack.common import log as logging
+from nova.openstack.common import uuidutils
 from nova import utils
 from nova import volume
 
@@ -365,6 +366,12 @@ class VolumeAttachmentController(wsgi.Controller):
             instance['uuid'],
             assigned_mountpoint)}
 
+    def _validate_volume_id(self, volume_id):
+        if not uuidutils.is_uuid_like(volume_id):
+            msg = _("Bad volumeId format: volumeId is "
+                    "not in proper format (%s)") % volume_id
+            raise exc.HTTPBadRequest(explanation=msg)
+
     @wsgi.serializers(xml=VolumeAttachmentTemplate)
     def create(self, req, server_id, body):
         """Attach a volume to an instance."""
@@ -377,6 +384,8 @@ class VolumeAttachmentController(wsgi.Controller):
         volume_id = body['volumeAttachment']['volumeId']
         device = body['volumeAttachment'].get('device')
 
+        self._validate_volume_id(volume_id)
+
         msg = _("Attach volume %(volume_id)s to instance %(server_id)s"
                 " at %(device)s") % locals()
         LOG.audit(msg, context=context)
diff --git a/nova/tests/api/openstack/compute/contrib/test_volumes.py b/nova/tests/api/openstack/compute/contrib/test_volumes.py
index e8a315edd..3119f55e8 100644
--- a/nova/tests/api/openstack/compute/contrib/test_volumes.py
+++ b/nova/tests/api/openstack/compute/contrib/test_volumes.py
@@ -289,6 +289,27 @@ class VolumeAttachTests(test.TestCase):
         self.assertEqual(result['volumeAttachment']['id'],
             '00000000-aaaa-aaaa-aaaa-000000000000')
 
+    def test_attach_volume_bad_id(self):
+        self.stubs.Set(compute_api.API,
+                       'attach_volume',
+                       fake_attach_volume)
+        attachments = volumes.VolumeAttachmentController()
+
+        body = {
+            'volumeAttachment': {
+                'device': None,
+                'volumeId': 'TESTVOLUME',
+            }
+        }
+
+        req = fakes.HTTPRequest.blank('/v2/fake/os-volumes/attach')
+        req.method = 'POST'
+        req.content_type = 'application/json'
+        req.body = jsonutils.dumps(body)
+
+        self.assertRaises(webob.exc.HTTPBadRequest, attachments.create,
+                          req, FAKE_UUID, body)
+
 
 class VolumeSerializerTest(test.TestCase):
     def _verify_volume_attachment(self, attach, tree):