diff --git a/cinder/tests/unit/api/v2/test_volume_metadata.py b/cinder/tests/unit/api/v2/test_volume_metadata.py index c86d54ded55..5d32f73b943 100644 --- a/cinder/tests/unit/api/v2/test_volume_metadata.py +++ b/cinder/tests/unit/api/v2/test_volume_metadata.py @@ -100,7 +100,9 @@ def fake_max_volume_metadata(): def get_volume(*args, **kwargs): vol = {'name': 'fake', - 'metadata': {}} + 'metadata': {}, + 'project_id': fake.PROJECT_ID + } return fake_volume.fake_volume_obj(args[0], **vol) @@ -136,7 +138,7 @@ class VolumeMetaDataTest(test.TestCase): "availability_zone": "zone1:host1", "metadata": {}} body = {"volume": vol} - req = fakes.HTTPRequest.blank('/v2/volumes') + req = fakes.HTTPRequest.blank('/v2/%s/volumes' % fake.PROJECT_ID) self.volume_controller.create(req, body=body) def test_index(self): diff --git a/cinder/tests/unit/api/v3/test_volume_metadata.py b/cinder/tests/unit/api/v3/test_volume_metadata.py index 8fdcebb003b..97431d4e63f 100644 --- a/cinder/tests/unit/api/v3/test_volume_metadata.py +++ b/cinder/tests/unit/api/v3/test_volume_metadata.py @@ -108,7 +108,8 @@ def stub_max_volume_metadata(): def get_volume(*args, **kwargs): vol = {'name': 'fake', - 'metadata': {}} + 'metadata': {}, + 'project_id': fake.PROJECT_ID} return fake_volume.fake_volume_obj(args[0], **vol) @@ -162,7 +163,7 @@ class VolumeMetaDataTest(test.TestCase): "availability_zone": "zone1:host1", "metadata": {}} body = {"volume": vol} - req = fakes.HTTPRequest.blank('/v2/volumes') + req = fakes.HTTPRequest.blank('/v3/%s/volumes' % fake.PROJECT_ID) self.volume_controller.create(req, body=body) def test_index(self): diff --git a/cinder/tests/unit/api/v3/test_volume_protection.py b/cinder/tests/unit/api/v3/test_volume_protection.py index 547aaf837ff..48bb5332d45 100644 --- a/cinder/tests/unit/api/v3/test_volume_protection.py +++ b/cinder/tests/unit/api/v3/test_volume_protection.py @@ -60,7 +60,8 @@ class VolumeProtectionTests(test.TestCase): fakes.wsgi_app(fake_auth_context=context) ) - def _create_fake_volume(self, context, status=None, attach_status=None): + def _create_fake_volume(self, context, status=None, attach_status=None, + metadata=None): vol = { 'display_name': 'fake_volume1', 'status': 'available', @@ -70,6 +71,8 @@ class VolumeProtectionTests(test.TestCase): vol['status'] = status if attach_status: vol['attach_status'] = attach_status + if metadata: + vol['metadata'] = metadata volume = objects.Volume(context=context, **vol) volume.create() return volume @@ -655,3 +658,175 @@ class VolumeProtectionTests(test.TestCase): response = self._get_request_response(non_owner_context, path, 'POST', body=body) self.assertEqual(http_client.FORBIDDEN, response.status_int) + + def test_admin_can_create_metadata(self): + admin_context = self.admin_context + + volume = self._create_fake_volume(admin_context, metadata={"k": "v"}) + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % { + 'project_id': admin_context.project_id, 'volume_id': volume.id + } + + body = {"metadata": {"k1": "v1"}} + response = self._get_request_response(admin_context, path, 'POST', + body=body) + self.assertEqual(http_client.OK, response.status_int) + + def test_admin_can_get_metadata(self): + admin_context = self.admin_context + + volume = self._create_fake_volume(admin_context, metadata={"k": "v"}) + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % { + 'project_id': admin_context.project_id, 'volume_id': volume.id + } + + response = self._get_request_response(admin_context, path, 'GET') + self.assertEqual(http_client.OK, response.status_int) + res_meta = response.json_body['metadata'] + self.assertIn('k', res_meta) + self.assertEqual('v', res_meta['k']) + + def test_admin_can_update_metadata(self): + admin_context = self.admin_context + + volume = self._create_fake_volume(admin_context, metadata={"k": "v"}) + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % { + 'project_id': admin_context.project_id, 'volume_id': volume.id + } + + body = {"metadata": {"k": "v2"}} + response = self._get_request_response(admin_context, path, 'PUT', + body=body) + self.assertEqual(http_client.OK, response.status_int) + res_meta = response.json_body['metadata'] + self.assertIn('k', res_meta) + self.assertEqual('v2', res_meta['k']) + + def test_admin_can_delete_metadata(self): + admin_context = self.admin_context + + volume = self._create_fake_volume(admin_context, metadata={"k": "v"}) + + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata/%(key)s' % { + 'project_id': admin_context.project_id, 'volume_id': volume.id, + 'key': 'k' + } + response = self._get_request_response(admin_context, path, 'DELETE') + self.assertEqual(http_client.OK, response.status_int) + + def test_owner_can_create_metadata(self): + user_context = self.user_context + + volume = self._create_fake_volume(user_context, metadata={"k": "v"}) + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % { + 'project_id': user_context.project_id, 'volume_id': volume.id + } + + body = {"metadata": {"k1": "v1"}} + response = self._get_request_response(user_context, path, 'POST', + body=body) + self.assertEqual(http_client.OK, response.status_int) + + def test_owner_can_get_metadata(self): + user_context = self.user_context + + volume = self._create_fake_volume(user_context, metadata={"k": "v"}) + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % { + 'project_id': user_context.project_id, 'volume_id': volume.id + } + + response = self._get_request_response(user_context, path, 'GET') + self.assertEqual(http_client.OK, response.status_int) + res_meta = response.json_body['metadata'] + self.assertIn('k', res_meta) + self.assertEqual('v', res_meta['k']) + + def test_owner_can_update_metadata(self): + user_context = self.user_context + + volume = self._create_fake_volume(user_context, metadata={"k": "v"}) + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % { + 'project_id': user_context.project_id, 'volume_id': volume.id + } + + body = {"metadata": {"k": "v2"}} + response = self._get_request_response(user_context, path, 'PUT', + body=body) + self.assertEqual(http_client.OK, response.status_int) + res_meta = response.json_body['metadata'] + self.assertIn('k', res_meta) + self.assertEqual('v2', res_meta['k']) + + def test_owner_can_delete_metadata(self): + user_context = self.user_context + + volume = self._create_fake_volume(user_context, metadata={"k": "v"}) + + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata/%(key)s' % { + 'project_id': user_context.project_id, 'volume_id': volume.id, + 'key': 'k' + } + response = self._get_request_response(user_context, path, 'DELETE') + self.assertEqual(http_client.OK, response.status_int) + + @mock.patch.object(volume_api.API, 'get') + def test_owner_cannot_create_metadata_for_others(self, mock_volume): + owner_context = self.user_context + non_owner_context = self.other_user_context + + volume = self._create_fake_volume(owner_context, metadata={"k": "v"}) + mock_volume.return_value = volume + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % { + 'project_id': non_owner_context.project_id, 'volume_id': volume.id + } + + body = {"metadata": {"k1": "v1"}} + response = self._get_request_response(non_owner_context, path, 'POST', + body=body) + self.assertEqual(http_client.FORBIDDEN, response.status_int) + + @mock.patch.object(volume_api.API, 'get') + def test_owner_cannot_get_metadata_for_others(self, mock_volume): + owner_context = self.user_context + non_owner_context = self.other_user_context + + volume = self._create_fake_volume(owner_context, metadata={"k": "v"}) + mock_volume.return_value = volume + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % { + 'project_id': non_owner_context.project_id, 'volume_id': volume.id + } + + response = self._get_request_response(non_owner_context, path, 'GET') + self.assertEqual(http_client.FORBIDDEN, response.status_int) + + @mock.patch.object(volume_api.API, 'get') + def test_owner_cannot_update_metadata_for_others(self, mock_volume): + owner_context = self.user_context + non_owner_context = self.other_user_context + + volume = self._create_fake_volume(owner_context, metadata={"k": "v"}) + mock_volume.return_value = volume + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata' % { + 'project_id': non_owner_context.project_id, 'volume_id': volume.id + } + + body = {"metadata": {"k": "v2"}} + response = self._get_request_response(non_owner_context, path, 'PUT', + body=body) + self.assertEqual(http_client.FORBIDDEN, response.status_int) + + @mock.patch.object(volume_api.API, 'get') + def test_owner_cannot_delete_metadata_for_others(self, mock_volume): + owner_context = self.user_context + non_owner_context = self.other_user_context + + volume = self._create_fake_volume(owner_context, metadata={"k": "v"}) + mock_volume.return_value = volume + path = '/v3/%(project_id)s/volumes/%(volume_id)s/metadata/%(key)s' % { + 'project_id': non_owner_context.project_id, + 'volume_id': volume.id, + 'key': 'k' + } + response = self._get_request_response(non_owner_context, path, + 'DELETE') + self.assertEqual(http_client.FORBIDDEN, response.status_int) diff --git a/cinder/tests/unit/policy.json b/cinder/tests/unit/policy.json index bebf0ae94ae..6edf75ff0d8 100644 --- a/cinder/tests/unit/policy.json +++ b/cinder/tests/unit/policy.json @@ -2,11 +2,6 @@ "admin_api": "is_admin:True", "admin_or_owner": "is_admin:True or project_id:%(project_id)s", - "volume:get_volume_metadata": "", - "volume:get_volume_image_metadata": "", - "volume:create_volume_metadata": "", - "volume:delete_volume_metadata": "", - "volume:update_volume_metadata": "", "volume:create_snapshot": "", "volume:delete_snapshot": "", "volume:get_snapshot": "", @@ -29,7 +24,6 @@ "volume_extension:types_extra_specs:update": "", "volume_extension:volume_type_access": "", "volume_extension:extended_snapshot_attributes": "", - "volume_extension:volume_image_metadata": "", "volume_extension:services:index": "", "volume_extension:services:update" : "rule:admin_api",