Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't pollute include_variables #54687

Merged
merged 7 commits into from
Apr 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions lib/ansible/executor/task_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,18 +559,18 @@ def _execute(self, variables=None):
# if this task is a TaskInclude, we just return now with a success code so the
# main thread can expand the task list for the given host
if self._task.action in ('include', 'include_tasks'):
include_variables = self._task.args.copy()
include_file = include_variables.pop('_raw_params', None)
include_args = self._task.args.copy()
include_file = include_args.pop('_raw_params', None)
if not include_file:
return dict(failed=True, msg="No include file was specified to the include")

include_file = templar.template(include_file)
return dict(include=include_file, include_variables=include_variables)
return dict(include=include_file, include_args=include_args)

# if this task is a IncludeRole, we just return now with a success code so the main thread can expand the task list for the given host
elif self._task.action == 'include_role':
include_variables = self._task.args.copy()
return dict(include_variables=include_variables)
include_args = self._task.args.copy()
return dict(include_args=include_args)

# Now we do final validation on the task, which sets all fields to their final values.
self._task.post_validate(templar=templar)
Expand Down
33 changes: 19 additions & 14 deletions lib/ansible/playbook/included_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@

class IncludedFile:

def __init__(self, filename, args, task, is_role=False):
def __init__(self, filename, args, vars, task, is_role=False):
self._filename = filename
self._args = args
self._vars = vars
self._task = task
self._hosts = []
self._is_role = is_role
Expand All @@ -47,10 +48,13 @@ def add_host(self, host):
raise ValueError()

def __eq__(self, other):
return other._filename == self._filename and other._args == self._args and other._task._parent._uuid == self._task._parent._uuid
return (other._filename == self._filename and
other._args == self._args and
other._vars == self._vars and
other._task._parent._uuid == self._task._parent._uuid)

def __repr__(self):
return "%s (%s): %s" % (self._filename, self._args, self._hosts)
return "%s (args=%s vars=%s): %s" % (self._filename, self._args, self._vars, self._hosts)

@staticmethod
def process_include_results(results, iterator, loader, variable_manager):
Expand Down Expand Up @@ -81,20 +85,21 @@ def process_include_results(results, iterator, loader, variable_manager):
except KeyError:
task_vars = task_vars_cache[cache_key] = variable_manager.get_vars(play=iterator._play, host=original_host, task=original_task)

include_variables = include_result.get('include_variables', dict())
include_args = include_result.get('include_args', dict())
special_vars = {}
loop_var = 'item'
index_var = None
if original_task.loop_control:
loop_var = original_task.loop_control.loop_var
index_var = original_task.loop_control.index_var
if loop_var in include_result:
task_vars[loop_var] = include_variables[loop_var] = include_result[loop_var]
task_vars[loop_var] = special_vars[loop_var] = include_result[loop_var]
if index_var and index_var in include_result:
task_vars[index_var] = include_variables[index_var] = include_result[index_var]
task_vars[index_var] = special_vars[index_var] = include_result[index_var]
if '_ansible_item_label' in include_result:
task_vars['_ansible_item_label'] = include_variables['_ansible_item_label'] = include_result['_ansible_item_label']
if original_task.no_log and '_ansible_no_log' not in include_variables:
task_vars['_ansible_no_log'] = include_variables['_ansible_no_log'] = original_task.no_log
task_vars['_ansible_item_label'] = special_vars['_ansible_item_label'] = include_result['_ansible_item_label']
if original_task.no_log and '_ansible_no_log' not in include_args:
task_vars['_ansible_no_log'] = special_vars['_ansible_no_log'] = original_task.no_log

# get search path for this task to pass to lookup plugins that may be used in pathing to
# the included file
Expand Down Expand Up @@ -166,21 +171,21 @@ def process_include_results(results, iterator, loader, variable_manager):
include_file = loader.path_dwim(include_result['include'])

include_file = templar.template(include_file)
inc_file = IncludedFile(include_file, include_variables, original_task)
inc_file = IncludedFile(include_file, include_args, special_vars, original_task)
else:
# template the included role's name here
role_name = include_variables.pop('name', include_variables.pop('role', None))
role_name = include_args.pop('name', include_args.pop('role', None))
if role_name is not None:
role_name = templar.template(role_name)

new_task = original_task.copy()
new_task._role_name = role_name
for from_arg in new_task.FROM_ARGS:
if from_arg in include_variables:
if from_arg in include_args:
from_key = from_arg.replace('_from', '')
new_task._from_files[from_key] = templar.template(include_variables.pop(from_arg))
new_task._from_files[from_key] = templar.template(include_args.pop(from_arg))

inc_file = IncludedFile(role_name, include_variables, new_task, is_role=True)
inc_file = IncludedFile(role_name, include_args, special_vars, new_task, is_role=True)

idx = 0
orig_inc_file = inc_file
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/strategy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ def _copy_included_file(self, included_file):
ti_copy._parent = included_file._task._parent

temp_vars = ti_copy.vars.copy()
temp_vars.update(included_file._args)
temp_vars.update(included_file._vars)

ti_copy.vars = temp_vars

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- assert:
that:
- name == 'name_from_loop_var'
- name != 'loop_name_assert'
3 changes: 3 additions & 0 deletions test/integration/targets/include_import/runme.sh
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,6 @@ test "$(egrep -c 'include handler task|ERROR! The requested handler '"'"'do_impo
# https://github.com/ansible/ansible/issues/49969
ansible-playbook -v parent_templating/playbook.yml 2>&1 | tee test_parent_templating.out
test "$(egrep -c 'Templating the path of the parent include_tasks failed.' test_parent_templating.out)" = 0

# https://github.com/ansible/ansible/issues/54618
ansible-playbook test_loop_var_bleed.yaml "$@"
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
- hosts: localhost
gather_facts: false
tasks:
- include_role:
name: loop_name_assert
loop:
- name_from_loop_var
loop_control:
loop_var: name
12 changes: 10 additions & 2 deletions test/units/playbook/test_included_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ def mock_variable_manager():
def test_included_file_instantiation():
filename = 'somefile.yml'

inc_file = IncludedFile(filename=filename, args=[], task=None)
inc_file = IncludedFile(filename=filename, args={}, vars={}, task=None)

assert isinstance(inc_file, IncludedFile)
assert inc_file._filename == filename
assert inc_file._args == []
assert inc_file._args == {}
assert inc_file._vars == {}
assert inc_file._task is None


Expand Down Expand Up @@ -84,6 +85,7 @@ def test_process_include_results(mock_iterator, mock_variable_manager):
assert res[0]._filename == os.path.join(os.getcwd(), 'include_test.yml')
assert res[0]._hosts == ['testhost1', 'testhost2']
assert res[0]._args == {}
assert res[0]._vars == {}


def test_process_include_diff_files(mock_iterator, mock_variable_manager):
Expand Down Expand Up @@ -124,6 +126,9 @@ def test_process_include_diff_files(mock_iterator, mock_variable_manager):
assert res[0]._args == {}
assert res[1]._args == {}

assert res[0]._vars == {}
assert res[1]._vars == {}


def test_process_include_simulate_free(mock_iterator, mock_variable_manager):
hostname = "testhost1"
Expand Down Expand Up @@ -159,3 +164,6 @@ def test_process_include_simulate_free(mock_iterator, mock_variable_manager):

assert res[0]._args == {}
assert res[1]._args == {}

assert res[0]._vars == {}
assert res[1]._vars == {}