[2.14] fix role argument spec error for invalid suboptions & fix reporting of deprecated arguments for modules (#79738)

* fix role argument spec error for invalid suboptions (#76578)

fixes https://github.com/ansible/ansible/issues/75536

(cherry picked from commit b5b239fd71)

* Fix reporting of deprecated arguments for modules. (#79681)

(cherry picked from commit 1a47a21b65)

Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com>

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.

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)."

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']))

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()
 

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"""
 

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

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),

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]"

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."