From: john-griffith <john.griffith@solidfire.com>
Date: Wed, 16 Jan 2013 06:38:11 +0000 (-0700)
Subject: Add capability to update volume metadata.
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=bd71987bbf67715c8b84519b3ba3ac80552a901e;p=openstack-build%2Fcinder-build.git

Add capability to update volume metadata.

This addresses some cleanup and bugs with volume metadata updates
on the cinder side.  Mostly this implements v1/volume_metadata api
and adds things like propery checks to cinder.volume.api.update_volume_meta.

NOTE: This is only for api V1, a seperate patch will be provided for
V2 once this lands.

The remainder of the bp will be implemented in a cinderclient patch to follow.
Implement cinder side of blueprint update-vol-metadata

Also fixes bug: 1096018

Change-Id: Ie11931f657d3dcd69680fe5fcae435ff89549d97
---

diff --git a/cinder/api/v1/router.py b/cinder/api/v1/router.py
index 87969a784..0524b94ae 100644
--- a/cinder/api/v1/router.py
+++ b/cinder/api/v1/router.py
@@ -26,6 +26,7 @@ import cinder.api.openstack
 from cinder.api.v1 import limits
 from cinder.api.v1 import snapshots
 from cinder.api.v1 import types
+from cinder.api.v1 import volume_metadata
 from cinder.api.v1 import volumes
 from cinder.api import versions
 from cinder.openstack.common import log as logging
@@ -68,3 +69,17 @@ class APIRouter(cinder.api.openstack.APIRouter):
         self.resources['limits'] = limits.create_resource()
         mapper.resource("limit", "limits",
                         controller=self.resources['limits'])
+        self.resources['volume_metadata'] = \
+            volume_metadata.create_resource()
+        volume_metadata_controller = self.resources['volume_metadata']
+
+        mapper.resource("volume_metadata", "metadata",
+                        controller=volume_metadata_controller,
+                        parent_resource=dict(member_name='volume',
+                        collection_name='volumes'))
+
+        mapper.connect("metadata",
+                       "/{project_id}/volumes/{volume_id}/metadata",
+                       controller=volume_metadata_controller,
+                       action='update_all',
+                       conditions={"method": ['PUT']})
diff --git a/cinder/api/v1/volume_metadata.py b/cinder/api/v1/volume_metadata.py
new file mode 100644
index 000000000..4f6df75b6
--- /dev/null
+++ b/cinder/api/v1/volume_metadata.py
@@ -0,0 +1,164 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import webob
+
+from cinder.api import common
+from cinder.api.openstack import wsgi
+from cinder import exception
+from cinder import volume
+from webob import exc
+
+
+class Controller(object):
+    """ The volume metadata API controller for the OpenStack API """
+
+    def __init__(self):
+        self.volume_api = volume.API()
+        super(Controller, self).__init__()
+
+    def _get_metadata(self, context, volume_id):
+        try:
+            volume = self.volume_api.get(context, volume_id)
+            meta = self.volume_api.get_volume_metadata(context, volume)
+        except exception.VolumeNotFound:
+            msg = _('volume does not exist')
+            raise exc.HTTPNotFound(explanation=msg)
+        return meta
+
+    @wsgi.serializers(xml=common.MetadataTemplate)
+    def index(self, req, volume_id):
+        """ Returns the list of metadata for a given volume"""
+        context = req.environ['cinder.context']
+        return {'metadata': self._get_metadata(context, volume_id)}
+
+    @wsgi.serializers(xml=common.MetadataTemplate)
+    @wsgi.deserializers(xml=common.MetadataDeserializer)
+    def create(self, req, volume_id, body):
+        try:
+            metadata = body['metadata']
+        except (KeyError, TypeError):
+            msg = _("Malformed request body")
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        context = req.environ['cinder.context']
+
+        new_metadata = self._update_volume_metadata(context,
+                                                    volume_id,
+                                                    metadata,
+                                                    delete=False)
+
+        return {'metadata': new_metadata}
+
+    @wsgi.serializers(xml=common.MetaItemTemplate)
+    @wsgi.deserializers(xml=common.MetaItemDeserializer)
+    def update(self, req, volume_id, id, body):
+        try:
+            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')
+            raise exc.HTTPBadRequest(explanation=expl)
+
+        if len(meta_item) > 1:
+            expl = _('Request body contains too many items')
+            raise exc.HTTPBadRequest(explanation=expl)
+
+        context = req.environ['cinder.context']
+        self._update_volume_metadata(context,
+                                     volume_id,
+                                     meta_item,
+                                     delete=False)
+
+        return {'meta': meta_item}
+
+    @wsgi.serializers(xml=common.MetadataTemplate)
+    @wsgi.deserializers(xml=common.MetadataDeserializer)
+    def update_all(self, req, volume_id, body):
+        try:
+            metadata = body['metadata']
+        except (TypeError, KeyError):
+            expl = _('Malformed request body')
+            raise exc.HTTPBadRequest(explanation=expl)
+
+        context = req.environ['cinder.context']
+        new_metadata = self._update_volume_metadata(context,
+                                                    volume_id,
+                                                    metadata,
+                                                    delete=True)
+
+        return {'metadata': new_metadata}
+
+    def _update_volume_metadata(self, context,
+                                volume_id, metadata,
+                                delete=False):
+        try:
+            volume = self.volume_api.get(context, volume_id)
+            return self.volume_api.update_volume_metadata(context,
+                                                          volume,
+                                                          metadata,
+                                                          delete)
+        except exception.VolumeNotFound:
+            msg = _('volume does not exist')
+            raise exc.HTTPNotFound(explanation=msg)
+
+        except (ValueError, AttributeError):
+            msg = _("Malformed request body")
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        except exception.InvalidVolumeMetadata as error:
+            raise exc.HTTPBadRequest(explanation=unicode(error))
+
+        except exception.InvalidVolumeMetadataSize as error:
+            raise exc.HTTPRequestEntityTooLarge(explanation=unicode(error))
+
+    @wsgi.serializers(xml=common.MetaItemTemplate)
+    def show(self, req, volume_id, id):
+        """ Return a single metadata item """
+        context = req.environ['cinder.context']
+        data = self._get_metadata(context, volume_id)
+
+        try:
+            return {'meta': {id: data[id]}}
+        except KeyError:
+            msg = _("Metadata item was not found")
+            raise exc.HTTPNotFound(explanation=msg)
+
+    def delete(self, req, volume_id, id):
+        """ Deletes an existing metadata """
+        context = req.environ['cinder.context']
+
+        metadata = self._get_metadata(context, volume_id)
+
+        if id not in metadata:
+            msg = _("Metadata item was not found")
+            raise exc.HTTPNotFound(explanation=msg)
+
+        try:
+            volume = self.volume_api.get(context, volume_id)
+            self.volume_api.delete_volume_metadata(context, volume, id)
+        except exception.VolumeNotFound:
+            msg = _('volume does not exist')
+            raise exc.HTTPNotFound(explanation=msg)
+        return webob.Response(status_int=200)
+
+
+def create_resource():
+    return wsgi.Resource(Controller())
diff --git a/cinder/exception.py b/cinder/exception.py
index 3abed63d8..daf003f94 100644
--- a/cinder/exception.py
+++ b/cinder/exception.py
@@ -255,6 +255,14 @@ class VolumeMetadataNotFound(NotFound):
                 "key %(metadata_key)s.")
 
 
