Merge "api: Add schema for share unmanage action"

manila/api/schemas/shares.py

@@ -88,6 +88,17 @@ shrink_request_body_v27['properties']['shrink'] = (
 )
 shrink_request_body_v27['required'] = ['shrink']
 
+unmanage_request_body = {
+    'type': 'object',
+    'properties': {
+        # TODO(stephenfin): We should restrict this to 'null' in a future
+        # microversion
+        'unmanage': {},
+    },
+    'required': ['unmanage'],
+    'additionalProperties': False,
+}
+
 soft_delete_response_body = {'type': 'null'}
 
 restore_response_body = {'type': 'null'}
@@ -95,3 +106,5 @@ restore_response_body = {'type': 'null'}
 extend_response_body = {'type': 'null'}
 
 shrink_response_body = {'type': 'null'}
+
+unmanage_response_body = {'type': 'null'}

manila/api/v2/shares.py

@@ -582,32 +582,27 @@ class ShareController(wsgi.Controller,
         """Shrink size of a share."""
         return self._shrink(req, id, body)
 
-    @wsgi.Controller.api_version('2.7', '2.7')
+    @wsgi.Controller.api_version('2.7')
     def manage(self, req, body):
-        body.get('share', {}).pop('is_public', None)
-        detail = self._manage(req, body, allow_dhss_true=False)
+        if req.api_version_request < api_version.APIVersionRequest('2.8'):
+            body.get('share', {}).pop('is_public', None)
+
+        allow_dhss_true = False
+        if req.api_version_request >= api_version.APIVersionRequest('2.49'):
+            allow_dhss_true = True
+
+        detail = self._manage(req, body, allow_dhss_true=allow_dhss_true)
         return detail
 
-    @wsgi.Controller.api_version("2.8", "2.48")  # noqa
-    def manage(self, req, body):  # pylint: disable=function-redefined  # noqa F811
-        detail = self._manage(req, body, allow_dhss_true=False)
-        return detail
-
-    @wsgi.Controller.api_version("2.49")  # noqa
-    def manage(self, req, body):  # pylint: disable=function-redefined  # noqa F811
-        detail = self._manage(req, body, allow_dhss_true=True)
-        return detail
-
-    @wsgi.Controller.api_version('2.7', '2.48')
+    @wsgi.Controller.api_version('2.7')
     @wsgi.action('unmanage')
-    def unmanage(self, req, id, body=None):
-        return self._unmanage(req, id, body, allow_dhss_true=False)
-
-    @wsgi.Controller.api_version('2.49')  # noqa
-    @wsgi.action('unmanage')
-    def unmanage(self, req, id,  # pylint: disable=function-redefined # noqa F811
-                 body=None):
-        return self._unmanage(req, id, body, allow_dhss_true=True)
+    @validation.request_body_schema(schema.unmanage_request_body)
+    @validation.response_body_schema(schema.unmanage_response_body)
+    def unmanage(self, req, id, body):
+        allow_dhss_true = False
+        if req.api_version_request >= api_version.APIVersionRequest('2.49'):
+            allow_dhss_true = True
+        return self._unmanage(req, id, body, allow_dhss_true=allow_dhss_true)
 
     @wsgi.Controller.api_version('2.27')
     @wsgi.action('revert')

manila/tests/api/v2/test_shares.py

@@ -2990,7 +2990,8 @@ class ShareUnmanageTest(test.TestCase):
             self.controller.share_api.db, 'share_snapshot_get_all_for_share',
             mock.Mock(return_value=[]))
 
