From cdb7ab61a0f23f98e7c01fc46ecb47c3fd078aec Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 25 Oct 2019 09:51:57 -0500 Subject: [PATCH] Introduce context manager for temporary templar context changes (#60513) * Introduce context manager for temporary templar context changes. Fixes #60106 * Rename and docstring * Make set_temporary_context more generic, don't hardcode each thing you can set, apply to template action too * not None * linting fix * Ignore invalid attrs * Catch the right things, loop the right things * Use set_temporary_context in a few extra action plugins --- .../60106-templar-contextmanager.yml | 4 +++ lib/ansible/plugins/action/ce_template.py | 4 +-- lib/ansible/plugins/action/network.py | 4 +-- lib/ansible/plugins/action/template.py | 23 ++++---------- lib/ansible/plugins/lookup/template.py | 19 +++--------- lib/ansible/template/__init__.py | 31 +++++++++++++++++++ 6 files changed, 50 insertions(+), 35 deletions(-) create mode 100644 changelogs/fragments/60106-templar-contextmanager.yml diff --git a/changelogs/fragments/60106-templar-contextmanager.yml b/changelogs/fragments/60106-templar-contextmanager.yml new file mode 100644 index 0000000000..45afc1544a --- /dev/null +++ b/changelogs/fragments/60106-templar-contextmanager.yml @@ -0,0 +1,4 @@ +bugfixes: +- template lookup - ensure changes to the templar in the lookup, do not + affect the templar context outside of the lookup + (https://github.com/ansible/ansible/issues/60106) diff --git a/lib/ansible/plugins/action/ce_template.py b/lib/ansible/plugins/action/ce_template.py index 8d62b25647..4a72fbbfa8 100644 --- a/lib/ansible/plugins/action/ce_template.py +++ b/lib/ansible/plugins/action/ce_template.py @@ -100,5 +100,5 @@ class ActionModule(_ActionModule): for role in dep_chain: searchpath.append(role._role_path) searchpath.append(os.path.dirname(source)) - self._templar.environment.loader.searchpath = searchpath - self._task.args['src'] = self._templar.template(template_data) + with self._templar.set_temporary_context(searchpath=searchpath): + self._task.args['src'] = self._templar.template(template_data) diff --git a/lib/ansible/plugins/action/network.py b/lib/ansible/plugins/action/network.py index f0d0ca3ba7..d91c9b2af9 100644 --- a/lib/ansible/plugins/action/network.py +++ b/lib/ansible/plugins/action/network.py @@ -160,8 +160,8 @@ class ActionModule(_ActionModule): for role in dep_chain: searchpath.append(role._role_path) searchpath.append(os.path.dirname(source)) - self._templar.environment.loader.searchpath = searchpath - self._task.args['src'] = self._templar.template(template_data, convert_data=convert_data) + with self._templar.set_temporary_context(searchpath=searchpath): + self._task.args['src'] = self._templar.template(template_data, convert_data=convert_data) def _get_network_os(self, task_vars): if 'network_os' in self._task.args and self._task.args['network_os']: diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index 8fb7393ff9..f400d7b96e 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -127,27 +127,16 @@ class ActionModule(ActionBase): newsearchpath.append(p) searchpath = newsearchpath - self._templar.environment.loader.searchpath = searchpath - self._templar.environment.newline_sequence = newline_sequence - if block_start_string is not None: - self._templar.environment.block_start_string = block_start_string - if block_end_string is not None: - self._templar.environment.block_end_string = block_end_string - if variable_start_string is not None: - self._templar.environment.variable_start_string = variable_start_string - if variable_end_string is not None: - self._templar.environment.variable_end_string = variable_end_string - self._templar.environment.trim_blocks = trim_blocks - self._templar.environment.lstrip_blocks = lstrip_blocks - # add ansible 'template' vars temp_vars = task_vars.copy() temp_vars.update(generate_ansible_template_vars(source, dest)) - 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.available_variables = old_vars + with self._templar.set_temporary_context(searchpath=searchpath, newline_sequence=newline_sequence, + block_start_string=block_start_string, block_end_string=block_end_string, + variable_start_string=variable_start_string, variable_end_string=variable_end_string, + trim_blocks=trim_blocks, lstrip_blocks=lstrip_blocks, + available_variables=temp_vars): + resultant = self._templar.do_template(template_data, preserve_trailing_newlines=True, escape_backslashes=False) except AnsibleAction: raise except Exception as e: diff --git a/lib/ansible/plugins/lookup/template.py b/lib/ansible/plugins/lookup/template.py index 4fd3584b65..666fe91ecd 100644 --- a/lib/ansible/plugins/lookup/template.py +++ b/lib/ansible/plugins/lookup/template.py @@ -68,8 +68,6 @@ 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 - for term in terms: display.debug("File lookup term: %s" % term) @@ -92,12 +90,6 @@ class LookupModule(LookupBase): searchpath = newsearchpath searchpath.insert(0, os.path.dirname(lookupfile)) - self._templar.environment.loader.searchpath = searchpath - if variable_start_string is not None: - self._templar.environment.variable_start_string = variable_start_string - if variable_end_string is not None: - self._templar.environment.variable_end_string = variable_end_string - # The template will have access to all existing variables, # plus some added by ansible (e.g., template_{path,mtime}), # plus anything passed to the lookup with the template_vars= @@ -105,17 +97,16 @@ class LookupModule(LookupBase): vars = deepcopy(variables) vars.update(generate_ansible_template_vars(lookupfile)) vars.update(lookup_template_vars) - self._templar.available_variables = vars # do the templating - res = self._templar.template(template_data, preserve_trailing_newlines=True, - convert_data=convert_data_p, escape_backslashes=False) + with self._templar.set_temporary_context(variable_start_string=variable_start_string, + variable_end_string=variable_end_string, + available_variables=vars, searchpath=searchpath): + res = self._templar.template(template_data, preserve_trailing_newlines=True, + convert_data=convert_data_p, escape_backslashes=False) ret.append(res) else: raise AnsibleError("the template file %s could not be found for the lookup" % term) - # restore old variables - self._templar.available_variables = old_vars - return ret diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index fa4b11afe1..7189dbc6a2 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -27,6 +27,7 @@ import pwd import re import time +from contextlib import contextmanager from numbers import Number try: @@ -512,6 +513,36 @@ class Templar: ) self.available_variables = variables + @contextmanager + def set_temporary_context(self, **kwargs): + """Context manager used to set temporary templating context, without having to worry about resetting + original values afterward + + Use a keyword that maps to the attr you are setting. Applies to ``self.environment`` by default, to + set context on another object, it must be in ``mapping``. + """ + mapping = { + 'available_variables': self, + 'searchpath': self.environment.loader, + } + original = {} + + for key, value in kwargs.items(): + obj = mapping.get(key, self.environment) + try: + original[key] = getattr(obj, key) + if value is not None: + setattr(obj, key, value) + except AttributeError: + # Ignore invalid attrs, lstrip_blocks was added in jinja2==2.7 + pass + + yield + + for key in original: + obj = mapping.get(key, self.environment) + setattr(obj, key, original[key]) + 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): '''