Skip to content

Commit

Permalink
fix role argument spec error for invalid suboptions (ansible#76578)
Browse files Browse the repository at this point in the history
fixes ansible#75536

(cherry picked from commit b5b239f)
  • Loading branch information
s-hertel authored and felixfontein committed Jan 14, 2023
1 parent 9e3363f commit c921224
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 10 deletions.
@@ -0,0 +1,2 @@
bugfixes:
- Module and role argument validation - include the valid suboption choices in the error when an invalid suboption is provided.
27 changes: 22 additions & 5 deletions lib/ansible/module_utils/common/arg_spec.py
Expand Up @@ -58,6 +58,7 @@ def __init__(self, parameters):
"""

self._unsupported_parameters = set()
self._supported_parameters = dict()
self._validated_parameters = deepcopy(parameters)
self._deprecations = []
self._warnings = []
Expand Down Expand Up @@ -204,7 +205,14 @@ def validate(self, parameters, *args, **kwargs):
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:
Expand Down Expand Up @@ -236,7 +244,8 @@ def validate(self, parameters, *args, **kwargs):
_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 = []
Expand All @@ -247,9 +256,17 @@ def validate(self, parameters, *args, **kwargs):
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

Expand Down
37 changes: 33 additions & 4 deletions lib/ansible/module_utils/common/parameters.py
Expand Up @@ -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.
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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.
Expand All @@ -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')
Expand Down Expand Up @@ -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)
Expand All @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/action/validate_argument_spec.py
Expand Up @@ -79,7 +79,7 @@ def run(self, tmp=None, task_vars=None):

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
Expand Down
Expand Up @@ -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."

0 comments on commit c921224

Please sign in to comment.