Make filter type errors 'loop friendly' (#70417) (#70574)

- 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 <shertel@redhat.com>

Co-authored-by: Sloane Hertel <shertel@redhat.com>
(cherry picked from commit cf89ca8a03)

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.

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

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):

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))
 

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

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 "$@"

test/integration/targets/filter_core/tasks/main.yml

@@ -501,7 +501,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'

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

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"),
+        (AnsibleFilterError, [{"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, {"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"),
+        (AnsibleFilterTypeError, ["string"], 'proto', "List item is not a valid dict"),
-        ([123], 'proto', "List item is not a valid dict"),
+        (AnsibleFilterTypeError, [123], 'proto', "List item is not a valid dict"),
-        ([[{'proto': 1}]], '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"),
+        (AnsibleFilterTypeError, "string", 'proto', "Type is not a valid list, set, or dict"),
-        (123, '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)
+    @pytest.mark.parametrize("expected_exception_type, list_original, key, expected", INVALID_ENTRIES)
-    def test_fail_rekey_on_member(self, list_original, key, expected):
+    def test_fail_rekey_on_member(self, expected_exception_type, list_original, key, expected):
-        with pytest.raises(AnsibleFilterError) as err:
+        with pytest.raises(expected_exception_type) as err:
             ms.rekey_on_member(list_original, key)
 
         assert err.value.message == expected