Skip to content
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

Don't convert numbers and booleans to strings. #10465

Merged
merged 1 commit into from
Apr 11, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ Ansible Changes By Release
## 2.0 "TBD" - ACTIVE DEVELOPMENT

Major Changes:
big_ip modules now support turning off ssl certificate validation (use only for self signed)
- big_ip modules now support turning off ssl certificate validation (use only for self signed)

- template code now retains types for bools and Numbers instead of turning them into strings
- If you need the old behaviour, quote the value and it will get passed around as a string

New Modules:
cloudtrail
Expand Down
30 changes: 24 additions & 6 deletions lib/ansible/utils/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import pwd
import ast
import traceback
from numbers import Number

from ansible.utils.string_functions import count_newlines_from_end
from ansible.utils import to_bytes, to_unicode
Expand Down Expand Up @@ -81,6 +82,11 @@ class Flags:

FILTER_PLUGINS = None
_LISTRE = re.compile(r"(\w+)\[(\d+)\]")

# A regex for checking to see if a variable we're trying to
# expand is just a single variable name.
SINGLE_VAR = re.compile(r"^{{\s*(\w*)\s*}}$")

JINJA2_OVERRIDE = '#jinja2:'
JINJA2_ALLOWED_OVERRIDES = ['trim_blocks', 'lstrip_blocks', 'newline_sequence', 'keep_trailing_newline']

Expand Down Expand Up @@ -109,7 +115,6 @@ def lookup(name, *args, **kwargs):
def template(basedir, varname, templatevars, lookup_fatal=True, depth=0, expand_lists=True, convert_bare=False, fail_on_undefined=False, filter_fatal=True):
''' templates a data structure by traversing it and substituting for other data structures '''
from ansible import utils

try:
if convert_bare and isinstance(varname, basestring):
first_part = varname.split(".")[0].split("[")[0]
Expand All @@ -123,10 +128,13 @@ def template(basedir, varname, templatevars, lookup_fatal=True, depth=0, expand_
except errors.AnsibleError, e:
raise errors.AnsibleError("Failed to template %s: %s" % (varname, str(e)))

if (varname.startswith("{") and not varname.startswith("{{")) or varname.startswith("["):
eval_results = utils.safe_eval(varname, locals=templatevars, include_exceptions=True)
if eval_results[1] is None:
varname = eval_results[0]
# template_from_string may return non strings for the case where the var is just
# a reference to a single variable, so we should re_check before we do further evals
if isinstance(varname, basestring):
if (varname.startswith("{") and not varname.startswith("{{")) or varname.startswith("["):
eval_results = utils.safe_eval(varname, locals=templatevars, include_exceptions=True)
if eval_results[1] is None:
varname = eval_results[0]

return varname

Expand Down Expand Up @@ -323,10 +331,20 @@ def my_finalize(thing):

def template_from_string(basedir, data, vars, fail_on_undefined=False):
''' run a string through the (Jinja2) templating engine '''

try:
if type(data) == str:
data = unicode(data, 'utf-8')

# Check to see if the string we are trying to render is just referencing a single
# var. In this case we don't wont to accidentally change the type of the variable
# to a string by using the jinja template renderer. We just want to pass it.
only_one = SINGLE_VAR.match(data)
if only_one:
var_name = only_one.group(1)
if var_name in vars:
resolved_val = vars[var_name]
if isinstance(resolved_val, (bool, Number)):
return resolved_val

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to handle filters. Then again, filters are a hard problem because they could change the type of the variable intentionally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I intentionally am only handling the use case of wanting to reference vars in a more complex structure and retaining type. filters or other jinja operations would make it very hard to infer type correctly and seems more error prone without hooking deeper into Jinja's parsing logic.

def my_finalize(thing):
return thing if thing is not None else ''
Expand Down
7 changes: 7 additions & 0 deletions test/integration/roles/test_template/files/foo.txt
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@
templated_var_loaded

{
"bool": true,
"multi_part": "1Foo",
"number": 5,
"string_num": "5"
}
2 changes: 2 additions & 0 deletions test/integration/roles/test_template/templates/foo.j2
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
{{ templated_var }}

{{ templated_dict | to_nice_json }}
13 changes: 13 additions & 0 deletions test/integration/roles/test_template/vars/main.yml
Original file line number Diff line number Diff line change
@@ -1 +1,14 @@
templated_var: templated_var_loaded

number_var: 5
string_num: "5"
bool_var: true
part_1: 1
part_2: "Foo"

templated_dict:
number: "{{ number_var }}"
string_num: "{{ string_num }}"
bool: "{{ bool_var }}"
multi_part: "{{ part_1 }}{{ part_2 }}"

21 changes: 21 additions & 0 deletions v2/ansible/template/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,17 @@
from ansible.template.vars import AnsibleJ2Vars
from ansible.utils.debug import debug

from numbers import Number

__all__ = ['Templar']

# A regex for checking to see if a variable we're trying to
# expand is just a single variable name.
SINGLE_VAR = re.compile(r"^{{\s*(\w*)\s*}}$")

# Primitive Types which we don't want Jinja to convert to strings.
NON_TEMPLATED_TYPES = ( bool, Number )

JINJA2_OVERRIDE = '#jinja2:'
JINJA2_ALLOWED_OVERRIDES = ['trim_blocks', 'lstrip_blocks', 'newline_sequence', 'keep_trailing_newline']

Expand Down Expand Up @@ -125,6 +134,18 @@ def template(self, variable, convert_bare=False, preserve_trailing_newlines=Fals
if isinstance(variable, basestring):
result = variable
if self._contains_vars(variable):

# Check to see if the string we are trying to render is just referencing a single
# var. In this case we don't wont to accidentally change the type of the variable
# to a string by using the jinja template renderer. We just want to pass it.
only_one = SINGLE_VAR.match(variable)
if only_one:
var_name = only_one.group(1)
if var_name in self._available_vars:
resolved_val = self._available_vars[var_name]
if isinstance(resolved_val, NON_TEMPLATED_TYPES):
return resolved_val

result = self._do_template(variable, preserve_trailing_newlines=preserve_trailing_newlines)

# if this looks like a dictionary or list, convert it to such using the safe_eval method
Expand Down