Argument spec alias handling: actually report deprecated aliases in suboptions, and fix warning message in suboptions when two aliases of the same option are used (#79740)

* Normalize deprecation records.

* Fix alias deprecations in suboptions.

* Report in which option an alias warning happened for suboptions.

* Add deprecation tests for suboptions.

* Also test deprecation in list of dicts.

* Adjust unit tests for toplevel alias deprecation field name change.

changelogs/fragments/79740-aliases-warnings-deprecations-in-suboptions.yml

@@ -0,0 +1,3 @@
+bugfixes:
+  - "argument spec validation - ensure that deprecated aliases in suboptions are also reported (https://github.com/ansible/ansible/pull/79740)."
+  - "argument spec validation - fix warning message when two aliases of the same option are used for suboptions to also mention the option's name they are in (https://github.com/ansible/ansible/pull/79740)."

lib/ansible/module_utils/common/arg_spec.py

@@ -195,7 +195,7 @@ class ArgumentSpecValidator:
 
         for deprecation in alias_deprecations:
             result._deprecations.append({
-                'name': deprecation['name'],
+                'msg': "Alias '%s' is deprecated. See the module docs for more information" % deprecation['name'],
                 'version': deprecation.get('version'),
                 'date': deprecation.get('date'),
                 'collection_name': deprecation.get('collection_name'),
@@ -248,11 +248,20 @@ class ArgumentSpecValidator:
 
         result._no_log_values.update(_set_defaults(self.argument_spec, result._validated_parameters))
 
+        alias_deprecations = []
         _validate_sub_spec(self.argument_spec, result._validated_parameters,
                            errors=result.errors,
                            no_log_values=result._no_log_values,
                            unsupported_parameters=result._unsupported_parameters,
-                           supported_parameters=result._supported_parameters,)
+                           supported_parameters=result._supported_parameters,
+                           alias_deprecations=alias_deprecations,)
+        for deprecation in alias_deprecations:
+            result._deprecations.append({
+                'msg': "Alias '%s' is deprecated. See the module docs for more information" % deprecation['name'],
+                'version': deprecation.get('version'),
+                'date': deprecation.get('date'),
+                'collection_name': deprecation.get('collection_name'),
+            })
 
         if result._unsupported_parameters:
             flattened_names = []
@@ -292,11 +301,6 @@ class ModuleArgumentSpecValidator(ArgumentSpecValidator):
         result = super(ModuleArgumentSpecValidator, self).validate(parameters)
 
         for d in result._deprecations:
-            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'))

lib/ansible/module_utils/common/parameters.py

@@ -705,6 +705,7 @@ def _validate_sub_spec(
     no_log_values=None,
     unsupported_parameters=None,
     supported_parameters=None,
+    alias_deprecations=None,
 ):
     """Validate sub argument spec.
 
@@ -761,15 +762,24 @@ def _validate_sub_spec(
                 new_prefix += '.'
 
                 alias_warnings = []
-                alias_deprecations = []
+                alias_deprecations_sub = []
                 try:
-                    options_aliases = _handle_aliases(sub_spec, sub_parameters, alias_warnings, alias_deprecations)
+                    options_aliases = _handle_aliases(sub_spec, sub_parameters, alias_warnings, alias_deprecations_sub)
                 except (TypeError, ValueError) as e:
                     options_aliases = {}
                     errors.append(AliasError(to_native(e)))
 
                 for option, alias in alias_warnings:
-                    warn('Both option %s and its alias %s are set.' % (option, alias))
+                    warn('Both option %s%s and its alias %s%s are set.' % (new_prefix, option, new_prefix, alias))
+
+                if alias_deprecations is not None:
+                    for deprecation in alias_deprecations_sub:
+                        alias_deprecations.append({
+                            'name': '%s%s' % (new_prefix, deprecation['name']),
+                            'version': deprecation.get('version'),
+                            'date': deprecation.get('date'),
+                            'collection_name': deprecation.get('collection_name'),
+                        })
 
                 try:
                     no_log_values.update(_list_no_log_values(sub_spec, sub_parameters))
@@ -811,7 +821,9 @@ def _validate_sub_spec(
                 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, supported_parameters)
+                _validate_sub_spec(
+                    sub_spec, sub_parameters, new_prefix, options_context, errors, no_log_values,
+                    unsupported_parameters, supported_parameters, alias_deprecations)
 
             options_context.pop()
 

test/integration/targets/argspec/library/argspec.py

@@ -34,7 +34,7 @@ def main():
                 'elements': 'dict',
                 'options': {
                     'thing': {},
-                    'other': {},
+                    'other': {'aliases': ['other_alias']},
                 },
             },
             'required_by': {
@@ -136,6 +136,7 @@ def main():
                     'bar': {
                         'type': 'str',
                         'default': 'baz',
+                        'aliases': ['bar_alias1', 'bar_alias2'],
                     },
                 },
             },
@@ -168,6 +169,78 @@ def main():
                 'removed_at_date': '2023-01-01',
                 'removed_from_collection': 'foo.bar',
             },
+            'subdeprecation': {
+                'aliases': [
+                    'subdeprecation_alias',
+                ],
+                'type': 'dict',
+                'options': {
+                    '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',
+                    },
+                },
+            },
+            'subdeprecation_list': {
+                'type': 'list',
+                'elements': 'dict',
+                'options': {
+                    '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

@@ -390,6 +390,106 @@
     deprecation_param_date: value
   register: deprecation_param_date
 
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation:
+      deprecation_aliases_version: value
+  register: sub_deprecation_alias_version
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation:
+      deprecation_aliases_date: value
+  register: sub_deprecation_alias_date
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation:
+      deprecation_param_version: value
+  register: sub_deprecation_param_version
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation:
+      deprecation_param_date: value
+  register: sub_deprecation_param_date
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation_alias:
+      deprecation_aliases_version: value
+  register: subalias_deprecation_alias_version
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation_alias:
+      deprecation_aliases_date: value
+  register: subalias_deprecation_alias_date
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation_alias:
+      deprecation_param_version: value
+  register: subalias_deprecation_param_version
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation_alias:
+      deprecation_param_date: value
+  register: subalias_deprecation_param_date
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation_list:
+      - deprecation_aliases_version: value
+  register: sublist_deprecation_alias_version
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation_list:
+      - deprecation_aliases_date: value
+  register: sublist_deprecation_alias_date
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation_list:
+      - deprecation_param_version: value
+  register: sublist_deprecation_param_version
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    subdeprecation_list:
+      - deprecation_param_date: value
+  register: sublist_deprecation_param_date
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    apply_defaults:
+      bar_alias1: foo
+      bar_alias2: baz
+  register: alias_warning_dict
+
+- argspec:
+    required: value
+    required_one_of_one: value
+    required_one_of:
+      - other: foo
+        other_alias: bar
+  register: alias_warning_listdict
+
 - assert:
     that:
       - argspec_required_fail is failed
@@ -491,3 +591,69 @@
       - 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]"
+
+      - sub_deprecation_alias_version.deprecations | length == 1
+      - sub_deprecation_alias_version.deprecations[0].msg == "Alias 'subdeprecation.deprecation_aliases_version' is deprecated. See the module docs for more information"
+      - sub_deprecation_alias_version.deprecations[0].collection_name == 'foo.bar'
+      - sub_deprecation_alias_version.deprecations[0].version == '2.0.0'
+      - "'date' not in sub_deprecation_alias_version.deprecations[0]"
+      - sub_deprecation_alias_date.deprecations | length == 1
+      - sub_deprecation_alias_date.deprecations[0].msg == "Alias 'subdeprecation.deprecation_aliases_date' is deprecated. See the module docs for more information"
+      - sub_deprecation_alias_date.deprecations[0].collection_name == 'foo.bar'
+      - sub_deprecation_alias_date.deprecations[0].date == '2023-01-01'
+      - "'version' not in sub_deprecation_alias_date.deprecations[0]"
+      - sub_deprecation_param_version.deprecations | length == 1
+      - sub_deprecation_param_version.deprecations[0].msg == "Param 'subdeprecation[\"deprecation_param_version\"]' is deprecated. See the module docs for more information"
+      - sub_deprecation_param_version.deprecations[0].collection_name == 'foo.bar'
+      - sub_deprecation_param_version.deprecations[0].version == '2.0.0'
+      - "'date' not in sub_deprecation_param_version.deprecations[0]"
+      - sub_deprecation_param_date.deprecations | length == 1
+      - sub_deprecation_param_date.deprecations[0].msg == "Param 'subdeprecation[\"deprecation_param_date\"]' is deprecated. See the module docs for more information"
+      - sub_deprecation_param_date.deprecations[0].collection_name == 'foo.bar'
+      - sub_deprecation_param_date.deprecations[0].date == '2023-01-01'
+      - "'version' not in sub_deprecation_param_date.deprecations[0]"
+
+      - subalias_deprecation_alias_version.deprecations | length == 1
+      - subalias_deprecation_alias_version.deprecations[0].msg == "Alias 'subdeprecation.deprecation_aliases_version' is deprecated. See the module docs for more information"
+      - subalias_deprecation_alias_version.deprecations[0].collection_name == 'foo.bar'
+      - subalias_deprecation_alias_version.deprecations[0].version == '2.0.0'
+      - "'date' not in subalias_deprecation_alias_version.deprecations[0]"
+      - subalias_deprecation_alias_date.deprecations | length == 1
+      - subalias_deprecation_alias_date.deprecations[0].msg == "Alias 'subdeprecation.deprecation_aliases_date' is deprecated. See the module docs for more information"
+      - subalias_deprecation_alias_date.deprecations[0].collection_name == 'foo.bar'
+      - subalias_deprecation_alias_date.deprecations[0].date == '2023-01-01'
+      - "'version' not in subalias_deprecation_alias_date.deprecations[0]"
+      - subalias_deprecation_param_version.deprecations | length == 1
+      - subalias_deprecation_param_version.deprecations[0].msg == "Param 'subdeprecation[\"deprecation_param_version\"]' is deprecated. See the module docs for more information"
+      - subalias_deprecation_param_version.deprecations[0].collection_name == 'foo.bar'
+      - subalias_deprecation_param_version.deprecations[0].version == '2.0.0'
+      - "'date' not in subalias_deprecation_param_version.deprecations[0]"
+      - subalias_deprecation_param_date.deprecations | length == 1
+      - subalias_deprecation_param_date.deprecations[0].msg == "Param 'subdeprecation[\"deprecation_param_date\"]' is deprecated. See the module docs for more information"
+      - subalias_deprecation_param_date.deprecations[0].collection_name == 'foo.bar'
+      - subalias_deprecation_param_date.deprecations[0].date == '2023-01-01'
+      - "'version' not in subalias_deprecation_param_date.deprecations[0]"
+
+      - sublist_deprecation_alias_version.deprecations | length == 1
+      - sublist_deprecation_alias_version.deprecations[0].msg == "Alias 'subdeprecation_list[0].deprecation_aliases_version' is deprecated. See the module docs for more information"
+      - sublist_deprecation_alias_version.deprecations[0].collection_name == 'foo.bar'
+      - sublist_deprecation_alias_version.deprecations[0].version == '2.0.0'
+      - "'date' not in sublist_deprecation_alias_version.deprecations[0]"
+      - sublist_deprecation_alias_date.deprecations | length == 1
+      - sublist_deprecation_alias_date.deprecations[0].msg == "Alias 'subdeprecation_list[0].deprecation_aliases_date' is deprecated. See the module docs for more information"
+      - sublist_deprecation_alias_date.deprecations[0].collection_name == 'foo.bar'
+      - sublist_deprecation_alias_date.deprecations[0].date == '2023-01-01'
+      - "'version' not in sublist_deprecation_alias_date.deprecations[0]"
+      - sublist_deprecation_param_version.deprecations | length == 1
+      - sublist_deprecation_param_version.deprecations[0].msg == "Param 'subdeprecation_list[\"deprecation_param_version\"]' is deprecated. See the module docs for more information"
+      - sublist_deprecation_param_version.deprecations[0].collection_name == 'foo.bar'
+      - sublist_deprecation_param_version.deprecations[0].version == '2.0.0'
+      - "'date' not in sublist_deprecation_param_version.deprecations[0]"
+      - sublist_deprecation_param_date.deprecations | length == 1
+      - sublist_deprecation_param_date.deprecations[0].msg == "Param 'subdeprecation_list[\"deprecation_param_date\"]' is deprecated. See the module docs for more information"
+      - sublist_deprecation_param_date.deprecations[0].collection_name == 'foo.bar'
+      - sublist_deprecation_param_date.deprecations[0].date == '2023-01-01'
+      - "'version' not in sublist_deprecation_param_date.deprecations[0]"
+
+      - "'Both option apply_defaults.bar and its alias apply_defaults.bar_alias2 are set.' in alias_warning_dict.warnings"
+      - "'Both option required_one_of[0].other and its alias required_one_of[0].other_alias are set.' in alias_warning_listdict.warnings"

test/units/module_utils/common/arg_spec/test_aliases.py

@@ -57,7 +57,12 @@ ALIAS_TEST_CASES = [
             'path': '/tmp',
             'not_yo_path': '/tmp',
         },
-        {'version': '1.7', 'date': None, 'collection_name': None, 'name': 'not_yo_path'},
+        {
+            'version': '1.7',
+            'date': None,
+            'collection_name': None,
+            'msg': "Alias 'not_yo_path' is deprecated. See the module docs for more information",
+        },
         "",
     )
 ]

test/units/module_utils/common/arg_spec/test_module_validate.py

@@ -49,7 +49,7 @@ def test_module_alias_deprecations_warnings(monkeypatch):
         {
             'collection_name': None,
             'date': '2020-03-04',
-            'name': 'flamethrower',
+            'msg': "Alias 'flamethrower' is deprecated. See the module docs for more information",
             'version': None,
         }
     ]