Skip to content

Commit

Permalink
Copy context dicts when rendering templates
Browse files Browse the repository at this point in the history
This avoid modifing `tmplpath` and `tmpldir` in-place which would effect
following renderings in the parent template. This would break e.g.
relative lookups as they would be relative to the override path of the
sub-template, not the actual parent template.

Unfortunatly, with yamlet not longer overriding tmpldir, using e.g.
`render` in jinja templates becomes difficult, as files would be loaded
relative to the original sls file, as `render` uses the context from
when it was injected (the sls file), and the path in not longer mutated
by tha yamlet !include handler.

This commit therefore removes the direct relative path handing from
`render`. Relative files must be included using `tmpldir`:

    {{ render(tmpldir + "/file") }}

This works as expected because `tmpldir` is taken from the copied
context injected by the yamlet renderer.
  • Loading branch information
jgraichen committed May 11, 2020
1 parent 730c72f commit baeaef2
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 26 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Fixed
- Fix mutation of context variables when rendering multiple files

## [1.3.0] - 2020-05-11
### Added
Expand Down
34 changes: 17 additions & 17 deletions salt_tower/pillar/tower.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,32 +246,32 @@ def _compile(
if default is None:
default = self._default_renderers

if context is None:
context = {}
ctx = {}

context["tmplpath"] = template
context["tmpldir"] = os.path.dirname(template)
context["minion_id"] = self.minion_id
context["pillar"] = self
context["tower"] = self
if context:
ctx.update(context)

def render(tmpl, renderer="text"):
if (tmpl.startswith("./") or tmpl.startswith("../")):
path = os.path.join(context["tmpldir"], tmpl)
elif not tmpl.startswith("/"):
path = os.path.join(context["basedir"], tmpl)
ctx["tmplpath"] = template
ctx["tmpldir"] = os.path.dirname(template)
ctx["minion_id"] = self.minion_id
ctx["pillar"] = self
ctx["tower"] = self

def render(path, renderer="text"):
if "basedir" in ctx:
path = os.path.join(ctx["basedir"], path)

file = os.path.abspath(path)

if not os.path.isfile(file):
raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), file)

return self._compile(file, renderer, None, None, context)
return self._compile(file, renderer, None, None, ctx)

context["render"] = render
ctx["render"] = render

kwargs["tower"] = context["tower"]
kwargs["minion_id"] = context["minion_id"]
kwargs["tower"] = ctx["tower"]
kwargs["minion_id"] = ctx["minion_id"]

try:
return salt.template.compile_template(
Expand All @@ -280,7 +280,7 @@ def render(tmpl, renderer="text"):
default=default,
blacklist=blacklist,
whitelist=whitelist,
context=context,
context=ctx,
**kwargs,
)
except Exception as err:
Expand Down
15 changes: 8 additions & 7 deletions salt_tower/renderers/yamlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"""
from __future__ import absolute_import

import copy
import io
import os
import six
Expand Down Expand Up @@ -70,21 +71,21 @@ def _read(self, source):
def _compile(self, source, default="jinja|yamlet", context=None):
source = self._resolve(source)

if context is None:
context = self.context
else:
context = dict(self.context, **context)
ctx = copy.copy(self.context)

if context:
ctx.update(context)

context["tmplpath"] = source
context["tmpldir"] = os.path.dirname(source)
ctx["tmplpath"] = source
ctx["tmpldir"] = os.path.dirname(source)

ret = salt.template.compile_template(
template=source,
renderers=self.renderers,
default=default,
blacklist=None,
whitelist=None,
context=context,
context=ctx,
)

if isinstance(ret, (six.StringIO, six.BytesIO, io.IOBase)):
Expand Down
7 changes: 5 additions & 2 deletions test/pillar/test_tower.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,9 @@ def test_render_func(env):


def test_render_func_relative(env):
"""
Relative paths must be specified using the `tmpldir` variable.
"""
env.setup({
'tower.sls':
'''
Expand All @@ -471,7 +474,7 @@ def test_render_func_relative(env):
'test/conf.j2':
'''
#! jinja | text strip
{{ render('./conf2.j2') }}
{{ render(tmpldir + '/conf2.j2') }}
''',
'test/conf2.j2':
'''
Expand All @@ -493,7 +496,7 @@ def test_render_func_in_top(env):
'test/conf.j2':
'''
#! jinja | text strip
{{ render('./conf2.j2') }}
{{ render(tmpldir + '/conf2.j2') }}
''',
'test/conf2.j2':
'''
Expand Down
17 changes: 17 additions & 0 deletions test/renderers/test_yamlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,20 @@ def fuubar():
return 'fuubar'

assert result(context={'fuubar': fuubar}) == {'key': 'fuubar'}


def test_include_does_not_mutate_context(env, result):
'''
The !include tag passes given ``context`` to downstream renderers.
'''
env.write('init.sls',
'''
key: !include template.sls
''')

env.write('template.sls', '')

context = {"key": "abc"}
result(context=context)

assert context == {"key": "abc"}

0 comments on commit baeaef2

Please sign in to comment.