From cc7a5228b02344658dac69c38ccb7d6580d2b4c6 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 19 Dec 2017 14:36:02 -0800 Subject: [PATCH] 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 | 13 ++++++ lib/ansible/module_utils/azure_rm_common.py | 2 +- lib/ansible/module_utils/basic.py | 19 +++++++- lib/ansible/modules/monitoring/bigpanda.py | 1 - lib/ansible/modules/net_tools/basics/uri.py | 12 +++-- lib/ansible/modules/storage/zfs/zfs.py | 51 +++++++++++++++++---- 6 files changed, 82 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39e8c620332..70ddf1a0ccd 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/lib/ansible/module_utils/azure_rm_common.py b/lib/ansible/module_utils/azure_rm_common.py index 7b03361900f..95c7e2c9bf5 100644 --- a/lib/ansible/module_utils/azure_rm_common.py +++ b/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): diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 4c26cb91d85..a7eef260aab 100644 --- a/lib/ansible/module_utils/basic.py +++ b/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): diff --git a/lib/ansible/modules/monitoring/bigpanda.py b/lib/ansible/modules/monitoring/bigpanda.py index ddd0776b9c7..8b2a073ad9b 100644 --- a/lib/ansible/modules/monitoring/bigpanda.py +++ b/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'] diff --git a/lib/ansible/modules/net_tools/basics/uri.py b/lib/ansible/modules/net_tools/basics/uri.py index 3f184051858..1d778858d2f 100644 --- a/lib/ansible/modules/net_tools/basics/uri.py +++ b/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 - release. Use I(headers) instead. + This option is deprecated as of C(2.1) and will be removed in Ansible-2.9. + 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 ' - 'will be removed in a future version. Please use ' - '`headers` to supply headers for the request') + module.deprecate('Supplying headers via HEADER_* is deprecated. Please use `headers` to' + ' supply headers for the request', version='2.9') 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 diff --git a/lib/ansible/modules/storage/zfs/zfs.py b/lib/ansible/modules/storage/zfs/zfs.py index 620307dcac6..bc4b17d80cc 100644 --- a/lib/ansible/modules/storage/zfs/zfs.py +++ b/lib/ansible/modules/storage/zfs/zfs.py @@ -2,6 +2,7 @@ # -*- coding: utf-8 -*- # Copyright: (c) 2013, Johan Wiren +# 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 - createparent=dict(type='bool'), + # No longer used. Deprecated and due for removal + 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():