Enforce integer API parameter checking

This patch is adding more checking to integer type parameters at the API
layer. It also adds a missing test case for the 'limit' parameter used in
event indexing.

Change-Id: If4588c9b9ded38db99b8727a1cfb5c88d6fa18de

heat/api/openstack/v1/resources.py

@@ -15,6 +15,7 @@ import itertools
 
 from heat.api.openstack.v1 import util
 from heat.common import identifier
+from heat.common import param_utils
 from heat.common import serializers
 from heat.common import wsgi
 from heat.rpc import api as rpc_api
@@ -81,9 +82,11 @@ class ResourceController(object):
         Lists summary information for all resources
         """
 
-        # Though nested_depth is defaulted in the RPC API, this prevents empty
+        nested_depth = 0
-        # strings from being passed, thus breaking the code in the engine.
+        key = rpc_api.PARAM_NESTED_DEPTH
-        nested_depth = int(req.params.get('nested_depth') or 0)
+        if key in req.params:
+            nested_depth = param_utils.extract_int(key, req.params[key])
+
         res_list = self.rpc_client.list_stack_resources(req.context,
                                                         identity,
                                                         nested_depth)

heat/api/openstack/v1/stacks.py

@@ -199,6 +199,11 @@ class StackController(object):
             params[rpc_api.PARAM_SHOW_NESTED] = param_utils.extract_bool(
                 params[rpc_api.PARAM_SHOW_NESTED])
             show_nested = params[rpc_api.PARAM_SHOW_NESTED]
+
+        key = rpc_api.PARAM_LIMIT
+        if key in params:
+            params[key] = param_utils.extract_int(key, params[key])
+
         # get the with_count value, if invalid, raise ValueError
         with_count = False
         if req.params.get('with_count'):
@@ -282,12 +287,17 @@ class StackController(object):
         """
         data = InstantiationData(body)
 
+        args = data.args()
+        key = rpc_api.PARAM_TIMEOUT
+        if key in args:
+            args[key] = param_utils.extract_int(key, args[key])
+
         result = self.rpc_client.create_stack(req.context,
                                               data.stack_name(),
                                               data.template(),
                                               data.environment(),
                                               data.files(),
-                                              data.args())
+                                              args)
 
         formatted_stack = stacks_view.format_stack(
             req,
@@ -354,12 +364,17 @@ class StackController(object):
         """
         data = InstantiationData(body)
 
+        args = data.args()
+        key = rpc_api.PARAM_TIMEOUT
+        if key in args:
+            args[key] = param_utils.extract_int(key, args[key])
+
         self.rpc_client.update_stack(req.context,
                                      identity,
                                      data.template(),
                                      data.environment(),
                                      data.files(),
-                                     data.args())
+                                     args)
 
         raise exc.HTTPAccepted()
 
@@ -370,12 +385,18 @@ class StackController(object):
         Add the flag patch to the args so the engine code can distinguish
         """
         data = InstantiationData(body, patch=True)
+
+        args = data.args()
+        key = rpc_api.PARAM_TIMEOUT
+        if key in args:
+            args[key] = param_utils.extract_int(key, args[key])
+
         self.rpc_client.update_stack(req.context,
                                      identity,
                                      data.template(),
                                      data.environment(),
                                      data.files(),
-                                     data.args())
+                                     args)
 
         raise exc.HTTPAccepted()
 

heat/rpc/api.py

