From 547c64f3268427e5d1f5d4c45c143b6d231efb1b Mon Sep 17 00:00:00 2001 From: Matt Davis <6775756+nitzmahone@users.noreply.github.com> Date: Tue, 24 Jun 2025 10:00:37 -0700 Subject: [PATCH] deprecate esoteric/undocumented playbook syntaxes (#85378) Co-authored-by: Matt Clay --- .../fragments/task_esoterica_deprecation.yml | 4 ++ lib/ansible/cli/adhoc.py | 7 ++- lib/ansible/parsing/mod_args.py | 56 ++++++++++++------- lib/ansible/playbook/helpers.py | 2 +- .../task-esoterica/action_plugins/echo.py | 8 +++ .../targets/task-esoterica/aliases | 4 ++ .../targets/task-esoterica/tasks/main.yml | 39 +++++++++++++ test/units/cli/test_adhoc.py | 2 +- 8 files changed, 99 insertions(+), 23 deletions(-) create mode 100644 changelogs/fragments/task_esoterica_deprecation.yml create mode 100644 test/integration/targets/task-esoterica/action_plugins/echo.py create mode 100644 test/integration/targets/task-esoterica/aliases create mode 100644 test/integration/targets/task-esoterica/tasks/main.yml diff --git a/changelogs/fragments/task_esoterica_deprecation.yml b/changelogs/fragments/task_esoterica_deprecation.yml new file mode 100644 index 00000000000..44bcc94366f --- /dev/null +++ b/changelogs/fragments/task_esoterica_deprecation.yml @@ -0,0 +1,4 @@ +deprecated_features: + - playbook syntax - Using a mapping with the ``action`` keyword is deprecated. (https://github.com/ansible/ansible/issues/84101) + - playbook syntax - Using ``key=value`` args and the task ``args`` keyword on the same task is deprecated. + - playbook syntax - Specifying the task ``args`` keyword without a value is deprecated. diff --git a/lib/ansible/cli/adhoc.py b/lib/ansible/cli/adhoc.py index 22a354e253c..352b4f1a64a 100755 --- a/lib/ansible/cli/adhoc.py +++ b/lib/ansible/cli/adhoc.py @@ -88,8 +88,11 @@ class AdHocCLI(CLI): if not module_args: module_args = parse_kv(module_args_raw, check_raw=check_raw) - mytask = {'action': {'module': context.CLIARGS['module_name'], 'args': module_args}, - 'timeout': context.CLIARGS['task_timeout']} + mytask = dict( + action=context.CLIARGS['module_name'], + args=module_args, + timeout=context.CLIARGS['task_timeout'], + ) mytask = Origin(description=f'').tag(mytask) diff --git a/lib/ansible/parsing/mod_args.py b/lib/ansible/parsing/mod_args.py index b156657f50d..09823d59dd5 100644 --- a/lib/ansible/parsing/mod_args.py +++ b/lib/ansible/parsing/mod_args.py @@ -20,13 +20,13 @@ from __future__ import annotations import ansible.constants as C from ansible.errors import AnsibleParserError, AnsibleError, AnsibleAssertionError from ansible.module_utils._internal._datatag import AnsibleTagHelper -from ansible.module_utils.six import string_types from ansible.module_utils.common.sentinel import Sentinel from ansible.module_utils.common.text.converters import to_text from ansible.parsing.splitter import parse_kv, split_args from ansible.parsing.vault import EncryptedString from ansible.plugins.loader import module_loader, action_loader -from ansible._internal._templating._engine import TemplateEngine +from ansible._internal._templating import _jinja_bits +from ansible.utils.display import Display from ansible.utils.fqcn import add_internal_fqcns @@ -152,38 +152,43 @@ class ModuleArgsParser: arguments can be fuzzy. Deal with all the forms. """ - additional_args = {} if additional_args is None else additional_args - # final args are the ones we'll eventually return, so first update # them with any additional args specified, which have lower priority # than those which may be parsed/normalized next final_args = dict() - if additional_args: - if isinstance(additional_args, (str, EncryptedString)): - # DTFIX5: should this be is_possibly_template? - if TemplateEngine().is_template(additional_args): - final_args['_variable_params'] = additional_args - else: - raise AnsibleParserError("Complex args containing variables cannot use bare variables (without Jinja2 delimiters), " - "and must use the full variable style ('{{var_name}}')") + + if additional_args is not Sentinel: + if isinstance(additional_args, str) and _jinja_bits.is_possibly_all_template(additional_args): + final_args['_variable_params'] = additional_args elif isinstance(additional_args, dict): final_args.update(additional_args) + elif additional_args is None: + Display().deprecated( + msg="Ignoring empty task `args` keyword.", + version="2.23", + help_text='A mapping or template which resolves to a mapping is required.', + obj=self._task_ds, + ) else: - raise AnsibleParserError('Complex args must be a dictionary or variable string ("{{var}}").') + raise AnsibleParserError( + message='The value of the task `args` keyword is invalid.', + help_text='A mapping or template which resolves to a mapping is required.', + obj=additional_args, + ) # how we normalize depends if we figured out what the module name is # yet. If we have already figured it out, it's a 'new style' invocation. # otherwise, it's not if action is not None: - args = self._normalize_new_style_args(thing, action) + args = self._normalize_new_style_args(thing, action, additional_args) else: (action, args) = self._normalize_old_style_args(thing) # this can occasionally happen, simplify if args and 'args' in args: tmp_args = args.pop('args') - if isinstance(tmp_args, string_types): + if isinstance(tmp_args, str): tmp_args = parse_kv(tmp_args) args.update(tmp_args) @@ -206,7 +211,7 @@ class ModuleArgsParser: return (action, final_args) - def _normalize_new_style_args(self, thing, action): + def _normalize_new_style_args(self, thing, action, additional_args): """ deals with fuzziness in new style module invocations accepting key=value pairs and dictionaries, and returns @@ -222,11 +227,23 @@ class ModuleArgsParser: if isinstance(thing, dict): # form is like: { xyz: { x: 2, y: 3 } } args = thing - elif isinstance(thing, string_types): + elif isinstance(thing, str): # form is like: copy: src=a dest=b check_raw = action in FREEFORM_ACTIONS args = parse_kv(thing, check_raw=check_raw) + args_keys = set(args) - {'_raw_params'} + + if args_keys and additional_args is not Sentinel: + kv_args = ', '.join(repr(arg) for arg in sorted(args_keys)) + + Display().deprecated( + msg=f"Merging legacy k=v args ({kv_args}) into task args.", + help_text="Include all task args in the task `args` mapping.", + version="2.23", + obj=thing, + ) elif isinstance(thing, EncryptedString): + # k=v parsing intentionally omitted args = dict(_raw_params=thing) elif thing is None: # this can happen with modules which take no params, like ping: @@ -253,6 +270,7 @@ class ModuleArgsParser: if isinstance(thing, dict): # form is like: action: { module: 'copy', src: 'a', dest: 'b' } + Display().deprecated("Using a mapping for `action` is deprecated.", version='2.23', help_text='Use a string value for `action`.', obj=thing) thing = thing.copy() if 'module' in thing: action, module_args = self._split_module_string(thing['module']) @@ -261,7 +279,7 @@ class ModuleArgsParser: args.update(parse_kv(module_args, check_raw=check_raw)) del args['module'] - elif isinstance(thing, string_types): + elif isinstance(thing, str): # form is like: action: copy src=a dest=b (action, args) = self._split_module_string(thing) check_raw = action in FREEFORM_ACTIONS @@ -287,7 +305,7 @@ class ModuleArgsParser: # This is the standard YAML form for command-type modules. We grab # the args and pass them in as additional arguments, which can/will # be overwritten via dict updates from the other arg sources below - additional_args = self._task_ds.get('args', dict()) + additional_args = self._task_ds.get('args', Sentinel) # We can have one of action, local_action, or module specified # action diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index f700bb2349a..f4d7a82a8ec 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -122,7 +122,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h except AnsibleParserError as ex: # if the raises exception was created with obj=ds args, then it includes the detail # so we dont need to add it so we can just re raise. - if ex.obj: + if ex.obj is not None: raise # But if it wasn't, we can add the yaml object now to get more detail # DTFIX-FUTURE: this *should* be unnecessary- check code coverage. diff --git a/test/integration/targets/task-esoterica/action_plugins/echo.py b/test/integration/targets/task-esoterica/action_plugins/echo.py new file mode 100644 index 00000000000..39ed4455215 --- /dev/null +++ b/test/integration/targets/task-esoterica/action_plugins/echo.py @@ -0,0 +1,8 @@ +from __future__ import annotations + +from ansible.plugins.action import ActionBase + + +class ActionModule(ActionBase): + def run(self, tmp=None, task_vars=None): + return dict(action_args=self._task.args) diff --git a/test/integration/targets/task-esoterica/aliases b/test/integration/targets/task-esoterica/aliases new file mode 100644 index 00000000000..569a8498d43 --- /dev/null +++ b/test/integration/targets/task-esoterica/aliases @@ -0,0 +1,4 @@ +shippable/posix/group3 +context/controller +gather_facts/no +env/set/ANSIBLE_DEPRECATION_WARNINGS/yes diff --git a/test/integration/targets/task-esoterica/tasks/main.yml b/test/integration/targets/task-esoterica/tasks/main.yml new file mode 100644 index 00000000000..8d2b452ad59 --- /dev/null +++ b/test/integration/targets/task-esoterica/tasks/main.yml @@ -0,0 +1,39 @@ +# Validates some deprecated (previously untested/undocumented) playbook syntax until support is removed. + +- name: action-as-dict (static-only, template-to-dict never worked anyway) + action: + module: "{{ 'echo' }}" + args: "{{ echo_args }}" # note that `module` and `args` are children of `action` + vars: + echo_args: + a: 1 + register: action_as_dict + +- assert: + that: action_as_dict.action_args == action_args + vars: + action_args: + a: 1 + +- name: kv and task args at the same time + echo: kv1=kv + args: + kv1: task_arg + kv2: task_arg + register: kv_and_task_args + +- assert: + that: kv_and_task_args.action_args == action_args + vars: + action_args: + kv1: kv + kv2: task_arg + +- name: task args with no value + echo: + args: + register: args_were_none + +- assert: + that: + - args_were_none.action_args == {} diff --git a/test/units/cli/test_adhoc.py b/test/units/cli/test_adhoc.py index bbb41b1f998..4b1c5fed6af 100644 --- a/test/units/cli/test_adhoc.py +++ b/test/units/cli/test_adhoc.py @@ -62,7 +62,7 @@ def test_play_ds_positive(): adhoc_cli.parse() ret = adhoc_cli._play_ds('command', 10, 2) assert ret['name'] == 'Ansible Ad-Hoc' - assert ret['tasks'] == [{'action': {'module': 'command', 'args': {}}, 'async_val': 10, 'poll': 2, 'timeout': 0}] + assert ret['tasks'] == [{'action': 'command', 'args': {}, 'async_val': 10, 'poll': 2, 'timeout': 0}] def test_play_ds_with_include_role():