From cf89ca8a03a8a84302ad27cb1fc7aa9120b743ca Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 10 Jul 2020 18:49:57 -0400 Subject: [PATCH] Make filter type errors 'loop friendly' (#70417) - ensure we preserve the typeerror part of the exception so loop defereed error handling can postpone those caused by undefined variables until the when check is done. - fix tests to comply with the 'new normal' - human_to_bytes and others can issue TypeError not only on 'non string' but also bad string that is not convertable. Co-authored-by: Sloane Hertel Co-authored-by: Sloane Hertel --- ...andle_undefined_in_type_errors_filters.yml | 2 ++ lib/ansible/errors/__init__.py | 5 +++ lib/ansible/plugins/filter/core.py | 14 ++++---- lib/ansible/plugins/filter/mathstuff.py | 18 ++++++---- .../handle_undefined_type_errors.yml | 29 +++++++++++++++ test/integration/targets/filter_core/runme.sh | 1 + .../targets/filter_core/tasks/main.yml | 2 +- .../targets/filter_mathstuff/tasks/main.yml | 2 +- test/units/plugins/filter/test_mathstuff.py | 36 +++++++++---------- 9 files changed, 76 insertions(+), 33 deletions(-) create mode 100644 changelogs/fragments/handle_undefined_in_type_errors_filters.yml create mode 100644 test/integration/targets/filter_core/handle_undefined_type_errors.yml diff --git a/changelogs/fragments/handle_undefined_in_type_errors_filters.yml b/changelogs/fragments/handle_undefined_in_type_errors_filters.yml new file mode 100644 index 00000000000..2f9cb20125d --- /dev/null +++ b/changelogs/fragments/handle_undefined_in_type_errors_filters.yml @@ -0,0 +1,2 @@ +bugfixes: + - Allow TypeErrors on Undefined variables in filters to be handled or deferred when processing for loops. diff --git a/lib/ansible/errors/__init__.py b/lib/ansible/errors/__init__.py index 79d9c5f4c32..563c5d25491 100644 --- a/lib/ansible/errors/__init__.py +++ b/lib/ansible/errors/__init__.py @@ -334,3 +334,8 @@ class AnsiblePluginCircularRedirect(AnsiblePluginError): class AnsibleCollectionUnsupportedVersionError(AnsiblePluginError): '''a collection is not supported by this version of Ansible''' pass + + +class AnsibleFilterTypeError(AnsibleTemplateError, TypeError): + ''' a Jinja filter templating failure due to bad type''' + pass diff --git a/lib/ansible/plugins/filter/core.py b/lib/ansible/plugins/filter/core.py index cbe03aeaf04..e497e305726 100644 --- a/lib/ansible/plugins/filter/core.py +++ b/lib/ansible/plugins/filter/core.py @@ -40,7 +40,7 @@ from random import Random, SystemRandom, shuffle from jinja2.filters import environmentfilter, do_groupby as _do_groupby -from ansible.errors import AnsibleError, AnsibleFilterError +from ansible.errors import AnsibleError, AnsibleFilterError, AnsibleFilterTypeError from ansible.module_utils.six import iteritems, string_types, integer_types, reraise from ansible.module_utils.six.moves import reduce, shlex_quote from ansible.module_utils._text import to_bytes, to_native, to_text @@ -509,7 +509,7 @@ def subelements(obj, subelements, skip_missing=False): elif isinstance(subelements, string_types): subelement_list = subelements.split('.') else: - raise AnsibleFilterError('subelements must be a list or a string') + raise AnsibleFilterTypeError('subelements must be a list or a string') results = [] @@ -524,9 +524,9 @@ def subelements(obj, subelements, skip_missing=False): break raise AnsibleFilterError("could not find %r key in iterated item %r" % (subelement, values)) except TypeError: - raise AnsibleFilterError("the key %s should point to a dictionary, got '%s'" % (subelement, values)) + raise AnsibleFilterTypeError("the key %s should point to a dictionary, got '%s'" % (subelement, values)) if not isinstance(values, list): - raise AnsibleFilterError("the key %r should point to a list, got %r" % (subelement, values)) + raise AnsibleFilterTypeError("the key %r should point to a list, got %r" % (subelement, values)) for value in values: results.append((element, value)) @@ -539,7 +539,7 @@ def dict_to_list_of_dict_key_value_elements(mydict, key_name='key', value_name=' with each having a 'key' and 'value' keys that correspond to the keys and values of the original ''' if not isinstance(mydict, Mapping): - raise AnsibleFilterError("dict2items requires a dictionary, got %s instead." % type(mydict)) + raise AnsibleFilterTypeError("dict2items requires a dictionary, got %s instead." % type(mydict)) ret = [] for key in mydict: @@ -552,7 +552,7 @@ def list_of_dict_key_value_elements_to_dict(mylist, key_name='key', value_name=' effectively as the reverse of dict2items ''' if not is_sequence(mylist): - raise AnsibleFilterError("items2dict requires a list, got %s instead." % type(mylist)) + raise AnsibleFilterTypeError("items2dict requires a list, got %s instead." % type(mylist)) return dict((item[key_name], item[value_name]) for item in mylist) @@ -565,7 +565,7 @@ def path_join(paths): elif is_sequence(paths): return os.path.join(*paths) else: - raise AnsibleFilterError("|path_join expects string or sequence, got %s instead." % type(paths)) + raise AnsibleFilterTypeError("|path_join expects string or sequence, got %s instead." % type(paths)) class FilterModule(object): diff --git a/lib/ansible/plugins/filter/mathstuff.py b/lib/ansible/plugins/filter/mathstuff.py index 4cddbdc68ec..64d0ba8b521 100644 --- a/lib/ansible/plugins/filter/mathstuff.py +++ b/lib/ansible/plugins/filter/mathstuff.py @@ -28,7 +28,7 @@ import math from jinja2.filters import environmentfilter -from ansible.errors import AnsibleFilterError +from ansible.errors import AnsibleFilterError, AnsibleFilterTypeError from ansible.module_utils.common.text import formatters from ansible.module_utils.six import binary_type, text_type from ansible.module_utils.six.moves import zip, zip_longest @@ -140,14 +140,14 @@ def logarithm(x, base=math.e): else: return math.log(x, base) except TypeError as e: - raise AnsibleFilterError('log() can only be used on numbers: %s' % to_native(e)) + raise AnsibleFilterTypeError('log() can only be used on numbers: %s' % to_native(e)) def power(x, y): try: return math.pow(x, y) except TypeError as e: - raise AnsibleFilterError('pow() can only be used on numbers: %s' % to_native(e)) + raise AnsibleFilterTypeError('pow() can only be used on numbers: %s' % to_native(e)) def inversepower(x, base=2): @@ -157,13 +157,15 @@ def inversepower(x, base=2): else: return math.pow(x, 1.0 / float(base)) except (ValueError, TypeError) as e: - raise AnsibleFilterError('root() can only be used on numbers: %s' % to_native(e)) + raise AnsibleFilterTypeError('root() can only be used on numbers: %s' % to_native(e)) def human_readable(size, isbits=False, unit=None): ''' Return a human readable string ''' try: return formatters.bytes_to_human(size, isbits, unit) + except TypeError as e: + raise AnsibleFilterTypeError("human_readable() failed on bad input: %s" % to_native(e)) except Exception: raise AnsibleFilterError("human_readable() can't interpret following string: %s" % size) @@ -172,6 +174,8 @@ def human_to_bytes(size, default_unit=None, isbits=False): ''' Return bytes count from a human readable string ''' try: return formatters.human_to_bytes(size, default_unit, isbits) + except TypeError as e: + raise AnsibleFilterTypeError("human_to_bytes() failed on bad input: %s" % to_native(e)) except Exception: raise AnsibleFilterError("human_to_bytes() can't interpret following string: %s" % size) @@ -195,16 +199,18 @@ def rekey_on_member(data, key, duplicates='error'): elif isinstance(data, Iterable) and not isinstance(data, (text_type, binary_type)): iterate_over = data else: - raise AnsibleFilterError("Type is not a valid list, set, or dict") + raise AnsibleFilterTypeError("Type is not a valid list, set, or dict") for item in iterate_over: if not isinstance(item, Mapping): - raise AnsibleFilterError("List item is not a valid dict") + raise AnsibleFilterTypeError("List item is not a valid dict") try: key_elem = item[key] except KeyError: raise AnsibleFilterError("Key {0} was not found".format(key)) + except TypeError as e: + raise AnsibleFilterTypeError(to_native(e)) except Exception as e: raise AnsibleFilterError(to_native(e)) diff --git a/test/integration/targets/filter_core/handle_undefined_type_errors.yml b/test/integration/targets/filter_core/handle_undefined_type_errors.yml new file mode 100644 index 00000000000..70628809a4b --- /dev/null +++ b/test/integration/targets/filter_core/handle_undefined_type_errors.yml @@ -0,0 +1,29 @@ +- hosts: localhost + gather_facts: false + tasks: + - debug: msg={{item}} + with_dict: '{{myundef}}' + when: + - myundef is defined + register: shouldskip + + - name: check if skipped + assert: + that: + - shouldskip is skipped + + - debug: msg={{item}} + loop: '{{myundef|dict2items}}' + when: + - myundef is defined + + - debug: msg={{item}} + with_dict: '{{myundef}}' + register: notskipped + ignore_errors: true + + - name: check it failed + assert: + that: + - notskipped is not skipped + - notskipped is failed diff --git a/test/integration/targets/filter_core/runme.sh b/test/integration/targets/filter_core/runme.sh index ac4361f3a45..c055603b0f4 100755 --- a/test/integration/targets/filter_core/runme.sh +++ b/test/integration/targets/filter_core/runme.sh @@ -3,3 +3,4 @@ set -eux ANSIBLE_ROLES_PATH=../ ansible-playbook runme.yml "$@" +ANSIBLE_ROLES_PATH=../ ansible-playbook handle_undefined_type_errors.yml "$@" diff --git a/test/integration/targets/filter_core/tasks/main.yml b/test/integration/targets/filter_core/tasks/main.yml index 4f7517004be..3b5cba39ee9 100644 --- a/test/integration/targets/filter_core/tasks/main.yml +++ b/test/integration/targets/filter_core/tasks/main.yml @@ -528,7 +528,7 @@ - subelements_1 is failed - 'subelements_1.msg == "obj must be a list of dicts or a nested dict"' - subelements_2 is failed - - 'subelements_2.msg == "subelements must be a list or a string"' + - '"subelements must be a list or a string" in subelements_2.msg' - 'subelements_demo|subelements("does not compute", skip_missing=True) == []' - subelements_3 is failed - '"could not find" in subelements_3.msg' diff --git a/test/integration/targets/filter_mathstuff/tasks/main.yml b/test/integration/targets/filter_mathstuff/tasks/main.yml index b78b9c4ee16..2a708be1d18 100644 --- a/test/integration/targets/filter_mathstuff/tasks/main.yml +++ b/test/integration/targets/filter_mathstuff/tasks/main.yml @@ -183,7 +183,7 @@ - '"0.10 GB" == 102400000|human_readable(unit="G")' - '"0.10 Gb" == 102400000|human_readable(isbits=True, unit="G")' - human_readable_exc1_res is failed - - '"interpret following string" in human_readable_exc1_res.msg' + - '"failed on bad input" in human_readable_exc1_res.msg' - name: Verify human_to_bytes tags: human_to_bytes diff --git a/test/units/plugins/filter/test_mathstuff.py b/test/units/plugins/filter/test_mathstuff.py index f0df17fc304..a0e78d338cb 100644 --- a/test/units/plugins/filter/test_mathstuff.py +++ b/test/units/plugins/filter/test_mathstuff.py @@ -9,7 +9,7 @@ import pytest from jinja2 import Environment import ansible.plugins.filter.mathstuff as ms -from ansible.errors import AnsibleFilterError +from ansible.errors import AnsibleFilterError, AnsibleFilterTypeError UNIQUE_DATA = (([1, 3, 4, 2], sorted([1, 2, 3, 4])), @@ -79,9 +79,9 @@ class TestMax: class TestLogarithm: def test_log_non_number(self): # Message changed in python3.6 - with pytest.raises(AnsibleFilterError, match='log\\(\\) can only be used on numbers: (a float is required|must be real number, not str)'): + with pytest.raises(AnsibleFilterTypeError, match='log\\(\\) can only be used on numbers: (a float is required|must be real number, not str)'): ms.logarithm('a') - with pytest.raises(AnsibleFilterError, match='log\\(\\) can only be used on numbers: (a float is required|must be real number, not str)'): + with pytest.raises(AnsibleFilterTypeError, match='log\\(\\) can only be used on numbers: (a float is required|must be real number, not str)'): ms.logarithm(10, base='a') def test_log_ten(self): @@ -98,10 +98,10 @@ class TestLogarithm: class TestPower: def test_power_non_number(self): # Message changed in python3.6 - with pytest.raises(AnsibleFilterError, match='pow\\(\\) can only be used on numbers: (a float is required|must be real number, not str)'): + with pytest.raises(AnsibleFilterTypeError, match='pow\\(\\) can only be used on numbers: (a float is required|must be real number, not str)'): ms.power('a', 10) - with pytest.raises(AnsibleFilterError, match='pow\\(\\) can only be used on numbers: (a float is required|must be real number, not str)'): + with pytest.raises(AnsibleFilterTypeError, match='pow\\(\\) can only be used on numbers: (a float is required|must be real number, not str)'): ms.power(10, 'a') def test_power_squared(self): @@ -114,13 +114,13 @@ class TestPower: class TestInversePower: def test_root_non_number(self): # Messages differed in python-2.6, python-2.7-3.5, and python-3.6+ - with pytest.raises(AnsibleFilterError, match="root\\(\\) can only be used on numbers:" + with pytest.raises(AnsibleFilterTypeError, match="root\\(\\) can only be used on numbers:" " (invalid literal for float\\(\\): a" "|could not convert string to float: a" "|could not convert string to float: 'a')"): ms.inversepower(10, 'a') - with pytest.raises(AnsibleFilterError, match="root\\(\\) can only be used on numbers: (a float is required|must be real number, not str)"): + with pytest.raises(AnsibleFilterTypeError, match="root\\(\\) can only be used on numbers: (a float is required|must be real number, not str)"): ms.inversepower('a', 10) def test_square_root(self): @@ -145,27 +145,27 @@ class TestRekeyOnMember(): # (Input data structure, member to rekey on, expected error message) INVALID_ENTRIES = ( # Fail when key is not found - ([{"proto": "eigrp", "state": "enabled"}], 'invalid_key', "Key invalid_key was not found"), - ({"eigrp": {"proto": "eigrp", "state": "enabled"}}, 'invalid_key', "Key invalid_key was not found"), + (AnsibleFilterError, [{"proto": "eigrp", "state": "enabled"}], 'invalid_key', "Key invalid_key was not found"), + (AnsibleFilterError, {"eigrp": {"proto": "eigrp", "state": "enabled"}}, 'invalid_key', "Key invalid_key was not found"), # Fail when key is duplicated - ([{"proto": "eigrp"}, {"proto": "ospf"}, {"proto": "ospf"}], + (AnsibleFilterError, [{"proto": "eigrp"}, {"proto": "ospf"}, {"proto": "ospf"}], 'proto', 'Key ospf is not unique, cannot correctly turn into dict'), # Fail when value is not a dict - (["string"], 'proto', "List item is not a valid dict"), - ([123], 'proto', "List item is not a valid dict"), - ([[{'proto': 1}]], 'proto', "List item is not a valid dict"), + (AnsibleFilterTypeError, ["string"], 'proto', "List item is not a valid dict"), + (AnsibleFilterTypeError, [123], 'proto', "List item is not a valid dict"), + (AnsibleFilterTypeError, [[{'proto': 1}]], 'proto', "List item is not a valid dict"), # Fail when we do not send a dict or list - ("string", 'proto', "Type is not a valid list, set, or dict"), - (123, 'proto', "Type is not a valid list, set, or dict"), + (AnsibleFilterTypeError, "string", 'proto', "Type is not a valid list, set, or dict"), + (AnsibleFilterTypeError, 123, 'proto', "Type is not a valid list, set, or dict"), ) @pytest.mark.parametrize("list_original, key, expected", VALID_ENTRIES) def test_rekey_on_member_success(self, list_original, key, expected): assert ms.rekey_on_member(list_original, key) == expected - @pytest.mark.parametrize("list_original, key, expected", INVALID_ENTRIES) - def test_fail_rekey_on_member(self, list_original, key, expected): - with pytest.raises(AnsibleFilterError) as err: + @pytest.mark.parametrize("expected_exception_type, list_original, key, expected", INVALID_ENTRIES) + def test_fail_rekey_on_member(self, expected_exception_type, list_original, key, expected): + with pytest.raises(expected_exception_type) as err: ms.rekey_on_member(list_original, key) assert err.value.message == expected