diff --git a/heat/api/openstack/v1/events.py b/heat/api/openstack/v1/events.py index 2fd4896bb2..aa8d360e2c 100644 --- a/heat/api/openstack/v1/events.py +++ b/heat/api/openstack/v1/events.py @@ -13,6 +13,7 @@ import itertools +import six from webob import exc from heat.api.openstack.v1 import util @@ -120,7 +121,11 @@ class EventController(object): key = rpc_api.PARAM_LIMIT if key in params: - limit = param_utils.extract_int(key, params[key], allow_zero=True) + try: + limit = param_utils.extract_int(key, params[key], + allow_zero=True) + except ValueError as e: + raise exc.HTTPBadRequest(six.text_type(e)) params[key] = limit if resource_name is None: diff --git a/heat/api/openstack/v1/resources.py b/heat/api/openstack/v1/resources.py index bed6e470c5..f1d13b7b23 100644 --- a/heat/api/openstack/v1/resources.py +++ b/heat/api/openstack/v1/resources.py @@ -13,6 +13,9 @@ import itertools +import six +from webob import exc + from heat.api.openstack.v1 import util from heat.common import identifier from heat.common import param_utils @@ -85,7 +88,10 @@ class ResourceController(object): nested_depth = 0 key = rpc_api.PARAM_NESTED_DEPTH if key in req.params: - nested_depth = param_utils.extract_int(key, req.params[key]) + try: + nested_depth = param_utils.extract_int(key, req.params[key]) + except ValueError as e: + raise exc.HTTPBadRequest(six.text_type(e)) res_list = self.rpc_client.list_stack_resources(req.context, identity, diff --git a/heat/api/openstack/v1/stacks.py b/heat/api/openstack/v1/stacks.py index 46254e4f7a..9cf5086e82 100644 --- a/heat/api/openstack/v1/stacks.py +++ b/heat/api/openstack/v1/stacks.py @@ -169,6 +169,26 @@ class StackController(object): def default(self, req, **args): raise exc.HTTPNotFound() + def _extract_bool_param(self, name, value): + try: + return param_utils.extract_bool(name, value) + except ValueError as e: + raise exc.HTTPBadRequest(six.text_type(e)) + + def _extract_int_param(self, name, value, + allow_zero=True, allow_negative=False): + try: + return param_utils.extract_int(name, value, + allow_zero, allow_negative) + except ValueError as e: + raise exc.HTTPBadRequest(six.text_type(e)) + + def _extract_tags_param(self, tags): + try: + return param_utils.extract_tags(tags) + except ValueError as e: + raise exc.HTTPBadRequest(six.text_type(e)) + def _index(self, req, tenant_safe=True): filter_whitelist = { 'id': 'mixed', @@ -196,55 +216,56 @@ class StackController(object): filter_params = util.get_allowed_params(req.params, filter_whitelist) show_deleted = False - if rpc_api.PARAM_SHOW_DELETED in params: - params[rpc_api.PARAM_SHOW_DELETED] = param_utils.extract_bool( - params[rpc_api.PARAM_SHOW_DELETED]) - show_deleted = params[rpc_api.PARAM_SHOW_DELETED] + p_name = rpc_api.PARAM_SHOW_DELETED + if p_name in params: + params[p_name] = self._extract_bool_param(p_name, params[p_name]) + show_deleted = params[p_name] show_nested = False - if rpc_api.PARAM_SHOW_NESTED in params: - params[rpc_api.PARAM_SHOW_NESTED] = param_utils.extract_bool( - params[rpc_api.PARAM_SHOW_NESTED]) - show_nested = params[rpc_api.PARAM_SHOW_NESTED] + p_name = rpc_api.PARAM_SHOW_NESTED + if p_name in params: + params[p_name] = self._extract_bool_param(p_name, params[p_name]) + show_nested = params[p_name] key = rpc_api.PARAM_LIMIT if key in params: - params[key] = param_utils.extract_int(key, params[key]) + params[key] = self._extract_int_param(key, params[key]) show_hidden = False - if rpc_api.PARAM_SHOW_HIDDEN in params: - params[rpc_api.PARAM_SHOW_HIDDEN] = param_utils.extract_bool( - params[rpc_api.PARAM_SHOW_HIDDEN]) - show_hidden = params[rpc_api.PARAM_SHOW_HIDDEN] + p_name = rpc_api.PARAM_SHOW_HIDDEN + if p_name in params: + params[p_name] = self._extract_bool_param(p_name, params[p_name]) + show_hidden = params[p_name] tags = None if rpc_api.PARAM_TAGS in params: - params[rpc_api.PARAM_TAGS] = param_utils.extract_tags( + params[rpc_api.PARAM_TAGS] = self._extract_tags_param( params[rpc_api.PARAM_TAGS]) tags = params[rpc_api.PARAM_TAGS] tags_any = None if rpc_api.PARAM_TAGS_ANY in params: - params[rpc_api.PARAM_TAGS_ANY] = param_utils.extract_tags( + params[rpc_api.PARAM_TAGS_ANY] = self._extract_tags_param( params[rpc_api.PARAM_TAGS_ANY]) tags_any = params[rpc_api.PARAM_TAGS_ANY] not_tags = None if rpc_api.PARAM_NOT_TAGS in params: - params[rpc_api.PARAM_NOT_TAGS] = param_utils.extract_tags( + params[rpc_api.PARAM_NOT_TAGS] = self._extract_tags_param( params[rpc_api.PARAM_NOT_TAGS]) not_tags = params[rpc_api.PARAM_NOT_TAGS] not_tags_any = None if rpc_api.PARAM_NOT_TAGS_ANY in params: - params[rpc_api.PARAM_NOT_TAGS_ANY] = param_utils.extract_tags( + params[rpc_api.PARAM_NOT_TAGS_ANY] = self._extract_tags_param( params[rpc_api.PARAM_NOT_TAGS_ANY]) not_tags_any = params[rpc_api.PARAM_NOT_TAGS_ANY] # get the with_count value, if invalid, raise ValueError with_count = False if req.params.get('with_count'): - with_count = param_utils.extract_bool( + with_count = self._extract_bool_param( + 'with_count', req.params.get('with_count')) if not filter_params: @@ -270,8 +291,8 @@ class StackController(object): tags_any=tags_any, not_tags=not_tags, not_tags_any=not_tags_any) - except AttributeError as exc: - LOG.warn(_LW("Old Engine Version: %s") % exc) + except AttributeError as ex: + LOG.warn(_LW("Old Engine Version: %s") % ex) return stacks_view.collection(req, stacks=stacks, count=count, tenant_safe=tenant_safe) @@ -286,9 +307,11 @@ class StackController(object): Lists summary information for all stacks """ global_tenant = False - if rpc_api.PARAM_GLOBAL_TENANT in req.params: - global_tenant = param_utils.extract_bool( - req.params.get(rpc_api.PARAM_GLOBAL_TENANT)) + name = rpc_api.PARAM_GLOBAL_TENANT + if name in req.params: + global_tenant = self._extract_bool_param( + name, + req.params.get(name)) if global_tenant: return self.global_index(req, req.context.tenant_id) @@ -332,7 +355,7 @@ class StackController(object): args = data.args() key = rpc_api.PARAM_TIMEOUT if key in args: - args[key] = param_utils.extract_int(key, args[key]) + args[key] = self._extract_int_param(key, args[key]) result = self.rpc_client.create_stack(req.context, data.stack_name(), @@ -409,7 +432,7 @@ class StackController(object): args = data.args() key = rpc_api.PARAM_TIMEOUT if key in args: - args[key] = param_utils.extract_int(key, args[key]) + args[key] = self._extract_int_param(key, args[key]) self.rpc_client.update_stack(req.context, identity, @@ -431,7 +454,7 @@ class StackController(object): args = data.args() key = rpc_api.PARAM_TIMEOUT if key in args: - args[key] = param_utils.extract_int(key, args[key]) + args[key] = self._extract_int_param(key, args[key]) self.rpc_client.update_stack(req.context, identity, diff --git a/heat/common/param_utils.py b/heat/common/param_utils.py index dc32ca6fe6..df42f58103 100644 --- a/heat/common/param_utils.py +++ b/heat/common/param_utils.py @@ -16,15 +16,16 @@ from oslo_utils import strutils from heat.common.i18n import _ -def extract_bool(subject): +def extract_bool(name, value): ''' Convert any true/false string to its corresponding boolean value, regardless of case. ''' - if str(subject).lower() not in ('true', 'false'): - raise ValueError(_('Unrecognized value "%(value)s", acceptable ' - 'values are: true, false.') % {'value': subject}) - return strutils.bool_from_string(subject, strict=True) + if str(value).lower() not in ('true', 'false'): + raise ValueError(_('Unrecognized value "%(value)s" for "%(name)s", ' + 'acceptable values are: true, false.') + % {'value': value, 'name': name}) + return strutils.bool_from_string(value, strict=True) def extract_int(name, value, allow_zero=True, allow_negative=False): diff --git a/heat/engine/api.py b/heat/engine/api.py index 08020ec886..4a416ec31e 100644 --- a/heat/engine/api.py +++ b/heat/engine/api.py @@ -46,14 +46,14 @@ def extract_args(params): else: raise ValueError(_('Invalid timeout value %s') % timeout) - if rpc_api.PARAM_DISABLE_ROLLBACK in params: - disable_rollback = param_utils.extract_bool( - params[rpc_api.PARAM_DISABLE_ROLLBACK]) - kwargs[rpc_api.PARAM_DISABLE_ROLLBACK] = disable_rollback + name = rpc_api.PARAM_DISABLE_ROLLBACK + if name in params: + disable_rollback = param_utils.extract_bool(name, params[name]) + kwargs[name] = disable_rollback - if rpc_api.PARAM_SHOW_DELETED in params: - params[rpc_api.PARAM_SHOW_DELETED] = param_utils.extract_bool( - params[rpc_api.PARAM_SHOW_DELETED]) + name = rpc_api.PARAM_SHOW_DELETED + if name in params: + params[name] = param_utils.extract_bool(name, params[name]) adopt_data = params.get(rpc_api.PARAM_ADOPT_STACK_DATA) if adopt_data: diff --git a/heat/tests/test_api_openstack_v1.py b/heat/tests/test_api_openstack_v1.py index 53d6fbf202..6742ed8fcf 100644 --- a/heat/tests/test_api_openstack_v1.py +++ b/heat/tests/test_api_openstack_v1.py @@ -427,7 +427,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): params = {'limit': 'not-an-int'} req = self._get('/stacks', params=params) - ex = self.assertRaises(ValueError, + ex = self.assertRaises(webob.exc.HTTPBadRequest, self.controller.index, req, tenant_id=self.tenant) self.assertEqual("Only integer is acceptable by 'limit'.", @@ -499,9 +499,10 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): params = {'with_count': 'invalid_value'} req = self._get('/stacks', params=params) - exc = self.assertRaises(ValueError, self.controller.index, + exc = self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.index, req, tenant_id=self.tenant) - excepted = ('Unrecognized value "invalid_value", ' + excepted = ('Unrecognized value "invalid_value" for "with_count", ' 'acceptable values are: true, false') self.assertIn(excepted, six.text_type(exc)) @@ -848,7 +849,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): req = self._post('/stacks', json.dumps(body)) mock_call = self.patchobject(rpc_client.EngineClient, 'call') - ex = self.assertRaises(ValueError, + ex = self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, req, tenant_id=self.tenant, body=body) @@ -1070,7 +1071,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): req = self._post('/stacks', json.dumps(body)) mock_call = self.patchobject(rpc_client.EngineClient, 'call') - ex = self.assertRaises(ValueError, + ex = self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, req, tenant_id=self.tenant, body=body) @@ -1588,7 +1589,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): json.dumps(body)) mock_call = self.patchobject(rpc_client.EngineClient, 'call') - ex = self.assertRaises(ValueError, + ex = self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, tenant_id=identity.tenant, stack_name=identity.stack_name, @@ -1708,7 +1709,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): json.dumps(body)) mock_call = self.patchobject(rpc_client.EngineClient, 'call') - ex = self.assertRaises(ValueError, + ex = self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update_patch, req, tenant_id=identity.tenant, stack_name=identity.stack_name, @@ -2295,7 +2296,7 @@ class ResourceControllerTest(ControllerTest, common.HeatTestCase): {'nested_depth': 'non-int'}) mock_call = self.patchobject(rpc_client.EngineClient, 'call') - ex = self.assertRaises(ValueError, + ex = self.assertRaises(webob.exc.HTTPBadRequest, self.controller.index, req, tenant_id=self.tenant, stack_name=stack_identity.stack_name, @@ -3059,7 +3060,7 @@ class EventControllerTest(ControllerTest, common.HeatTestCase): req = self._get(sid._tenant_path() + '/events', params={'limit': 'not-an-int'}) - ex = self.assertRaises(ValueError, + ex = self.assertRaises(webob.exc.HTTPBadRequest, self.controller.index, req, tenant_id=self.tenant, stack_name=sid.stack_name, diff --git a/heat/tests/test_common_param_utils.py b/heat/tests/test_common_param_utils.py index c4c6cfb690..2b1f7bce81 100644 --- a/heat/tests/test_common_param_utils.py +++ b/heat/tests/test_common_param_utils.py @@ -18,11 +18,12 @@ from heat.tests import common class TestExtractBool(common.HeatTestCase): def test_extract_bool(self): for value in ('True', 'true', 'TRUE', True): - self.assertTrue(param_utils.extract_bool(value)) + self.assertTrue(param_utils.extract_bool('bool', value)) for value in ('False', 'false', 'FALSE', False): - self.assertFalse(param_utils.extract_bool(value)) + self.assertFalse(param_utils.extract_bool('bool', value)) for value in ('foo', 't', 'f', 'yes', 'no', 'y', 'n', '1', '0', None): - self.assertRaises(ValueError, param_utils.extract_bool, value) + self.assertRaises(ValueError, param_utils.extract_bool, + 'bool', value) class TestExtractInt(common.HeatTestCase):