Role arg spec validation implementation

[Shrews]

SUMMARY

This is a modified version of the solution provided by @alikins in PR #44983, but updated for the latest engineering spec and core code changes.

Still needed:

• Migrate/add more extensive testing.
• Add docs.
• Change from using AnsibleModule to new, common validation code in module_utils.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

core
roles

ADDITIONAL INFORMATION

[ansibot]

The test ansible-test sanity --test package-data [explain] failed with the error:

Command "/usr/bin/python3.6 /root/ansible/test/sanity/code-smell/package-data.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 376, in <module>
    main()
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 353, in main
    sdist_path = create_sdist(tmp_dir)
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 167, in create_sdist
    raise Exception('make snapshot failed:\n%s' % stderr)
Exception: make snapshot failed:
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
ERROR! module validate_arg_spec missing documentation (or could not parse documentation): while parsing a block mapping
  in "<unicode string>", line 2, column 1
did not find expected key
  in "<unicode string>", line 26, column 2
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/antsibull_changelog/cli.py", line 220, in run
    return arguments.func(arguments)
  File "/usr/local/lib/python3.6/dist-packages/antsibull_changelog/cli.py", line 423, in command_release
    use_ansible_doc=args.use_ansible_doc)
  File "/usr/local/lib/python3.6/dist-packages/antsibull_changelog/plugins.py", line 353, in load_plugins
    paths, None, plugin_type, collection_name, use_ansible_doc=use_ansible_doc)
  File "/usr/local/lib/python3.6/dist-packages/antsibull_changelog/plugins.py", line 266, in load_plugin_metadata
    plugins_data = run_ansible_doc(paths, playbook_dir, plugin_type, plugins_list)
  File "/usr/local/lib/python3.6/dist-packages/antsibull_changelog/plugins.py", line 238, in run_ansible_doc
    output = subprocess.check_output(command)
  File "/usr/lib/python3.6/subprocess.py", line 356, in check_output
    **kwargs).stdout
  File "/usr/lib/python3.6/subprocess.py", line 438, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['ansible-doc', '--json', '-t', 'module', 'add_host', 'apt', 'apt_key', 'apt_repository', 'assemble', 'assert', 'async_status', 'blockinfile', 'command', 'copy', 'cron', 'debconf', 'debug', 'dnf', 'dpkg_selections', 'expect', 'fail', 'fetch', 'file', 'find', 'gather_facts', 'get_url', 'getent', 'git', 'group', 'group_by', 'hostname', 'import_playbook', 'import_role', 'import_tasks', 'include', 'include_role', 'include_tasks', 'include_vars', 'iptables', 'known_hosts', 'lineinfile', 'meta', 'package', 'package_facts', 'pause', 'ping', 'pip', 'raw', 'reboot', 'replace', 'rpm_key', 'script', 'service', 'service_facts', 'set_fact', 'set_stats', 'setup', 'shell', 'slurp', 'stat', 'subversion', 'systemd', 'sysvinit', 'tempfile', 'template', 'unarchive', 'uri', 'user', 'validate_arg_spec', 'wait_for', 'wait_for_connection', 'yum', 'yum_repository']' returned non-zero exit status 1.
make: *** [changelog] Error 1

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Command "ansible-doc -t module add_host apt apt_key apt_repository assemble assert async_status blockinfile command copy cron debconf debug dnf dpkg_selections expect fail fetch file find gather_facts get_url getent git group group_by hostname import_playbook import_role import_tasks include include_role include_tasks include_vars iptables known_hosts lineinfile meta package package_facts pause ping pip raw reboot replace rpm_key script service service_facts set_fact set_stats setup shell slurp stat subversion systemd sysvinit tempfile template unarchive uri user validate_arg_spec wait_for wait_for_connection yum yum_repository" returned exit status 1.
>>> Standard Error
ERROR! module validate_arg_spec missing documentation (or could not parse documentation): while parsing a block mapping
  in "<unicode string>", line 2, column 1
did not find expected key
  in "<unicode string>", line 26, column 2

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/validate_arg_spec.py:0:0: documentation-error: Unknown DOCUMENTATION error, see TRACE: while parsing a block mapping
  in "<unicode string>", line 2, column 1
did not find expected key
  in "<unicode string>", line 26, column 2
lib/ansible/modules/validate_arg_spec.py:63:2: return-syntax-error: RETURN is not valid YAML

The test ansible-test sanity --test yamllint [explain] failed with 2 errors:

lib/ansible/modules/validate_arg_spec.py:63:2: error: RETURN: syntax error: expected <block end>, but found '<block mapping start>' (syntax)
lib/ansible/modules/validate_arg_spec.py:63:2: unparsable-with-libyaml: while parsing a block mapping - did not find expected key

click here for bot help

[ansibot]

The test ansible-test sanity --test docs-build [explain] failed with the error:

