summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBrian Coca <bcoca@users.noreply.github.com>2024-09-20 16:06:15 +0200
committerGitHub <noreply@github.com>2024-09-20 16:06:15 +0200
commit6efb30b43e89e56061311235b3ee97181039a1c9 (patch)
treeb0299aaf93af54d72b74cd6222097d5e16b152cd
parentAdd mount_facts module (#83508) (diff)
downloadansible-6efb30b43e89e56061311235b3ee97181039a1c9.tar.xz
ansible-6efb30b43e89e56061311235b3ee97181039a1c9.zip
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>
-rw-r--r--changelogs/fragments/fix_floating_ints.yml2
-rw-r--r--lib/ansible/config/manager.py15
-rw-r--r--lib/ansible/module_utils/common/validation.py33
-rw-r--r--lib/ansible/playbook/base.py9
-rw-r--r--test/integration/targets/config/types.yml1
-rw-r--r--test/integration/targets/roles_arg_spec/test.yml3
-rw-r--r--test/integration/targets/roles_arg_spec/test_complex_role_fails.yml78
-rw-r--r--test/units/module_utils/basic/test_argument_spec.py10
-rw-r--r--test/units/module_utils/common/arg_spec/test_validate_invalid.py2
9 files changed, 43 insertions, 110 deletions
diff --git a/changelogs/fragments/fix_floating_ints.yml b/changelogs/fragments/fix_floating_ints.yml
new file mode 100644
index 0000000000..7a8df434a0
--- /dev/null
+++ b/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.
diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py
index 2336ae1f4a..e4e7b6a8e9 100644
--- a/lib/ansible/config/manager.py
+++ b/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')
diff --git a/lib/ansible/module_utils/common/validation.py b/lib/ansible/module_utils/common/validation.py
index c37d9d3097..399767e775 100644
--- a/lib/ansible/module_utils/common/validation.py
+++ b/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):
- return value
-
- if isinstance(value, string_types):
+ if not isinstance(value, int):
try:
- return int(value)
- except ValueError:
- pass
-
- raise TypeError('%s cannot be converted to an int' % type(value))
+ if (decimal_value := decimal.Decimal(value)) != (int_value := int(decimal_value)):
+ raise ValueError("Significant decimal part found")
+ else:
+ value = int_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):
- return value
-
- if isinstance(value, (binary_type, text_type, int)):
+ if not isinstance(value, float):
try:
- return float(value)
- except ValueError:
- pass
-
- raise TypeError('%s cannot be converted to a float' % type(value))
+ value = float(value)
+ except (TypeError, ValueError) as e:
+ raise TypeError(f'{type(value)} cannot be converted to a float')
+ return value
def check_type_path(value,):
diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py
index 552dfbb140..f93fb5ef5f 100644
--- a/lib/ansible/playbook/base.py
+++ b/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':
diff --git a/test/integration/targets/config/types.yml b/test/integration/targets/config/types.yml
index fe7e6df37f..7e67710eec 100644
--- a/test/integration/targets/config/types.yml
+++ b/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"'
diff --git a/test/integration/targets/roles_arg_spec/test.yml b/test/integration/targets/roles_arg_spec/test.yml
index b88d2e183d..26beb21055 100644
--- a/test/integration/targets/roles_arg_spec/test.yml
+++ b/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:
- <class 'NoneType'> cannot be converted to an int
+ argument 'c_int' is of type <class 'NoneType'> and we were unable to convert to int: "None" cannot be converted to an int
- name: test type coercion fails on None for required list
block:
diff --git a/test/integration/targets/roles_arg_spec/test_complex_role_fails.yml b/test/integration/targets/roles_arg_spec/test_complex_role_fails.yml
index 81abdaa8c2..99db01d8f4 100644
--- a/test/integration/targets/roles_arg_spec/test_complex_role_fails.yml
+++ b/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:
diff --git a/test/units/module_utils/basic/test_argument_spec.py b/test/units/module_utils/basic/test_argument_spec.py
index f75883a44b..46a3e9d9f3 100644
--- a/test/units/module_utils/basic/test_argument_spec.py
+++ b/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 "
- "an int".format(type('int'))),
+ ({'arg': {'type': 'list', 'elements': 'int'}}, {'arg': [1, "bad"]},
+ "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
diff --git a/test/units/module_utils/common/arg_spec/test_validate_invalid.py b/test/units/module_utils/common/arg_spec/test_validate_invalid.py
index 2e905849c4..200cd37a4f 100644
--- a/test/units/module_utils/common/arg_spec/test_validate_invalid.py
+++ b/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',