@@ -17,10 +17,12 @@ PARAM_KEYS = (
     PARAM_TIMEOUT, PARAM_DISABLE_ROLLBACK, PARAM_ADOPT_STACK_DATA,
     PARAM_SHOW_DELETED, PARAM_SHOW_NESTED, PARAM_EXISTING,
     PARAM_CLEAR_PARAMETERS, PARAM_GLOBAL_TENANT, PARAM_LIMIT,
+    PARAM_NESTED_DEPTH,
 ) = (
     'timeout_mins', 'disable_rollback', 'adopt_stack_data',
     'show_deleted', 'show_nested', 'existing',
     'clear_parameters', 'global_tenant', 'limit',
+    'nested_depth',
 )
 
 STACK_KEYS = (

heat/tests/test_api_openstack_v1.py

@@ -393,7 +393,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
     def test_index_whitelists_pagination_params(self, mock_call, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'index', True)
         params = {
-            'limit': 'fake limit',
+            'limit': 10,
             'sort_keys': 'fake sort keys',
             'marker': 'fake marker',
             'sort_dir': 'fake sort dir',
@@ -415,6 +415,19 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
         self.assertIn('tenant_safe', engine_args)
         self.assertNotIn('balrog', engine_args)
 
+    @mock.patch.object(rpc_client.EngineClient, 'call')
+    def test_index_limit_not_int(self, mock_call, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'index', True)
+        params = {'limit': 'not-an-int'}
+        req = self._get('/stacks', params=params)
+
+        ex = self.assertRaises(ValueError,
+                               self.controller.index, req,
+                               tenant_id=self.tenant)
+        self.assertEqual("Only integer is acceptable by 'limit'.",
+                         six.text_type(ex))
+        self.assertFalse(mock_call.called)
+
     @mock.patch.object(rpc_client.EngineClient, 'call')
     def test_index_whitelist_filter_params(self, mock_call, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'index', True)
@@ -802,6 +815,27 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
         self.assertEqual(expected, response)
         self.m.VerifyAll()
 
+    def test_adopt_timeout_not_int(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'create', True)
+        identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1')
+
+        body = {'template': None,
+                'stack_name': identity.stack_name,
+                'parameters': {},
+                'timeout_mins': 'not-an-int',
+                'adopt_stack_data': 'does not matter'}
+
+        req = self._post('/stacks', json.dumps(body))
+
+        mock_call = self.patchobject(rpc_client.EngineClient, 'call')
+        ex = self.assertRaises(ValueError,
+                               self.controller.create, req,
+                               tenant_id=self.tenant, body=body)
+
+        self.assertEqual("Only integer is acceptable by 'timeout_mins'.",
+                         six.text_type(ex))
+        self.assertFalse(mock_call.called)
+
     def test_adopt_error(self, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'create', True)
         identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1')
@@ -993,6 +1027,27 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
         self.assertEqual('StackExists', resp.json['error']['type'])
         self.m.VerifyAll()
 
+    def test_create_timeout_not_int(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'create', True)
+        stack_name = "wordpress"
+        template = {u'Foo': u'bar'}
+        parameters = {u'InstanceType': u'm1.xlarge'}
+        body = {'template': template,
+                'stack_name': stack_name,
+                'parameters': parameters,
+                'timeout_mins': 'not-an-int'}
+
+        req = self._post('/stacks', json.dumps(body))
+
+        mock_call = self.patchobject(rpc_client.EngineClient, 'call')
+        ex = self.assertRaises(ValueError,
+                               self.controller.create, req,
+                               tenant_id=self.tenant, body=body)
+
+        self.assertEqual("Only integer is acceptable by 'timeout_mins'.",
+                         six.text_type(ex))
+        self.assertFalse(mock_call.called)
+
     def test_create_err_denied_policy(self, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'create', False)
         stack_name = "wordpress"
@@ -1485,6 +1540,30 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
         self.assertEqual('StackNotFound', resp.json['error']['type'])
         self.m.VerifyAll()
 
+    def test_update_timeout_not_int(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'update', True)
+        identity = identifier.HeatIdentifier(self.tenant, 'wibble', '6')
+        template = {u'Foo': u'bar'}
+        parameters = {u'InstanceType': u'm1.xlarge'}
+        body = {'template': template,
+                'parameters': parameters,
+                'files': {},
+                'timeout_mins': 'not-int'}
+
+        req = self._put('/stacks/%(stack_name)s/%(stack_id)s' % identity,
+                        json.dumps(body))
+
+        mock_call = self.patchobject(rpc_client.EngineClient, 'call')
+        ex = self.assertRaises(ValueError,
+                               self.controller.update, req,
+                               tenant_id=identity.tenant,
+                               stack_name=identity.stack_name,
+                               stack_id=identity.stack_id,
+                               body=body)
+        self.assertEqual("Only integer is acceptable by 'timeout_mins'.",
+                         six.text_type(ex))
+        self.assertFalse(mock_call.called)
+
     def test_update_err_denied_policy(self, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'update', False)
         identity = identifier.HeatIdentifier(self.tenant, 'wibble', '6')
@@ -1579,6 +1658,30 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
                           body=body)
         self.m.VerifyAll()
 
+    def test_update_with_patch_timeout_not_int(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'update_patch', True)
+        identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '6')
+        template = {u'Foo': u'bar'}
+        parameters = {u'InstanceType': u'm1.xlarge'}
+        body = {'template': template,
+                'parameters': parameters,
+                'files': {},
+                'timeout_mins': 'not-int'}
+
+        req = self._patch('/stacks/%(stack_name)s/%(stack_id)s' % identity,
+                          json.dumps(body))
+
+        mock_call = self.patchobject(rpc_client.EngineClient, 'call')
+        ex = self.assertRaises(ValueError,
+                               self.controller.update_patch, req,
+                               tenant_id=identity.tenant,
+                               stack_name=identity.stack_name,
+                               stack_id=identity.stack_id,
+                               body=body)
+        self.assertEqual("Only integer is acceptable by 'timeout_mins'.",
+                         six.text_type(ex))
+        self.assertFalse(mock_call.called)
+
     def test_update_with_existing_and_default_parameters(
             self, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'update_patch', True)
