From f52a0888622788a4d202d2a563dfd54dd90b65a3 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Fri, 22 Feb 2019 16:44:32 -0500 Subject: [PATCH] Add option to ignore, warn, or error when a module parameter is converted to a string (#51404) * Add new module property to Windows modules * Add brief pause to file tests to ensure the stat times are not equal, which was happening sometimes. * Raise TypeError on error rather than fail_json() * Rework error message to be less verbose * Add porting guide entry --- ...heck_type_string-option-when-converting.yaml | 2 ++ .../rst/porting_guides/porting_guide_2.8.rst | 15 ++++++++++----- lib/ansible/config/base.yml | 15 +++++++++++++++ lib/ansible/module_utils/basic.py | 17 ++++++++++++++--- .../module_utils/csharp/Ansible.Basic.cs | 1 + lib/ansible/plugins/action/__init__.py | 3 +++ test/integration/targets/file/tasks/main.yml | 4 ++++ 7 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/check_type_string-option-when-converting.yaml diff --git a/changelogs/fragments/check_type_string-option-when-converting.yaml b/changelogs/fragments/check_type_string-option-when-converting.yaml new file mode 100644 index 00000000000..d7cfaa23ee7 --- /dev/null +++ b/changelogs/fragments/check_type_string-option-when-converting.yaml @@ -0,0 +1,2 @@ +minor_changes: + - 'add ``STRING_CONVERSION_ACTION`` option to warn, error, or ignore when a module parameter is string type but the value from YAML is not a string type and it is converted (https://github.com/ansible/ansible/issues/50503)' diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst index 8df162d97b6..5126f4dcaaa 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst @@ -50,16 +50,22 @@ In Ansible 2.8:: In Ansible 2.7 and older:: {{ ((foo | default({})).bar | default({})).baz | default('DEFAULT') }} - + or - + {{ foo.bar.baz if (foo is defined and foo.bar is defined and foo.bar.baz is defined) else 'DEFAULT' }} +Module option conversion to string +---------------------------------- + +Beginning in version 2.8, Ansible will warn if a module expects a string, but a non-string value is passed and automatically converted to a string. This highlights potential problems where, for example, a ``yes`` or ``true`` (parsed as truish boolean value) would be converted to the string ``'True'``, or where a version number ``1.10`` (parsed as float value) would be converted to ``'1.0'``. Such conversions can result in unexpected behavior depending on context. + +This behavior can be changed to be an error or to be ignored by setting the ``ANSIBLE_STRING_CONVERSION_ACTION`` environment variable, or by setting the ``string_conversion_action`` configuration in the ``defaults`` section of ``ansible.cfg``. + Command line facts ------------------ -``cmdline`` facts returned in system will be deprecated in favor of ``proc_cmdline``. This change handles special case where Kernel command line parameter -contains multiple values with the same key. +``cmdline`` facts returned in system will be deprecated in favor of ``proc_cmdline``. This change handles special case where Kernel command line parameter contains multiple values with the same key. Command Line ============ @@ -122,7 +128,6 @@ PowerShell module options and option choices are currently case insensitive to w specification. This behaviour is deprecated and a warning displayed to the user if a case insensitive match was found. A future release of Ansible will make these checks case sensitive. - Modules removed --------------- diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index ce7ec70b0b2..a31b7e20cf0 100644 --- a/lib/ansible/config/base.yml +++ b/lib/ansible/config/base.yml @@ -1704,4 +1704,19 @@ NETCONF_SSH_CONFIG: - {key: ssh_config, section: netconf_connection} yaml: {key: netconf_connection.ssh_config} default: null +STRING_CONVERSION_ACTION: + version_added: '2.8' + description: + - Action to take when a module parameter value is converted to a string (this does not affect variables). + For string parameters, values such as '1.00', "['a', 'b',]", and 'yes', 'y', etc. + will be converted by the YAML parser unless fully quoted. + - Valid options are 'error', 'warn', and 'ignore'. + - Since 2.8, this option defaults to 'warn' but will change to 'error' in 2.12. + default: 'warn' + env: + - name: ANSIBLE_STRING_CONVERSION_ACTION + ini: + - section: defaults + key: string_conversion_action + type: string ... diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 905bc1b282c..eb7c77f50b1 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -51,6 +51,7 @@ PASS_VARS = { 'shell_executable': '_shell', 'socket': '_socket_path', 'syslog_facility': '_syslog_facility', + 'string_conversion_action': '_string_conversion_action', 'tmpdir': '_tmpdir', 'verbosity': '_verbosity', 'version': 'ansible_version', @@ -790,6 +791,7 @@ class AnsibleModule(object): self._warnings = [] self._deprecations = [] self._clean = {} + self._string_conversion_action = '' self.aliases = {} self._legal_inputs = ['_ansible_%s' % k for k in PASS_VARS] @@ -1859,9 +1861,18 @@ class AnsibleModule(object): def _check_type_str(self, value): if isinstance(value, string_types): return value - # Note: This could throw a unicode error if value's __str__() method - # returns non-ascii. Have to port utils.to_bytes() if that happens - return str(value) + + # Ignore, warn, or error when converting to a string. + # The current default is to warn. Change this in Anisble 2.12 to error. + common_msg = 'quote the entire value to ensure it does not change.' + if self._string_conversion_action == 'error': + msg = common_msg.capitalize() + raise TypeError(msg) + elif self._string_conversion_action == 'warn': + msg = ('The value {0!r} (type {0.__class__.__name__}) in a string field was converted to {1!r} (type string). ' + 'If this does not look like what you expect, {2}').format(value, to_text(value), common_msg) + self.warn(msg) + return to_native(value, errors='surrogate_or_strict') def _check_type_list(self, value): if isinstance(value, list): diff --git a/lib/ansible/module_utils/csharp/Ansible.Basic.cs b/lib/ansible/module_utils/csharp/Ansible.Basic.cs index 54c3e9d70fa..845d25eeb41 100644 --- a/lib/ansible/module_utils/csharp/Ansible.Basic.cs +++ b/lib/ansible/module_utils/csharp/Ansible.Basic.cs @@ -63,6 +63,7 @@ namespace Ansible.Basic { "selinux_special_fs", null }, { "shell_executable", null }, { "socket", null }, + { "string_conversion_action", null }, { "syslog_facility", null }, { "tmpdir", "tmpdir" }, { "verbosity", "Verbosity" }, diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 4c28ef215aa..59e7c85e8a1 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -683,6 +683,9 @@ class ActionBase(with_metaclass(ABCMeta, object)): # let module know about filesystems that selinux treats specially module_args['_ansible_selinux_special_fs'] = C.DEFAULT_SELINUX_SPECIAL_FS + # what to do when parameter values are converted to strings + module_args['_ansible_string_conversion_action'] = C.STRING_CONVERSION_ACTION + # give the module the socket for persistent connections module_args['_ansible_socket'] = getattr(self._connection, 'socket_path') if not module_args['_ansible_socket']: diff --git a/test/integration/targets/file/tasks/main.yml b/test/integration/targets/file/tasks/main.yml index dfdada0b318..52957fff054 100644 --- a/test/integration/targets/file/tasks/main.yml +++ b/test/integration/targets/file/tasks/main.yml @@ -493,6 +493,10 @@ - name: create file as root with all write permissions file: dest=/tmp/write_utime state=touch mode=0666 owner={{ansible_user}} +- name: Pause to ensure stat times are not the exact same + pause: + seconds: 1 + - block: - name: get previous time stat: path=/tmp/write_utime