diff options
9 files changed, 180 insertions, 13 deletions
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 0000000000..58f2f5f924 --- /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/changelogs/fragments/79681-argspec-param-deprecation.yml b/changelogs/fragments/79681-argspec-param-deprecation.yml new file mode 100644 index 0000000000..ac19a47a16 --- /dev/null +++ b/changelogs/fragments/79681-argspec-param-deprecation.yml @@ -0,0 +1,2 @@ +bugfixes: + - "argument spec validation - again report deprecated parameters for Python-based modules. This was accidentally removed in ansible-core 2.11 when argument spec validation was refactored (https://github.com/ansible/ansible/issues/79680, https://github.com/ansible/ansible/pull/79681)." diff --git a/lib/ansible/module_utils/common/arg_spec.py b/lib/ansible/module_utils/common/arg_spec.py index 9fa2b4d2c9..176268d096 100644 --- a/lib/ansible/module_utils/common/arg_spec.py +++ b/lib/ansible/module_utils/common/arg_spec.py @@ -12,6 +12,7 @@ from ansible.module_utils.common.parameters import ( _get_legal_inputs, _get_unsupported_parameters, _handle_aliases, + _list_deprecations, _list_no_log_values, _set_defaults, _validate_argument_types, @@ -31,6 +32,7 @@ from ansible.module_utils.common.validation import ( from ansible.module_utils.errors import ( AliasError, AnsibleValidationErrorMultiple, + DeprecationError, MutuallyExclusiveError, NoLogError, RequiredDefaultError, @@ -58,6 +60,7 @@ class ValidationResult: """ self._unsupported_parameters = set() + self._supported_parameters = dict() self._validated_parameters = deepcopy(parameters) self._deprecations = [] self._warnings = [] @@ -204,7 +207,19 @@ 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._deprecations.extend(_list_deprecations(self.argument_spec, result._validated_parameters)) + except TypeError as te: + result.errors.append(DeprecationError(to_native(te))) + + try: + 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 +251,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 +263,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 @@ -268,9 +292,14 @@ class ModuleArgumentSpecValidator(ArgumentSpecValidator): result = super(ModuleArgumentSpecValidator, self).validate(parameters) for d in result._deprecations: - deprecate("Alias '{name}' is deprecated. See the module docs for more information".format(name=d['name']), - version=d.get('version'), date=d.get('date'), - collection_name=d.get('collection_name')) + if 'name' in d: + deprecate("Alias '{name}' is deprecated. See the module docs for more information".format(name=d['name']), + version=d.get('version'), date=d.get('date'), + collection_name=d.get('collection_name')) + if 'msg' in d: + deprecate(d['msg'], + version=d.get('version'), date=d.get('date'), + collection_name=d.get('collection_name')) for w in result._warnings: warn('Both option {option} and its alias {alias} are set.'.format(option=w['option'], alias=w['alias'])) diff --git a/lib/ansible/module_utils/common/parameters.py b/lib/ansible/module_utils/common/parameters.py index 93b80431c3..c1df1c06e9 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/module_utils/errors.py b/lib/ansible/module_utils/errors.py index 3274b85b63..cbbd86c01c 100644 --- a/lib/ansible/module_utils/errors.py +++ b/lib/ansible/module_utils/errors.py @@ -75,6 +75,10 @@ class ArgumentValueError(AnsibleValidationError): """Error with parameter value""" +class DeprecationError(AnsibleValidationError): + """Error processing parameter deprecations""" + + class ElementError(AnsibleValidationError): """Error when validating elements""" diff --git a/lib/ansible/plugins/action/validate_argument_spec.py b/lib/ansible/plugins/action/validate_argument_spec.py index 8c4432d111..dc7d6cb3b9 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/argspec/library/argspec.py b/test/integration/targets/argspec/library/argspec.py index 1a1d288d23..2f24bb7492 100644 --- a/test/integration/targets/argspec/library/argspec.py +++ b/test/integration/targets/argspec/library/argspec.py @@ -139,6 +139,35 @@ def main(): }, }, }, + 'deprecation_aliases': { + 'type': 'str', + 'aliases': [ + 'deprecation_aliases_version', + 'deprecation_aliases_date', + ], + 'deprecated_aliases': [ + { + 'name': 'deprecation_aliases_version', + 'version': '2.0.0', + 'collection_name': 'foo.bar', + }, + { + 'name': 'deprecation_aliases_date', + 'date': '2023-01-01', + 'collection_name': 'foo.bar', + }, + ], + }, + 'deprecation_param_version': { + 'type': 'str', + 'removed_in_version': '2.0.0', + 'removed_from_collection': 'foo.bar', + }, + 'deprecation_param_date': { + 'type': 'str', + 'removed_at_date': '2023-01-01', + 'removed_from_collection': 'foo.bar', + }, }, required_if=( ('state', 'present', ('path', 'content'), True), diff --git a/test/integration/targets/argspec/tasks/main.yml b/test/integration/targets/argspec/tasks/main.yml index 283c922d78..a00d33d7ab 100644 --- a/test/integration/targets/argspec/tasks/main.yml +++ b/test/integration/targets/argspec/tasks/main.yml @@ -366,6 +366,30 @@ foo: bar register: argspec_apply_defaults_one +- argspec: + required: value + required_one_of_one: value + deprecation_aliases_version: value + register: deprecation_alias_version + +- argspec: + required: value + required_one_of_one: value + deprecation_aliases_date: value + register: deprecation_alias_date + +- argspec: + required: value + required_one_of_one: value + deprecation_param_version: value + register: deprecation_param_version + +- argspec: + required: value + required_one_of_one: value + deprecation_param_date: value + register: deprecation_param_date + - assert: that: - argspec_required_fail is failed @@ -446,3 +470,24 @@ - "argspec_apply_defaults_none.apply_defaults == {'foo': none, 'bar': 'baz'}" - "argspec_apply_defaults_empty.apply_defaults == {'foo': none, 'bar': 'baz'}" - "argspec_apply_defaults_one.apply_defaults == {'foo': 'bar', 'bar': 'baz'}" + + - deprecation_alias_version.deprecations | length == 1 + - deprecation_alias_version.deprecations[0].msg == "Alias 'deprecation_aliases_version' is deprecated. See the module docs for more information" + - deprecation_alias_version.deprecations[0].collection_name == 'foo.bar' + - deprecation_alias_version.deprecations[0].version == '2.0.0' + - "'date' not in deprecation_alias_version.deprecations[0]" + - deprecation_alias_date.deprecations | length == 1 + - deprecation_alias_date.deprecations[0].msg == "Alias 'deprecation_aliases_date' is deprecated. See the module docs for more information" + - deprecation_alias_date.deprecations[0].collection_name == 'foo.bar' + - deprecation_alias_date.deprecations[0].date == '2023-01-01' + - "'version' not in deprecation_alias_date.deprecations[0]" + - deprecation_param_version.deprecations | length == 1 + - deprecation_param_version.deprecations[0].msg == "Param 'deprecation_param_version' is deprecated. See the module docs for more information" + - deprecation_param_version.deprecations[0].collection_name == 'foo.bar' + - deprecation_param_version.deprecations[0].version == '2.0.0' + - "'date' not in deprecation_param_version.deprecations[0]" + - deprecation_param_date.deprecations | length == 1 + - deprecation_param_date.deprecations[0].msg == "Param 'deprecation_param_date' is deprecated. See the module docs for more information" + - deprecation_param_date.deprecations[0].collection_name == 'foo.bar' + - deprecation_param_date.deprecations[0].date == '2023-01-01' + - "'version' not in deprecation_param_date.deprecations[0]" 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 8764d38275..81abdaa8c2 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." |