Deprecate check_invalid_arguments (#34004)

* Deprecate check_invalid_arguments

Check_invalid_arguments is a piece of functionality from the early days
of Ansible that should not be used.  We'll remove it in Ansible 2.9.
Deprecating it for now.

CHANGELOG.md

@@ -19,6 +19,19 @@ Ansible Changes By Release
   and these options will become undocumented in Ansible 2.9 and removed in a
   later version.
 * The `redis_kv` lookup in favor of new `redis` lookup
+* Passing arbitrary parameters that begin with `HEADER_` to the uri module,
+  used for passing http headers, is deprecated.  Use the ``headers`` parameter
+  with a dictionary of header names to value instead.  This will be removed in
+  Ansible-2.9
+* Passing arbitrary parameters to the zfs module to set zfs properties is
+  deprecated.  Use the ``extra_zfs_properties`` parameter with a dictionary of
+  property names to values instead.  This will be removed in Ansible-2.9.
+* Use of the AnsibleModule parameter, check_invalid_arguments, in custom modules
+  is deprecated.  In the future, all parameters will be checked to see whether
+  they are listed in the arg spec and an error raised if they are not listed.
+  This behaviour is the current and future default so most custom modules can
+  simply remove check_invalid_arguments if they set it to the default of True.
+  check_invalid_arguments will be removed in Ansible-2.9.
 
 ### Minor Changes
 * added a few new magic vars corresponding to configuration/command line options:

lib/ansible/module_utils/azure_rm_common.py

@@ -191,7 +191,7 @@ AZURE_MIN_RELEASE = '2.0.0'
 
 class AzureRMModuleBase(object):
     def __init__(self, derived_arg_spec, bypass_checks=False, no_log=False,
-                 check_invalid_arguments=True, mutually_exclusive=None, required_together=None,
+                 check_invalid_arguments=None, mutually_exclusive=None, required_together=None,
                  required_one_of=None, add_file_common_args=False, supports_check_mode=False,
                  required_if=None, supports_tags=True, facts_module=False, skip_exec=False):
 

lib/ansible/module_utils/basic.py

@@ -771,7 +771,7 @@ class AnsibleFallbackNotFound(Exception):
 
 class AnsibleModule(object):
     def __init__(self, argument_spec, bypass_checks=False, no_log=False,
-                 check_invalid_arguments=True, mutually_exclusive=None, required_together=None,
+                 check_invalid_arguments=None, mutually_exclusive=None, required_together=None,
                  required_one_of=None, add_file_common_args=False, supports_check_mode=False,
                  required_if=None):
 
@@ -787,7 +787,15 @@ class AnsibleModule(object):
         self.check_mode = False
         self.bypass_checks = bypass_checks
         self.no_log = no_log
+
+        # Check whether code set this explicitly for deprecation purposes
+        if check_invalid_arguments is None:
+            check_invalid_arguments = True
+            module_set_check_invalid_arguments = False
+        else:
+            module_set_check_invalid_arguments = True
         self.check_invalid_arguments = check_invalid_arguments
+
         self.mutually_exclusive = mutually_exclusive
         self.required_together = required_together
         self.required_one_of = required_one_of
@@ -876,6 +884,15 @@ class AnsibleModule(object):
         # finally, make sure we're in a sane working dir
         self._set_cwd()
 
+        # Do this at the end so that logging parameters have been set up
+        # This is to warn third party module authors that the functionatlity is going away.
+        # We exclude uri and zfs as they have their own deprecation warnings for users and we'll
+        # make sure to update their code to stop using check_invalid_arguments when 2.9 rolls around
+        if module_set_check_invalid_arguments and self._name not in ('uri', 'zfs'):
+            self.deprecate('Setting check_invalid_arguments is deprecated and will be removed.'
+                           ' Update the code for this module  In the future, AnsibleModule will'
+                           ' always check for invalid arguments.', version='2.9')
+
     def warn(self, warning):
 
         if isinstance(warning, string_types):

lib/ansible/modules/monitoring/bigpanda.py

@@ -135,7 +135,6 @@ def main():
             url=dict(required=False, default='https://api.bigpanda.io'),
         ),
         supports_check_mode=True,
-        check_invalid_arguments=False,
     )
 
     token = module.params['token']

lib/ansible/modules/net_tools/basics/uri.py

@@ -101,8 +101,8 @@ options:
       - Any parameter starting with "HEADER_" is a sent with your request as a header.
         For example, HEADER_Content-Type="application/json" would send the header
         "Content-Type" along with your request with a value of "application/json".
-        This option is deprecated as of C(2.1) and may be removed in a future
+        This option is deprecated as of C(2.1) and will be removed in Ansible-2.9.
-        release. Use I(headers) instead.
+        Use I(headers) instead.
   headers:
     description:
         - Add custom HTTP headers to a request in the format of a YAML hash. As
@@ -386,6 +386,7 @@ def main():
 
     module = AnsibleModule(
         argument_spec=argument_spec,
+        # TODO: Remove check_invalid_arguments in 2.9
         check_invalid_arguments=False,
         add_file_common_args=True
     )
