From 8a7185c224eb6e116d1233d37d57bbbb223cb5bb Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 19 Jan 2023 17:58:02 +0100 Subject: [PATCH] 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. --- ...es-warnings-deprecations-in-suboptions.yml | 3 + lib/ansible/module_utils/common/arg_spec.py | 24 +-- lib/ansible/module_utils/common/parameters.py | 20 ++- .../targets/argspec/library/argspec.py | 75 +++++++- .../targets/argspec/tasks/main.yml | 166 ++++++++++++++++++ .../common/arg_spec/test_aliases.py | 7 +- .../common/arg_spec/test_module_validate.py | 2 +- 7 files changed, 280 insertions(+), 17 deletions(-) create mode 100644 changelogs/fragments/79740-aliases-warnings-deprecations-in-suboptions.yml diff --git a/changelogs/fragments/79740-aliases-warnings-deprecations-in-suboptions.yml b/changelogs/fragments/79740-aliases-warnings-deprecations-in-suboptions.yml new file mode 100644 index 00000000000..1e839850190 --- /dev/null +++ b/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)." diff --git a/lib/ansible/module_utils/common/arg_spec.py b/lib/ansible/module_utils/common/arg_spec.py index 176268d096f..d9f716efce6 100644 --- a/lib/ansible/module_utils/common/arg_spec.py +++ b/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,14 +301,9 @@ 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')) + 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 c1df1c06e92..059ca0af513 100644 --- a/lib/ansible/module_utils/common/parameters.py +++ b/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() diff --git a/test/integration/targets/argspec/library/argspec.py b/test/integration/targets/argspec/library/argspec.py index 2f24bb7492e..b6d6d110f26 100644 --- a/test/integration/targets/argspec/library/argspec.py +++ b/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), diff --git a/test/integration/targets/argspec/tasks/main.yml b/test/integration/targets/argspec/tasks/main.yml index a00d33d7aba..6e8ec054fe0 100644 --- a/test/integration/targets/argspec/tasks/main.yml +++ b/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" diff --git a/test/units/module_utils/common/arg_spec/test_aliases.py b/test/units/module_utils/common/arg_spec/test_aliases.py index 1c1e243a7b1..7d30fb0f0cc 100644 --- a/test/units/module_utils/common/arg_spec/test_aliases.py +++ b/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", + }, "", ) ] diff --git a/test/units/module_utils/common/arg_spec/test_module_validate.py b/test/units/module_utils/common/arg_spec/test_module_validate.py index 5041d521bd2..2c2211c9d41 100644 --- a/test/units/module_utils/common/arg_spec/test_module_validate.py +++ b/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, } ]