V3 jsonschema validation: Clusters

This patch adds jsonschema validation for below Clusters API's
* PUT  /v3/{project_id}/clusters/action

Change-Id: I7ba0d9fb5292d0076fc99004cf1326d7f3fc86ee
Partial-Implements: bp json-schema-validation

cinder/api/schemas/clusters.py

@@ -0,0 +1,47 @@
+# Copyright (C) 2018 NTT DATA
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+Schema for V3 Clusters API.
+
+"""
+
+
+from cinder.api.validation import parameter_types
+
+
+disable_cluster = {
+    'type': 'object',
+    'properties': {
+        'name': parameter_types.name,
+        'binary': parameter_types.nullable_string,
+        'disabled_reason': {
+            'type': ['string', 'null'], 'format': 'disabled_reason'
+        }
+    },
+    'required': ['name'],
+    'additionalProperties': False,
+}
+
+
+enable_cluster = {
+    'type': 'object',
+    'properties': {
+        'name': parameter_types.name,
+        'binary': parameter_types.nullable_string
+    },
+    'required': ['name'],
+    'additionalProperties': False,
+}

cinder/api/v3/clusters.py

@@ -15,7 +15,9 @@
 
 from cinder.api import microversions as mv
 from cinder.api.openstack import wsgi
+from cinder.api.schemas import clusters as cluster
 from cinder.api.v3.views import clusters as clusters_view
+from cinder.api import validation
 from cinder.common import constants
 from cinder import exception
 from cinder.i18n import _
@@ -102,15 +104,11 @@ class ClusterController(wsgi.Controller):
             raise exception.NotFound(message=_("Unknown action"))
 
         disabled = id != 'enable'
-        disabled_reason = self._get_disabled_reason(body) if disabled else None
+        disabled_reason = self._disable_cluster(
+            req, body=body) if disabled else self._enable_cluster(
+            req, body=body)
 
-        if not disabled and disabled_reason:
+        name = body['name']
-            msg = _("Unexpected 'disabled_reason' found on enable request.")
-            raise exception.InvalidInput(reason=msg)
-
-        name = body.get('name')
-        if not name:
-            raise exception.MissingRequired(element='name')
 
         binary = body.get('binary', constants.VOLUME_BINARY)
 
@@ -129,15 +127,17 @@ class ClusterController(wsgi.Controller):
 
         return ret_val
 
-    def _get_disabled_reason(self, body):
+    @validation.schema(cluster.disable_cluster)
+    def _disable_cluster(self, req, body):
         reason = body.get('disabled_reason')
         if reason:
-            # Let wsgi handle InvalidInput exception
             reason = reason.strip()
-            utils.check_string_length(reason, 'Disabled reason', min_length=1,
-                                      max_length=255)
         return reason
 
+    @validation.schema(cluster.enable_cluster)
+    def _enable_cluster(self, req, body):
+        pass
+
 
 def create_resource():
     return wsgi.Resource(ClusterController())

cinder/api/validation/validators.py

@@ -26,10 +26,12 @@ from jsonschema import exceptions as jsonschema_exc
 from oslo_utils import timeutils
 from oslo_utils import uuidutils
 import six
+import webob.exc
 
 from cinder import exception
 from cinder.i18n import _
 from cinder.objects import fields as c_fields
+from cinder import utils
 
 
 def _soft_validate_additional_properties(
@@ -85,6 +87,34 @@ def _soft_validate_additional_properties(
             del param_value[prop]
 
 
+def _validate_string_length(value, entity_name, mandatory=False,
+                            min_length=0, max_length=None,
+                            remove_whitespaces=False):
+    """Check the length of specified string.
+
+    :param value: the value of the string
+    :param entity_name: the name of the string
+    :mandatory: string is mandatory or not
+    :param min_length: the min_length of the string
+    :param max_length: the max_length of the string
+    :param remove_whitespaces: True if trimming whitespaces is needed
+                                   else False
+    """
+    if not mandatory and not value:
+        return True
+
+    if mandatory and not value:
+        msg = _("The '%s' can not be None.") % entity_name
+        raise webob.exc.HTTPBadRequest(explanation=msg)
+
+    if remove_whitespaces:
+        value = value.strip()
+
+    utils.check_string_length(value, entity_name,
+                              min_length=min_length,
+                              max_length=max_length)
+
+
 @jsonschema.FormatChecker.cls_checks('date-time')
 def _validate_datetime_format(param_value):
     try:
@@ -157,6 +187,14 @@ def _validate_base64_format(instance):
     return True
 
 
+@jsonschema.FormatChecker.cls_checks('disabled_reason')
+def _validate_disabled_reason(param_value):
+    _validate_string_length(param_value, 'disabled_reason',
+                            mandatory=False, min_length=1, max_length=255,
+                            remove_whitespaces=True)
+    return True
+
+
 class FormatChecker(jsonschema.FormatChecker):
     """A FormatChecker can output the message from cause exception
 

