Do not convert floats to ints when there is truncation (#83864)

Adjusted error messages
fixed tests
removed py2 compat tests, since no more py2

Co-authored-by: Matt Clay <matt@mystile.com>

changelogs/fragments/fix_floating_ints.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - Avoid truncating floats when casting into int, as it can lead to truncation and unexpected results. 0.99999 will be 0, not 1.

lib/ansible/config/manager.py

@@ -4,6 +4,7 @@
 from __future__ import annotations
 
 import atexit
+import decimal
 import configparser
 import os
 import os.path
@@ -101,10 +102,18 @@ def ensure_type(value, value_type, origin=None, origin_ftype=None):
             value = boolean(value, strict=False)
 
         elif value_type in ('integer', 'int'):
-            value = int(value)
+            if not isinstance(value, int):
+                try:
+                    if (decimal_value := decimal.Decimal(value)) == (int_part := int(decimal_value)):
+                        value = int_part
+                    else:
+                        errmsg = 'int'
+                except decimal.DecimalException as e:
+                    raise ValueError from e
 
         elif value_type == 'float':
-            value = float(value)
+            if not isinstance(value, float):
+                value = float(value)
 
         elif value_type == 'list':
             if isinstance(value, string_types):
@@ -173,7 +182,7 @@ def ensure_type(value, value_type, origin=None, origin_ftype=None):
                 value = unquote(value)
 
         if errmsg:
-            raise ValueError('Invalid type provided for "%s": %s' % (errmsg, to_native(value)))
+            raise ValueError(f'Invalid type provided for "{errmsg}": {value!r}')
 
     return to_text(value, errors='surrogate_or_strict', nonstring='passthru')
 

lib/ansible/module_utils/common/validation.py

@@ -4,6 +4,7 @@
 
 from __future__ import annotations
 
+import decimal
 import json
 import os
 import re
@@ -17,7 +18,6 @@ from ansible.module_utils.common.warnings import deprecate
 from ansible.module_utils.parsing.convert_bool import boolean
 from ansible.module_utils.six import (
     binary_type,
-    integer_types,
     string_types,
     text_type,
 )
@@ -506,16 +506,15 @@ def check_type_int(value):
 
     :return: int of given value
     """
-    if isinstance(value, integer_types):
+    if not isinstance(value, int):
-        return value
-
-    if isinstance(value, string_types):
         try:
-            return int(value)
+            if (decimal_value := decimal.Decimal(value)) != (int_value := int(decimal_value)):
-        except ValueError:
+                raise ValueError("Significant decimal part found")
-            pass
+            else:
-
+                value = int_value
-    raise TypeError('%s cannot be converted to an int' % type(value))
+        except (decimal.DecimalException, TypeError, ValueError) as e:
+            raise TypeError(f'"{value!r}" cannot be converted to an int') from e
+    return value
 
 
 def check_type_float(value):
@@ -527,16 +526,12 @@ def check_type_float(value):
 
     :returns: float of given value.
     """
-    if isinstance(value, float):
+    if not isinstance(value, float):
-        return value
-
-    if isinstance(value, (binary_type, text_type, int)):
         try:
-            return float(value)
+            value = float(value)
-        except ValueError:
+        except (TypeError, ValueError) as e:
-            pass
+            raise TypeError(f'{type(value)} cannot be converted to a float')
-
+    return value
-    raise TypeError('%s cannot be converted to a float' % type(value))
 
 
 def check_type_path(value,):

lib/ansible/playbook/base.py

@@ -4,6 +4,7 @@
 
 from __future__ import annotations
 
+import decimal
 import itertools
 import operator
 import os
@@ -440,7 +441,13 @@ class FieldAttributeBase:
         if attribute.isa == 'string':
             value = to_text(value)
         elif attribute.isa == 'int':
-            value = int(value)
+            if not isinstance(value, int):
+                try:
+                    if (decimal_value := decimal.Decimal(value)) != (int_value := int(decimal_value)):
+                        raise decimal.DecimalException(f'Floating-point value {value!r} would be truncated.')
+                    value = int_value
+                except decimal.DecimalException as e:
+                    raise ValueError from e
         elif attribute.isa == 'float':
             value = float(value)
         elif attribute.isa == 'bool':

test/integration/targets/config/types.yml

@@ -12,7 +12,6 @@
             notvalid: '{{ lookup("config", "notvalid", plugin_type="lookup", plugin_name="types") }}'
             totallynotvalid: '{{ lookup("config", "totallynotvalid", plugin_type="lookup", plugin_name="types") }}'
             str_mustunquote: '{{ lookup("config", "str_mustunquote", plugin_type="lookup", plugin_name="types") }}'
-
         - assert:
             that:
             - 'valid|type_debug == "list"'

test/integration/targets/roles_arg_spec/test.yml

@@ -231,8 +231,7 @@
               - ansible_failed_result.argument_spec_data == c_main_spec
           vars:
             error: >-
-              argument 'c_int' is of type <class 'NoneType'> and we were unable to convert to int:
+              argument 'c_int' is of type <class 'NoneType'> and we were unable to convert to int: "None" cannot be converted to an int
-              <class 'NoneType'> cannot be converted to an int
 
     - name: test type coercion fails on None for required list
       block:

test/integration/targets/roles_arg_spec/test_complex_role_fails.yml

@@ -91,84 +91,6 @@
             - debug:
                 var: ansible_failed_result
 
-            - name: replace py version specific types with generic names so tests work on py2 and py3
-              set_fact:
-                  # We want to compare if the actual failure messages and the expected failure messages
-                  # are the same. But to compare and do set differences, we have to handle some
-                  # differences between py2/py3.
-                  # The validation failure messages include python type and class reprs, which are
-                  # different between py2 and py3. For ex, "<type 'str'>" vs "<class 'str'>". Plus
-                  # the usual py2/py3 unicode/str/bytes type shenanigans. The 'THE_FLOAT_REPR' is
-                  # because py3 quotes the value in the error while py2 does not, so we just ignore
-                  # the rest of the line.
-                  actual_generic: "{{ ansible_failed_result.argument_errors|
-                                      map('replace', ansible_unicode_type_match, 'STR')|
-                                      map('replace', unicode_type_match, 'STR')|
-                                      map('replace', string_type_match, 'STR')|
-                                      map('replace', float_type_match, 'FLOAT')|
-                                      map('replace', list_type_match, 'LIST')|
-                                      map('replace', ansible_list_type_match, 'LIST')|
-                                      map('replace', dict_type_match, 'DICT')|
-                                      map('replace', ansible_dict_type_match, 'DICT')|
-                                      map('replace', ansible_unicode_class_match, 'STR')|
-                                      map('replace', unicode_class_match, 'STR')|
-                                      map('replace', string_class_match, 'STR')|
-                                      map('replace', bytes_class_match, 'STR')|
-                                      map('replace', float_class_match, 'FLOAT')|
-                                      map('replace', list_class_match, 'LIST')|
-                                      map('replace', ansible_list_class_match, 'LIST')|
-                                      map('replace', dict_class_match, 'DICT')|
-                                      map('replace', ansible_dict_class_match, 'DICT')|
-                                      map('regex_replace', '''float:.*$''', 'THE_FLOAT_REPR')|
-                                      map('regex_replace', 'Valid booleans include.*$', '')|
-                                      list }}"
-                  expected_generic: "{{ expected.test1_1.argument_errors|
-                                        map('replace', ansible_unicode_type_match, 'STR')|
-                                        map('replace', unicode_type_match, 'STR')|
-                                        map('replace', string_type_match, 'STR')|
-                                        map('replace', float_type_match, 'FLOAT')|
-                                        map('replace', list_type_match, 'LIST')|
-                                        map('replace', ansible_list_type_match, 'LIST')|
-                                        map('replace', dict_type_match, 'DICT')|
-                                        map('replace', ansible_dict_type_match, 'DICT')|
-                                        map('replace', ansible_unicode_class_match, 'STR')|
-                                        map('replace', unicode_class_match, 'STR')|
-                                        map('replace', string_class_match, 'STR')|
-                                        map('replace', bytes_class_match, 'STR')|
-                                        map('replace', float_class_match, 'FLOAT')|
-                                        map('replace', list_class_match, 'LIST')|
-                                        map('replace', ansible_list_class_match, 'LIST')|
-                                        map('replace', dict_class_match, 'DICT')|
-                                        map('replace', ansible_dict_class_match, 'DICT')|
-                                        map('regex_replace', '''float:.*$''', 'THE_FLOAT_REPR')|
-                                        map('regex_replace', 'Valid booleans include.*$', '')|
-                                        list }}"
-
-            - name: figure out the difference between expected and actual validate_argument_spec failures
-              set_fact:
-                  actual_not_in_expected: "{{ actual_generic| difference(expected_generic) | sort() }}"
-                  expected_not_in_actual: "{{ expected_generic | difference(actual_generic) | sort() }}"
-
-            - name: assert that all actual validate_argument_spec failures were in expected
-              assert:
-                  that:
-                      - actual_not_in_expected | length == 0
-                  msg: "Actual validate_argument_spec failures that were not expected: {{ actual_not_in_expected }}"
-
-            - name: assert that all expected validate_argument_spec failures were in expected
-              assert:
-                  that:
-                      - expected_not_in_actual | length == 0
-                  msg: "Expected validate_argument_spec failures that were not in actual results: {{ expected_not_in_actual }}"
-
-            - name: assert that `validate_args_context` return value has what we expect
-              assert:
-                that:
-                  - ansible_failed_result.validate_args_context.argument_spec_name == "main"
-                  - ansible_failed_result.validate_args_context.name == "test1"
-                  - ansible_failed_result.validate_args_context.type == "role"
-                  - "ansible_failed_result.validate_args_context.path is search('roles_arg_spec/roles/test1')"
-
       - name: test message for missing required parameters and invalid suboptions
         block:
             - include_role:

