From 34e9d6781b24fb8a6ec8f33701275fca351833c1 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Mon, 20 May 2019 17:49:54 +0200 Subject: [PATCH] Templar: encapsulate _available_variables (#55435) Ensure variables are reset between iterations --- lib/ansible/executor/task_executor.py | 2 +- lib/ansible/playbook/base.py | 2 +- lib/ansible/playbook/conditional.py | 2 +- lib/ansible/plugins/action/template.py | 6 +++--- lib/ansible/plugins/inventory/__init__.py | 4 ++-- lib/ansible/plugins/inventory/azure_rm.py | 2 +- lib/ansible/plugins/inventory/generator.py | 5 ++--- lib/ansible/plugins/lookup/template.py | 6 +++--- lib/ansible/plugins/lookup/vars.py | 2 +- lib/ansible/template/__init__.py | 14 +++++++++++++- lib/ansible/template/vars.py | 10 +++++----- lib/ansible/vars/manager.py | 6 +++--- test/units/plugins/action/test_action.py | 2 +- test/units/template/test_templar.py | 14 ++++++++++---- 14 files changed, 47 insertions(+), 30 deletions(-) diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 55edb249cb0..2eb16e76491 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -345,7 +345,7 @@ class TaskExecutor: task_vars['ansible_loop']['previtem'] = items[item_index - 1] # Update template vars to reflect current loop iteration - templar.set_available_variables(task_vars) + templar.available_variables = task_vars # pause between loop iterations if loop_pause and ran_once: diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index 74357e39f96..5bc132d6985 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -342,7 +342,7 @@ class FieldAttributeBase(with_metaclass(BaseMeta, object)): ''' # save the omit value for later checking - omit_value = templar._available_variables.get('omit') + omit_value = templar.available_variables.get('omit') for (name, attribute) in iteritems(self._valid_attrs): diff --git a/lib/ansible/playbook/conditional.py b/lib/ansible/playbook/conditional.py index 04b49d7167e..08c0ba52e71 100644 --- a/lib/ansible/playbook/conditional.py +++ b/lib/ansible/playbook/conditional.py @@ -125,7 +125,7 @@ class Conditional: 'templating delimiters such as {{ }} or {%% %%}. ' 'Found: %s' % conditional) # make sure the templar is using the variables specified with this method - templar.set_available_variables(variables=all_vars) + templar.available_variables = all_vars try: # if the conditional is "unsafe", disable lookups diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index d205af77e25..8fb7393ff93 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -144,10 +144,10 @@ class ActionModule(ActionBase): temp_vars = task_vars.copy() temp_vars.update(generate_ansible_template_vars(source, dest)) - old_vars = self._templar._available_variables - self._templar.set_available_variables(temp_vars) + old_vars = self._templar.available_variables + self._templar.available_variables = temp_vars resultant = self._templar.do_template(template_data, preserve_trailing_newlines=True, escape_backslashes=False) - self._templar.set_available_variables(old_vars) + self._templar.available_variables = old_vars except AnsibleAction: raise except Exception as e: diff --git a/lib/ansible/plugins/inventory/__init__.py b/lib/ansible/plugins/inventory/__init__.py index 21bdac360d8..9791dbbeb0b 100644 --- a/lib/ansible/plugins/inventory/__init__.py +++ b/lib/ansible/plugins/inventory/__init__.py @@ -349,7 +349,7 @@ class Constructable(object): def _compose(self, template, variables): ''' helper method for plugins to compose variables for Ansible based on jinja2 expression and inventory vars''' t = self.templar - t.set_available_variables(variables) + t.available_variables = variables return t.template('%s%s%s' % (t.environment.variable_start_string, template, t.environment.variable_end_string), disable_lookups=True) def _set_composite_vars(self, compose, variables, host, strict=False): @@ -369,7 +369,7 @@ class Constructable(object): # process each 'group entry' if groups and isinstance(groups, dict): variables = combine_vars(variables, self.inventory.get_host(host).get_vars()) - self.templar.set_available_variables(variables) + self.templar.available_variables = variables for group_name in groups: conditional = "{%% if %s %%} True {%% else %%} False {%% endif %%}" % groups[group_name] group_name = self._sanitize_group_name(group_name) diff --git a/lib/ansible/plugins/inventory/azure_rm.py b/lib/ansible/plugins/inventory/azure_rm.py index 910535698e0..245cee3a4e2 100644 --- a/lib/ansible/plugins/inventory/azure_rm.py +++ b/lib/ansible/plugins/inventory/azure_rm.py @@ -348,7 +348,7 @@ class InventoryModule(BaseInventoryPlugin, Constructable): # FUTURE: fix underlying inventory stuff to allow us to quickly access known groupvars from reconciled host def _filter_host(self, inventory_hostname, hostvars): - self.templar.set_available_variables(hostvars) + self.templar.available_variables = hostvars for condition in self._filters: # FUTURE: should warn/fail if conditional doesn't return True or False diff --git a/lib/ansible/plugins/inventory/generator.py b/lib/ansible/plugins/inventory/generator.py index ab0e45d581b..1af3dfa9015 100644 --- a/lib/ansible/plugins/inventory/generator.py +++ b/lib/ansible/plugins/inventory/generator.py @@ -101,9 +101,8 @@ class InventoryModule(BaseInventoryPlugin): return valid def template(self, pattern, variables): - t = self.templar - t.set_available_variables(variables) - return t.do_template(pattern) + self.templar.available_variables = variables + return self.templar.do_template(pattern) def add_parents(self, inventory, child, parents, template_vars): for parent in parents: diff --git a/lib/ansible/plugins/lookup/template.py b/lib/ansible/plugins/lookup/template.py index 959a2c096e0..4fd3584b653 100644 --- a/lib/ansible/plugins/lookup/template.py +++ b/lib/ansible/plugins/lookup/template.py @@ -68,7 +68,7 @@ class LookupModule(LookupBase): variable_start_string = kwargs.get('variable_start_string', None) variable_end_string = kwargs.get('variable_end_string', None) - old_vars = self._templar._available_variables + old_vars = self._templar.available_variables for term in terms: display.debug("File lookup term: %s" % term) @@ -105,7 +105,7 @@ class LookupModule(LookupBase): vars = deepcopy(variables) vars.update(generate_ansible_template_vars(lookupfile)) vars.update(lookup_template_vars) - self._templar.set_available_variables(vars) + self._templar.available_variables = vars # do the templating res = self._templar.template(template_data, preserve_trailing_newlines=True, @@ -116,6 +116,6 @@ class LookupModule(LookupBase): raise AnsibleError("the template file %s could not be found for the lookup" % term) # restore old variables - self._templar.set_available_variables(old_vars) + self._templar.available_variables = old_vars return ret diff --git a/lib/ansible/plugins/lookup/vars.py b/lib/ansible/plugins/lookup/vars.py index a9dad34f248..73e55219d3e 100644 --- a/lib/ansible/plugins/lookup/vars.py +++ b/lib/ansible/plugins/lookup/vars.py @@ -66,7 +66,7 @@ class LookupModule(LookupBase): def run(self, terms, variables=None, **kwargs): if variables is not None: - self._templar.set_available_variables(variables) + self._templar.available_variables = variables myvars = getattr(self._templar, '_available_variables', {}) self.set_options(direct=kwargs) diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index 599d9edbd86..3dfce04d68c 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -451,7 +451,12 @@ class Templar: return jinja_exts - def set_available_variables(self, variables): + @property + def available_variables(self): + return self._available_variables + + @available_variables.setter + def available_variables(self, variables): ''' Sets the list of template variables this Templar instance will use to template things, so we don't have to pass them around between @@ -464,6 +469,13 @@ class Templar: self._available_variables = variables self._cached_result = {} + def set_available_variables(self, variables): + display.deprecated( + 'set_available_variables is being deprecated. Use "@available_variables.setter" instead.', + version='2.13' + ) + self.available_variables = variables + def template(self, variable, convert_bare=False, preserve_trailing_newlines=True, escape_backslashes=True, fail_on_undefined=None, overrides=None, convert_data=True, static_vars=None, cache=True, disable_lookups=False): ''' diff --git a/lib/ansible/template/vars.py b/lib/ansible/template/vars.py index e2c95b047cb..164cf1a1abe 100644 --- a/lib/ansible/template/vars.py +++ b/lib/ansible/template/vars.py @@ -60,7 +60,7 @@ class AnsibleJ2Vars(Mapping): self._locals[key] = val def __contains__(self, k): - if k in self._templar._available_variables: + if k in self._templar.available_variables: return True if k in self._locals: return True @@ -73,16 +73,16 @@ class AnsibleJ2Vars(Mapping): def __iter__(self): keys = set() - keys.update(self._templar._available_variables, self._locals, self._globals, *self._extras) + keys.update(self._templar.available_variables, self._locals, self._globals, *self._extras) return iter(keys) def __len__(self): keys = set() - keys.update(self._templar._available_variables, self._locals, self._globals, *self._extras) + keys.update(self._templar.available_variables, self._locals, self._globals, *self._extras) return len(keys) def __getitem__(self, varname): - if varname not in self._templar._available_variables: + if varname not in self._templar.available_variables: if varname in self._locals: return self._locals[varname] for i in self._extras: @@ -93,7 +93,7 @@ class AnsibleJ2Vars(Mapping): else: raise KeyError("undefined variable: %s" % varname) - variable = self._templar._available_variables[varname] + variable = self._templar.available_variables[varname] # HostVars is special, return it as-is, as is the special variable # 'vars', which contains the vars structure diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py index 50346e43e85..3d2733282f2 100644 --- a/lib/ansible/vars/manager.py +++ b/lib/ansible/vars/manager.py @@ -318,7 +318,7 @@ class VariableManager: # and magic vars so we can properly template the vars_files entries temp_vars = combine_vars(all_vars, self._extra_vars) temp_vars = combine_vars(temp_vars, magic_variables) - self._templar.set_available_variables(temp_vars) + self._templar.available_variables = temp_vars # we assume each item in the list is itself a list, as we # support "conditional includes" for vars_files, which mimics @@ -498,7 +498,7 @@ class VariableManager: # as we're fetching vars before post_validate has been called on # the task that has been passed in vars_copy = existing_variables.copy() - self._templar.set_available_variables(vars_copy) + self._templar.available_variables = vars_copy items = [] has_loop = True @@ -533,7 +533,7 @@ class VariableManager: if item is not None: vars_copy[item_var] = item - self._templar.set_available_variables(vars_copy) + self._templar.available_variables = vars_copy delegated_host_name = self._templar.template(task.delegate_to, fail_on_undefined=False) if delegated_host_name != task.delegate_to: cache_items = True diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 249561cce16..faa6c6e6391 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -214,7 +214,7 @@ class TestActionBase(unittest.TestCase): self.assertEqual(env_string, "FOO=foo") # test environment with a variable in it - templar.set_available_variables(variables=dict(the_var='bar')) + templar.available_variables = dict(the_var='bar') mock_task.environment = [dict(FOO='{{the_var}}')] env_string = action_base._compute_environment_string() self.assertEqual(env_string, "FOO=bar") diff --git a/test/units/template/test_templar.py b/test/units/template/test_templar.py index a57a0a655f0..02022ce7af3 100644 --- a/test/units/template/test_templar.py +++ b/test/units/template/test_templar.py @@ -217,11 +217,17 @@ class TestTemplarMisc(BaseTemplar, unittest.TestCase): # test with fail_on_undefined=False self.assertEqual(templar.template("{{bad_var}}", fail_on_undefined=False), "{{bad_var}}") - # test set_available_variables() - templar.set_available_variables(variables=dict(foo="bam")) + # test setting available_variables + templar.available_variables = dict(foo="bam") self.assertEqual(templar.template("{{foo}}"), "bam") - # variables must be a dict() for set_available_variables() - self.assertRaises(AssertionError, templar.set_available_variables, "foo=bam") + # variables must be a dict() for available_variables setter + # FIXME Use assertRaises() as a context manager (added in 2.7) once we do not run tests on Python 2.6 anymore. + try: + templar.available_variables = "foo=bam" + except AssertionError as e: + pass + except Exception: + self.fail(e) def test_templar_escape_backslashes(self): # Rule of thumb: If escape backslashes is True you should end up with