openstack/cinder
Code Issues Proposed changes

Separate create and update rules for volume metadata

Browse Source 
This patch allows different policy rules for create and
update volume metadata.

Change-Id: I23dabd8866a9358c41eb3e048d91011a53c41cc3
Closes-Bug: #1472042
This commit is contained in:
Yuriy Nesenenko 2016-08-17 20:15:03 +03:00
parent f33fc3b69b
commit 9771c2cd4e
6 changed files with 82 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
cinder/api/v2/volume_metadata.py
View File

@ -50,11 +50,9 @@ class Controller(wsgi.Controller):
context = req.environ['cinder.context'] context = req.environ['cinder.context']
metadata = body['metadata'] metadata = body['metadata']
new_metadata = self._update_volume_metadata(context, new_metadata = self._update_volume_metadata(context, volume_id,
volume_id, metadata, delete=False,
metadata, use_create=True)
delete=False)
return {'metadata': new_metadata} return {'metadata': new_metadata}
def update(self, req, volume_id, id, body): def update(self, req, volume_id, id, body):
@ -89,17 +87,17 @@ class Controller(wsgi.Controller):
return {'metadata': new_metadata} return {'metadata': new_metadata}
def _update_volume_metadata(self, context, def _update_volume_metadata(self, context, volume_id, metadata,
volume_id, metadata, delete=False, use_create=False):
delete=False):
try: try:
volume = self.volume_api.get(context, volume_id) volume = self.volume_api.get(context, volume_id)
return self.volume_api.update_volume_metadata( if use_create:
context, return self.volume_api.create_volume_metadata(context, volume,
volume, metadata)
metadata, else:
delete, return self.volume_api.update_volume_metadata(
meta_type=common.METADATA_TYPES.user) context, volume, metadata, delete,
meta_type=common.METADATA_TYPES.user)
# Not found exception will be handled at the wsgi level # Not found exception will be handled at the wsgi level
except (ValueError, AttributeError): except (ValueError, AttributeError):
msg = _("Malformed request body") msg = _("Malformed request body")

1
cinder/tests/unit/policy.json
View File

@ -8,6 +8,7 @@
"volume:get_all": "", "volume:get_all": "",
"volume:get_volume_metadata": "", "volume:get_volume_metadata": "",
"volume:get_volume_image_metadata": "", "volume:get_volume_image_metadata": "",
"volume:create_volume_metadata": "",
"volume:delete_volume_metadata": "", "volume:delete_volume_metadata": "",
"volume:update_volume_metadata": "", "volume:update_volume_metadata": "",
"volume:get_volume_admin_metadata": "rule:admin_api", "volume:get_volume_admin_metadata": "rule:admin_api",

61
cinder/tests/unit/test_volume.py
View File