+class InvalidVolumeMetadata(Invalid):
+    message = _("Invalid metadata") + ": %(reason)s"
+
+
+class InvalidVolumeMetadataSize(Invalid):
+    message = _("Invalid metadata size") + ": %(reason)s"
+
+
 class VolumeTypeNotFound(NotFound):
     message = _("Volume type %(volume_type_id)s could not be found.")
 
diff --git a/cinder/tests/api/fakes.py b/cinder/tests/api/fakes.py
index edcfad0d6..e2bbe21e2 100644
--- a/cinder/tests/api/fakes.py
+++ b/cinder/tests/api/fakes.py
@@ -30,6 +30,7 @@ from cinder.api.v2 import limits
 from cinder.api.v2 import router
 from cinder.api import versions
 from cinder import context
+from cinder import exception as exc
 from cinder.openstack.common import timeutils
 from cinder import wsgi
 
@@ -97,6 +98,20 @@ def stub_out_rate_limiting(stubs):
     #     '__call__', fake_wsgi)
 
 
+def stub_out_key_pair_funcs(stubs, have_key_pair=True):
+    def key_pair(context, user_id):
+        return [dict(name='key', public_key='public_key')]
+
+    def one_key_pair(context, user_id, name):
+        if name == 'key':
+            return dict(name='key', public_key='public_key')
+        else:
+            raise exc.KeypairNotFound(user_id=user_id, name=name)
+
+    def no_key_pair(context, user_id):
+        return []
+
+
 class FakeToken(object):
     id_count = 0
 
