From 3d5a7d6dc2e50575872f1dc15fc4f8016dac5db5 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 4 May 2018 11:10:50 -0500 Subject: [PATCH] Allow using action/local_action on includes and imports (#37260) * Prevent using action/local_action on includes and imports. Fixes #28822 * Use ModuleArgsParser to determine action instead of disallowing action/local_action with import/include * Add to_native * switch back to block in task_ds, use ModuleArgsParse otherwise * var should be task_ds * Add test validating action+include_tasks --- lib/ansible/playbook/helpers.py | 29 ++++++++++++++----- .../tasks/test_include_tasks.yml | 3 ++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index b0b9c9cbdcc..14b4f480d53 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -22,7 +22,9 @@ import os from ansible import constants as C from ansible.errors import AnsibleParserError, AnsibleUndefinedVariable, AnsibleFileNotFound, AnsibleAssertionError +from ansible.module_utils._text import to_native from ansible.module_utils.six import string_types +from ansible.parsing.mod_args import ModuleArgsParser try: from __main__ import display @@ -118,7 +120,18 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h ) task_list.append(t) else: - if 'include' in task_ds or 'import_tasks' in task_ds or 'include_tasks' in task_ds: + args_parser = ModuleArgsParser(task_ds) + try: + (action, args, delegate_to) = args_parser.parse() + except AnsibleParserError as e: + # 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 e._obj: + raise + # But if it wasn't, we can add the yaml object now to get more detail + raise AnsibleParserError(to_native(e), obj=task_ds, orig_exc=e) + + if action in ('include', 'import_tasks', 'include_tasks'): if use_handlers: include_class = HandlerTaskInclude @@ -140,9 +153,9 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h # check to see if this include is dynamic or static: # 1. the user has set the 'static' option to false or true # 2. one of the appropriate config options was set - if 'include_tasks' in task_ds: + if action == 'include_tasks': is_static = False - elif 'import_tasks' in task_ds: + elif action == 'import_tasks': is_static = True elif t.static is not None: display.deprecated("The use of 'static' has been deprecated. " @@ -155,7 +168,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h if is_static: if t.loop is not None: - if 'import_tasks' in task_ds: + if action == 'import_tasks': raise AnsibleParserError("You cannot use loops on 'import_tasks' statements. You should use 'include_tasks' instead.", obj=task_ds) else: raise AnsibleParserError("You cannot use 'static' on an include with a loop", obj=task_ds) @@ -262,7 +275,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h tags = tags.split(',') if len(tags) > 0: - if 'include_tasks' in task_ds or 'import_tasks' in task_ds: + if action in ('include_tasks', 'import_tasks'): raise AnsibleParserError('You cannot specify "tags" inline to the task, it is a task keyword') if len(ti_copy.tags) > 0: raise AnsibleParserError( @@ -292,7 +305,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h t.is_static = False task_list.append(t) - elif 'include_role' in task_ds or 'import_role' in task_ds: + elif action in ('include_role', 'import_role'): ir = IncludeRole.load( task_ds, block=block, @@ -305,7 +318,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h # 1. the user has set the 'static' option to false or true # 2. one of the appropriate config options was set is_static = False - if 'import_role' in task_ds: + if action == 'import_role': is_static = True elif ir.static is not None: @@ -315,7 +328,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h if is_static: if ir.loop is not None: - if 'import_role' in task_ds: + if action == 'import_role': raise AnsibleParserError("You cannot use loops on 'import_role' statements. You should use 'include_role' instead.", obj=task_ds) else: raise AnsibleParserError("You cannot use 'static' on an include_role with a loop", obj=task_ds) diff --git a/test/integration/targets/include_import/tasks/test_include_tasks.yml b/test/integration/targets/include_import/tasks/test_include_tasks.yml index 76471ffd4cc..ebe2273e896 100644 --- a/test/integration/targets/include_import/tasks/test_include_tasks.yml +++ b/test/integration/targets/include_import/tasks/test_include_tasks.yml @@ -39,3 +39,6 @@ assert: that: - set_in_tasks4 + + - name: include_tasks + action + action: include_tasks tasks1.yml