@@ -411,15 +412,16 @@ def main():
         if 'content-type' not in lower_header_keys:
             dict_headers['Content-Type'] = 'application/json'
 
+    # TODO: Deprecated section.  Remove in Ansible 2.9
     # Grab all the http headers. Need this hack since passing multi-values is
     # currently a bit ugly. (e.g. headers='{"Content-Type":"application/json"}')
     for key, value in six.iteritems(module.params):
         if key.startswith("HEADER_"):
-            module.deprecate('Supplying headers via HEADER_* is deprecated and '
+            module.deprecate('Supplying headers via HEADER_* is deprecated. Please use `headers` to'
-                             'will be removed in a future version. Please use '
+                             ' supply headers for the request', version='2.9')
-                             '`headers` to supply headers for the request')
             skey = key.replace("HEADER_", "")
             dict_headers[skey] = value
+    # End deprecated section
 
     if creates is not None:
         # do not run the command if the line contains creates=filename

lib/ansible/modules/storage/zfs/zfs.py

@@ -2,6 +2,7 @@
 # -*- coding: utf-8 -*-
 
 # Copyright: (c) 2013, Johan Wiren <johan.wiren.se@gmail.com>
+# Copyright: (c) 2017, Ansible Project
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
@@ -35,8 +36,15 @@ options:
       - Snapshot from which to create a clone.
   key_value:
     description:
+      - (**DEPRECATED**) This will be removed in Ansible-2.9.  Set these values in the
+      - C(extra_zfs_properties) option instead.
       - The C(zfs) module takes key=value pairs for zfs properties to be set.
       - See the zfs(8) man page for more information.
+  extra_zfs_properties:
+    description:
+      - A dictionary of zfs properties to be set.
+      - See the zfs(8) man page for more information.
+    version_added: "2.5"
 author:
 - Johan Wiren (@johanwiren)
 '''
@@ -46,13 +54,15 @@ EXAMPLES = '''
   zfs:
     name: rpool/myfs
     state: present
-    setuid: off
+    extra_zfs_properties:
+      setuid: off
 
 - name: Create a new volume called myvol in pool rpool.
   zfs:
     name: rpool/myvol
     state: present
-    volsize: 10M
+    extra_zfs_properties:
+      volsize: 10M
 
 - name: Create a snapshot of rpool/myfs file system.
   zfs:
@@ -63,13 +73,15 @@ EXAMPLES = '''
   zfs:
     name: rpool/myfs2
     state: present
-    snapdir: enabled
+    extra_zfs_properties:
+      snapdir: enabled
 
 - name: Create a new file system by cloning a snapshot
   zfs:
     name: rpool/cloned_fs
     state: present
-    origin: rpool/myfs@mysnapshot
+    extra_zfs_properties:
+      origin: rpool/myfs@mysnapshot
 
 - name: Destroy a filesystem
   zfs:
@@ -216,22 +228,24 @@ def main():
         argument_spec=dict(
             name=dict(type='str', required=True),
             state=dict(type='str', required=True, choices=['absent', 'present']),
-            # No longer used. Kept here to not interfere with zfs properties
+            # No longer used. Deprecated and due for removal
-            createparent=dict(type='bool'),
+            createparent=dict(type='bool', default=None),
+            extra_zfs_properties=dict(type='dict', default={}),
         ),
         supports_check_mode=True,
+        # Remove this in Ansible 2.9
         check_invalid_arguments=False,
     )
 
     state = module.params.pop('state')
     name = module.params.pop('name')
 
+    # The following is deprecated.  Remove in Ansible 2.9
     # Get all valid zfs-properties
     properties = dict()
     for prop, value in module.params.items():
         # All freestyle params are zfs properties
         if prop not in module.argument_spec:
-            # Reverse the boolification of freestyle zfs properties
             if isinstance(value, bool):
                 if value is True:
                     properties[prop] = 'on'
@@ -240,12 +254,33 @@ def main():
             else:
                 properties[prop] = value
 
+    if properties:
+        module.deprecate('Passing zfs properties as arbitrary parameters to the zfs module is'
+                         ' deprecated.  Send them as a dictionary in the extra_zfs_properties'
+                         ' parameter instead.', version='2.9')
+        # Merge, giving the module_params precedence
+        for prop, value in module.params['extra_zfs_properties'].items():
+            properties[prop] = value
+
+        module.params['extras_zfs_properties'] = properties
+    # End deprecated section
+
+    # Reverse the boolification of zfs properties
+    for prop, value in module.params['extra_zfs_properties'].items():
+        if isinstance(value, bool):
+            if value is True:
+                module.params['extra_zfs_properties'][prop] = 'on'
+            else:
+                module.params['extra_zfs_properties'][prop] = 'off'
+        else:
+            module.params['extra_zfs_properties'][prop] = value
+
     result = dict(
         name=name,
         state=state,
     )
 
-    zfs = Zfs(module, name, properties)
+    zfs = Zfs(module, name, module.params['extra_zfs_properties'])
 
     if state == 'present':
         if zfs.exists():