From e2f14d94b8492970cc82a963879f07d48b5a7b35 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Wed, 4 Aug 2021 22:36:23 +0200 Subject: [PATCH] [2.11] Improve handling of AnsibleModule arguments in validate-modules (#75360) * Improve handling of AnsibleModule arguments in validate-modules (#75332) * Make sure AnsibleModule positional arguments are validated. * Extract ANSIBLE_MODULE_CONSTURCTOR_ARGS with inspection. * Remove no longer necessary return value. * Fix PR #. * argument_spec might not have been specified, as in community.general.xenserver_facts. * Fix typo. (cherry picked from commit 7726b70d9a780248229316407fb1aac0c762eabd) * Make sure self doesn't end up in fake.args (#75403) (cherry picked from commit 2455d82c1427eca00fd5790228107b052df0c459) Co-authored-by: Matt Martz --- ...st-validate-modules-AnsibleModule-args.yml | 2 ++ .../validate-modules/validate_modules/main.py | 2 +- .../validate_modules/module_args.py | 27 ++++++++++++------- 3 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 changelogs/fragments/75332-ansible-test-validate-modules-AnsibleModule-args.yml diff --git a/changelogs/fragments/75332-ansible-test-validate-modules-AnsibleModule-args.yml b/changelogs/fragments/75332-ansible-test-validate-modules-AnsibleModule-args.yml new file mode 100644 index 00000000000..fed73ba78e6 --- /dev/null +++ b/changelogs/fragments/75332-ansible-test-validate-modules-AnsibleModule-args.yml @@ -0,0 +1,2 @@ +bugfixes: +- "ansible-test validate-modules - correctly validate positional parameters to ``AnsibleModules`` (https://github.com/ansible/ansible/pull/75332)." 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 1a75193318d..5b61f44b385 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 @@ -1221,7 +1221,7 @@ class ModuleValidator(Validator): def _validate_ansible_module_call(self, docs): try: - spec, args, kwargs = get_argument_spec(self.path, self.collection) + spec, kwargs = get_argument_spec(self.path, self.collection) except AnsibleModuleNotInitialized: self.reporter.error( path=self.object_path, diff --git a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py index ac0252914b7..8cd0e5e5607 100644 --- a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py +++ b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py @@ -19,6 +19,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import runpy +import inspect import json import os import subprocess @@ -27,13 +28,16 @@ import sys from contextlib import contextmanager from ansible.executor.powershell.module_manifest import PSModuleDepFinder -from ansible.module_utils.basic import FILE_COMMON_ARGUMENTS +from ansible.module_utils.basic import FILE_COMMON_ARGUMENTS, AnsibleModule from ansible.module_utils.six import reraise from ansible.module_utils._text import to_bytes, to_text from .utils import CaptureStd, find_executable, get_module_name_from_filename +ANSIBLE_MODULE_CONSTRUCTOR_ARGS = tuple(list(inspect.signature(AnsibleModule.__init__).parameters)[1:]) + + class AnsibleModuleCallError(RuntimeError): pass @@ -53,7 +57,12 @@ class _FakeAnsibleModuleInit: self.called = False def __call__(self, *args, **kwargs): - self.args = args + if args and isinstance(args[0], AnsibleModule): + # Make sure, due to creative calling, that we didn't end up with + # ``self`` in ``args`` + self.args = args[1:] + else: + self.args = args self.kwargs = kwargs self.called = True raise AnsibleModuleCallError('AnsibleModuleCallError') @@ -126,7 +135,7 @@ def get_ps_argument_spec(filename, collection): # the validate-modules code expects the options spec to be under the argument_spec key not options as set in PS kwargs['argument_spec'] = kwargs.pop('options', {}) - return kwargs['argument_spec'], (), kwargs + return kwargs['argument_spec'], kwargs def get_py_argument_spec(filename, collection): @@ -146,11 +155,11 @@ def get_py_argument_spec(filename, collection): raise AnsibleModuleNotInitialized() try: + # Convert positional arguments to kwargs to make sure that all parameters are actually checked + for arg, arg_name in zip(fake.args, ANSIBLE_MODULE_CONSTRUCTOR_ARGS): + fake.kwargs[arg_name] = arg # for ping kwargs == {'argument_spec':{'data':{'type':'str','default':'pong'}}, 'supports_check_mode':True} - if 'argument_spec' in fake.kwargs: - argument_spec = fake.kwargs['argument_spec'] - else: - argument_spec = fake.args[0] + argument_spec = fake.kwargs.get('argument_spec') or {} # If add_file_common_args is truish, add options from FILE_COMMON_ARGUMENTS when not present. # This is the only modification to argument_spec done by AnsibleModule itself, and which is # not caught by setup_env's AnsibleModule replacement @@ -158,9 +167,9 @@ def get_py_argument_spec(filename, collection): for k, v in FILE_COMMON_ARGUMENTS.items(): if k not in argument_spec: argument_spec[k] = v - return argument_spec, fake.args, fake.kwargs + return argument_spec, fake.kwargs except (TypeError, IndexError): - return {}, (), {} + return {}, {} def get_argument_spec(filename, collection):