-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Support Jinja imports with relative paths. Issue #13889 #47490
Changes from 4 commits
485e151
9409c73
13a364f
19e53fe
43ed9bc
bec2517
9dd9430
7a3bd63
cc22068
e6773db
80bb6e4
540bcde
1be2561
c1c8b36
a7e633e
a6a5441
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,28 +98,60 @@ def check_cache(self, template): | |
self.cached.append(template) | ||
|
||
def get_source(self, environment, template): | ||
# checks for relative '..' paths | ||
if '..' in template: | ||
log.warning( | ||
'Discarded template path \'%s\', relative paths are ' | ||
'prohibited', template | ||
) | ||
raise TemplateNotFound(template) | ||
''' | ||
Salt-specific loader to find imported jinja files. | ||
|
||
Jinja imports will be interpreted as originating from the top | ||
of each of the directories in the searchpath when the template | ||
name does not begin with './' or '../'. When a template name | ||
begins with './' or '../' then the import will be relative to | ||
the importing file. | ||
|
||
''' | ||
# FIXME: somewhere do seprataor replacement: '\\' => '/' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Translation of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dwoz @twangboy When using jinja imports on Windows, are Windows-specific path-separators supported? If so, are they the only ones supported, or are forward slashes also supported? I would assume both are supported, since they are in Python proper, but I don't want to assume without knowing for sure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use Windows separators in my Jinja. I don't even have to escape them. Jinja apparently does some escaping for you. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
_template = template | ||
if template.split('/', 1)[0] in ('..', '.'): | ||
is_relative = True | ||
else: | ||
is_relative = False | ||
# checks for relative '..' paths that step-out of file_roots | ||
if is_relative: | ||
# Starts with a relative path indicator | ||
|
||
if not environment or 'tpldir' not in environment.globals: | ||
log.warning( | ||
'Relative path "%s" cannot be resolved without an environment', | ||
template | ||
) | ||
print('Relative path "{0}" cannot be resolved without an environment'.format(template)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not be printing to the CLI here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well that's embarrassing - that should have been removed. |
||
raise TemplateNotFound | ||
base_path = environment.globals['tpldir'] | ||
_template = os.path.normpath('/'.join((base_path, _template))) | ||
if _template.split('/', 1)[0] == '..': | ||
log.warning( | ||
'Discarded template path "%s": attempts to' | ||
' ascend outside of file roots', template | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we have multiple backends, we can't assume people will be using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay. |
||
) | ||
raise TemplateNotFound(template) | ||
|
||
self.check_cache(template) | ||
self.check_cache(_template) | ||
|
||
if environment and template: | ||
tpldir = os.path.dirname(template).replace('\\', '/') | ||
tpldir = os.path.dirname(_template).replace('\\', '/') | ||
tplfile = _template | ||
if is_relative: | ||
tpldir = environment.globals.get('tpldir', tpldir) | ||
tplfile = template | ||
tpldata = { | ||
'tplfile': template, | ||
'tplfile': tplfile, | ||
'tpldir': '.' if tpldir == '' else tpldir, | ||
'tpldot': tpldir.replace('/', '.'), | ||
} | ||
environment.globals.update(tpldata) | ||
|
||
# pylint: disable=cell-var-from-loop | ||
for spath in self.searchpath: | ||
filepath = os.path.join(spath, template) | ||
filepath = os.path.join(spath, _template) | ||
try: | ||
with salt.utils.files.fopen(filepath, 'rb') as ifile: | ||
contents = ifile.read().decode(self.encoding) | ||
|
@@ -205,7 +237,7 @@ def skip_filter(data): | |
''' | ||
Suppress data output | ||
|
||
.. code-balock:: yaml | ||
.. code-block:: yaml | ||
|
||
{% my_string = "foo" %} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -332,7 +332,7 @@ def parse_args(self, args=None, values=None): | |
os.path.basename(fpath).startswith('test_'): | ||
self.options.name.append(fpath) | ||
continue | ||
self.exit(status=1, msg='\'{}\' is not a valid test module'.format(fpath)) | ||
self.exit(status=1, msg='\'{}\' is not a valid test module\n'.format(fpath)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you help me understand this change? I don't know what adding a newline here fixes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without the newline then the error message is not on a line by itself and the command prompt is left on the same line as the error message. |
||
|
||
print_header(u'', inline=True, width=self.options.output_columns) | ||
self.pre_execution_cleanup() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
FAILURE |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{% import '../../rescape' as xfail -%} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
{% from './rmacro' import rmacro with context -%} | ||
{{ rmacro('Hey') ~ rmacro(a|default('a'), b|default('b')) }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{% from '../macro' import mymacro with context %} | ||
{% macro rmacro(greeting, greetee='world') -%} | ||
{{ mymacro(greeting, greetee) }} | ||
{%- endmacro %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import behavior needs to be in the documentation. Where is the best location for it?