diff --git a/docs/docsite/rst/dev_guide/testing_validate-modules.rst b/docs/docsite/rst/dev_guide/testing_validate-modules.rst index 0caff15f7b1..e60f762562d 100644 --- a/docs/docsite/rst/dev_guide/testing_validate-modules.rst +++ b/docs/docsite/rst/dev_guide/testing_validate-modules.rst @@ -110,6 +110,7 @@ Errors 321 ``Exception`` attempting to import module for ``argument_spec`` introspection 322 argument is listed in the argument_spec, but not documented in the module 323 argument is listed in DOCUMENTATION.options, but not accepted by the module + 332 ``AnsibleModule`` schema validation error .. --------- ------------------- **4xx** **Syntax** diff --git a/lib/ansible/modules/cloud/amazon/ecs_attribute.py b/lib/ansible/modules/cloud/amazon/ecs_attribute.py index ce8455ce3c7..3723e7b5963 100644 --- a/lib/ansible/modules/cloud/amazon/ecs_attribute.py +++ b/lib/ansible/modules/cloud/amazon/ecs_attribute.py @@ -257,7 +257,7 @@ def main(): attributes=dict(required=True, type='list'), )) - required_together = (['cluster', 'ec2_instance_id', 'attributes']) + required_together = [['cluster', 'ec2_instance_id', 'attributes']] module = AnsibleModule(argument_spec=argument_spec, supports_check_mode=True, required_together=required_together) diff --git a/lib/ansible/modules/cloud/amazon/ecs_cluster.py b/lib/ansible/modules/cloud/amazon/ecs_cluster.py index a4a9503d25a..eedd9892d8b 100644 --- a/lib/ansible/modules/cloud/amazon/ecs_cluster.py +++ b/lib/ansible/modules/cloud/amazon/ecs_cluster.py @@ -162,7 +162,7 @@ def main(): delay=dict(required=False, type='int', default=10), repeat=dict(required=False, type='int', default=10) )) - required_together = (['state', 'name']) + required_together = [['state', 'name']] module = AnsibleModule(argument_spec=argument_spec, supports_check_mode=True, required_together=required_together) diff --git a/lib/ansible/modules/cloud/amazon/elb_application_lb.py b/lib/ansible/modules/cloud/amazon/elb_application_lb.py index 2878dceb24b..1dfda756f1b 100644 --- a/lib/ansible/modules/cloud/amazon/elb_application_lb.py +++ b/lib/ansible/modules/cloud/amazon/elb_application_lb.py @@ -989,9 +989,9 @@ def main(): required_if=[ ('state', 'present', ['subnets', 'security_groups']) ], - required_together=( + required_together=[ ['access_logs_enabled', 'access_logs_s3_bucket', 'access_logs_s3_prefix'] - ) + ] ) # Quick check of listeners parameters diff --git a/lib/ansible/modules/cloud/amazon/elb_application_lb_facts.py b/lib/ansible/modules/cloud/amazon/elb_application_lb_facts.py index 268d96d52e5..0e601a66622 100644 --- a/lib/ansible/modules/cloud/amazon/elb_application_lb_facts.py +++ b/lib/ansible/modules/cloud/amazon/elb_application_lb_facts.py @@ -258,7 +258,7 @@ def main(): ) module = AnsibleModule(argument_spec=argument_spec, - mutually_exclusive=['load_balancer_arns', 'names'], + mutually_exclusive=[['load_balancer_arns', 'names']], supports_check_mode=True ) diff --git a/lib/ansible/modules/cloud/amazon/elb_target.py b/lib/ansible/modules/cloud/amazon/elb_target.py index 5b2143d0994..6a1074dd75c 100644 --- a/lib/ansible/modules/cloud/amazon/elb_target.py +++ b/lib/ansible/modules/cloud/amazon/elb_target.py @@ -296,7 +296,7 @@ def main(): ) module = AnsibleModule(argument_spec=argument_spec, - mutually_exclusive=['target_group_arn', 'target_group_name'] + mutually_exclusive=[['target_group_arn', 'target_group_name']] ) if not HAS_BOTO3: diff --git a/lib/ansible/modules/cloud/amazon/elb_target_group_facts.py b/lib/ansible/modules/cloud/amazon/elb_target_group_facts.py index 95dbf928704..31bbc818476 100644 --- a/lib/ansible/modules/cloud/amazon/elb_target_group_facts.py +++ b/lib/ansible/modules/cloud/amazon/elb_target_group_facts.py @@ -247,7 +247,7 @@ def main(): ) module = AnsibleModule(argument_spec=argument_spec, - mutually_exclusive=['load_balancer_arn', 'target_group_arns', 'names'], + mutually_exclusive=[['load_balancer_arn', 'target_group_arns', 'names']], supports_check_mode=True ) diff --git a/lib/ansible/modules/cloud/digital_ocean/digital_ocean_floating_ip.py b/lib/ansible/modules/cloud/digital_ocean/digital_ocean_floating_ip.py index 81f11e3753d..a207f3cf8ab 100644 --- a/lib/ansible/modules/cloud/digital_ocean/digital_ocean_floating_ip.py +++ b/lib/ansible/modules/cloud/digital_ocean/digital_ocean_floating_ip.py @@ -308,12 +308,12 @@ def main(): validate_certs=dict(type='bool', default=True), timeout=dict(type='int', default=30), ), - required_if=([ + required_if=[ ('state', 'delete', ['ip']) - ]), - mutually_exclusive=( + ], + mutually_exclusive=[ ['region', 'droplet_id'] - ), + ], ) core(module) diff --git a/lib/ansible/modules/cloud/vmware/vmware_guest_tools_wait.py b/lib/ansible/modules/cloud/vmware/vmware_guest_tools_wait.py index ddd40602d64..87817803308 100644 --- a/lib/ansible/modules/cloud/vmware/vmware_guest_tools_wait.py +++ b/lib/ansible/modules/cloud/vmware/vmware_guest_tools_wait.py @@ -163,7 +163,7 @@ def main(): module = AnsibleModule( argument_spec=argument_spec, required_one_of=[['name', 'uuid']], - required_together=['name', 'folder'] + required_together=[['name', 'folder']], ) # FindByInventoryPath() does not require an absolute path diff --git a/lib/ansible/modules/clustering/k8s/_kubernetes.py b/lib/ansible/modules/clustering/k8s/_kubernetes.py index 5e57e69e71b..d635f6341ab 100755 --- a/lib/ansible/modules/clustering/k8s/_kubernetes.py +++ b/lib/ansible/modules/clustering/k8s/_kubernetes.py @@ -349,7 +349,7 @@ def main(): mutually_exclusive=(('file_reference', 'inline_data'), ('url_username', 'insecure'), ('url_password', 'insecure')), - required_one_of=(('file_reference', 'inline_data')), + required_one_of=(('file_reference', 'inline_data'),), ) if not HAS_LIB_YAML: diff --git a/lib/ansible/modules/net_tools/basics/get_url.py b/lib/ansible/modules/net_tools/basics/get_url.py index 46692d76912..b9642debd98 100644 --- a/lib/ansible/modules/net_tools/basics/get_url.py +++ b/lib/ansible/modules/net_tools/basics/get_url.py @@ -396,7 +396,7 @@ def main(): argument_spec=argument_spec, add_file_common_args=True, supports_check_mode=True, - mutually_exclusive=(['checksum', 'sha256sum']), + mutually_exclusive=[['checksum', 'sha256sum']], ) url = module.params['url'] diff --git a/lib/ansible/modules/net_tools/dnsimple.py b/lib/ansible/modules/net_tools/dnsimple.py index 5739ee8a04f..7da7fb47df4 100644 --- a/lib/ansible/modules/net_tools/dnsimple.py +++ b/lib/ansible/modules/net_tools/dnsimple.py @@ -188,9 +188,9 @@ def main(): state=dict(required=False, choices=['present', 'absent']), solo=dict(required=False, type='bool'), ), - required_together=( + required_together=[ ['record', 'value'] - ), + ], supports_check_mode=True, ) diff --git a/lib/ansible/modules/net_tools/dnsmadeeasy.py b/lib/ansible/modules/net_tools/dnsmadeeasy.py index e4093359afd..85ae876b2e1 100644 --- a/lib/ansible/modules/net_tools/dnsmadeeasy.py +++ b/lib/ansible/modules/net_tools/dnsmadeeasy.py @@ -584,9 +584,9 @@ def main(): ip5=dict(required=False), validate_certs=dict(default='yes', type='bool'), ), - required_together=( + required_together=[ ['record_value', 'record_ttl', 'record_type'] - ), + ], required_if=[ ['failover', True, ['autoFailover', 'port', 'protocol', 'ip1', 'ip2']], ['monitor', True, ['port', 'protocol', 'maxEmails', 'systemDescription', 'ip1']] diff --git a/lib/ansible/modules/network/vyos/vyos_interface.py b/lib/ansible/modules/network/vyos/vyos_interface.py index 78cc60fda43..63f441c6bb8 100644 --- a/lib/ansible/modules/network/vyos/vyos_interface.py +++ b/lib/ansible/modules/network/vyos/vyos_interface.py @@ -393,7 +393,7 @@ def main(): required_one_of = [['name', 'aggregate']] mutually_exclusive = [['name', 'aggregate']] - required_together = (['speed', 'duplex']) + required_together = [['speed', 'duplex']] module = AnsibleModule(argument_spec=argument_spec, required_one_of=required_one_of, mutually_exclusive=mutually_exclusive, diff --git a/lib/ansible/modules/packaging/language/pear.py b/lib/ansible/modules/packaging/language/pear.py index f1353328240..b45a40ead3d 100644 --- a/lib/ansible/modules/packaging/language/pear.py +++ b/lib/ansible/modules/packaging/language/pear.py @@ -204,10 +204,9 @@ def check_packages(module, packages, state): def main(): module = AnsibleModule( argument_spec=dict( - name=dict(aliases=['pkg']), + name=dict(aliases=['pkg'], required=True), state=dict(default='present', choices=['present', 'installed', "latest", 'absent', 'removed']), executable=dict(default=None, required=False, type='path')), - required_one_of=[['name']], supports_check_mode=True) p = module.params diff --git a/test/sanity/validate-modules/main.py b/test/sanity/validate-modules/main.py index fc9f81e00f2..867410ee6ab 100755 --- a/test/sanity/validate-modules/main.py +++ b/test/sanity/validate-modules/main.py @@ -43,7 +43,7 @@ from ansible.utils.plugin_docs import BLACKLIST, add_fragments, get_docstring from module_args import AnsibleModuleImportError, get_argument_spec -from schema import doc_schema, metadata_1_1_schema, return_schema +from schema import ansible_module_kwargs_schema, doc_schema, metadata_1_1_schema, return_schema from utils import CaptureStd, NoArgsAnsibleModule, compare_unordered_lists, is_empty, parse_yaml from voluptuous.humanize import humanize_error @@ -1015,19 +1015,7 @@ class ModuleValidator(Validator): msg='version_added should be %s. Currently %s' % (should_be, version_added) ) - def _validate_argument_spec(self, docs): - if not self.analyze_arg_spec: - return - - if docs is None: - docs = {} - - try: - add_fragments(docs, self.object_path, fragment_loader=fragment_loader) - except Exception: - # Cannot merge fragments - return - + def _validate_ansible_module_call(self, docs): try: spec, args, kwargs = get_argument_spec(self.path) except AnsibleModuleImportError as e: @@ -1042,6 +1030,23 @@ class ModuleValidator(Validator): ) return + self._validate_docs_schema(kwargs, ansible_module_kwargs_schema, 'AnsibleModule', 332) + + self._validate_argument_spec(docs, spec, kwargs) + + def _validate_argument_spec(self, docs, spec, kwargs): + if not self.analyze_arg_spec: + return + + if docs is None: + docs = {} + + try: + add_fragments(docs, self.object_path, fragment_loader=fragment_loader) + except Exception: + # Cannot merge fragments + return + # Use this to access type checkers later module = NoArgsAnsibleModule({}) @@ -1352,7 +1357,7 @@ class ModuleValidator(Validator): # FIXME if +2 then file should be empty? - maybe add this only in the future if self._python_module() and not self._just_docs() and not end_of_deprecation_should_be_docs_only: - self._validate_argument_spec(docs) + self._validate_ansible_module_call(docs) self._check_for_sys_exit() self._find_blacklist_imports() main = self._find_main_call() diff --git a/test/sanity/validate-modules/schema.py b/test/sanity/validate-modules/schema.py index 64f7b5e4400..b9a2510fa16 100644 --- a/test/sanity/validate-modules/schema.py +++ b/test/sanity/validate-modules/schema.py @@ -16,10 +16,41 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from voluptuous import PREVENT_EXTRA, Any, Required, Schema, Self +from voluptuous import PREVENT_EXTRA, All, Any, Length, Required, Schema, Self from ansible.module_utils.six import string_types list_string_types = list(string_types) + +def sequence_of_sequences(min=None, max=None): + return All( + Any( + None, + [Length(min=min, max=max)], + tuple([Length(min=min, max=max)]), + ), + Any( + None, + [Any(list, tuple)], + tuple([Any(list, tuple)]), + ), + ) + + +ansible_module_kwargs_schema = Schema( + { + 'argument_spec': dict, + 'bypass_checks': bool, + 'no_log': bool, + 'check_invalid_arguments': Any(None, bool), + 'mutually_exclusive': sequence_of_sequences(min=2), + 'required_together': sequence_of_sequences(min=2), + 'required_one_of': sequence_of_sequences(min=2), + 'add_file_common_args': bool, + 'supports_check_mode': bool, + 'required_if': sequence_of_sequences(min=3), + } +) + suboption_schema = Schema( { Required('description'): Any(list_string_types, *string_types),