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.

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

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

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.

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)

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

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