diff --git a/cinder/tests/api/v1/test_volume_metadata.py b/cinder/tests/api/v1/test_volume_metadata.py
new file mode 100644
index 000000000..97c7075e6
--- /dev/null
+++ b/cinder/tests/api/v1/test_volume_metadata.py
@@ -0,0 +1,441 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import uuid
+
+import webob
+
+from cinder.api import extensions
+from cinder.api.v1 import volume_metadata
+from cinder.api.v1 import volumes
+import cinder.db
+from cinder import exception
+from cinder.openstack.common import cfg
+from cinder.openstack.common import jsonutils
+from cinder import test
+from cinder.tests.api import fakes
+
+
+CONF = cfg.CONF
+
+
+def return_create_volume_metadata_max(context, volume_id, metadata, delete):
+    return stub_max_volume_metadata()
+
+
+def return_create_volume_metadata(context, volume_id, metadata, delete):
+    return stub_volume_metadata()
+
+
+def return_volume_metadata(context, volume_id):
+    if not isinstance(volume_id, str) or not len(volume_id) == 36:
+        msg = 'id %s must be a uuid in return volume metadata' % volume_id
+        raise Exception(msg)
+    return stub_volume_metadata()
+
+
+def return_empty_volume_metadata(context, volume_id):
+    return {}
+
+
+def delete_volume_metadata(context, volume_id, key):
+    pass
+
+
+def stub_volume_metadata():
+    metadata = {
+        "key1": "value1",
+        "key2": "value2",
+        "key3": "value3",
+    }
+    return metadata
+
+
+def stub_max_volume_metadata():
+    metadata = {"metadata": {}}
+    for num in range(CONF.quota_metadata_items):
+        metadata['metadata']['key%i' % num] = "blah"
+    return metadata
+
+
+def return_volume(context, volume_id):
+    return {'id': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
+            'name': 'fake',
+            'metadata': {}}
+
+
+def return_volume_nonexistent(context, volume_id):
+    raise exception.VolumeNotFound('bogus test message')
+
+
+def fake_update_volume_metadata(self, context, volume, diff):
+    pass
+
+
+class volumeMetaDataTest(test.TestCase):
+
+    def setUp(self):
+        super(volumeMetaDataTest, self).setUp()
+        self.volume_api = cinder.volume.api.API()
+        fakes.stub_out_key_pair_funcs(self.stubs)
+        self.stubs.Set(cinder.db, 'volume_get', return_volume)
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_volume_metadata)
+
+        self.stubs.Set(self.volume_api, 'update_volume_metadata',
+                       fake_update_volume_metadata)
+
+        self.ext_mgr = extensions.ExtensionManager()
+        self.ext_mgr.extensions = {}
+        self.volume_controller = volumes.VolumeController(self.ext_mgr)
+        self.controller = volume_metadata.Controller()
+        self.id = str(uuid.uuid4())
+        self.url = '/v1/fake/volumes/%s/metadata' % self.id
+
+        vol = {"size": 100,
+               "display_name": "Volume Test Name",
+               "display_description": "Volume Test Desc",
+               "availability_zone": "zone1:host1",
+               "metadata": {}}
+        body = {"volume": vol}
+        req = fakes.HTTPRequest.blank('/v1/volumes')
+        self.volume_controller.create(req, body)
+
+    def test_index(self):
+        req = fakes.HTTPRequest.blank(self.url)
+        res_dict = self.controller.index(req, self.id)
+
+        expected = {
+            'metadata': {
+                'key1': 'value1',
+                'key2': 'value2',
+                'key3': 'value3',
+            },
+        }
+        self.assertEqual(expected, res_dict)
+
+    def test_index_nonexistent_volume(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_volume_nonexistent)
+        req = fakes.HTTPRequest.blank(self.url)
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.index, req, self.url)
+
+    def test_index_no_data(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_empty_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url)
+        res_dict = self.controller.index(req, self.id)
+        expected = {'metadata': {}}
+        self.assertEqual(expected, res_dict)
+
+    def test_show(self):
+        req = fakes.HTTPRequest.blank(self.url + '/key2')
+        res_dict = self.controller.show(req, self.id, 'key2')
+        expected = {'meta': {'key2': 'value2'}}
+        self.assertEqual(expected, res_dict)
+
+    def test_show_nonexistent_volume(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_volume_nonexistent)
+        req = fakes.HTTPRequest.blank(self.url + '/key2')
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.show, req, self.id, 'key2')
+
+    def test_show_meta_not_found(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_empty_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key6')
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.show, req, self.id, 'key6')
+
+    def test_delete(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_volume_metadata)
+        self.stubs.Set(cinder.db, 'volume_metadata_delete',
+                       delete_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key2')
+        req.method = 'DELETE'
+        res = self.controller.delete(req, self.id, 'key2')
+
+        self.assertEqual(200, res.status_int)
+
+    def test_delete_nonexistent_volume(self):
+        self.stubs.Set(cinder.db, 'volume_get',
+                       return_volume_nonexistent)
+        req = fakes.HTTPRequest.blank(self.url + '/key1')
+        req.method = 'DELETE'
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.delete, req, self.id, 'key1')
+
+    def test_delete_meta_not_found(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_empty_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key6')
+        req.method = 'DELETE'
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.delete, req, self.id, 'key6')
+
+    def test_create(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_empty_volume_metadata)
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+
+        req = fakes.HTTPRequest.blank('/v1/volume_metadata')
+        req.method = 'POST'
+        req.content_type = "application/json"
+        body = {"metadata": {"key9": "value9"}}
+        req.body = jsonutils.dumps(body)
+        res_dict = self.controller.create(req, self.id, body)
+        self.assertEqual(body, res_dict)
+
+    def test_create_empty_body(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url)
+        req.method = 'POST'
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.create, req, self.id, None)
+
+    def test_create_item_empty_key(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key1')
+        req.method = 'PUT'
+        body = {"meta": {"": "value1"}}
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.create, req, self.id, body)
+
+    def test_create_item_key_too_long(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key1')
+        req.method = 'PUT'
+        body = {"meta": {("a" * 260): "value1"}}
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.create,
+                          req, self.id, body)
+
+    def test_create_nonexistent_volume(self):
+        self.stubs.Set(cinder.db, 'volume_get',
+                       return_volume_nonexistent)
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_volume_metadata)
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+
+        req = fakes.HTTPRequest.blank('/v1/volume_metadata')
+        req.method = 'POST'
+        req.content_type = "application/json"
+        body = {"metadata": {"key9": "value9"}}
+        req.body = jsonutils.dumps(body)
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.create, req, self.id, body)
+
+    def test_update_all(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url)
+        req.method = 'PUT'
+        req.content_type = "application/json"
+        expected = {
+            'metadata': {
+                'key10': 'value10',
+                'key99': 'value99',
+            },
+        }
+        req.body = jsonutils.dumps(expected)
+        res_dict = self.controller.update_all(req, self.id, expected)
+
+        self.assertEqual(expected, res_dict)
+
+    def test_update_all_empty_container(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url)
+        req.method = 'PUT'
+        req.content_type = "application/json"
+        expected = {'metadata': {}}
+        req.body = jsonutils.dumps(expected)
+        res_dict = self.controller.update_all(req, self.id, expected)
+
+        self.assertEqual(expected, res_dict)
+
+    def test_update_all_malformed_container(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url)
+        req.method = 'PUT'
+        req.content_type = "application/json"
+        expected = {'meta': {}}
+        req.body = jsonutils.dumps(expected)
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.update_all, req, self.id, expected)
+
+    def test_update_all_malformed_data(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url)
+        req.method = 'PUT'
+        req.content_type = "application/json"
+        expected = {'metadata': ['asdf']}
+        req.body = jsonutils.dumps(expected)
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.update_all, req, self.id, expected)
+
+    def test_update_all_nonexistent_volume(self):
+        self.stubs.Set(cinder.db, 'volume_get', return_volume_nonexistent)
+        req = fakes.HTTPRequest.blank(self.url)
+        req.method = 'PUT'
+        req.content_type = "application/json"
+        body = {'metadata': {'key10': 'value10'}}
+        req.body = jsonutils.dumps(body)
+
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.update_all, req, '100', body)
+
+    def test_update_item(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key1')
+        req.method = 'PUT'
+        body = {"meta": {"key1": "value1"}}
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+        res_dict = self.controller.update(req, self.id, 'key1', body)
+        expected = {'meta': {'key1': 'value1'}}
+        self.assertEqual(expected, res_dict)
+
+    def test_update_item_nonexistent_volume(self):
+        self.stubs.Set(cinder.db, 'volume_get',
+                       return_volume_nonexistent)
+        req = fakes.HTTPRequest.blank('/v1.1/fake/volumes/asdf/metadata/key1')
+        req.method = 'PUT'
+        body = {"meta": {"key1": "value1"}}
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.update, req, self.id, 'key1', body)
+
+    def test_update_item_empty_body(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key1')
+        req.method = 'PUT'
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.update, req, self.id, 'key1', None)
+
+    def test_update_item_empty_key(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key1')
+        req.method = 'PUT'
+        body = {"meta": {"": "value1"}}
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.update, req, self.id, '', body)
+
+    def test_update_item_key_too_long(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key1')
+        req.method = 'PUT'
+        body = {"meta": {("a" * 260): "value1"}}
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPRequestEntityTooLarge,
+                          self.controller.update,
+                          req, self.id, ("a" * 260), body)
+
+    def test_update_item_value_too_long(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key1')
+        req.method = 'PUT'
+        body = {"meta": {"key1": ("a" * 260)}}
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPRequestEntityTooLarge,
+                          self.controller.update,
+                          req, self.id, "key1", body)
+
+    def test_update_item_too_many_keys(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/key1')
+        req.method = 'PUT'
+        body = {"meta": {"key1": "value1", "key2": "value2"}}
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.update, req, self.id, 'key1', body)
+
+    def test_update_item_body_uri_mismatch(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url + '/bad')
+        req.method = 'PUT'
+        body = {"meta": {"key1": "value1"}}
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.update, req, self.id, 'bad', body)
+
+    def test_invalid_metadata_items_on_create(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url)
+        req.method = 'POST'
+        req.headers["content-type"] = "application/json"
+
+        #test for long key
+        data = {"metadata": {"a" * 260: "value1"}}
+        req.body = jsonutils.dumps(data)
+        self.assertRaises(webob.exc.HTTPRequestEntityTooLarge,
+                          self.controller.create, req, self.id, data)
+
+        #test for long value
+        data = {"metadata": {"key": "v" * 260}}
+        req.body = jsonutils.dumps(data)
+        self.assertRaises(webob.exc.HTTPRequestEntityTooLarge,
+                          self.controller.create, req, self.id, data)
+
+        #test for empty key.
+        data = {"metadata": {"": "value1"}}
+        req.body = jsonutils.dumps(data)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.create, req, self.id, data)
diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 291a0c79c..98b634db1 100644
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -141,6 +141,21 @@ class VolumeTestCase(test.TestCase):
                           self.context,
                           volume_id)
 