@ -318,6 +318,7 @@ class AvailabilityZoneTestCase(BaseVolumeTestCase):
self.assertEqual(expected, azs) self.assertEqual(expected, azs)
@ddt.ddt
class VolumeTestCase(BaseVolumeTestCase): class VolumeTestCase(BaseVolumeTestCase):
def setUp(self): def setUp(self):
@ -754,12 +755,34 @@ class VolumeTestCase(BaseVolumeTestCase):
self.context, self.context,
volume_id) volume_id)
@mock.patch('cinder.db.volume_metadata_update')
def test_create_volume_metadata(self, metadata_update):
metadata = {'fake_key': 'fake_value'}
metadata_update.return_value = metadata
volume = tests_utils.create_volume(self.context, **self.volume_params)
res = self.volume_api.create_volume_metadata(self.context,
volume, metadata)
metadata_update.assert_called_once_with(self.context, volume.id,
metadata, False,
common.METADATA_TYPES.user)
self.assertEqual(metadata, res)
@ddt.data('maintenance', 'uploading')
def test_create_volume_metadata_maintenance(self, status):
metadata = {'fake_key': 'fake_value'}
volume = tests_utils.create_volume(self.context, **self.volume_params)
volume['status'] = status
self.assertRaises(exception.InvalidVolume,
self.volume_api.create_volume_metadata,
self.context,
volume,
metadata)
def test_create_volume_with_invalid_metadata(self): def test_create_volume_with_invalid_metadata(self):
"""Test volume create with too much metadata fails.""" """Test volume create with too much metadata fails."""
volume_api = cinder.volume.api.API()
test_meta = {'fake_key': 'fake_value' * 256} test_meta = {'fake_key': 'fake_value' * 256}
self.assertRaises(exception.InvalidVolumeMetadataSize, self.assertRaises(exception.InvalidVolumeMetadataSize,
volume_api.create, self.volume_api.create,
self.context, self.context,
1, 1,
'name', 'name',
@ -777,11 +800,8 @@ class VolumeTestCase(BaseVolumeTestCase):
volume = tests_utils.create_volume(self.context, metadata=test_meta1, volume = tests_utils.create_volume(self.context, metadata=test_meta1,
**self.volume_params) **self.volume_params)
self.volume.create_volume(self.context, volume.id, volume=volume) self.volume.create_volume(self.context, volume.id, volume=volume)
volume_api = cinder.volume.api.API()
# update user metadata associated with the volume. # update user metadata associated with the volume.
result_meta = volume_api.update_volume_metadata( result_meta = self.volume_api.update_volume_metadata(
self.context, self.context,
volume, volume,
test_meta2, test_meta2,
@ -790,7 +810,7 @@ class VolumeTestCase(BaseVolumeTestCase):
self.assertEqual(test_meta2, result_meta) self.assertEqual(test_meta2, result_meta)
# create image metadata associated with the volume. # create image metadata associated with the volume.
result_meta = volume_api.update_volume_metadata( result_meta = self.volume_api.update_volume_metadata(
self.context, self.context,
volume, volume,
test_meta1, test_meta1,
@ -799,7 +819,7 @@ class VolumeTestCase(BaseVolumeTestCase):
self.assertEqual(test_meta1, result_meta) self.assertEqual(test_meta1, result_meta)
# update image metadata associated with the volume. # update image metadata associated with the volume.
result_meta = volume_api.update_volume_metadata( result_meta = self.volume_api.update_volume_metadata(
self.context, self.context,
volume, volume,
test_meta2, test_meta2,
@ -809,7 +829,7 @@ class VolumeTestCase(BaseVolumeTestCase):
# update volume metadata with invalid metadta type. # update volume metadata with invalid metadta type.
self.assertRaises(exception.InvalidMetadataType, self.assertRaises(exception.InvalidMetadataType,
volume_api.update_volume_metadata, self.volume_api.update_volume_metadata,
self.context, self.context,
volume, volume,
test_meta1, test_meta1,
@ -823,9 +843,8 @@ class VolumeTestCase(BaseVolumeTestCase):
volume = tests_utils.create_volume(self.context, metadata=test_meta1, volume = tests_utils.create_volume(self.context, metadata=test_meta1,
**self.volume_params) **self.volume_params)
volume['status'] = 'maintenance' volume['status'] = 'maintenance'
volume_api = cinder.volume.api.API()
self.assertRaises(exception.InvalidVolume, self.assertRaises(exception.InvalidVolume,
volume_api.update_volume_metadata, self.volume_api.update_volume_metadata,
self.context, self.context,
volume, volume,
test_meta1, test_meta1,
@ -836,9 +855,8 @@ class VolumeTestCase(BaseVolumeTestCase):
def test_update_with_ovo(self, volume_update): def test_update_with_ovo(self, volume_update):
"""Test update volume using oslo_versionedobject.""" """Test update volume using oslo_versionedobject."""
volume = tests_utils.create_volume(self.context, **self.volume_params) volume = tests_utils.create_volume(self.context, **self.volume_params)
volume_api = cinder.volume.api.API()
updates = {'display_name': 'foobbar'} updates = {'display_name': 'foobbar'}
volume_api.update(self.context, volume, updates) self.volume_api.update(self.context, volume, updates)
volume_update.assert_called_once_with(self.context, volume.id, volume_update.assert_called_once_with(self.context, volume.id,
updates) updates)
self.assertEqual('foobbar', volume.display_name) self.assertEqual('foobbar', volume.display_name)
@ -852,11 +870,8 @@ class VolumeTestCase(BaseVolumeTestCase):
**self.volume_params) **self.volume_params)
volume_id = volume['id'] volume_id = volume['id']
self.volume.create_volume(self.context, volume_id, volume=volume) self.volume.create_volume(self.context, volume_id, volume=volume)
volume_api = cinder.volume.api.API()
# delete user metadata associated with the volume. # delete user metadata associated with the volume.
volume_api.delete_volume_metadata( self.volume_api.delete_volume_metadata(
self.context, self.context,
volume, volume,
'fake_key2', 'fake_key2',
@ -866,7 +881,7 @@ class VolumeTestCase(BaseVolumeTestCase):
db.volume_metadata_get(self.context, volume_id)) db.volume_metadata_get(self.context, volume_id))
# create image metadata associated with the volume. # create image metadata associated with the volume.
result_meta = volume_api.update_volume_metadata( result_meta = self.volume_api.update_volume_metadata(
self.context, self.context,
volume, volume,
test_meta1, test_meta1,
@ -876,7 +891,7 @@ class VolumeTestCase(BaseVolumeTestCase):
self.assertEqual(test_meta1, result_meta) self.assertEqual(test_meta1, result_meta)
# delete image metadata associated with the volume. # delete image metadata associated with the volume.
volume_api.delete_volume_metadata( self.volume_api.delete_volume_metadata(
self.context, self.context,
volume, volume,
'fake_key2', 'fake_key2',
@ -891,7 +906,7 @@ class VolumeTestCase(BaseVolumeTestCase):
# delete volume metadata with invalid metadta type. # delete volume metadata with invalid metadta type.
self.assertRaises(exception.InvalidMetadataType, self.assertRaises(exception.InvalidMetadataType,
volume_api.delete_volume_metadata, self.volume_api.delete_volume_metadata,
self.context, self.context,
volume, volume,
'fake_key1', 'fake_key1',
@ -904,9 +919,8 @@ class VolumeTestCase(BaseVolumeTestCase):
volume = tests_utils.create_volume(self.context, metadata=test_meta1, volume = tests_utils.create_volume(self.context, metadata=test_meta1,
**self.volume_params) **self.volume_params)
volume['status'] = 'maintenance' volume['status'] = 'maintenance'
volume_api = cinder.volume.api.API()
self.assertRaises(exception.InvalidVolume, self.assertRaises(exception.InvalidVolume,
volume_api.delete_volume_metadata, self.volume_api.delete_volume_metadata,
self.context, self.context,
volume, volume,
'fake_key1', 'fake_key1',
@ -918,9 +932,8 @@ class VolumeTestCase(BaseVolumeTestCase):
volume = tests_utils.create_volume(self.context, metadata=test_meta1, volume = tests_utils.create_volume(self.context, metadata=test_meta1,
**self.volume_params) **self.volume_params)
volume['status'] = 'maintenance' volume['status'] = 'maintenance'
volume_api = cinder.volume.api.API()
self.assertRaises(exception.InvalidVolume, self.assertRaises(exception.InvalidVolume,
volume_api.attach, self.volume_api.attach,
self.context, self.context,
volume, None, None, None, None) volume, None, None, None, None)

39
cinder/volume/api.py
View File

@ -954,6 +954,15 @@ class API(base.Base):
resource=volume) resource=volume)
return dict(rv) return dict(rv)
@wrap_check_policy
def create_volume_metadata(self, context, volume, metadata):
"""Creates volume metadata."""
db_meta = self._update_volume_metadata(context, volume, metadata)
LOG.info(_LI("Create volume metadata completed successfully."),
resource=volume)
return db_meta
@wrap_check_policy @wrap_check_policy
def delete_volume_metadata(self, context, volume, def delete_volume_metadata(self, context, volume,
key, meta_type=common.METADATA_TYPES.user): key, meta_type=common.METADATA_TYPES.user):
@ -967,26 +976,28 @@ class API(base.Base):
LOG.info(_LI("Delete volume metadata completed successfully."), LOG.info(_LI("Delete volume metadata completed successfully."),
resource=volume) resource=volume)
@wrap_check_policy def _update_volume_metadata(self, context, volume, metadata, delete=False,
def update_volume_metadata(self, context, volume, meta_type=common.METADATA_TYPES.user):
metadata, delete=False,
meta_type=common.METADATA_TYPES.user):
"""Updates or creates volume metadata.
If delete is True, metadata items that are not specified in the
`metadata` argument will be deleted.
"""
if volume['status'] in ('maintenance', 'uploading'): if volume['status'] in ('maintenance', 'uploading'):
msg = _('Updating volume metadata is not allowed for volumes in ' msg = _('Updating volume metadata is not allowed for volumes in '
'%s status.') % volume['status'] '%s status.') % volume['status']
LOG.info(msg, resource=volume) LOG.info(msg, resource=volume)
raise exception.InvalidVolume(reason=msg) raise exception.InvalidVolume(reason=msg)
utils.check_metadata_properties(metadata) utils.check_metadata_properties(metadata)
db_meta = self.db.volume_metadata_update(context, volume['id'], return self.db.volume_metadata_update(context, volume['id'],
metadata, metadata, delete, meta_type)
delete,
meta_type) @wrap_check_policy
def update_volume_metadata(self, context, volume, metadata, delete=False,
meta_type=common.METADATA_TYPES.user):
"""Updates volume metadata.
If delete is True, metadata items that are not specified in the
`metadata` argument will be deleted.
"""
db_meta = self._update_volume_metadata(context, volume, metadata,
delete, meta_type)
# TODO(jdg): Implement an RPC call for drivers that may use this info # TODO(jdg): Implement an RPC call for drivers that may use this info

1
etc/cinder/policy.json
View File

@ -10,6 +10,7 @@
"volume:get": "rule:admin_or_owner", "volume:get": "rule:admin_or_owner",
"volume:get_all": "rule:admin_or_owner", "volume:get_all": "rule:admin_or_owner",
"volume:get_volume_metadata": "rule:admin_or_owner", "volume:get_volume_metadata": "rule:admin_or_owner",
"volume:create_volume_metadata": "rule:admin_or_owner",
"volume:delete_volume_metadata": "rule:admin_or_owner", "volume:delete_volume_metadata": "rule:admin_or_owner",
"volume:update_volume_metadata": "rule:admin_or_owner", "volume:update_volume_metadata": "rule:admin_or_owner",
"volume:get_volume_admin_metadata": "rule:admin_api", "volume:get_volume_admin_metadata": "rule:admin_api",

6
releasenotes/notes/create-update-rules-b46cf9c07c5a3966.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- Separate create and update rules for volume metadata.
upgrade:
- If policy for update volume metadata is modified in a desired way
it's needed to add a desired rule for create volume metadata.