Command "/usr/bin/python3.6 /root/ansible/test/sanity/code-smell/docs-build.py" returned exit status 1.
>>> Standard Error
Command 'make base_singlehtmldocs' failed with status code: 2
--> Standard Output
../../hacking/build-ansible.py collection-meta --template-file=../templates/collections_galaxy_meta.rst.j2 --output-dir=rst/dev_guide/ ../../lib/ansible/galaxy/data/collections_galaxy_meta.yml
../../hacking/build-ansible.py document-config --template-file=../templates/config.rst.j2 --output-dir=rst/reference_appendices/ ../../lib/ansible/config/base.yml
mkdir -p rst/cli
../../hacking/build-ansible.py generate-man --template-file=../templates/cli_rst.j2 --output-dir=rst/cli/ --output-format rst ../../lib/ansible/cli/*.py
../../hacking/build-ansible.py document-keywords --template-dir=../templates --output-dir=rst/reference_appendices/ ../../lib/ansible/keyword_desc.yml
../../hacking/build-ansible.py docs-build base -o rst ;\

Makefile:133: recipe for target 'base_plugins' failed
--> Standard Error
Traceback (most recent call last):
  File "../../hacking/build-ansible.py", line 103, in <module>
    main()
  File "../../hacking/build-ansible.py", line 92, in main
    retval = command.main(args)
  File "/root/ansible/hacking/build_library/build_ansible/command_plugins/docs_build.py", line 164, in main
    return generate_base_docs(args)
  File "/root/ansible/hacking/build_library/build_ansible/command_plugins/docs_build.py", line 57, in generate_base_docs
    '--dest-dir', args.output_dir])
  File "/usr/local/lib/python3.6/dist-packages/antsibull/cli/antsibull_docs.py", line 280, in run
    return ARGS_MAP[args.command]()
  File "/usr/local/lib/python3.6/dist-packages/antsibull/cli/doc_commands/stable.py", line 395, in generate_docs
    venv.install_package(ansible_base_path)
  File "/usr/local/lib/python3.6/dist-packages/antsibull/venv.py", line 66, in install_package
    return self._python('-m', 'pip', 'install', package_name, _env=get_clean_environment())
  File "/usr/local/lib/python3.6/dist-packages/sh.py", line 1520, in __call__
    return RunningCommand(cmd, call_args, stdin, stdout, stderr)
  File "/usr/local/lib/python3.6/dist-packages/sh.py", line 784, in __init__
    self.wait()
  File "/usr/local/lib/python3.6/dist-packages/sh.py", line 841, in wait
    self.handle_command_exit_code(exit_code)
  File "/usr/local/lib/python3.6/dist-packages/sh.py", line 865, in handle_command_exit_code
    raise exc
sh.ErrorReturnCode_1: 

  RAN: /tmp/tmpht9idn0k/ansible-base-venv/bin/python -m pip install /tmp/tmpht9idn0k/ansiblec7a42pm2/ansible-core-2.11.0.dev0.tar.gz

  STDOUT:
Processing /tmp/tmpht9idn0k/ansiblec7a42pm2/ansible-core-2.11.0.dev0.tar.gz
Collecting PyYAML (from ansible-core==2.11.0.dev0)
  Downloading https://files.pythonhosted.org/packages/7a/5b/bc0b5ab38247bba158504a410112b6c03f153c652734ece1849749e5f518/PyYAML-5.4.1-cp36-cp36m-manylinux1_x86_64.whl (640kB)
�[?25l
�[K    1% |▌                               | 10kB 39.2MB/s eta 0:00:01
�[K    3% |█                               | 20kB 32.4MB/s eta 0:00:01
�[K    4% |█▌                              | 30kB 23.2MB/s eta 0:00:01
�[K    6% |██                              | 40kB 5.2MB/s eta 0:00:01
�[K    7% |██▋                             | 51kB 6.3MB/s eta 0:00:01
�[K    9% |███                             | 61kB 7.4MB/s eta 0:... (13050 more, please see e.stdout)

  STDERR:
Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-build-qpyfefvk/cryptography/

make: *** [base_plugins] Error 1

click here for bot help

[s-hertel]

Should there be a way for a user to opt out of this? They may be using a 3rd party role that has an argspec, and if they don't want this to run they'd need to create their own version of the role. I also noticed that the yaml callback blows up on the msg field for failed validation.

[Shrews]

Hrm, yes, I think there should be. Would we want to disable globally for all roles, or per role?

[samdoran]

Very good point. Probably should be per role.

[bcoca]

meta/ parameter + include/import_role option. Allows both for role author to handle manually as for role consumers to 'know better' and override default behaviour.

[bcoca]

this should really go into detail using suboptions to describe the spec

[Shrews]

The spec is described in good detail in the .rst file in this change. I think we should have 1 source describing the spec, otherwise they will go out of sync for certain.

[bcoca]

why i would keep in in the module (closer to the actual code) as any updates will come here naturally but not to the docs page (which can just point at module). Also would help with arg_spec/docs validation.

[bcoca]

but not a blocker for this PR, can be addressed afterwards

[Shrews]

👍