cmueller/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Don't pollute include_variables (#54687)

Browse source 
* Don't pollute include_variables. Fixes #51667. Fixes #54618.

* Rename include_variables to include_args, so we can make the distinction about what they are

* Track args and vars separately

* oops

* oops again

* linting fix

* Add test
This commit is contained in:
Matt Martz 2019-04-09 10:14:42 -05:00 committed by GitHub
parent dd20c7c04e
commit fbf2d5d2f4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 51 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
lib/ansible/executor/task_executor.py
View file

@ -559,18 +559,18 @@ class TaskExecutor:
# 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)

33
lib/ansible/playbook/included_file.py
View file

@ -33,9 +33,10 @@ display = Display()
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
@ -47,10 +48,13 @@ class IncludedFile:
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):
@ -81,20 +85,21 @@ class IncludedFile:
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
@ -166,21 +171,21 @@ class IncludedFile:
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

2
lib/ansible/plugins/strategy/__init__.py
View file

@ -762,7 +762,7 @@ class StrategyBase:
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

4
test/integration/targets/include_import/roles/loop_name_assert/tasks/main.yml Normal file
View file

@ -0,0 +1,4 @@
- assert:
that:
- name == 'name_from_loop_var'
- name != 'loop_name_assert'

3
test/integration/targets/include_import/runme.sh
View file

@ -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 "$@"

9
test/integration/targets/include_import/test_loop_var_bleed.yaml Normal file
View file

@ -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
test/units/playbook/test_included_file.py
View file

@ -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
@ -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):
@ -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"
@ -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 == {}