From 1f8d70346a610133cf8c0aa82e11afbffabc3071 Mon Sep 17 00:00:00 2001 From: Crag Wolfe Date: Tue, 23 Aug 2016 19:44:43 -0400 Subject: [PATCH] Add the HOT fuction str_replace_vstrict The already existing str_replace_strict function raises an error if a param is not present in the template. str_replace_vstrict, newly added in this patch, is identical but also raises an error if any of the params have an empty value. Change-Id: I5407135cc0435cfbad2d18964fe2119c999f67a3 --- doc/source/template_guide/hot_spec.rst | 15 ++++++- heat/engine/hot/functions.py | 43 +++++++++++++++---- heat/engine/hot/template.py | 1 + heat/tests/test_hot.py | 59 ++++++++++++++++++++++++-- 4 files changed, 104 insertions(+), 14 deletions(-) diff --git a/doc/source/template_guide/hot_spec.rst b/doc/source/template_guide/hot_spec.rst index 7f9f8e7d71..c5548282b3 100644 --- a/doc/source/template_guide/hot_spec.rst +++ b/doc/source/template_guide/hot_spec.rst @@ -294,8 +294,9 @@ for the ``heat_template_version`` key: The key with value ``2017-09-01`` or ``pike`` indicates that the YAML document is a HOT template and it may contain features added and/or removed up until the Pike release. This version adds the ``make_url`` function for - assembling URLs and the ``list_concat`` function for combining multiple - lists. The complete list of supported functions is:: + assembling URLs, the ``list_concat`` function for combining multiple + lists, and ``string_replace_vstrict`` which raises errors for + missing and empty params. The complete list of supported functions is:: digest filter @@ -312,6 +313,7 @@ for the ``heat_template_version`` key: resource_facade str_replace str_replace_strict + str_replace_vstrict str_split yaql if @@ -1494,6 +1496,15 @@ in the template. This may help catch typo's or other issues sooner rather than later when processing a template. +str_replace_vstrict +------------------ +``str_replace_vstrict`` behaves identically to the +``str_replace_strict`` function, only an error is raised if any of the +params are empty. This may help catch issues (i.e., prevent +resources from being created with bogus values) sooner rather than later if +it is known that all the params should be non-empty. + + str_split --------- The ``str_split`` function allows for splitting a string into a list by diff --git a/heat/engine/hot/functions.py b/heat/engine/hot/functions.py index 4c49ee45e4..40fe316b88 100644 --- a/heat/engine/hot/functions.py +++ b/heat/engine/hot/functions.py @@ -359,6 +359,7 @@ class Replace(function.Function): """ _strict = False + _allow_empty_value = True def __init__(self, stack, fn_name, args): super(Replace, self).__init__(stack, fn_name, args) @@ -388,15 +389,16 @@ class Replace(function.Function): else: return mapping, string - def _validate_replacement(self, value): + def _validate_replacement(self, value, param): if value is None: return '' if not isinstance(value, (six.string_types, six.integer_types, float, bool)): - raise TypeError(_('"%s" params must be strings or numbers') % - self.fn_name) + raise TypeError(_('"%(name)s" params must be strings or numbers, ' + 'param %(param)s is not valid') % + {'name': self.fn_name, 'param': param}) return six.text_type(value) @@ -423,7 +425,8 @@ class Replace(function.Function): self.fn_name) remaining_keys = keys[1:] - value = self._validate_replacement(mapping[placeholder]) + value = self._validate_replacement(mapping[placeholder], + placeholder) def string_split(s): ss = s.split(placeholder) @@ -467,13 +470,25 @@ class ReplaceJson(Replace): being substituted in. """ - def _validate_replacement(self, value): + def _validate_replacement(self, value, param): + + def _raise_empty_param_value_error(): + raise ValueError( + _('%(name)s has an undefined or empty value for param ' + '%(param)s, must be a defined non-empty value') % + {'name': self.fn_name, 'param': param}) + if value is None: - return '' + if self._allow_empty_value: + return '' + else: + _raise_empty_param_value_error() if not isinstance(value, (six.string_types, six.integer_types, float, bool)): if isinstance(value, (collections.Mapping, collections.Sequence)): + if not self._allow_empty_value and len(value) == 0: + _raise_empty_param_value_error() try: return jsonutils.dumps(value, default=None, sort_keys=True) except TypeError: @@ -486,7 +501,10 @@ class ReplaceJson(Replace): raise TypeError(_('"%s" params must be strings, numbers, ' 'list or map.') % self.fn_name) - return six.text_type(value) + ret_value = six.text_type(value) + if not self._allow_empty_value and not ret_value: + _raise_empty_param_value_error() + return ret_value class ReplaceJsonStrict(ReplaceJson): @@ -496,10 +514,19 @@ class ReplaceJsonStrict(ReplaceJson): a ValueError is raised if any of the params are not present in the template. """ - _strict = True +class ReplaceJsonVeryStrict(ReplaceJsonStrict): + """A function for performing string substituions. + + str_replace_vstrict is identical to the str_replace_strict + function, only a ValueError is raised if any of the params are + None or empty. + """ + _allow_empty_value = False + + class GetFile(function.Function): """A function for including a file inline. diff --git a/heat/engine/hot/template.py b/heat/engine/hot/template.py index 44ca6cea81..e0fdf29ae8 100644 --- a/heat/engine/hot/template.py +++ b/heat/engine/hot/template.py @@ -591,6 +591,7 @@ class HOTemplate20170901(HOTemplate20170224): # functions added in 2017-09-01 'make_url': hot_funcs.MakeURL, 'list_concat': hot_funcs.ListConcat, + 'str_replace_vstrict': hot_funcs.ReplaceJsonVeryStrict, # functions removed from 2015-10-15 'Fn::Select': hot_funcs.Removed, diff --git a/heat/tests/test_hot.py b/heat/tests/test_hot.py index 1f16894a04..108aa0c693 100644 --- a/heat/tests/test_hot.py +++ b/heat/tests/test_hot.py @@ -586,7 +586,18 @@ class HOTemplateTest(common.HeatTestCase): self.assertEqual(snippet_resolved, self.resolve(snippet, tmpl)) def test_str_replace_map_param(self): - """Test str_replace function with non-string params.""" + """Test old str_replace function with non-string map param.""" + + snippet = {'str_replace': {'template': 'jsonvar1', + 'params': {'jsonvar1': {'foo': 123}}}} + + tmpl = template.Template(hot_tpl_empty) + ex = self.assertRaises(TypeError, self.resolve, snippet, tmpl) + self.assertIn('"str_replace" params must be strings or numbers, ' + 'param jsonvar1 is not valid', six.text_type(ex)) + + def test_liberty_str_replace_map_param(self): + """Test str_replace function with non-string map param.""" snippet = {'str_replace': {'template': 'jsonvar1', 'params': {'jsonvar1': {'foo': 123}}}} @@ -596,7 +607,18 @@ class HOTemplateTest(common.HeatTestCase): self.assertEqual(snippet_resolved, self.resolve(snippet, tmpl)) def test_str_replace_list_param(self): - """Test str_replace function with non-string params.""" + """Test old str_replace function with non-string list param.""" + + snippet = {'str_replace': {'template': 'listvar1', + 'params': {'listvar1': ['foo', 123]}}} + + tmpl = template.Template(hot_tpl_empty) + ex = self.assertRaises(TypeError, self.resolve, snippet, tmpl) + self.assertIn('"str_replace" params must be strings or numbers, ' + 'param listvar1 is not valid', six.text_type(ex)) + + def test_liberty_str_replace_list_param(self): + """Test str_replace function with non-string param.""" snippet = {'str_replace': {'template': 'listvar1', 'params': {'listvar1': ['foo', 123]}}} @@ -717,7 +739,7 @@ class HOTemplateTest(common.HeatTestCase): self.assertEqual(snippet_resolved, self.resolve(snippet, tmpl)) def test_str_replace_strict_missing_param(self): - """Test str_replace_strict function missing param (s)raises error.""" + """Test str_replace_strict function missing param(s) raises error.""" snippet = {'str_replace_strict': {'template': 'Template var1 string var2', @@ -738,16 +760,45 @@ class HOTemplateTest(common.HeatTestCase): self.assertEqual('The following params were not found in the ' 'template: var0', six.text_type(ex)) - snippet = {'str_replace_strict': + # str_replace_vstrict has same behaviour + snippet = {'str_replace_vstrict': {'template': 'Template var1 string var2', 'params': {'var1': 'foo', 'var2': 'bar', 'var0': 'zed', 'var': 'z', 'longvarname': 'q'}}} + tmpl = template.Template(hot_pike_tpl_empty) ex = self.assertRaises(ValueError, self.resolve, snippet, tmpl) self.assertEqual('The following params were not found in the ' 'template: longvarname,var0,var', six.text_type(ex)) + def test_str_replace_strict_empty_param_ok(self): + """Test str_replace_strict function with empty params.""" + + snippet = {'str_replace_strict': + {'template': 'Template var1 string var2', + 'params': {'var1': 'foo', 'var2': ''}}} + + tmpl = template.Template(hot_ocata_tpl_empty) + self.assertEqual('Template foo string ', self.resolve(snippet, tmpl)) + + def test_str_replace_vstrict_empty_param_not_ok(self): + """Test str_replace_vstrict function with empty params. + + Raise ValueError when any of the params are None or empty. + """ + + snippet = {'str_replace_vstrict': + {'template': 'Template var1 string var2', + 'params': {'var1': 'foo', 'var2': ''}}} + + tmpl = template.Template(hot_pike_tpl_empty) + for val in (None, '', {}, []): + snippet['str_replace_vstrict']['params']['var2'] = val + ex = self.assertRaises(ValueError, self.resolve, snippet, tmpl) + self.assertIn('str_replace_vstrict has an undefined or empty ' + 'value for param var2', six.text_type(ex)) + def test_str_replace_invalid_param_keys(self): """Test str_replace function parameter keys.