cinder/tests/unit/api/v3/test_cluster.py

@@ -218,15 +218,15 @@ class ClustersTestCase(test.TestCase):
     def test_show(self, get_mock):
         req = FakeRequest()
         expected = {'cluster': self._get_expected()[0]}
-        cluster = self.controller.show(req, mock.sentinel.name,
+        cluster = self.controller.show(req, 'cluster_name',
-                                       mock.sentinel.binary)
+                                       'cinder-volume')
         self.assertEqual(expected, cluster)
         get_mock.assert_called_once_with(
             req.environ['cinder.context'],
             None,
             services_summary=True,
-            name=mock.sentinel.name,
+            name='cluster_name',
-            binary=mock.sentinel.binary)
+            binary='cinder-volume')
 
     def test_show_unauthorized(self):
         req = FakeRequest(is_admin=False)
@@ -249,13 +249,13 @@ class ClustersTestCase(test.TestCase):
                                 'status': 'enabled',
                                 'disabled_reason': None}}
         res = self.controller.update(req, 'enable',
-                                     {'name': mock.sentinel.name,
+                                     body={'name': 'cluster_name',
-                                      'binary': mock.sentinel.binary})
+                                           'binary': 'cinder-volume'})
         self.assertEqual(expected, res)
         ctxt = req.environ['cinder.context']
         get_mock.assert_called_once_with(ctxt,
-                                         None, binary=mock.sentinel.binary,
+                                         None, binary='cinder-volume',
-                                         name=mock.sentinel.name)
+                                         name='cluster_name')
         update_mock.assert_called_once_with(ctxt, get_mock.return_value.id,
                                             {'disabled': False,
                                              'disabled_reason': None})
@@ -272,40 +272,55 @@ class ClustersTestCase(test.TestCase):
                                 'status': 'disabled',
                                 'disabled_reason': disabled_reason}}
         res = self.controller.update(req, 'disable',
-                                     {'name': mock.sentinel.name,
+                                     body={'name': 'cluster_name',
-                                      'binary': mock.sentinel.binary,
+                                           'binary': 'cinder-volume',
-                                      'disabled_reason': disabled_reason})
+                                           'disabled_reason': disabled_reason})
         self.assertEqual(expected, res)
         ctxt = req.environ['cinder.context']
         get_mock.assert_called_once_with(ctxt,
-                                         None, binary=mock.sentinel.binary,
+                                         None, binary='cinder-volume',
-                                         name=mock.sentinel.name)
+                                         name='cluster_name')
         update_mock.assert_called_once_with(
             ctxt, get_mock.return_value.id,
             {'disabled': True, 'disabled_reason': disabled_reason})
 
     def test_update_wrong_action(self):
         req = FakeRequest()
-        self.assertRaises(exception.NotFound, self.controller.update, req,
+        self.assertRaises(exception.NotFound, self.controller.update,
-                          'action', {})
+                          req, 'action', body={'name': 'cluster_name'})
 
     @ddt.data('enable', 'disable')
     def test_update_missing_name(self, action):
         req = FakeRequest()
-        self.assertRaises(exception.MissingRequired, self.controller.update,
+        self.assertRaises(exception.ValidationError, self.controller.update,
-                          req, action, {'binary': mock.sentinel.binary})
+                          req, action, body={'binary': 'cinder-volume'})
 
-    def test_update_wrong_disabled_reason(self):
+    def test_update_with_binary_more_than_255_characters(self):
         req = FakeRequest()
-        self.assertRaises(exception.InvalidInput, self.controller.update, req,
+        self.assertRaises(exception.ValidationError, self.controller.update,
-                          'disable', {'name': mock.sentinel.name,
+                          req, 'enable', body={'name': 'cluster_name',
-                                      'disabled_reason': '   '})
+                                               'binary': 'a' * 256})
+
+    def test_update_with_name_more_than_255_characters(self):
+        req = FakeRequest()
+        self.assertRaises(exception.ValidationError, self.controller.update,
+                          req, 'enable', body={'name': 'a' * 256,
+                                               'binary': 'cinder-volume'})
+
+    @ddt.data('a' * 256, '   ')
+    def test_update_wrong_disabled_reason(self, disabled_reason):
+        req = FakeRequest()
+        self.assertRaises(exception.InvalidInput, self.controller.update,
+                          req, 'disable',
+                          body={'name': 'cluster_name',
+                                'disabled_reason': disabled_reason})
 
     @ddt.data('enable', 'disable')
     def test_update_unauthorized(self, action):
         req = FakeRequest(is_admin=False)
         self.assertRaises(exception.PolicyNotAuthorized,
-                          self.controller.update, req, action, {})
+                          self.controller.update, req, action,
+                          body={'name': 'fake_name'})
 
     @ddt.data('enable', 'disable')
     def test_update_wrong_version(self, action):