diff options
11 files changed, 450 insertions, 21 deletions
diff --git a/changelogs/fragments/77035-ansible-doc-metadata-dump-errors.yml b/changelogs/fragments/77035-ansible-doc-metadata-dump-errors.yml new file mode 100644 index 0000000000..045e80a571 --- /dev/null +++ b/changelogs/fragments/77035-ansible-doc-metadata-dump-errors.yml @@ -0,0 +1,2 @@ +minor_changes: + - "ansible-doc metadata dump - add option ``--no-fail-on-errors`` which allows to not fail the ansible-doc invocation when errors happen during docs parsing or processing. Instead they are reported in the JSON result in an ``error`` key for the affected plugins (https://github.com/ansible/ansible/pull/77035)." diff --git a/lib/ansible/cli/doc.py b/lib/ansible/cli/doc.py index 314e517a47..98ff8705a6 100755 --- a/lib/ansible/cli/doc.py +++ b/lib/ansible/cli/doc.py @@ -255,7 +255,7 @@ class RoleMixin(object): return (fqcn, doc) - def _create_role_list(self): + def _create_role_list(self, fail_on_errors=True): """Return a dict describing the listing of all roles with arg specs. :param role_paths: A tuple of one or more role paths. @@ -296,22 +296,37 @@ class RoleMixin(object): result = {} for role, role_path in roles: - argspec = self._load_argspec(role, role_path=role_path) - fqcn, summary = self._build_summary(role, '', argspec) - result[fqcn] = summary + try: + argspec = self._load_argspec(role, role_path=role_path) + fqcn, summary = self._build_summary(role, '', argspec) + result[fqcn] = summary + except Exception as e: + if fail_on_errors: + raise + result[role] = { + 'error': 'Error while loading role argument spec: %s' % to_native(e), + } for role, collection, collection_path in collroles: - argspec = self._load_argspec(role, collection_path=collection_path) - fqcn, summary = self._build_summary(role, collection, argspec) - result[fqcn] = summary + try: + argspec = self._load_argspec(role, collection_path=collection_path) + fqcn, summary = self._build_summary(role, collection, argspec) + result[fqcn] = summary + except Exception as e: + if fail_on_errors: + raise + result['%s.%s' % (collection, role)] = { + 'error': 'Error while loading role argument spec: %s' % to_native(e), + } return result - def _create_role_doc(self, role_names, entry_point=None): + def _create_role_doc(self, role_names, entry_point=None, fail_on_errors=True): """ :param role_names: A tuple of one or more role names. :param role_paths: A tuple of one or more role paths. :param entry_point: A role entry point name for filtering. + :param fail_on_errors: When set to False, include errors in the JSON output instead of raising errors :returns: A dict indexed by role name, with 'collection', 'entry_points', and 'path' keys per role. """ @@ -322,16 +337,26 @@ class RoleMixin(object): result = {} for role, role_path in roles: - argspec = self._load_argspec(role, role_path=role_path) - fqcn, doc = self._build_doc(role, role_path, '', argspec, entry_point) - if doc: - result[fqcn] = doc + try: + argspec = self._load_argspec(role, role_path=role_path) + fqcn, doc = self._build_doc(role, role_path, '', argspec, entry_point) + if doc: + result[fqcn] = doc + except Exception as e: # pylint:disable=broad-except + result[role] = { + 'error': 'Error while processing role: %s' % to_native(e), + } for role, collection, collection_path in collroles: - argspec = self._load_argspec(role, collection_path=collection_path) - fqcn, doc = self._build_doc(role, collection_path, collection, argspec, entry_point) - if doc: - result[fqcn] = doc + try: + argspec = self._load_argspec(role, collection_path=collection_path) + fqcn, doc = self._build_doc(role, collection_path, collection, argspec, entry_point) + if doc: + result[fqcn] = doc + except Exception as e: # pylint:disable=broad-except + result['%s.%s' % (collection, role)] = { + 'error': 'Error while processing role: %s' % to_native(e), + } return result @@ -438,6 +463,10 @@ class DocCLI(CLI, RoleMixin): exclusive.add_argument("--metadata-dump", action="store_true", default=False, dest='dump', help='**For internal use only** Dump json metadata for all entries, ignores other options.') + self.parser.add_argument("--no-fail-on-errors", action="store_true", default=False, dest='no_fail_on_errors', + help='**For internal use only** Only used for --metadata-dump. ' + 'Do not fail on errors. Report the error message in the JSON instead.') + def post_process_args(self, options): options = super(DocCLI, self).post_process_args(options) @@ -626,7 +655,7 @@ class DocCLI(CLI, RoleMixin): self.plugin_list = set() # reset for next iteration return results - def _get_plugins_docs(self, plugin_type, names): + def _get_plugins_docs(self, plugin_type, names, fail_on_errors=True): loader = DocCLI._prep_loader(plugin_type) search_paths = DocCLI.print_paths(loader) @@ -640,6 +669,11 @@ class DocCLI(CLI, RoleMixin): display.warning("%s %s not found in:\n%s\n" % (plugin_type, plugin, search_paths)) continue except Exception as e: + if not fail_on_errors: + plugin_docs[plugin] = { + 'error': 'Missing documentation or could not parse documentation: %s' % to_native(e), + } + continue display.vvv(traceback.format_exc()) raise AnsibleError("%s %s missing documentation (or could not parse" " documentation): %s\n" % @@ -647,9 +681,24 @@ class DocCLI(CLI, RoleMixin): if not doc: # The doc section existed but was empty + if not fail_on_errors: + plugin_docs[plugin] = { + 'error': 'No valid documentation found', + } continue - plugin_docs[plugin] = DocCLI._combine_plugin_doc(plugin, plugin_type, doc, plainexamples, returndocs, metadata) + docs = DocCLI._combine_plugin_doc(plugin, plugin_type, doc, plainexamples, returndocs, metadata) + if not fail_on_errors: + # Check whether JSON serialization would break + try: + json.dumps(docs, cls=AnsibleJSONEncoder) + except Exception as e: # pylint:disable=broad-except + plugin_docs[plugin] = { + 'error': 'Cannot serialize documentation as JSON: %s' % to_native(e), + } + continue + + plugin_docs[plugin] = docs return plugin_docs @@ -719,14 +768,16 @@ class DocCLI(CLI, RoleMixin): docs['all'] = {} for ptype in ptypes: if ptype == 'role': - roles = self._create_role_list() - docs['all'][ptype] = self._create_role_doc(roles.keys(), context.CLIARGS['entry_point']) + roles = self._create_role_list(fail_on_errors=not context.CLIARGS['no_fail_on_errors']) + docs['all'][ptype] = self._create_role_doc( + roles.keys(), context.CLIARGS['entry_point'], fail_on_errors=not context.CLIARGS['no_fail_on_errors']) elif ptype == 'keyword': names = DocCLI._list_keywords() docs['all'][ptype] = DocCLI._get_keywords_docs(names.keys()) else: plugin_names = self._list_plugins(ptype, None) - docs['all'][ptype] = self._get_plugins_docs(ptype, plugin_names) + docs['all'][ptype] = self._get_plugins_docs( + ptype, plugin_names, fail_on_errors=not context.CLIARGS['no_fail_on_errors']) # reset list after each type to avoid polution elif listing: if plugin_type == 'keyword': diff --git a/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/MANIFEST.json b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/MANIFEST.json new file mode 100644 index 0000000000..243a5e4372 --- /dev/null +++ b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/MANIFEST.json @@ -0,0 +1,30 @@ +{ + "collection_info": { + "description": null, + "repository": "", + "tags": [], + "dependencies": {}, + "authors": [ + "Ansible (https://ansible.com)" + ], + "issues": "", + "name": "testcol", + "license": [ + "GPL-3.0-or-later" + ], + "documentation": "", + "namespace": "testns", + "version": "0.1.1231", + "readme": "README.md", + "license_file": "COPYING", + "homepage": "", + }, + "file_manifest_file": { + "format": 1, + "ftype": "file", + "chksum_sha256": "4c15a867ceba8ba1eaf2f4a58844bb5dbb82fec00645fc7eb74a3d31964900f6", + "name": "FILES.json", + "chksum_type": "sha256" + }, + "format": 1 +} diff --git a/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/cache/notjsonfile.py b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/cache/notjsonfile.py new file mode 100644 index 0000000000..9fa25b400a --- /dev/null +++ b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/cache/notjsonfile.py @@ -0,0 +1,70 @@ +# (c) 2020 Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +DOCUMENTATION = ''' + cache: notjsonfile + broken: + short_description: JSON formatted files. + description: + - This cache uses JSON formatted, per host, files saved to the filesystem. + author: Ansible Core (@ansible-core) + version_added: 0.7.0 + options: + _uri: + required: True + description: + - Path in which the cache plugin will save the JSON files + env: + - name: ANSIBLE_CACHE_PLUGIN_CONNECTION + version_added: 1.2.0 + ini: + - key: fact_caching_connection + section: defaults + deprecated: + alternative: none + why: Test deprecation + version: '2.0.0' + _prefix: + description: User defined prefix to use when creating the JSON files + env: + - name: ANSIBLE_CACHE_PLUGIN_PREFIX + version_added: 1.1.0 + ini: + - key: fact_caching_prefix + section: defaults + deprecated: + alternative: none + why: Another test deprecation + removed_at_date: '2050-01-01' + _timeout: + default: 86400 + description: Expiration timeout for the cache plugin data + env: + - name: ANSIBLE_CACHE_PLUGIN_TIMEOUT + ini: + - key: fact_caching_timeout + section: defaults + vars: + - name: notsjonfile_fact_caching_timeout + version_added: 1.5.0 + deprecated: + alternative: do not use a variable + why: Test deprecation + version: '3.0.0' + type: integer + extends_documentation_fragment: + - testns.testcol2.plugin +''' + +from ansible.plugins.cache import BaseFileCacheModule + + +class CacheModule(BaseFileCacheModule): + """ + A caching module backed by json files. + """ + pass diff --git a/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/inventory/statichost.py b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/inventory/statichost.py new file mode 100644 index 0000000000..caec2ed699 --- /dev/null +++ b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/inventory/statichost.py @@ -0,0 +1,36 @@ +# Copyright (c) 2018 Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +DOCUMENTATION = ''' + inventory: statichost + broken: + short_description: Add a single host + description: Add a single host + extends_documentation_fragment: + - inventory_cache + options: + plugin: + description: plugin name (must be statichost) + required: true + hostname: + description: Toggle display of stderr even when script was successful + required: True +''' + +from ansible.errors import AnsibleParserError +from ansible.plugins.inventory import BaseInventoryPlugin, Cacheable + + +class InventoryModule(BaseInventoryPlugin, Cacheable): + + NAME = 'testns.content_adj.statichost' + + def verify_file(self, path): + pass + + def parse(self, inventory, loader, path, cache=None): + + pass diff --git a/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/lookup/noop.py b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/lookup/noop.py new file mode 100644 index 0000000000..d456986957 --- /dev/null +++ b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/lookup/noop.py @@ -0,0 +1,45 @@ +# (c) 2020 Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) + +__metaclass__ = type + +DOCUMENTATION = """ + lookup: noop + broken: + author: Ansible core team + short_description: returns input + description: + - this is a noop + deprecated: + alternative: Use some other lookup + why: Test deprecation + removed_in: '3.0.0' + extends_documentation_fragment: + - testns.testcol2.version_added +""" + +EXAMPLES = """ +- name: do nothing + debug: msg="{{ lookup('testns.testcol.noop', [1,2,3,4] }}" +""" + +RETURN = """ + _list: + description: input given + version_added: 1.0.0 +""" + +from ansible.module_utils.common._collections_compat import Sequence +from ansible.plugins.lookup import LookupBase +from ansible.errors import AnsibleError + + +class LookupModule(LookupBase): + + def run(self, terms, **kwargs): + if not isinstance(terms, Sequence): + raise AnsibleError("testns.testcol.noop expects a list") + return terms diff --git a/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/modules/fakemodule.py b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/modules/fakemodule.py new file mode 100644 index 0000000000..a1caeb148b --- /dev/null +++ b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/modules/fakemodule.py @@ -0,0 +1,28 @@ +#!/usr/bin/python +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +DOCUMENTATION = """ + module: fakemodule + broken: + short_desciption: fake module + description: + - this is a fake module + version_added: 1.0.0 + options: + _notreal: + description: really not a real option + author: + - me +""" + +import json + + +def main(): + print(json.dumps(dict(changed=False, source='testns.testcol.fakemodule'))) + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/modules/notrealmodule.py b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/modules/notrealmodule.py new file mode 100644 index 0000000000..4479f23fa5 --- /dev/null +++ b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/modules/notrealmodule.py @@ -0,0 +1,13 @@ +#!/usr/bin/python +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import json + + +def main(): + print(json.dumps(dict(changed=False, source='testns.testcol.notrealmodule'))) + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/modules/randommodule.py b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/modules/randommodule.py new file mode 100644 index 0000000000..fb0e319d14 --- /dev/null +++ b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/modules/randommodule.py @@ -0,0 +1,96 @@ +#!/usr/bin/python +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +DOCUMENTATION = ''' +--- +module: randommodule +short_description: A random module +description: + - A random module. +author: + - Ansible Core Team +version_added: 1.0.0 +deprecated: + alternative: Use some other module + why: Test deprecation + removed_in: '3.0.0' +options: + test: + description: Some text. + type: str + version_added: 1.2.0 + sub: + description: Suboptions. + type: dict + suboptions: + subtest: + description: A suboption. + type: int + version_added: 1.1.0 + # The following is the wrong syntax, and should not get processed + # by add_collection_to_versions_and_dates() + options: + subtest2: + description: Another suboption. + type: float + version_added: 1.1.0 + # The following is not supported in modules, and should not get processed + # by add_collection_to_versions_and_dates() + env: + - name: TEST_ENV + version_added: 1.0.0 + deprecated: + alternative: none + why: Test deprecation + removed_in: '2.0.0' + version: '2.0.0' +extends_documentation_fragment: + - testns.testcol2.module +''' + +EXAMPLES = ''' +''' + +RETURN = ''' +z_last: + description: A last result. + broken: + type: str + returned: success + version_added: 1.3.0 + +m_middle: + description: + - This should be in the middle. + - Has some more data + type: dict + returned: success and 1st of month + contains: + suboption: + description: A suboption. + type: str + choices: [ARF, BARN, c_without_capital_first_letter] + version_added: 1.4.0 + +a_first: + description: A first result. + type: str + returned: success +''' + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/vars/noop_vars_plugin.py b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/vars/noop_vars_plugin.py new file mode 100644 index 0000000000..ae0f75e0b3 --- /dev/null +++ b/test/integration/targets/ansible-doc/broken-docs/collections/ansible_collections/testns/testcol/plugins/vars/noop_vars_plugin.py @@ -0,0 +1,30 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +DOCUMENTATION = ''' + vars: noop_vars_plugin + broken: + short_description: Do NOT load host and group vars + description: don't test loading host and group vars from a collection + options: + stage: + default: all + choices: ['all', 'inventory', 'task'] + type: str + ini: + - key: stage + section: testns.testcol.noop_vars_plugin + env: + - name: ANSIBLE_VARS_PLUGIN_STAGE + extends_documentation_fragment: + - testns.testcol2.deprecation +''' + +from ansible.plugins.vars import BaseVarsPlugin + + +class VarsModule(BaseVarsPlugin): + + def get_vars(self, loader, path, entities, cache=True): + super(VarsModule, self).get_vars(loader, path, entities) + return {'collection': 'yes', 'notreal': 'value'} diff --git a/test/integration/targets/ansible-doc/runme.sh b/test/integration/targets/ansible-doc/runme.sh index bab614bc97..81eba61946 100755 --- a/test/integration/targets/ansible-doc/runme.sh +++ b/test/integration/targets/ansible-doc/runme.sh @@ -103,3 +103,31 @@ test "$current_out" == "$expected_out" # just ensure it runs ANSIBLE_LIBRARY='./nolibrary' ansible-doc --metadata-dump --playbook-dir /dev/null + +# create broken role argument spec +mkdir -p broken-docs/collections/ansible_collections/testns/testcol/roles/testrole/meta +cat <<EOF > broken-docs/collections/ansible_collections/testns/testcol/roles/testrole/meta/main.yml +--- +dependencies: +galaxy_info: + +argument_specs: + main: + short_description: testns.testcol.testrole short description for main entry point + description: + - Longer description for testns.testcol.testrole main entry point. + author: Ansible Core (@ansible) + options: + opt1: + description: opt1 description + broken: + type: "str" + required: true +EOF + +# ensure that --metadata-dump does not fail when --no-fail-on-errors is supplied +ANSIBLE_LIBRARY='./nolibrary' ansible-doc --metadata-dump --no-fail-on-errors --playbook-dir broken-docs testns.testcol + +# ensure that --metadata-dump does fail when --no-fail-on-errors is not supplied +output=$(ANSIBLE_LIBRARY='./nolibrary' ansible-doc --metadata-dump --playbook-dir broken-docs testns.testcol 2>&1 | grep -c 'ERROR!' || true) +test "$output" -eq 1 |