-        actual_result = self.controller.unmanage(self.request, share['id'])
+        actual_result = self.controller.unmanage(
+            self.request, share['id'], body={'unmanage': None})
 
         self.assertEqual(202, actual_result.status_int)
         (self.controller.share_api.db.share_snapshot_get_all_for_share.
@@ -3001,16 +3002,18 @@ class ShareUnmanageTest(test.TestCase):
         share_api.API.unmanage.assert_called_once_with(
             self.request.environ['manila.context'], share)
 
-    def test__unmanage(self):
-        body = {}
+    def test_unmanage_v249(self):
+        body = {'unmanage': None}
         req = fakes.HTTPRequest.blank('/v2/fake/shares/1/action',
                                       use_admin_context=False,
                                       version='2.49')
         share = dict(status=constants.STATUS_AVAILABLE, id='foo_id',
                      instance={})
-        mock_unmanage = self.mock_object(self.controller, '_unmanage')
+        mock_unmanage = self.mock_object(
+            self.controller, '_unmanage',
+            mock.Mock(return_value=None))
 
-        self.controller.unmanage(req, share['id'], body)
+        self.controller.unmanage(req, share['id'], body=body)
 
         mock_unmanage.assert_called_once_with(
             req, share['id'], body, allow_dhss_true=True
@@ -3030,7 +3033,8 @@ class ShareUnmanageTest(test.TestCase):
 
         self.assertRaises(
             webob.exc.HTTPForbidden,
-            self.controller.unmanage, self.request, share['id'])
+            self.controller.unmanage, self.request, share['id'],
+            body={'unmanage': None})
 
         self.assertFalse(self.controller.share_api.unmanage.called)
         (self.controller.share_api.db.share_snapshot_get_all_for_share.
@@ -3047,7 +3051,8 @@ class ShareUnmanageTest(test.TestCase):
 
         self.assertRaises(
             webob.exc.HTTPForbidden,
-            self.controller.unmanage, self.request, share['id'])
+            self.controller.unmanage, self.request, share['id'],
+            body={'unmanage': None})
 
         self.controller.share_api.get.assert_called_once_with(
             self.request.environ['manila.context'], share['id'])
@@ -3061,7 +3066,8 @@ class ShareUnmanageTest(test.TestCase):
 
         self.assertRaises(
             webob.exc.HTTPForbidden,
-            self.controller.unmanage, self.request, share['id'])
+            self.controller.unmanage, self.request, share['id'],
+            body={'unmanage': None})
 
         self.controller.share_api.get.assert_called_once_with(
             self.request.environ['manila.context'], share['id'])
@@ -3071,9 +3077,10 @@ class ShareUnmanageTest(test.TestCase):
             side_effect=exception.NotFound))
         self.mock_object(share_api.API, 'unmanage', mock.Mock())
 
-        self.assertRaises(webob.exc.HTTPNotFound,
-                          self.controller.unmanage,
-                          self.request, self.share_id)
+        self.assertRaises(
+            webob.exc.HTTPNotFound,
+            self.controller.unmanage, self.request, self.share_id,
+            body={'unmanage': None})
 
     @ddt.data(exception.InvalidShare(reason="fake"),
               exception.PolicyNotAuthorized(action="fake"),)
@@ -3084,29 +3091,28 @@ class ShareUnmanageTest(test.TestCase):
         self.mock_object(share_api.API, 'unmanage', mock.Mock(
             side_effect=side_effect))
 
-        self.assertRaises(webob.exc.HTTPForbidden,
-                          self.controller.unmanage,
-                          self.request, self.share_id)
+        self.assertRaises(
+            webob.exc.HTTPForbidden,
+            self.controller.unmanage, self.request, self.share_id,
+            body={'unmanage': None})
 
     def test_wrong_permissions(self):
         share_id = 'fake'
         req = fakes.HTTPRequest.blank('/v2/fake/share/%s/unmanage' % share_id,
                                       use_admin_context=False, version='2.7')
 
-        self.assertRaises(webob.exc.HTTPForbidden,
-                          self.controller.unmanage,
-                          req,
-                          share_id)
+        self.assertRaises(
+            webob.exc.HTTPForbidden,
+            self.controller.unmanage, req, share_id, body={'unmanage': None})
 
     def test_unsupported_version(self):
         share_id = 'fake'
         req = fakes.HTTPRequest.blank('/v2/fake/share/%s/unmanage' % share_id,
                                       use_admin_context=False, version='2.6')
 
-        self.assertRaises(exception.VersionNotFoundForAPIMethod,
-                          self.controller.unmanage,
-                          req,
-                          share_id)
+        self.assertRaises(
+            exception.VersionNotFoundForAPIMethod,
+            self.controller.unmanage, req, share_id, body={'unmanage': None})
 
 
 def get_fake_manage_body(export_path='/fake', service_host='fake@host#POOL',