@@ -2122,6 +2225,25 @@ class ResourceControllerTest(ControllerTest, common.HeatTestCase):
         self.assertEqual([], result['resources'])
         self.m.VerifyAll()
 
+    def test_index_nested_depth_not_int(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'index', True)
+        stack_identity = identifier.HeatIdentifier(self.tenant,
+                                                   'rubbish', '1')
+
+        req = self._get(stack_identity._tenant_path() + '/resources',
+                        {'nested_depth': 'non-int'})
+
+        mock_call = self.patchobject(rpc_client.EngineClient, 'call')
+        ex = self.assertRaises(ValueError,
+                               self.controller.index, req,
+                               tenant_id=self.tenant,
+                               stack_name=stack_identity.stack_name,
+                               stack_id=stack_identity.stack_id)
+
+        self.assertEqual("Only integer is acceptable by 'nested_depth'.",
+                         six.text_type(ex))
+        self.assertFalse(mock_call.called)
+
     def test_index_denied_policy(self, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'index', False)
         res_name = 'WikiDatabase'
@@ -2868,6 +2990,23 @@ class EventControllerTest(ControllerTest, common.HeatTestCase):
         self.assertIsNone(engine_args['filters'])
         self.assertNotIn('balrog', engine_args)
 
+    @mock.patch.object(rpc_client.EngineClient, 'call')
+    def test_index_limit_not_int(self, mock_call, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'index', True)
+        sid = identifier.HeatIdentifier(self.tenant, 'wibble', '6')
+
+        req = self._get(sid._tenant_path() + '/events',
+                        params={'limit': 'not-an-int'})
+
+        ex = self.assertRaises(ValueError,
+                               self.controller.index, req,
+                               tenant_id=self.tenant,
+                               stack_name=sid.stack_name,
+                               stack_id=sid.stack_id)
+        self.assertEqual("Only integer is acceptable by 'limit'.",
+                         six.text_type(ex))
+        self.assertFalse(mock_call.called)
+
     @mock.patch.object(rpc_client.EngineClient, 'call')
     def test_index_whitelist_filter_params(self, mock_call, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'index', True)