From f6815040fd5903eb6883acf02146136aa6ba6ce7 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Wed, 12 Feb 2020 07:44:11 +0100 Subject: [PATCH] add_file_common_arguments: fix defaults and tpyes, improve sanity checking (#67243) * Make validate-modules stop ignore FILE_COMMON_ARGUMENTS. * Add types to FILE_COMMON_ARGUMENTS and update document fragment to match it. * Update ignore.txt. * Add changelog. --- ...43-file_common_arguments-defaults-sanity.yml | 2 ++ lib/ansible/module_utils/basic.py | 16 ++++++++-------- lib/ansible/plugins/doc_fragments/files.py | 3 +-- .../validate-modules/validate_modules/main.py | 17 ----------------- .../validate_modules/module_args.py | 11 ++++++++++- test/sanity/ignore.txt | 8 ++++++++ 6 files changed, 29 insertions(+), 28 deletions(-) create mode 100644 changelogs/fragments/67243-file_common_arguments-defaults-sanity.yml diff --git a/changelogs/fragments/67243-file_common_arguments-defaults-sanity.yml b/changelogs/fragments/67243-file_common_arguments-defaults-sanity.yml new file mode 100644 index 00000000000..a055ce411b4 --- /dev/null +++ b/changelogs/fragments/67243-file_common_arguments-defaults-sanity.yml @@ -0,0 +1,2 @@ +minor_changes: + - "ansible-test - module validation will now consider arguments added by ``add_file_common_arguments=True`` correctly." diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 84dcc3b9039..a1a8f023a13 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -237,14 +237,14 @@ FILE_COMMON_ARGUMENTS = dict( # created files (these are used by set_fs_attributes_if_different and included in # load_file_common_arguments) mode=dict(type='raw'), - owner=dict(), - group=dict(), - seuser=dict(), - serole=dict(), - selevel=dict(), - setype=dict(), - attributes=dict(aliases=['attr']), - unsafe_writes=dict(type='bool'), # should be available to any module using atomic_move + owner=dict(type='str'), + group=dict(type='str'), + seuser=dict(type='str'), + serole=dict(type='str'), + selevel=dict(type='str'), + setype=dict(type='str'), + attributes=dict(type='str', aliases=['attr']), + unsafe_writes=dict(type='bool', default=False), # should be available to any module using atomic_move ) PASSWD_ARG_RE = re.compile(r'^[-]{0,2}pass[-]?(word|wd)?') diff --git a/lib/ansible/plugins/doc_fragments/files.py b/lib/ansible/plugins/doc_fragments/files.py index 2cb3b3f6546..766b933a409 100644 --- a/lib/ansible/plugins/doc_fragments/files.py +++ b/lib/ansible/plugins/doc_fragments/files.py @@ -25,7 +25,7 @@ options: C(u=rw,g=r,o=r)). - As of Ansible 2.6, the mode may also be the special string C(preserve). - When set to C(preserve) the file will be given the same permissions as the source file. - type: str + type: raw owner: description: - Name of the user that should own the file/directory, as would be fed to I(chown). @@ -56,7 +56,6 @@ options: - This is the MLS/MCS attribute, sometimes known as the C(range). - When set to C(_default), it will use the C(level) portion of the policy if available. type: str - default: s0 unsafe_writes: description: - Influence when to use atomic operation to prevent data corruption or inconsistent reads from the target file. diff --git a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py index 2898d062762..563decb2aef 100644 --- a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py +++ b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py @@ -51,7 +51,6 @@ from .utils import CaptureStd, NoArgsAnsibleModule, compare_unordered_lists, is_ from voluptuous.humanize import humanize_error from ansible.module_utils.six import PY3, with_metaclass -from ansible.module_utils.basic import FILE_COMMON_ARGUMENTS if PY3: # Because there is no ast.TryExcept in Python 3 ast module @@ -1529,26 +1528,14 @@ class ModuleValidator(Validator): ) if docs: - file_common_arguments = set() - for arg, data in FILE_COMMON_ARGUMENTS.items(): - file_common_arguments.add(arg) - file_common_arguments.update(data.get('aliases', [])) - args_from_docs = set() for arg, data in doc_options.items(): args_from_docs.add(arg) args_from_docs.update(data.get('aliases', [])) - # add_file_common_args is only of interest on top-level - add_file_common_args = kwargs.get('add_file_common_args', False) and not context - args_missing_from_docs = args_from_argspec.difference(args_from_docs) docs_missing_from_args = args_from_docs.difference(args_from_argspec | deprecated_args_from_argspec) for arg in args_missing_from_docs: - # args_from_argspec contains undocumented argument - if add_file_common_args and arg in file_common_arguments: - # add_file_common_args is handled in AnsibleModule, and not exposed earlier - continue if arg in provider_args: # Provider args are being removed from network module top level # So they are likely not documented on purpose @@ -1563,10 +1550,6 @@ class ModuleValidator(Validator): msg=msg ) for arg in docs_missing_from_args: - # args_from_docs contains argument not in the argument_spec - if add_file_common_args and arg in file_common_arguments: - # add_file_common_args is handled in AnsibleModule, and not exposed earlier - continue msg = "Argument '%s'" % arg if context: msg += " found in %s" % " -> ".join(context) diff --git a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py index 666aa4347ca..ef1f83778ed 100644 --- a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py +++ b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py @@ -26,6 +26,7 @@ import sys from contextlib import contextmanager +from ansible.module_utils.basic import FILE_COMMON_ARGUMENTS from ansible.module_utils.six import reraise from .utils import CaptureStd, find_executable, get_module_name_from_filename @@ -127,7 +128,15 @@ def get_py_argument_spec(filename, collection): try: try: # for ping kwargs == {'argument_spec':{'data':{'type':'str','default':'pong'}}, 'supports_check_mode':True} - return fake.kwargs['argument_spec'], fake.args, fake.kwargs + argument_spec = fake.kwargs['argument_spec'] + # If add_file_common_args is truish, add options from FILE_COMMON_ARGUMENTS when not present. + # This is the only modification to argument_spec done by AnsibleModule itself, and which is + # not caught by setup_env's AnsibleModule replacement + if fake.kwargs.get('add_file_common_args'): + for k, v in FILE_COMMON_ARGUMENTS.items(): + if k not in argument_spec: + argument_spec[k] = v + return argument_spec, fake.args, fake.kwargs except KeyError: return fake.args[0], fake.args, fake.kwargs except (TypeError, IndexError): diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 4d383744c97..2de05d27253 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -2530,6 +2530,7 @@ lib/ansible/modules/files/blockinfile.py validate-modules:doc-choices-do-not-mat lib/ansible/modules/files/blockinfile.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/files/copy.py pylint:blacklisted-name lib/ansible/modules/files/copy.py validate-modules:doc-default-does-not-match-spec +lib/ansible/modules/files/copy.py validate-modules:doc-type-does-not-match-spec lib/ansible/modules/files/copy.py validate-modules:nonexistent-parameter-documented lib/ansible/modules/files/copy.py validate-modules:undocumented-parameter lib/ansible/modules/files/file.py pylint:ansible-bad-function @@ -4444,6 +4445,8 @@ lib/ansible/modules/network/f5/bigip_ike_peer.py validate-modules:doc-required-m lib/ansible/modules/network/f5/bigip_ike_peer.py validate-modules:parameter-list-no-elements lib/ansible/modules/network/f5/bigip_imish_config.py validate-modules:doc-required-mismatch lib/ansible/modules/network/f5/bigip_imish_config.py validate-modules:parameter-list-no-elements +lib/ansible/modules/network/f5/bigip_imish_config.py validate-modules:parameter-type-not-in-doc +lib/ansible/modules/network/f5/bigip_imish_config.py validate-modules:undocumented-parameter lib/ansible/modules/network/f5/bigip_ipsec_policy.py validate-modules:doc-required-mismatch lib/ansible/modules/network/f5/bigip_irule.py validate-modules:doc-required-mismatch lib/ansible/modules/network/f5/bigip_log_destination.py validate-modules:doc-required-mismatch @@ -4552,6 +4555,8 @@ lib/ansible/modules/network/f5/bigip_tunnel.py validate-modules:doc-required-mis lib/ansible/modules/network/f5/bigip_tunnel.py validate-modules:invalid-ansiblemodule-schema lib/ansible/modules/network/f5/bigip_ucs.py validate-modules:doc-required-mismatch lib/ansible/modules/network/f5/bigip_ucs_fetch.py validate-modules:doc-required-mismatch +lib/ansible/modules/network/f5/bigip_ucs_fetch.py validate-modules:parameter-type-not-in-doc +lib/ansible/modules/network/f5/bigip_ucs_fetch.py validate-modules:undocumented-parameter lib/ansible/modules/network/f5/bigip_user.py validate-modules:doc-required-mismatch lib/ansible/modules/network/f5/bigip_user.py validate-modules:parameter-list-no-elements lib/ansible/modules/network/f5/bigip_vcmp_guest.py validate-modules:doc-required-mismatch @@ -7330,6 +7335,8 @@ lib/ansible/modules/system/iptables.py pylint:blacklisted-name lib/ansible/modules/system/iptables.py validate-modules:parameter-list-no-elements lib/ansible/modules/system/java_cert.py pylint:blacklisted-name lib/ansible/modules/system/java_keystore.py validate-modules:doc-missing-type +lib/ansible/modules/system/java_keystore.py validate-modules:parameter-type-not-in-doc +lib/ansible/modules/system/java_keystore.py validate-modules:undocumented-parameter lib/ansible/modules/system/kernel_blacklist.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/system/known_hosts.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/system/known_hosts.py validate-modules:doc-missing-type @@ -7457,6 +7464,7 @@ lib/ansible/modules/web_infrastructure/apache2_module.py validate-modules:doc-mi lib/ansible/modules/web_infrastructure/apache2_module.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/web_infrastructure/deploy_helper.py validate-modules:doc-missing-type lib/ansible/modules/web_infrastructure/deploy_helper.py validate-modules:parameter-type-not-in-doc +lib/ansible/modules/web_infrastructure/deploy_helper.py validate-modules:undocumented-parameter lib/ansible/modules/web_infrastructure/django_manage.py validate-modules:doc-choices-do-not-match-spec lib/ansible/modules/web_infrastructure/django_manage.py validate-modules:doc-missing-type lib/ansible/modules/web_infrastructure/django_manage.py validate-modules:no-default-for-required-parameter