From bd71987bbf67715c8b84519b3ba3ac80552a901e Mon Sep 17 00:00:00 2001 From: john-griffith Date: Tue, 15 Jan 2013 23:38:11 -0700 Subject: [PATCH] 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 --- cinder/api/v1/router.py | 15 + cinder/api/v1/volume_metadata.py | 164 ++++++++ cinder/exception.py | 8 + cinder/tests/api/fakes.py | 15 + cinder/tests/api/v1/test_volume_metadata.py | 441 ++++++++++++++++++++ cinder/tests/test_volume.py | 15 + cinder/volume/api.py | 27 +- 7 files changed, 684 insertions(+), 1 deletion(-) create mode 100644 cinder/api/v1/volume_metadata.py create mode 100644 cinder/tests/api/v1/test_volume_metadata.py 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): -- 2.45.2