Raise StackValidationFailed instead of InvalidCondition*

Now that we raise them in only a single place each, there's not a lot of
value in having a custom exception type for condition-related errors in the
template any more, and in fact there was no code to handle them
appropriately in heat-api so it would have resulted in 500 errors to the
client if they ever escaped heat-engine anyway.

Just use the existing StackValidationFailed exception type. This provides
more flexible control over the path than InvalidConditionReference did.
Also, raising ValueError in the Invalid function means that
StackValidationFailed will be raised with the path, so it will be much
easier for users to find where they have used an invalid function.

Change-Id: Iaa6ca030fc144efc75e707910f84bfa7385fa9e2
This commit is contained in:
Zane Bitter 2016-09-07 12:21:14 -04:00
parent f18e57e004
commit d98d821ac5
8 changed files with 42 additions and 60 deletions

View File

@ -129,19 +129,6 @@ class InvalidTemplateSection(HeatException):
msg_fmt = _("The template section is invalid: %(section)s")
class InvalidConditionFunction(HeatException):
msg_fmt = _("The function is not supported in condition: %(func)s")
class InvalidConditionDefinition(HeatException):
msg_fmt = _("The definition of condition (%(cd)s) is "
"invalid: %(definition)s")
class InvalidConditionReference(HeatException):
msg_fmt = _('Invalid condition "%(cd)s" (in %(path)s)')
class ImmutableParameterModified(HeatException):
msg_fmt = _("The following parameters are immutable and may not be "
"updated: %(keys)s")

View File

@ -155,10 +155,7 @@ class CfnTemplateBase(template_common.CommonTemplate):
cond_name = defn.condition_name()
if cond_name is not None:
path = '.'.join([self.RESOURCES,
name,
self.RES_CONDITION])
path = [self.RESOURCES, name, self.RES_CONDITION]
if not conditions.is_enabled(cond_name, path):
continue

View File

@ -18,7 +18,7 @@ import weakref
import six
from heat.common import exception
from heat.common.i18n import _
@six.add_metaclass(abc.ABCMeta)
@ -262,7 +262,8 @@ class Invalid(Function):
"""
def __init__(self, stack, fn_name, args):
raise exception.InvalidConditionFunction(func=fn_name)
raise ValueError(_('The function "%s" '
'is invalid in this context') % fn_name)
def result(self):
return super(Invalid, self).result()

View File

@ -244,9 +244,7 @@ class HOTemplate20130523(template_common.CommonTemplate):
cond_name = defn.condition_name()
if cond_name is not None:
path = '.'.join([self.RESOURCES,
name,
self.RES_CONDITION])
path = [self.RESOURCES, name, self.RES_CONDITION]
if not conditions.is_enabled(cond_name, path):
continue

View File

@ -318,9 +318,9 @@ class Template(collections.Mapping):
def parse(self, stack, snippet, path=''):
return parse(self.functions, stack, snippet, path, self)
def parse_condition(self, stack, snippet):
def parse_condition(self, stack, snippet, path=''):
return parse(self._parser_condition_functions, stack, snippet,
template=self)
path, self)
def validate(self):
"""Validate the template.

View File

@ -92,7 +92,7 @@ class CommonTemplate(template.Template):
# hasn't been resolved yet
if not isinstance(cd_value, bool):
condition_func = self.parse_condition(
stack, cd_value)
stack, cd_value, '.'.join([self.CONDITIONS, cd_key]))
resolved_cd_value = function.resolve(condition_func)
result[cd_key] = resolved_cd_value
else:
@ -110,9 +110,12 @@ class CommonTemplate(template.Template):
if resolved_cds:
for cd_key, cd_value in six.iteritems(resolved_cds):
if not isinstance(cd_value, bool):
raise exception.InvalidConditionDefinition(
cd=cd_key,
definition=cd_value)
msg_data = {'cd': cd_key, 'definition': cd_value}
message = _('The definition of condition "%(cd)s" is '
'invalid: %(definition)s') % msg_data
raise exception.StackValidationFailed(
error='Condition validation error',
message=message)
self._conditions = resolved_cds
@ -145,9 +148,7 @@ class CommonTemplate(template.Template):
if hasattr(self, 'OUTPUT_CONDITION'):
cond_name = val.get(self.OUTPUT_CONDITION)
path = '.'.join([self.OUTPUTS,
key,
self.OUTPUT_CONDITION])
path = [self.OUTPUTS, key, self.OUTPUT_CONDITION]
if not conditions.is_enabled(cond_name, path):
yield key, output.OutputDefinition(key, None,
description)
@ -166,12 +167,13 @@ class Conditions(object):
def __init__(self, conditions_dict):
self._conditions = conditions_dict
def is_enabled(self, condition_name, path):
def is_enabled(self, condition_name, path=None):
if condition_name is None:
return True
if condition_name not in self._conditions:
raise exception.InvalidConditionReference(cd=condition_name,
path=path)
message = _('Invalid condition "%s"') % condition_name
raise exception.StackValidationFailed(path=path,
message=message)
return self._conditions[condition_name]

View File

