diff --git a/changelogs/fragments/76578-fix-role-argspec-suboptions-error.yml b/changelogs/fragments/76578-fix-role-argspec-suboptions-error.yml new file mode 100644 index 00000000000..58f2f5f924f --- /dev/null +++ b/changelogs/fragments/76578-fix-role-argspec-suboptions-error.yml @@ -0,0 +1,2 @@ +bugfixes: + - Module and role argument validation - include the valid suboption choices in the error when an invalid suboption is provided. diff --git a/lib/ansible/module_utils/common/arg_spec.py b/lib/ansible/module_utils/common/arg_spec.py index 9fa2b4d2c9a..237279d0456 100644 --- a/lib/ansible/module_utils/common/arg_spec.py +++ b/lib/ansible/module_utils/common/arg_spec.py @@ -58,6 +58,7 @@ class ValidationResult: """ self._unsupported_parameters = set() + self._supported_parameters = dict() self._validated_parameters = deepcopy(parameters) self._deprecations = [] self._warnings = [] @@ -204,7 +205,14 @@ class ArgumentSpecValidator: result.errors.append(NoLogError(to_native(te))) try: - result._unsupported_parameters.update(_get_unsupported_parameters(self.argument_spec, result._validated_parameters, legal_inputs)) + result._unsupported_parameters.update( + _get_unsupported_parameters( + self.argument_spec, + result._validated_parameters, + legal_inputs, + store_supported=result._supported_parameters, + ) + ) except TypeError as te: result.errors.append(RequiredDefaultError(to_native(te))) except ValueError as ve: @@ -236,7 +244,8 @@ class ArgumentSpecValidator: _validate_sub_spec(self.argument_spec, result._validated_parameters, errors=result.errors, no_log_values=result._no_log_values, - unsupported_parameters=result._unsupported_parameters) + unsupported_parameters=result._unsupported_parameters, + supported_parameters=result._supported_parameters,) if result._unsupported_parameters: flattened_names = [] @@ -247,9 +256,17 @@ class ArgumentSpecValidator: flattened_names.append(item) unsupported_string = ", ".join(sorted(list(flattened_names))) - supported_string = ", ".join(self._valid_parameter_names) - result.errors.append( - UnsupportedError("{0}. Supported parameters include: {1}.".format(unsupported_string, supported_string))) + supported_params = supported_aliases = [] + if result._supported_parameters.get(item): + supported_params = sorted(list(result._supported_parameters[item][0])) + supported_aliases = sorted(list(result._supported_parameters[item][1])) + supported_string = ", ".join(supported_params) + if supported_aliases: + aliases_string = ", ".join(supported_aliases) + supported_string += " (%s)" % aliases_string + + msg = "{0}. Supported parameters include: {1}.".format(unsupported_string, supported_string) + result.errors.append(UnsupportedError(msg)) return result diff --git a/lib/ansible/module_utils/common/parameters.py b/lib/ansible/module_utils/common/parameters.py index 93b80431c36..c1df1c06e92 100644 --- a/lib/ansible/module_utils/common/parameters.py +++ b/lib/ansible/module_utils/common/parameters.py @@ -154,7 +154,7 @@ def _get_legal_inputs(argument_spec, parameters, aliases=None): return list(aliases.keys()) + list(argument_spec.keys()) -def _get_unsupported_parameters(argument_spec, parameters, legal_inputs=None, options_context=None): +def _get_unsupported_parameters(argument_spec, parameters, legal_inputs=None, options_context=None, store_supported=None): """Check keys in parameters against those provided in legal_inputs to ensure they contain legal values. If legal_inputs are not supplied, they will be generated using the argument_spec. @@ -182,6 +182,16 @@ def _get_unsupported_parameters(argument_spec, parameters, legal_inputs=None, op unsupported_parameters.add(context) + if store_supported is not None: + supported_aliases = _handle_aliases(argument_spec, parameters) + supported_params = [] + for option in legal_inputs: + if option in supported_aliases: + continue + supported_params.append(option) + + store_supported.update({context: (supported_params, supported_aliases)}) + return unsupported_parameters @@ -686,7 +696,16 @@ def _validate_argument_values(argument_spec, parameters, options_context=None, e errors.append(ArgumentTypeError(msg)) -def _validate_sub_spec(argument_spec, parameters, prefix='', options_context=None, errors=None, no_log_values=None, unsupported_parameters=None): +def _validate_sub_spec( + argument_spec, + parameters, + prefix="", + options_context=None, + errors=None, + no_log_values=None, + unsupported_parameters=None, + supported_parameters=None, +): """Validate sub argument spec. This function is recursive. @@ -703,6 +722,8 @@ def _validate_sub_spec(argument_spec, parameters, prefix='', options_context=Non if unsupported_parameters is None: unsupported_parameters = set() + if supported_parameters is None: + supported_parameters = dict() for param, value in argument_spec.items(): wanted = value.get('type') @@ -756,7 +777,15 @@ def _validate_sub_spec(argument_spec, parameters, prefix='', options_context=Non errors.append(NoLogError(to_native(te))) legal_inputs = _get_legal_inputs(sub_spec, sub_parameters, options_aliases) - unsupported_parameters.update(_get_unsupported_parameters(sub_spec, sub_parameters, legal_inputs, options_context)) + unsupported_parameters.update( + _get_unsupported_parameters( + sub_spec, + sub_parameters, + legal_inputs, + options_context, + store_supported=supported_parameters, + ) + ) try: check_mutually_exclusive(value.get('mutually_exclusive'), sub_parameters, options_context) @@ -782,7 +811,7 @@ def _validate_sub_spec(argument_spec, parameters, prefix='', options_context=Non no_log_values.update(_set_defaults(sub_spec, sub_parameters)) # Handle nested specs - _validate_sub_spec(sub_spec, sub_parameters, new_prefix, options_context, errors, no_log_values, unsupported_parameters) + _validate_sub_spec(sub_spec, sub_parameters, new_prefix, options_context, errors, no_log_values, unsupported_parameters, supported_parameters) options_context.pop() diff --git a/lib/ansible/plugins/action/validate_argument_spec.py b/lib/ansible/plugins/action/validate_argument_spec.py index 8c4432d111a..dc7d6cb3b90 100644 --- a/lib/ansible/plugins/action/validate_argument_spec.py +++ b/lib/ansible/plugins/action/validate_argument_spec.py @@ -79,7 +79,7 @@ class ActionModule(ActionBase): args_from_vars = self.get_args_from_task_vars(argument_spec_data, task_vars) validator = ArgumentSpecValidator(argument_spec_data) - validation_result = validator.validate(combine_vars(args_from_vars, provided_arguments)) + validation_result = validator.validate(combine_vars(args_from_vars, provided_arguments), validate_role_argument_spec=True) if validation_result.error_messages: result['failed'] = True diff --git a/test/integration/targets/roles_arg_spec/test_complex_role_fails.yml b/test/integration/targets/roles_arg_spec/test_complex_role_fails.yml index 8764d382753..81abdaa8c27 100644 --- a/test/integration/targets/roles_arg_spec/test_complex_role_fails.yml +++ b/test/integration/targets/roles_arg_spec/test_complex_role_fails.yml @@ -168,3 +168,30 @@ - ansible_failed_result.validate_args_context.name == "test1" - ansible_failed_result.validate_args_context.type == "role" - "ansible_failed_result.validate_args_context.path is search('roles_arg_spec/roles/test1')" + + - name: test message for missing required parameters and invalid suboptions + block: + - include_role: + name: test1 + vars: + some_json: '{}' + some_jsonarg: '{}' + multi_level_option: + second_level: + not_a_supported_suboption: true + + - fail: + msg: "Should not get here" + + rescue: + - debug: + var: ansible_failed_result + + - assert: + that: + - ansible_failed_result.argument_errors | length == 2 + - missing_required in ansible_failed_result.argument_errors + - got_unexpected in ansible_failed_result.argument_errors + vars: + missing_required: "missing required arguments: third_level found in multi_level_option -> second_level" + got_unexpected: "multi_level_option.second_level.not_a_supported_suboption. Supported parameters include: third_level."