From 784e5076711620c49097b9f27f9de4c9ea98c950 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 21 Nov 2019 21:33:27 +0100 Subject: [PATCH] module sanity checks: improve alias handling (#59060) * add_file_common_args is only of interest on top-level. * Handle undocumented arguments in one place. * Update ignore.txt * Add changelog --- .../59060-validate-modules-aliases.yml | 5 ++ .../dev_guide/testing_validate-modules.rst | 3 + .../validate-modules/validate_modules/main.py | 65 ++++++++++++++++--- test/sanity/ignore.txt | 20 +++--- 4 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 changelogs/fragments/59060-validate-modules-aliases.yml diff --git a/changelogs/fragments/59060-validate-modules-aliases.yml b/changelogs/fragments/59060-validate-modules-aliases.yml new file mode 100644 index 00000000000..12a845a7436 --- /dev/null +++ b/changelogs/fragments/59060-validate-modules-aliases.yml @@ -0,0 +1,5 @@ +bugfixes: +- "ansible-test - during module validation, improve alias handling." +- "ansible-test - during module validation, handle add_file_common_args only for top-level arguments." +minor_changes: +- "ansible-test - extend alias validation." \ No newline at end of file diff --git a/docs/docsite/rst/dev_guide/testing_validate-modules.rst b/docs/docsite/rst/dev_guide/testing_validate-modules.rst index 59c3cdbf69c..c8386dd79f1 100644 --- a/docs/docsite/rst/dev_guide/testing_validate-modules.rst +++ b/docs/docsite/rst/dev_guide/testing_validate-modules.rst @@ -119,6 +119,9 @@ Codes parameter-invalid-elements Documentation Error Value for "elements" is valid only when value of "type" is ``list`` implied-parameter-type-mismatch Documentation Error Argument_spec implies ``type="str"`` but documentation defines it as different data type parameter-type-not-in-doc Documentation Error Type value is defined in ``argument_spec`` but documentation doesn't specify a type + parameter-alias-repeated Parameters Error argument in argument_spec has at least one alias specified multiple times in aliases + parameter-alias-self Parameters Error argument in argument_spec is specified as its own alias + parameter-documented-multiple-times Documentation Error argument in argument_spec with aliases is documented multiple times python-syntax-error Syntax Error Python ``SyntaxError`` while parsing module return-syntax-error Documentation Error ``RETURN`` is not valid YAML, ``RETURN`` fragments missing or invalid subdirectory-missing-init Naming Error Ansible module subdirectories must contain an ``__init__.py`` 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 694ce36ccc5..00e12024f40 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 @@ -1176,6 +1176,7 @@ class ModuleValidator(Validator): provider_args = set() args_from_argspec = set() deprecated_args_from_argspec = set() + doc_options = docs.get('options', {}) for arg, data in spec.items(): if not isinstance(data, dict): msg = "Argument '%s' in argument_spec" % arg @@ -1188,12 +1189,33 @@ class ModuleValidator(Validator): msg=msg, ) continue + aliases = data.get('aliases', []) + if arg in aliases: + msg = "Argument '%s' in argument_spec" % arg + if context: + msg += " found in %s" % " -> ".join(context) + msg += " is specified as its own alias" + self.reporter.error( + path=self.object_path, + code='parameter-alias-self', + msg=msg + ) + if len(aliases) > len(set(aliases)): + msg = "Argument '%s' in argument_spec" % arg + if context: + msg += " found in %s" % " -> ".join(context) + msg += " has at least one alias specified multiple times in aliases" + self.reporter.error( + path=self.object_path, + code='parameter-alias-repeated', + msg=msg + ) if not data.get('removed_in_version', None): args_from_argspec.add(arg) - args_from_argspec.update(data.get('aliases', [])) + args_from_argspec.update(aliases) else: deprecated_args_from_argspec.add(arg) - deprecated_args_from_argspec.update(data.get('aliases', [])) + deprecated_args_from_argspec.update(aliases) if arg == 'provider' and self.object_path.startswith('lib/ansible/modules/network/'): if data.get('options') is not None and not isinstance(data.get('options'), Mapping): self.reporter.error( @@ -1264,9 +1286,31 @@ class ModuleValidator(Validator): elif data.get('default') is None and _type == 'bool' and 'options' not in data: arg_default = False + doc_options_args = [] + for alias in sorted(set([arg] + list(aliases))): + if alias in doc_options: + doc_options_args.append(alias) + if len(doc_options_args) == 0: + # Undocumented arguments will be handled later (search for undocumented-parameter) + doc_options_arg = {} + else: + doc_options_arg = doc_options[doc_options_args[0]] + if len(doc_options_args) > 1: + msg = "Argument '%s' in argument_spec" % arg + if context: + msg += " found in %s" % " -> ".join(context) + msg += " with aliases %s is documented multiple times, namely as %s" % ( + ", ".join([("'%s'" % alias) for alias in aliases]), + ", ".join([("'%s'" % alias) for alias in doc_options_args]) + ) + self.reporter.error( + path=self.object_path, + code='parameter-documented-multiple-times', + msg=msg + ) + try: doc_default = None - doc_options_arg = (docs.get('options', {}) or {}).get(arg, {}) if 'default' in doc_options_arg and not is_empty(doc_options_arg['default']): with CaptureStd(): doc_default = _type_checker(doc_options_arg['default']) @@ -1295,7 +1339,7 @@ class ModuleValidator(Validator): msg=msg ) - doc_type = docs.get('options', {}).get(arg, {}).get('type') + doc_type = doc_options_arg.get('type') if 'type' in data and data['type'] is not None: if doc_type is None: if not arg.startswith('_'): # hidden parameter, for example _raw_params @@ -1342,7 +1386,7 @@ class ModuleValidator(Validator): doc_choices = [] try: - for choice in docs.get('options', {}).get(arg, {}).get('choices', []): + for choice in doc_options_arg.get('choices', []): try: with CaptureStd(): doc_choices.append(_type_checker(choice)) @@ -1392,7 +1436,7 @@ class ModuleValidator(Validator): ) spec_suboptions = data.get('options') - doc_suboptions = docs.get('options', {}).get(arg, {}).get('suboptions', {}) + doc_suboptions = doc_options_arg.get('suboptions', {}) if spec_suboptions: if not doc_suboptions: msg = "Argument '%s' in argument_spec" % arg @@ -1425,15 +1469,18 @@ class ModuleValidator(Validator): file_common_arguments.update(data.get('aliases', [])) args_from_docs = set() - for arg, data in docs.get('options', {}).items(): + 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 kwargs.get('add_file_common_args', False) and arg in file_common_arguments: + 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: @@ -1451,7 +1498,7 @@ class ModuleValidator(Validator): ) for arg in docs_missing_from_args: # args_from_docs contains argument not in the argument_spec - if kwargs.get('add_file_common_args', False) and arg in file_common_arguments: + 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 diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index efa3e8ff7e1..23012e9efb3 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -1477,7 +1477,6 @@ lib/ansible/modules/cloud/vmware/vmware_vmkernel.py validate-modules:undocumente lib/ansible/modules/cloud/vmware/vmware_vspan_session.py validate-modules:missing-suboption-docs lib/ansible/modules/cloud/vmware/vmware_vspan_session.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/cloud/vmware/vmware_vspan_session.py validate-modules:undocumented-parameter -lib/ansible/modules/cloud/vmware/vsphere_copy.py validate-modules:doc-missing-type lib/ansible/modules/cloud/vmware/vsphere_copy.py validate-modules:undocumented-parameter lib/ansible/modules/cloud/vultr/_vultr_block_storage_facts.py validate-modules:return-syntax-error lib/ansible/modules/cloud/vultr/_vultr_dns_domain_facts.py validate-modules:return-syntax-error @@ -1579,17 +1578,13 @@ lib/ansible/modules/database/mongodb/mongodb_user.py validate-modules:undocument lib/ansible/modules/database/mssql/mssql_db.py validate-modules:doc-missing-type lib/ansible/modules/database/mysql/mysql_db.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/database/mysql/mysql_db.py validate-modules:use-run-command-not-popen -lib/ansible/modules/database/mysql/mysql_user.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/database/mysql/mysql_user.py validate-modules:undocumented-parameter lib/ansible/modules/database/postgresql/postgresql_db.py use-argspec-type-path -lib/ansible/modules/database/postgresql/postgresql_db.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/database/postgresql/postgresql_db.py validate-modules:use-run-command-not-popen -lib/ansible/modules/database/postgresql/postgresql_ext.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/database/postgresql/postgresql_pg_hba.py validate-modules:parameter-type-not-in-doc -lib/ansible/modules/database/postgresql/postgresql_schema.py validate-modules:parameter-type-not-in-doc +lib/ansible/modules/database/postgresql/postgresql_privs.py validate-modules:parameter-documented-multiple-times lib/ansible/modules/database/postgresql/postgresql_tablespace.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/database/postgresql/postgresql_user.py validate-modules:doc-choices-do-not-match-spec -lib/ansible/modules/database/postgresql/postgresql_user.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/database/proxysql/proxysql_backend_servers.py validate-modules:doc-missing-type lib/ansible/modules/database/proxysql/proxysql_backend_servers.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/database/proxysql/proxysql_backend_servers.py validate-modules:undocumented-parameter @@ -1794,8 +1789,8 @@ lib/ansible/modules/net_tools/dnsmadeeasy.py validate-modules:parameter-type-not lib/ansible/modules/net_tools/ip_netns.py validate-modules:doc-missing-type lib/ansible/modules/net_tools/ipinfoio_facts.py validate-modules:doc-missing-type lib/ansible/modules/net_tools/ipinfoio_facts.py validate-modules:parameter-type-not-in-doc -lib/ansible/modules/net_tools/ldap/ldap_entry.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/net_tools/ldap/ldap_entry.py validate-modules:doc-missing-type +lib/ansible/modules/net_tools/ldap/ldap_entry.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/net_tools/ldap/ldap_passwd.py validate-modules:doc-missing-type lib/ansible/modules/net_tools/netbox/netbox_device.py validate-modules:doc-missing-type lib/ansible/modules/net_tools/netbox/netbox_device.py validate-modules:parameter-type-not-in-doc @@ -1825,11 +1820,13 @@ lib/ansible/modules/net_tools/nios/nios_dns_view.py validate-modules:parameter-t lib/ansible/modules/net_tools/nios/nios_dns_view.py validate-modules:undocumented-parameter lib/ansible/modules/net_tools/nios/nios_fixed_address.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/net_tools/nios/nios_fixed_address.py validate-modules:doc-missing-type +lib/ansible/modules/net_tools/nios/nios_fixed_address.py validate-modules:parameter-alias-self lib/ansible/modules/net_tools/nios/nios_fixed_address.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/net_tools/nios/nios_fixed_address.py validate-modules:undocumented-parameter lib/ansible/modules/net_tools/nios/nios_host_record.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/net_tools/nios/nios_host_record.py validate-modules:doc-missing-type lib/ansible/modules/net_tools/nios/nios_host_record.py validate-modules:nonexistent-parameter-documented +lib/ansible/modules/net_tools/nios/nios_host_record.py validate-modules:parameter-alias-self lib/ansible/modules/net_tools/nios/nios_host_record.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/net_tools/nios/nios_host_record.py validate-modules:undocumented-parameter lib/ansible/modules/net_tools/nios/nios_member.py validate-modules:doc-default-does-not-match-spec @@ -1872,6 +1869,7 @@ lib/ansible/modules/net_tools/nios/nios_txt_record.py validate-modules:parameter lib/ansible/modules/net_tools/nios/nios_txt_record.py validate-modules:undocumented-parameter lib/ansible/modules/net_tools/nios/nios_zone.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/net_tools/nios/nios_zone.py validate-modules:doc-missing-type +lib/ansible/modules/net_tools/nios/nios_zone.py validate-modules:parameter-alias-self lib/ansible/modules/net_tools/nios/nios_zone.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/net_tools/nios/nios_zone.py validate-modules:undocumented-parameter lib/ansible/modules/net_tools/nmcli.py validate-modules:parameter-type-not-in-doc @@ -1882,6 +1880,10 @@ lib/ansible/modules/network/a10/a10_server_axapi3.py validate-modules:parameter- lib/ansible/modules/network/a10/a10_service_group.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/network/a10/a10_virtual_server.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/network/a10/a10_virtual_server.py validate-modules:parameter-type-not-in-doc +lib/ansible/modules/network/aci/aci_fabric_scheduler.py validate-modules:parameter-alias-self +lib/ansible/modules/network/aci/aci_firmware_group.py validate-modules:parameter-alias-self +lib/ansible/modules/network/aci/aci_firmware_group_node.py validate-modules:parameter-alias-self +lib/ansible/modules/network/aci/aci_firmware_policy.py validate-modules:parameter-alias-self lib/ansible/modules/network/aireos/aireos_command.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/network/aireos/aireos_command.py validate-modules:doc-missing-type lib/ansible/modules/network/aireos/aireos_command.py validate-modules:parameter-type-not-in-doc @@ -4223,7 +4225,6 @@ lib/ansible/modules/notification/cisco_spark.py validate-modules:doc-missing-typ lib/ansible/modules/notification/cisco_spark.py validate-modules:undocumented-parameter lib/ansible/modules/notification/flowdock.py validate-modules:doc-missing-type lib/ansible/modules/notification/grove.py validate-modules:parameter-type-not-in-doc -lib/ansible/modules/notification/hipchat.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/notification/hipchat.py validate-modules:doc-missing-type lib/ansible/modules/notification/hipchat.py validate-modules:undocumented-parameter lib/ansible/modules/notification/irc.py validate-modules:doc-choices-do-not-match-spec @@ -4235,7 +4236,6 @@ lib/ansible/modules/notification/jabber.py validate-modules:doc-missing-type lib/ansible/modules/notification/jabber.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/notification/logentries_msg.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/notification/mail.py validate-modules:doc-default-does-not-match-spec -lib/ansible/modules/notification/mail.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/notification/mail.py validate-modules:undocumented-parameter lib/ansible/modules/notification/matrix.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/notification/mattermost.py validate-modules:parameter-type-not-in-doc @@ -4322,6 +4322,7 @@ lib/ansible/modules/packaging/os/homebrew.py validate-modules:parameter-invalid lib/ansible/modules/packaging/os/homebrew.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/packaging/os/homebrew_cask.py validate-modules:doc-choices-do-not-match-spec lib/ansible/modules/packaging/os/homebrew_cask.py validate-modules:doc-missing-type +lib/ansible/modules/packaging/os/homebrew_cask.py validate-modules:parameter-documented-multiple-times lib/ansible/modules/packaging/os/homebrew_cask.py validate-modules:parameter-invalid lib/ansible/modules/packaging/os/homebrew_cask.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/packaging/os/homebrew_tap.py validate-modules:doc-missing-type @@ -4590,7 +4591,6 @@ lib/ansible/modules/storage/netapp/_na_cdot_user.py validate-modules:doc-missing lib/ansible/modules/storage/netapp/_na_cdot_user.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/storage/netapp/_na_cdot_user_role.py validate-modules:doc-missing-type lib/ansible/modules/storage/netapp/_na_cdot_user_role.py validate-modules:parameter-type-not-in-doc -lib/ansible/modules/storage/netapp/_na_cdot_volume.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/storage/netapp/_na_cdot_volume.py validate-modules:doc-missing-type lib/ansible/modules/storage/netapp/_na_cdot_volume.py validate-modules:no-default-for-required-parameter lib/ansible/modules/storage/netapp/_na_cdot_volume.py validate-modules:parameter-type-not-in-doc