+    def test_create_volume_with_invalid_metadata(self):
+        """Test volume create with too much metadata fails."""
+        volume_api = cinder.volume.api.API()
+        test_meta = {'fake_key': 'fake_value' * 256}
+        self.assertRaises(exception.InvalidVolumeMetadataSize,
+                          volume_api.create,
+                          self.context,
+                          1,
+                          'name',
+                          'description',
+                          None,
+                          None,
+                          None,
+                          test_meta)
+
     def test_create_volume_with_volume_type(self):
         """Test volume creation with default volume type."""
         def fake_reserve(context, expire=None, **deltas):
diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index e17ec141f..cbbd8f61e 100644
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -181,6 +181,7 @@ class API(base.Base):
         else:
             volume_type_id = volume_type.get('id')
 
+        self._check_metadata_properties(context, metadata)
         options = {'size': size,
                    'user_id': context.user_id,
                    'project_id': context.project_id,
@@ -512,6 +513,24 @@ class API(base.Base):
         """Delete the given metadata item from a volume."""
         self.db.volume_metadata_delete(context, volume['id'], key)
 
+    def _check_metadata_properties(self, context, metadata=None):
+        if not metadata:
+            metadata = {}
+
+        for k, v in metadata.iteritems():
+            if len(k) == 0:
+                msg = _("Metadata property key blank")
+                LOG.warn(msg)
+                raise exception.InvalidVolumeMetadata(reason=msg)
+            if len(k) > 255:
+                msg = _("Metadata property key greater than 255 characters")
+                LOG.warn(msg)
+                raise exception.InvalidVolumeMetadataSize(reason=msg)
+            if len(v) > 255:
+                msg = _("Metadata property value greater than 255 characters")
+                LOG.warn(msg)
+                raise exception.InvalidVolumeMetadataSize(reason=msg)
+
     @wrap_check_policy
     def update_volume_metadata(self, context, volume, metadata, delete=False):
         """Updates or creates volume metadata.
@@ -520,13 +539,19 @@ class API(base.Base):
         `metadata` argument will be deleted.
 
         """
+        orig_meta = self.get_volume_metadata(context, volume)
         if delete:
             _metadata = metadata
         else:
-            _metadata = self.get_volume_metadata(context, volume['id'])
+            _metadata = orig_meta.copy()
             _metadata.update(metadata)
 
+        self._check_metadata_properties(context, _metadata)
+
         self.db.volume_metadata_update(context, volume['id'], _metadata, True)
+
+        # TODO(jdg): Implement an RPC call for drivers that may use this info
+
         return _metadata
 
     def get_volume_metadata_value(self, volume, key):