[BugFix] Add method policy in attachment APIs

This patch adds policy in attachment APIs, also
add related testcases in API unit testcases.

Closes-Bug: #1680836

Change-Id: I310fec39719ead39f26d97ee4ba95187e1fb2069

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:
+        except Exception:
-            err_msg = _("Unable to create attachment for volume.")
+            err_msg = _("Unable to update the attachment.")
             LOG.exception(err_msg)
         finally:
             if err_msg:

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,

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": "",

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",