diff --git a/cinder/api/v3/attachments.py b/cinder/api/v3/attachments.py index 093443adf..d5f19428e 100644 --- a/cinder/api/v3/attachments.py +++ b/cinder/api/v3/attachments.py @@ -169,6 +169,8 @@ class AttachmentsController(wsgi.Controller): volume_ref, instance_uuid, connector=connector)) + except exception.NotAuthorized: + raise except exception.CinderException as ex: err_msg = _( "Unable to create attachment for volume (%s).") % ex.msg @@ -222,13 +224,14 @@ class AttachmentsController(wsgi.Controller): self.volume_api.attachment_update(context, attachment_ref, connector)) - + except exception.NotAuthorized: + raise except exception.CinderException as ex: err_msg = ( - _("Unable to create attachment for volume (%s).") % ex.msg) + _("Unable to update attachment.(%s).") % ex.msg) LOG.exception(err_msg) - except Exception as ex: - err_msg = _("Unable to create attachment for volume.") + except Exception: + err_msg = _("Unable to update the attachment.") LOG.exception(err_msg) finally: if err_msg: diff --git a/cinder/tests/unit/api/v3/test_attachments.py b/cinder/tests/unit/api/v3/test_attachments.py index c3f06fd4e..3cae68082 100644 --- a/cinder/tests/unit/api/v3/test_attachments.py +++ b/cinder/tests/unit/api/v3/test_attachments.py @@ -18,15 +18,18 @@ Tests for attachments Api. """ import ddt +import mock import webob from cinder.api.v3 import attachments as v3_attachments from cinder import context +from cinder import exception from cinder import objects from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_constants as fake from cinder.volume import api as volume_api +from cinder.volume import rpcapi as volume_rpcapi ATTACHMENTS_MICRO_VERSION = '3.27' @@ -70,9 +73,98 @@ class AttachmentsAPITestCase(test.TestCase): volume = objects.Volume(ctxt) volume.display_name = display_name volume.project_id = project_id + volume.status = 'available' + volume.attach_status = 'attached' volume.create() return volume + def test_create_attachment(self): + req = fakes.HTTPRequest.blank('/v3/%s/attachments' % + fake.PROJECT_ID, + version=ATTACHMENTS_MICRO_VERSION) + body = { + "attachment": + { + "connector": None, + "instance_uuid": fake.UUID1, + "volume_uuid": self.volume1.id + }, + } + + attachment = self.controller.create(req, body) + + self.assertEqual(self.volume1.id, + attachment['attachment']['volume_id']) + self.assertEqual(fake.UUID1, + attachment['attachment']['instance']) + + @mock.patch.object(volume_rpcapi.VolumeAPI, 'attachment_update') + def test_update_attachment(self, mock_update): + fake_connector = {'fake_key': 'fake_value'} + mock_update.return_value = fake_connector + req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' % + (fake.PROJECT_ID, self.attachment1.id), + version=ATTACHMENTS_MICRO_VERSION, + use_admin_context=True) + body = { + "attachment": + { + "connector": {'fake_key': 'fake_value'}, + }, + } + + attachment = self.controller.update(req, self.attachment1.id, body) + + self.assertEqual(fake_connector, + attachment['attachment']['connection_info']) + self.assertEqual(fake.UUID1, attachment['attachment']['instance']) + + @mock.patch.object(objects.VolumeAttachment, 'get_by_id') + def test_attachment_operations_not_authorized(self, mock_get): + mock_get.return_value = {'project_id': fake.PROJECT2_ID} + req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' % + (fake.PROJECT_ID, self.attachment1.id), + version=ATTACHMENTS_MICRO_VERSION, + use_admin_context=False) + body = { + "attachment": + { + "connector": {'fake_key': 'fake_value'}, + }, + } + self.assertRaises(exception.NotAuthorized, + self.controller.update, req, + self.attachment1.id, body) + self.assertRaises(exception.NotAuthorized, + self.controller.delete, req, + self.attachment1.id) + + @ddt.data('reserved', 'attached') + @mock.patch.object(volume_rpcapi.VolumeAPI, 'attachment_delete') + def test_delete_attachment(self, status, mock_delete): + volume1 = self._create_volume(display_name='fake_volume_1', + project_id=fake.PROJECT_ID) + attachment = self._create_attachement( + volume_uuid=volume1.id, instance_uuid=fake.UUID1, + attach_status=status) + req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' % + (fake.PROJECT_ID, attachment.id), + version=ATTACHMENTS_MICRO_VERSION, + use_admin_context=True) + + self.controller.delete(req, attachment.id) + + volume2 = objects.Volume.get_by_id(self.ctxt, volume1.id) + if status == 'reserved': + self.assertEqual('detached', volume2.attach_status) + self.assertRaises( + exception.VolumeAttachmentNotFound, + objects.VolumeAttachment.get_by_id, self.ctxt, attachment.id) + else: + self.assertEqual('attached', volume2.attach_status) + mock_delete.assert_called_once_with(req.environ['cinder.context'], + attachment.id, mock.ANY) + def _create_attachement(self, ctxt=None, volume_uuid=None, instance_uuid=None, mountpoint=None, attach_time=None, detach_time=None, diff --git a/cinder/tests/unit/policy.json b/cinder/tests/unit/policy.json index 8cb8da205..d944799b4 100644 --- a/cinder/tests/unit/policy.json +++ b/cinder/tests/unit/policy.json @@ -103,6 +103,10 @@ "backup:update": "rule:admin_or_owner", "backup:backup_project_attribute": "rule:admin_api", + "volume:attachment_create": "", + "volume:attachment_update": "rule:admin_or_owner", + "volume:attachment_delete": "rule:admin_or_owner", + "consistencygroup:create" : "", "consistencygroup:delete": "", "consistencygroup:update": "", diff --git a/etc/cinder/policy.json b/etc/cinder/policy.json index 6a651220e..a32ad1ccd 100644 --- a/etc/cinder/policy.json +++ b/etc/cinder/policy.json @@ -93,6 +93,10 @@ "backup:update": "rule:admin_or_owner", "backup:backup_project_attribute": "rule:admin_api", + "volume:attachment_create": "", + "volume:attachment_update": "rule:admin_or_owner", + "volume:attachment_delete": "rule:admin_or_owner", + "snapshot_extension:snapshot_actions:update_snapshot_status": "", "snapshot_extension:snapshot_manage": "rule:admin_api", "snapshot_extension:snapshot_unmanage": "rule:admin_api",