@ -1157,10 +1157,9 @@ class HOTemplateTest(common.HeatTestCase):
snippet = {'equals': [{'get_attr': [None, 'att1']},
{'get_attr': [None, 'att2']}]}
exc = self.assertRaises(exception.InvalidConditionFunction,
exc = self.assertRaises(exception.StackValidationFailed,
self.resolve_condition, snippet, tmpl)
error_msg = 'The function is not supported in condition: get_attr'
self.assertIn(error_msg, six.text_type(exc))
self.assertIn('"get_attr" is invalid', six.text_type(exc))
def test_if(self):
snippet = {'if': ['create_prod', 'value_if_true', 'value_if_false']}

View File

@ -321,26 +321,25 @@ class TestTemplateConditionParser(common.HeatTestCase):
# test with get_attr in equals
tmpl = template.Template(t)
stk = stack.Stack(self.ctx, 'test_condition_with_get_attr_func', tmpl)
ex = self.assertRaises(exception.InvalidConditionFunction,
ex = self.assertRaises(exception.StackValidationFailed,
tmpl._resolve_conditions, stk)
self.assertIn('The function is not supported in condition: get_attr',
self.assertIn('"get_attr" is invalid', six.text_type(ex))
self.assertIn('conditions.prod_env.equals[1].get_attr',
six.text_type(ex))
# test with get_resource in top level of a condition
tmpl.t['conditions']['prod_env'] = {'get_resource': 'R1'}
stk = stack.Stack(self.ctx, 'test_condition_with_get_attr_func', tmpl)
ex = self.assertRaises(exception.InvalidConditionFunction,
ex = self.assertRaises(exception.StackValidationFailed,
tmpl._resolve_conditions, stk)
self.assertIn('The function is not supported in condition: '
'get_resource', six.text_type(ex))
self.assertIn('"get_resource" is invalid', six.text_type(ex))
# test with get_attr in top level of a condition
tmpl.t['conditions']['prod_env'] = {'get_attr': [None, 'att']}
stk = stack.Stack(self.ctx, 'test_condition_with_get_attr_func', tmpl)
ex = self.assertRaises(exception.InvalidConditionFunction,
ex = self.assertRaises(exception.StackValidationFailed,
tmpl._resolve_conditions, stk)
self.assertIn('The function is not supported in condition: get_attr',
six.text_type(ex))
self.assertIn('"get_attr" is invalid', six.text_type(ex))
def test_condition_resolved_not_boolean(self):
t = {
@ -358,9 +357,9 @@ class TestTemplateConditionParser(common.HeatTestCase):
tmpl = template.Template(t)
stk = stack.Stack(self.ctx, 'test_condition_not_boolean', tmpl)
ex = self.assertRaises(exception.InvalidConditionDefinition,
ex = self.assertRaises(exception.StackValidationFailed,
tmpl.conditions, stk)
self.assertIn('The definition of condition (prod_env) is invalid',
self.assertIn('The definition of condition "prod_env" is invalid',
six.text_type(ex))
def test_get_res_condition_invalid(self):
@ -369,17 +368,17 @@ class TestTemplateConditionParser(common.HeatTestCase):
stk = stack.Stack(self.ctx, 'test_res_invalid_condition', tmpl)
conds = template_common.Conditions(tmpl.conditions(stk))
ex = self.assertRaises(exception.InvalidConditionReference,
ex = self.assertRaises(exception.StackValidationFailed,
conds.is_enabled, 'invalid_cd',
'resources.r1.condition')
self.assertIn('Invalid condition "invalid_cd" '
'(in resources.r1.condition)', six.text_type(ex))
self.assertIn('Invalid condition "invalid_cd"', six.text_type(ex))
self.assertIn('resources.r1.condition', six.text_type(ex))
# test condition name is not string
ex = self.assertRaises(exception.InvalidConditionReference,
ex = self.assertRaises(exception.StackValidationFailed,
conds.is_enabled, 111,
'resources.r1.condition')
self.assertIn('Invalid condition "111" (in resources.r1.condition)',
six.text_type(ex))
self.assertIn('Invalid condition "111"', six.text_type(ex))
self.assertIn('resources.r1.condition', six.text_type(ex))
def test_parse_output_condition_invalid(self):
stk = stack.Stack(self.ctx,
@ -388,17 +387,16 @@ class TestTemplateConditionParser(common.HeatTestCase):
# test condition name is invalid
self.tmpl.t['outputs']['foo']['condition'] = 'invalid_cd'
ex = self.assertRaises(exception.InvalidConditionReference,
ex = self.assertRaises(exception.StackValidationFailed,
lambda: stk.outputs)
self.assertIn('Invalid condition "invalid_cd" '
'(in outputs.foo.condition)',
six.text_type(ex))
self.assertIn('Invalid condition "invalid_cd"', six.text_type(ex))
self.assertIn('outputs.foo.condition', six.text_type(ex))
# test condition name is not string
self.tmpl.t['outputs']['foo']['condition'] = 222
ex = self.assertRaises(exception.InvalidConditionReference,
ex = self.assertRaises(exception.StackValidationFailed,
lambda: stk.outputs)
self.assertIn('Invalid condition "222" (in outputs.foo.condition)',
six.text_type(ex))
self.assertIn('Invalid condition "222"', six.text_type(ex))
self.assertIn('outputs.foo.condition', six.text_type(ex))
class TestTemplateValidate(common.HeatTestCase):