diff --git a/changelogs/fragments/53698-alias-collision-warning.yaml b/changelogs/fragments/53698-alias-collision-warning.yaml new file mode 100644 index 00000000000..ad7a7877c52 --- /dev/null +++ b/changelogs/fragments/53698-alias-collision-warning.yaml @@ -0,0 +1,2 @@ +minor_changes: +- Ansible will now warn if two aliases of the same option are used for Python modules. diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index d07c0cd5108..d25915c8247 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1413,14 +1413,17 @@ class AnsibleModule(object): self.fail_json(msg="An unknown error was encountered while attempting to validate the locale: %s" % to_native(e), exception=traceback.format_exc()) - def _handle_aliases(self, spec=None, param=None): + def _handle_aliases(self, spec=None, param=None, option_prefix=''): if spec is None: spec = self.argument_spec if param is None: param = self.params # this uses exceptions as it happens before we can safely call fail_json - alias_results, self._legal_inputs = handle_aliases(spec, param) + alias_warnings = [] + alias_results, self._legal_inputs = handle_aliases(spec, param, alias_warnings=alias_warnings) + for option, alias in alias_warnings: + self._warnings.append('Both option %s and its alias %s are set.' % (option_prefix + option, option_prefix + alias)) return alias_results def _handle_no_log_values(self, spec=None, param=None): @@ -1665,7 +1668,7 @@ class AnsibleModule(object): def _check_type_bits(self, value): return check_type_bits(value) - def _handle_options(self, argument_spec=None, params=None): + def _handle_options(self, argument_spec=None, params=None, prefix=''): ''' deal with options to create sub spec ''' if argument_spec is None: argument_spec = self.argument_spec @@ -1692,12 +1695,17 @@ class AnsibleModule(object): else: elements = params[k] - for param in elements: + for idx, param in enumerate(elements): if not isinstance(param, dict): self.fail_json(msg="value of %s must be of type dict or list of dict" % k) + new_prefix = prefix + k + if wanted == 'list': + new_prefix += '[%d]' % idx + new_prefix += '.' + self._set_fallbacks(spec, param) - options_aliases = self._handle_aliases(spec, param) + options_aliases = self._handle_aliases(spec, param, option_prefix=new_prefix) self._handle_no_log_values(spec, param) options_legal_inputs = list(spec.keys()) + list(options_aliases.keys()) @@ -1723,7 +1731,7 @@ class AnsibleModule(object): self._set_defaults(pre=False, spec=spec, param=param) # handle multi level options (sub argspec) - self._handle_options(spec, param) + self._handle_options(spec, param, new_prefix) self._options_context.pop() def _get_wanted_type(self, wanted, k): diff --git a/lib/ansible/module_utils/common/parameters.py b/lib/ansible/module_utils/common/parameters.py index 71285fbd2d8..9b87af85577 100644 --- a/lib/ansible/module_utils/common/parameters.py +++ b/lib/ansible/module_utils/common/parameters.py @@ -112,9 +112,13 @@ def list_deprecations(argument_spec, params): return deprecations -def handle_aliases(argument_spec, params): +def handle_aliases(argument_spec, params, alias_warnings=None): """Return a two item tuple. The first is a dictionary of aliases, the second is - a list of legal inputs.""" + a list of legal inputs. + + If a list is provided to the alias_warnings parameter, it will be filled with tuples + (option, alias) in every case where both an option and its alias are specified. + """ legal_inputs = ['_ansible_%s' % k for k in PASS_VARS] aliases_results = {} # alias:canon @@ -135,6 +139,8 @@ def handle_aliases(argument_spec, params): legal_inputs.append(alias) aliases_results[alias] = k if alias in params: + if k in params and alias_warnings is not None: + alias_warnings.append((k, alias)) params[k] = params[alias] return aliases_results, legal_inputs diff --git a/test/units/module_utils/basic/test_argument_spec.py b/test/units/module_utils/basic/test_argument_spec.py index e068714837d..c081ff2f62b 100644 --- a/test/units/module_utils/basic/test_argument_spec.py +++ b/test/units/module_utils/basic/test_argument_spec.py @@ -233,6 +233,14 @@ class TestComplexArgSpecs: assert isinstance(am.params['foo'], str) assert am.params['foo'] == 'hello' + @pytest.mark.parametrize('stdin', [{'foo': 'hello1', 'dup': 'hello2'}], indirect=['stdin']) + def test_complex_duplicate_warning(self, stdin, complex_argspec): + """Test that the complex argspec issues a warning if we specify an option both with its canonical name and its alias""" + am = basic.AnsibleModule(**complex_argspec) + assert isinstance(am.params['foo'], str) + assert 'Both option foo and its alias dup are set.' in am._warnings + assert am.params['foo'] == 'hello2' + @pytest.mark.parametrize('stdin', [{'foo': 'hello', 'bam': 'test'}], indirect=['stdin']) def test_complex_type_fallback(self, mocker, stdin, complex_argspec): """Test that the complex argspec works if we get a required parameter via fallback"""