summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--changelogs/fragments/76578-fix-role-argspec-suboptions-error.yml2
-rw-r--r--changelogs/fragments/79681-argspec-param-deprecation.yml2
-rw-r--r--lib/ansible/module_utils/common/arg_spec.py45
-rw-r--r--lib/ansible/module_utils/common/parameters.py37
-rw-r--r--lib/ansible/module_utils/errors.py4
-rw-r--r--lib/ansible/plugins/action/validate_argument_spec.py2
-rw-r--r--test/integration/targets/argspec/library/argspec.py29
-rw-r--r--test/integration/targets/argspec/tasks/main.yml45
-rw-r--r--test/integration/targets/roles_arg_spec/test_complex_role_fails.yml27
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."