Revert "Improve validation for some functions"

This reverts commit 36bf170f34.

The construction and validation phases of intrinsic functions are
deliberately separated, to:

- Reduce the amount of unnecessary work done
- Reduce the risk of changes to validation breaking the loading of existing
  stacks from the database
- Ensure that the most critical errors are reported to the user first

This patch reverts a commit that sought to combine the two phases. There
was no linked bug and no explanation other than that this 'improved'
things.

It does not, however, restore the check for the Removed function during
properties translation (and the associated test), because I believe its
original existence was based on a misconception. There is no way in
practice for a hot.functions.Removed object to appear in the properties a
translation rule is operating on.

Change-Id: I59d02e540771baf4fe078081cd0993a3251eb9a6
This commit is contained in:
Zane Bitter 2016-09-06 16:05:12 -04:00
parent 4d7257a3c9
commit f18e57e004
4 changed files with 16 additions and 15 deletions

View File

@ -614,9 +614,10 @@ class Removed(function.Function):
Check the HOT guide for an equivalent native function.
"""
def __init__(self, stack, fn_name, args):
def validate(self):
exp = (_("The function %s is not supported in this version of HOT.") %
fn_name)
self.fn_name)
raise exception.InvalidTemplateVersion(explanation=exp)
def result(self):
@ -656,9 +657,10 @@ class Repeat(function.Function):
for_each:
%var%: ['a', 'b', 'c']''')
raise KeyError(_('"repeat" syntax should be %s') % example)
self.validate_args()
def validate_args(self):
def validate(self):
super(Repeat, self).validate()
if not isinstance(self._for_each, function.Function):
if not isinstance(self._for_each, collections.Mapping):
raise TypeError(_('The "for_each" argument to "%s" must '
@ -872,6 +874,8 @@ class Yaql(function.Function):
raise KeyError(_('"%(name)s" syntax should be %(example)s') % {
'name': self.fn_name, 'example': example})
def validate(self):
super(Yaql, self).validate()
if not isinstance(self._expression, function.Function):
self._parse(self._expression)

View File

@ -407,8 +407,7 @@ def parse(functions, stack, snippet, path='', template=None):
template)
else:
return Func(stack, fn_name, recurse(args, path))
except (ValueError, TypeError, KeyError,
exception.InvalidTemplateVersion) as e:
except (ValueError, TypeError, KeyError) as e:
raise exception.StackValidationFailed(
path=path,
message=six.text_type(e))

View File

@ -182,9 +182,6 @@ class TranslationRule(object):
def _resolve_param(self, param):
"""Check whether given item is param and resolve, if it is."""
# NOTE(prazumovsky): If property uses removed in HOT function,
# we should not translate it for correct validating and raising
# validation error.
if isinstance(param, (hot_funcs.GetParam, cfn_funcs.ParamRef)):
try:
return function.resolve(param)

View File

@ -1089,8 +1089,8 @@ class HOTemplateTest(common.HeatTestCase):
snippet = {'yaql': {'expression': 'invalid(',
'data': {'var1': [1, 2, 3, 4]}}}
tmpl = template.Template(hot_newton_tpl_empty)
self.assertRaises(exception.StackValidationFailed,
tmpl.parse, None, snippet)
yaql = tmpl.parse(None, snippet)
self.assertRaises(ValueError, function.validate, yaql)
def test_yaql_data_as_function(self):
snippet = {'yaql': {'expression': '$.data.var1.len()',
@ -1320,8 +1320,8 @@ class HOTemplateTest(common.HeatTestCase):
# for_each is not a map
snippet = {'repeat': {'template': 'this is %var%',
'for_each': '%var%'}}
self.assertRaises(exception.StackValidationFailed,
tmpl.parse, None, snippet)
repeat = tmpl.parse(None, snippet)
self.assertRaises(TypeError, function.validate, repeat)
def test_digest(self):
snippet = {'digest': ['md5', 'foobar']}
@ -1564,8 +1564,9 @@ class HOTemplateTest(common.HeatTestCase):
snippet = {'Fn::GetAZs': ''}
stack = parser.Stack(utils.dummy_context(), 'test_stack',
template.Template(hot_juno_tpl_empty))
error = self.assertRaises(exception.StackValidationFailed,
stack.t.parse, stack, snippet)
error = self.assertRaises(exception.InvalidTemplateVersion,
function.validate,
stack.t.parse(stack, snippet))
self.assertIn(next(iter(snippet)), six.text_type(error))
def test_add_resource(self):