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

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

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

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

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

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" },

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']:

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