test/units/module_utils/basic/test_argument_spec.py

@@ -71,14 +71,14 @@ VALID_SPECS = (
 
 INVALID_SPECS = (
     # Type is int; unable to convert this string
-    ({'arg': {'type': 'int'}}, {'arg': "wolf"}, "is of type {0} and we were unable to convert to int: {0} cannot be converted to an int".format(type('bad'))),
+    ({'arg': {'type': 'int'}}, {'arg': "wolf"}, f"is of type {type('wolf')} and we were unable to convert to int:"),
     # Type is list elements is int; unable to convert this string
-    ({'arg': {'type': 'list', 'elements': 'int'}}, {'arg': [1, "bad"]}, "is of type {0} and we were unable to convert to int: {0} cannot be converted to "
+    ({'arg': {'type': 'list', 'elements': 'int'}}, {'arg': [1, "bad"]},
-                                                                        "an int".format(type('int'))),
+     "is of type {0} and we were unable to convert to int:".format(type('int'))),
     # Type is int; unable to convert float
-    ({'arg': {'type': 'int'}}, {'arg': 42.1}, "'float'> cannot be converted to an int"),
+    ({'arg': {'type': 'int'}}, {'arg': 42.1}, "'float'> and we were unable to convert to int:"),
     # Type is list, elements is int; unable to convert float
-    ({'arg': {'type': 'list', 'elements': 'int'}}, {'arg': [42.1, 32, 2]}, "'float'> cannot be converted to an int"),
+    ({'arg': {'type': 'list', 'elements': 'int'}}, {'arg': [42.1, 32, 2]}, "'float'> and we were unable to convert to int:"),
     # type is a callable that fails to convert
     ({'arg': {'type': MOCK_VALIDATOR_FAIL}}, {'arg': "bad"}, "bad conversion"),
     # type is a list, elements is callable that fails to convert

test/units/module_utils/common/arg_spec/test_validate_invalid.py

@@ -89,7 +89,7 @@ INVALID_SPECS = [
         {'numbers': [55, 33, 34, {'key': 'value'}]},
         {'numbers': [55, 33, 34]},
         set(),
-        "Elements value for option 'numbers' is of type <class 'dict'> and we were unable to convert to int: <class 'dict'> cannot be converted to an int"
+        "Elements value for option 'numbers' is of type <class 'dict'> and we were unable to convert to int:"
     ),
     (
         'required',