From 9771c2cd4e32979358f8647e57b4bab355221c0d Mon Sep 17 00:00:00 2001 From: Yuriy Nesenenko Date: Wed, 17 Aug 2016 20:15:03 +0300 Subject: [PATCH] Separate create and update rules for volume metadata This patch allows different policy rules for create and update volume metadata. Change-Id: I23dabd8866a9358c41eb3e048d91011a53c41cc3 Closes-Bug: #1472042 --- cinder/api/v2/volume_metadata.py | 26 ++++---- cinder/tests/unit/policy.json | 1 + cinder/tests/unit/test_volume.py | 61 +++++++++++-------- cinder/volume/api.py | 39 +++++++----- etc/cinder/policy.json | 1 + .../create-update-rules-b46cf9c07c5a3966.yaml | 6 ++ 6 files changed, 82 insertions(+), 52 deletions(-) create mode 100644 releasenotes/notes/create-update-rules-b46cf9c07c5a3966.yaml diff --git a/cinder/api/v2/volume_metadata.py b/cinder/api/v2/volume_metadata.py index e1d9c8d3e7a..37c3e52d5c1 100644 --- a/cinder/api/v2/volume_metadata.py +++ b/cinder/api/v2/volume_metadata.py @@ -50,11 +50,9 @@ class Controller(wsgi.Controller): context = req.environ['cinder.context'] metadata = body['metadata'] - new_metadata = self._update_volume_metadata(context, - volume_id, - metadata, - delete=False) - + new_metadata = self._update_volume_metadata(context, volume_id, + metadata, delete=False, + use_create=True) return {'metadata': new_metadata} def update(self, req, volume_id, id, body): @@ -89,17 +87,17 @@ class Controller(wsgi.Controller): return {'metadata': new_metadata} - def _update_volume_metadata(self, context, - volume_id, metadata, - delete=False): + def _update_volume_metadata(self, context, volume_id, metadata, + delete=False, use_create=False): try: volume = self.volume_api.get(context, volume_id) - return self.volume_api.update_volume_metadata( - context, - volume, - metadata, - delete, - meta_type=common.METADATA_TYPES.user) + if use_create: + return self.volume_api.create_volume_metadata(context, volume, + metadata) + else: + return self.volume_api.update_volume_metadata( + context, volume, metadata, delete, + meta_type=common.METADATA_TYPES.user) # Not found exception will be handled at the wsgi level except (ValueError, AttributeError): msg = _("Malformed request body") diff --git a/cinder/tests/unit/policy.json b/cinder/tests/unit/policy.json index 20c92fb20b6..89154e5a2dd 100644 --- a/cinder/tests/unit/policy.json +++ b/cinder/tests/unit/policy.json @@ -8,6 +8,7 @@ "volume:get_all": "", "volume:get_volume_metadata": "", "volume:get_volume_image_metadata": "", + "volume:create_volume_metadata": "", "volume:delete_volume_metadata": "", "volume:update_volume_metadata": "", "volume:get_volume_admin_metadata": "rule:admin_api", diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 4d50a3f2795..fb7f3793b13 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -318,6 +318,7 @@ class AvailabilityZoneTestCase(BaseVolumeTestCase): self.assertEqual(expected, azs) +@ddt.ddt class VolumeTestCase(BaseVolumeTestCase): def setUp(self): @@ -754,12 +755,34 @@ class VolumeTestCase(BaseVolumeTestCase): self.context, 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): """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.volume_api.create, self.context, 1, 'name', @@ -777,11 +800,8 @@ class VolumeTestCase(BaseVolumeTestCase): volume = tests_utils.create_volume(self.context, metadata=test_meta1, **self.volume_params) self.volume.create_volume(self.context, volume.id, volume=volume) - - volume_api = cinder.volume.api.API() - # update user metadata associated with the volume. - result_meta = volume_api.update_volume_metadata( + result_meta = self.volume_api.update_volume_metadata( self.context, volume, test_meta2, @@ -790,7 +810,7 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertEqual(test_meta2, result_meta) # create image metadata associated with the volume. - result_meta = volume_api.update_volume_metadata( + result_meta = self.volume_api.update_volume_metadata( self.context, volume, test_meta1, @@ -799,7 +819,7 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertEqual(test_meta1, result_meta) # update image metadata associated with the volume. - result_meta = volume_api.update_volume_metadata( + result_meta = self.volume_api.update_volume_metadata( self.context, volume, test_meta2, @@ -809,7 +829,7 @@ class VolumeTestCase(BaseVolumeTestCase): # update volume metadata with invalid metadta type. self.assertRaises(exception.InvalidMetadataType, - volume_api.update_volume_metadata, + self.volume_api.update_volume_metadata, self.context, volume, test_meta1, @@ -823,9 +843,8 @@ class VolumeTestCase(BaseVolumeTestCase): volume = tests_utils.create_volume(self.context, metadata=test_meta1, **self.volume_params) volume['status'] = 'maintenance' - volume_api = cinder.volume.api.API() self.assertRaises(exception.InvalidVolume, - volume_api.update_volume_metadata, + self.volume_api.update_volume_metadata, self.context, volume, test_meta1, @@ -836,9 +855,8 @@ class VolumeTestCase(BaseVolumeTestCase): def test_update_with_ovo(self, volume_update): """Test update volume using oslo_versionedobject.""" volume = tests_utils.create_volume(self.context, **self.volume_params) - volume_api = cinder.volume.api.API() 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, updates) self.assertEqual('foobbar', volume.display_name) @@ -852,11 +870,8 @@ class VolumeTestCase(BaseVolumeTestCase): **self.volume_params) volume_id = volume['id'] self.volume.create_volume(self.context, volume_id, volume=volume) - - volume_api = cinder.volume.api.API() - # delete user metadata associated with the volume. - volume_api.delete_volume_metadata( + self.volume_api.delete_volume_metadata( self.context, volume, 'fake_key2', @@ -866,7 +881,7 @@ class VolumeTestCase(BaseVolumeTestCase): db.volume_metadata_get(self.context, volume_id)) # create image metadata associated with the volume. - result_meta = volume_api.update_volume_metadata( + result_meta = self.volume_api.update_volume_metadata( self.context, volume, test_meta1, @@ -876,7 +891,7 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertEqual(test_meta1, result_meta) # delete image metadata associated with the volume. - volume_api.delete_volume_metadata( + self.volume_api.delete_volume_metadata( self.context, volume, 'fake_key2', @@ -891,7 +906,7 @@ class VolumeTestCase(BaseVolumeTestCase): # delete volume metadata with invalid metadta type. self.assertRaises(exception.InvalidMetadataType, - volume_api.delete_volume_metadata, + self.volume_api.delete_volume_metadata, self.context, volume, 'fake_key1', @@ -904,9 +919,8 @@ class VolumeTestCase(BaseVolumeTestCase): volume = tests_utils.create_volume(self.context, metadata=test_meta1, **self.volume_params) volume['status'] = 'maintenance' - volume_api = cinder.volume.api.API() self.assertRaises(exception.InvalidVolume, - volume_api.delete_volume_metadata, + self.volume_api.delete_volume_metadata, self.context, volume, 'fake_key1', @@ -918,9 +932,8 @@ class VolumeTestCase(BaseVolumeTestCase): volume = tests_utils.create_volume(self.context, metadata=test_meta1, **self.volume_params) volume['status'] = 'maintenance' - volume_api = cinder.volume.api.API() self.assertRaises(exception.InvalidVolume, - volume_api.attach, + self.volume_api.attach, self.context, volume, None, None, None, None) diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 4209bf194f8..6eb5fb22b88 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -954,6 +954,15 @@ class API(base.Base): resource=volume) 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 def delete_volume_metadata(self, context, volume, key, meta_type=common.METADATA_TYPES.user): @@ -967,26 +976,28 @@ class API(base.Base): LOG.info(_LI("Delete volume metadata completed successfully."), resource=volume) - @wrap_check_policy - def update_volume_metadata(self, context, volume, - 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. - - """ + def _update_volume_metadata(self, context, volume, metadata, delete=False, + meta_type=common.METADATA_TYPES.user): if volume['status'] in ('maintenance', 'uploading'): msg = _('Updating volume metadata is not allowed for volumes in ' '%s status.') % volume['status'] LOG.info(msg, resource=volume) raise exception.InvalidVolume(reason=msg) utils.check_metadata_properties(metadata) - db_meta = self.db.volume_metadata_update(context, volume['id'], - metadata, - delete, - meta_type) + return self.db.volume_metadata_update(context, volume['id'], + metadata, 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 diff --git a/etc/cinder/policy.json b/etc/cinder/policy.json index 69d638d07f3..4c599cd95c2 100644 --- a/etc/cinder/policy.json +++ b/etc/cinder/policy.json @@ -10,6 +10,7 @@ "volume:get": "rule:admin_or_owner", "volume:get_all": "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:update_volume_metadata": "rule:admin_or_owner", "volume:get_volume_admin_metadata": "rule:admin_api", diff --git a/releasenotes/notes/create-update-rules-b46cf9c07c5a3966.yaml b/releasenotes/notes/create-update-rules-b46cf9c07c5a3966.yaml new file mode 100644 index 00000000000..8ccc773d0e5 --- /dev/null +++ b/releasenotes/notes/create-update-rules-b46cf9c07c5a3966.yaml @@ -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.