From 4209d714dbe10e23161bbdc7fbc54c21b276ce7a Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Wed, 3 Sep 2025 23:52:22 +0200 Subject: [PATCH] validate-modules and return fragments: fix bug in markup check, fix bug in missing doc fragment check, add tests (#85638) * Prevent crashing on invalid structure. * Also process return doc fragments. * Fix handling of missing doc fragments. --- ...le-test-validate-modules-doc-fragments.yml | 2 + lib/ansible/utils/plugin_docs.py | 12 +++-- .../doc_fragments/return_doc_fragment.py | 19 +++++++ .../modules/doc_fragments_not_exist.py | 28 +++++++++++ .../col/plugins/modules/return_fragments.py | 29 +++++++++++ .../modules/return_fragments_not_exist.py | 29 +++++++++++ .../expected.txt | 3 ++ .../validate-modules/validate_modules/main.py | 49 ++++++++++++------- 8 files changed, 149 insertions(+), 22 deletions(-) create mode 100644 changelogs/fragments/85638-ansible-test-validate-modules-doc-fragments.yml create mode 100644 test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/doc_fragments/return_doc_fragment.py create mode 100644 test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/doc_fragments_not_exist.py create mode 100644 test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments.py create mode 100644 test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments_not_exist.py diff --git a/changelogs/fragments/85638-ansible-test-validate-modules-doc-fragments.yml b/changelogs/fragments/85638-ansible-test-validate-modules-doc-fragments.yml new file mode 100644 index 00000000000..7f265d13e07 --- /dev/null +++ b/changelogs/fragments/85638-ansible-test-validate-modules-doc-fragments.yml @@ -0,0 +1,2 @@ +bugfixes: + - "validate-modules sanity test - fix handling of missing doc fragments (https://github.com/ansible/ansible/pull/85638)." diff --git a/lib/ansible/utils/plugin_docs.py b/lib/ansible/utils/plugin_docs.py index a0960b960f7..3bff309f4d9 100644 --- a/lib/ansible/utils/plugin_docs.py +++ b/lib/ansible/utils/plugin_docs.py @@ -126,6 +126,10 @@ def remove_current_collection_from_versions_and_dates(fragment, collection_name, _process_versions_and_dates(fragment, is_module, return_docs, remove) +class AnsibleFragmentError(AnsibleError): + pass + + def add_fragments(doc, filename, fragment_loader, is_module=False, section='DOCUMENTATION'): if section not in _FRAGMENTABLE: @@ -185,7 +189,7 @@ def add_fragments(doc, filename, fragment_loader, is_module=False, section='DOCU doc[doc_key].extend(entries) if 'options' not in fragment and 'attributes' not in fragment: - raise Exception("missing options or attributes in fragment (%s), possibly misformatted?: %s" % (fragment_name, filename)) + raise AnsibleFragmentError("missing options or attributes in fragment (%s), possibly misformatted?: %s" % (fragment_name, filename)) # ensure options themselves are directly merged for doc_key in ['options', 'attributes']: @@ -194,7 +198,7 @@ def add_fragments(doc, filename, fragment_loader, is_module=False, section='DOCU try: merge_fragment(doc[doc_key], fragment.pop(doc_key)) except Exception as e: - raise AnsibleError("%s %s (%s) of unknown type: %s" % (to_native(e), doc_key, fragment_name, filename)) + raise AnsibleFragmentError("%s %s (%s) of unknown type: %s" % (to_native(e), doc_key, fragment_name, filename)) else: doc[doc_key] = fragment.pop(doc_key) @@ -202,10 +206,10 @@ def add_fragments(doc, filename, fragment_loader, is_module=False, section='DOCU try: merge_fragment(doc, fragment) except Exception as e: - raise AnsibleError("%s (%s) of unknown type: %s" % (to_native(e), fragment_name, filename)) + raise AnsibleFragmentError("%s (%s) of unknown type: %s" % (to_native(e), fragment_name, filename)) if unknown_fragments: - raise AnsibleError('unknown doc_fragment(s) in file {0}: {1}'.format(filename, to_native(', '.join(unknown_fragments)))) + raise AnsibleFragmentError('unknown doc_fragment(s) in file {0}: {1}'.format(filename, to_native(', '.join(unknown_fragments)))) def get_docstring(filename, fragment_loader, verbose=False, ignore_errors=False, collection_name=None, is_module=None, plugin_type=None): diff --git a/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/doc_fragments/return_doc_fragment.py b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/doc_fragments/return_doc_fragment.py new file mode 100644 index 00000000000..1c885b4b35b --- /dev/null +++ b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/doc_fragments/return_doc_fragment.py @@ -0,0 +1,19 @@ +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import annotations + + +class ModuleDocFragment: + DOCUMENTATION = r""" +options: {} +""" + + RETURN = r""" +bar: + description: + - Some foo bar. + - P(a.b.asfd#dfsa) this is an error. + returned: success + type: int + sample: 42 +""" diff --git a/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/doc_fragments_not_exist.py b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/doc_fragments_not_exist.py new file mode 100644 index 00000000000..6116d869693 --- /dev/null +++ b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/doc_fragments_not_exist.py @@ -0,0 +1,28 @@ +#!/usr/bin/python +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import annotations + +DOCUMENTATION = """ +module: doc_fragments_not_exist +short_description: Non-existing doc fragment +description: A module with a non-existing doc fragment +author: + - Ansible Core Team +extends_documentation_fragment: + - does.not.exist +""" + +EXAMPLES = """#""" + +RETURN = """""" + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + AnsibleModule().exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments.py b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments.py new file mode 100644 index 00000000000..963d368c32c --- /dev/null +++ b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments.py @@ -0,0 +1,29 @@ +#!/usr/bin/python +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import annotations + +DOCUMENTATION = """ +module: return_fragments +short_description: Uses return fragments +description: A module with a return doc fragment. +author: + - Ansible Core Team +""" + +EXAMPLES = """#""" + +RETURN = """ +extends_documentation_fragment: + - ns.col.return_doc_fragment +""" + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + AnsibleModule().exit_json(bar=42) + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments_not_exist.py b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments_not_exist.py new file mode 100644 index 00000000000..69879b3156f --- /dev/null +++ b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments_not_exist.py @@ -0,0 +1,29 @@ +#!/usr/bin/python +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import annotations + +DOCUMENTATION = """ +module: return_fragments_not_exist +short_description: Non-existing return doc fragment +description: A module with a non-existing return doc fragment. +author: + - Ansible Core Team +""" + +EXAMPLES = """#""" + +RETURN = """ +extends_documentation_fragment: + - does.not.exist +""" + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + AnsibleModule().exit_json(bar=42) + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-test-sanity-validate-modules/expected.txt b/test/integration/targets/ansible-test-sanity-validate-modules/expected.txt index d3a1ffa70ba..d178a8b9207 100644 --- a/test/integration/targets/ansible-test-sanity-validate-modules/expected.txt +++ b/test/integration/targets/ansible-test-sanity-validate-modules/expected.txt @@ -3,6 +3,7 @@ plugins/modules/check_mode_attribute_1.py:0:0: attributes-check-mode: The module plugins/modules/check_mode_attribute_2.py:0:0: attributes-check-mode: The module does not declare support for check mode, but the check_mode attribute's support value is 'partial' and not 'none' plugins/modules/check_mode_attribute_3.py:0:0: attributes-check-mode: The module does declare support for check mode, but the check_mode attribute's support value is 'none' plugins/modules/check_mode_attribute_4.py:0:0: attributes-check-mode-details: The module declares it does not fully support check mode, but has no details on what exactly that means +plugins/modules/doc_fragments_not_exist.py:0:0: doc-fragment-error: Error while adding fragments: unknown doc_fragment(s) in file plugins/modules/doc_fragments_not_exist.py: does.not.exist plugins/modules/import_order.py:7:0: import-before-documentation: Import found before documentation variables. All imports must appear below DOCUMENTATION/EXAMPLES/RETURN. plugins/modules/invalid_argument_spec_extra_key.py:0:0: invalid-ansiblemodule-schema: AnsibleModule.argument_spec.foo.extra_key: extra keys not allowed @ data['argument_spec']['foo']['extra_key']. Got 'bar' plugins/modules/invalid_argument_spec_incorrect_context.py:0:0: invalid-ansiblemodule-schema: AnsibleModule.argument_spec.foo.context: expected dict for dictionary value @ data['argument_spec']['foo']['context']. Got 'bar' @@ -14,6 +15,8 @@ plugins/modules/invalid_yaml_syntax.py:11:15: invalid-examples: EXAMPLES is not plugins/modules/invalid_yaml_syntax.py:15:15: return-syntax-error: RETURN is not valid YAML plugins/modules/option_name_casing.py:0:0: option-equal-up-to-casing: Multiple options/aliases are equal up to casing: option 'Bar', alias 'baR' of option 'bam', alias 'bar' of option 'foo' plugins/modules/option_name_casing.py:0:0: option-equal-up-to-casing: Multiple options/aliases are equal up to casing: option 'Foo', option 'foo' +plugins/modules/return_fragments.py:0:0: invalid-documentation-markup: RETURN.bar.description.1: Directive "P(a.b.asfd#dfsa)" must contain a valid plugin type; found "dfsa" @ data['bar']['description'][1]. Got 'P(a.b.asfd#dfsa) this is an error.' +plugins/modules/return_fragments_not_exist.py:0:0: return-fragment-error: Error while adding fragments: unknown doc_fragment(s) in file plugins/modules/return_fragments_not_exist.py: does.not.exist plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a11.suboptions.b1.description.0: While parsing "V(C\(" at index 1: Unnecessarily escaped "(" @ data['options']['a11']['suboptions']['b1']['description'][0]. Got 'V(C\\(foo\\)).' plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a11.suboptions.b1.description.2: While parsing "P(foo.bar#baz)" at index 1: Plugin name "foo.bar" is not a FQCN @ data['options']['a11']['suboptions']['b1']['description'][2]. Got 'P(foo.bar#baz).' plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a11.suboptions.b1.description.3: While parsing "P(foo.bar.baz)" at index 1: Parameter "foo.bar.baz" is not of the form FQCN#type @ data['options']['a11']['suboptions']['b1']['description'][3]. Got 'P(foo.bar.baz).' diff --git a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py index cd2a301eaa5..341c37f3789 100644 --- a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py +++ b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py @@ -72,7 +72,7 @@ from ansible.module_utils.compat.version import StrictVersion, LooseVersion from ansible.module_utils.basic import to_bytes from ansible.plugins.loader import fragment_loader from ansible.plugins.list import IGNORE as REJECTLIST -from ansible.utils.plugin_docs import add_collection_to_versions_and_dates, add_fragments, get_docstring +from ansible.utils.plugin_docs import AnsibleFragmentError, add_collection_to_versions_and_dates, add_fragments, get_docstring from ansible.utils.version import SemanticVersion from .module_args import AnsibleModuleImportError, AnsibleModuleNotInitialized, get_py_argument_spec, get_ps_argument_spec @@ -1003,21 +1003,15 @@ class ModuleValidator(Validator): add_collection_to_versions_and_dates(doc, self.collection_name, is_module=self.plugin_type == 'module') - missing_fragment = False with CaptureStd(): try: get_docstring(os.path.abspath(self.path), fragment_loader=fragment_loader, verbose=True, collection_name=self.collection_name, plugin_type=self.plugin_type) - except AssertionError: - fragment = doc['extends_documentation_fragment'] - self.reporter.error( - path=self.object_path, - code='missing-doc-fragment', - msg='DOCUMENTATION fragment missing: %s' % fragment - ) - missing_fragment = True + except AnsibleFragmentError: + # Will be re-triggered below when explicitly calling add_fragments() + pass except Exception as e: self.reporter.trace( path=self.object_path, @@ -1029,9 +1023,16 @@ class ModuleValidator(Validator): msg='Unknown DOCUMENTATION error, see TRACE: %s' % e ) - if not missing_fragment: + try: add_fragments(doc, os.path.abspath(self.object_path), fragment_loader=fragment_loader, - is_module=self.plugin_type == 'module') + is_module=self.plugin_type == 'module', section='DOCUMENTATION') + except AnsibleFragmentError as exc: + error = str(exc).replace(os.path.abspath(self.object_path), self.object_path) + self.reporter.error( + path=self.object_path, + code='doc-fragment-error', + msg=f'Error while adding fragments: {error}' + ) if 'options' in doc and doc['options'] is None: self.reporter.error( @@ -1130,6 +1131,16 @@ class ModuleValidator(Validator): self.collection_name, is_module=self.plugin_type == 'module', return_docs=True) + try: + add_fragments(returns, os.path.abspath(self.object_path), fragment_loader=fragment_loader, + is_module=self.plugin_type == 'module', section='RETURN') + except AnsibleFragmentError as exc: + error = str(exc).replace(os.path.abspath(self.object_path), self.object_path) + self.reporter.error( + path=self.object_path, + code='return-fragment-error', + msg=f'Error while adding fragments: {error}' + ) self._validate_docs_schema( returns, return_schema(for_collection=bool(self.collection), plugin_type=self.plugin_type), @@ -1268,16 +1279,18 @@ class ModuleValidator(Validator): if not isinstance(options, dict): return for key, value in options.items(): - self._validate_semantic_markup(value.get('description')) - self._validate_semantic_markup_options(value.get('suboptions')) + if isinstance(value, dict): + self._validate_semantic_markup(value.get('description')) + self._validate_semantic_markup_options(value.get('suboptions')) def _validate_semantic_markup_return_values(self, return_vars): if not isinstance(return_vars, dict): return for key, value in return_vars.items(): - self._validate_semantic_markup(value.get('description')) - self._validate_semantic_markup(value.get('returned')) - self._validate_semantic_markup_return_values(value.get('contains')) + if isinstance(value, dict): + self._validate_semantic_markup(value.get('description')) + self._validate_semantic_markup(value.get('returned')) + self._validate_semantic_markup_return_values(value.get('contains')) def _validate_all_semantic_markup(self, docs, return_docs): if not isinstance(docs, dict): @@ -1617,7 +1630,7 @@ class ModuleValidator(Validator): try: if not context: add_fragments(docs, os.path.abspath(self.object_path), fragment_loader=fragment_loader, - is_module=self.plugin_type == 'module') + is_module=self.plugin_type == 'module', section='DOCUMENTATION') except Exception: # Cannot merge fragments return