From 501fc7a248c4ad09c2593a200b3fbbdff0e48c70 Mon Sep 17 00:00:00 2001 From: Zac Medico Date: Wed, 9 Aug 2017 15:50:53 -0700 Subject: [PATCH] template: fix KeyError: 'undefined variable: 0 (#27972) * template: fix KeyError: 'undefined variable: 0 For compatibility with the Context.get_all() implementation in jinja 2.9, make AnsibleJ2Vars implement collections.Mapping. Also, make AnsibleJ2Template.newcontext() handle dict type for the 'vars' parameter. See: https://github.com/pallets/jinja/commit/d67f0fd4cc2a4af08f51f4466150d49da7798729 Fixes: https://github.com/ansible/ansible/issues/20494 * add units/template/test_vars * intg tests for jinja-2.9 issues like 20494 test cases here are based on https://github.com/ansible/ansible/issues/20494#issue-202108318 --- lib/ansible/template/template.py | 9 ++- lib/ansible/template/vars.py | 14 ++++- .../targets/template/files/import_as.expected | 3 + .../files/import_as_with_context.expected | 2 + .../files/import_with_context.expected | 3 + .../targets/template/tasks/main.yml | 54 ++++++++++++++++ .../targets/template/templates/bar | 1 + .../targets/template/templates/import_as.j2 | 4 ++ .../templates/import_as_with_context.j2 | 3 + .../templates/import_as_with_context.js | 3 + .../template/templates/import_with_context.j2 | 4 ++ .../targets/template/templates/qux | 1 + test/units/template/test_vars.py | 61 +++++++++++++++++++ 13 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 test/integration/targets/template/files/import_as.expected create mode 100644 test/integration/targets/template/files/import_as_with_context.expected create mode 100644 test/integration/targets/template/files/import_with_context.expected create mode 100644 test/integration/targets/template/templates/bar create mode 100644 test/integration/targets/template/templates/import_as.j2 create mode 100644 test/integration/targets/template/templates/import_as_with_context.j2 create mode 100644 test/integration/targets/template/templates/import_as_with_context.js create mode 100644 test/integration/targets/template/templates/import_with_context.j2 create mode 100644 test/integration/targets/template/templates/qux diff --git a/lib/ansible/template/template.py b/lib/ansible/template/template.py index 50bf2a2dfaa..fe11471d6c8 100644 --- a/lib/ansible/template/template.py +++ b/lib/ansible/template/template.py @@ -33,4 +33,11 @@ class AnsibleJ2Template(jinja2.environment.Template): ''' def new_context(self, vars=None, shared=False, locals=None): - return self.environment.context_class(self.environment, vars.add_locals(locals), self.name, self.blocks) + if vars is not None: + if isinstance(vars, dict): + vars = vars.copy() + if locals is not None: + vars.update(locals) + else: + vars = vars.add_locals(locals) + return self.environment.context_class(self.environment, vars, self.name, self.blocks) diff --git a/lib/ansible/template/vars.py b/lib/ansible/template/vars.py index 854af5be950..86837a9a799 100644 --- a/lib/ansible/template/vars.py +++ b/lib/ansible/template/vars.py @@ -19,6 +19,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +from collections import Mapping + from jinja2.utils import missing from ansible.module_utils.six import iteritems @@ -28,7 +30,7 @@ from ansible.module_utils._text import to_native __all__ = ['AnsibleJ2Vars'] -class AnsibleJ2Vars: +class AnsibleJ2Vars(Mapping): ''' Helper class to template all variable content before jinja2 sees it. This is done by hijacking the variable storage that jinja2 uses, and overriding __contains__ @@ -69,6 +71,16 @@ class AnsibleJ2Vars: return True return False + def __iter__(self): + keys = set() + 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) + return len(keys) + def __getitem__(self, varname): if varname not in self._templar._available_variables: if varname in self._locals: diff --git a/test/integration/targets/template/files/import_as.expected b/test/integration/targets/template/files/import_as.expected new file mode 100644 index 00000000000..fc6ea0218d4 --- /dev/null +++ b/test/integration/targets/template/files/import_as.expected @@ -0,0 +1,3 @@ +hello world import as +WIBBLE +Goodbye diff --git a/test/integration/targets/template/files/import_as_with_context.expected b/test/integration/targets/template/files/import_as_with_context.expected new file mode 100644 index 00000000000..7099a47a9e9 --- /dev/null +++ b/test/integration/targets/template/files/import_as_with_context.expected @@ -0,0 +1,2 @@ +hello world as qux with context +WIBBLE diff --git a/test/integration/targets/template/files/import_with_context.expected b/test/integration/targets/template/files/import_with_context.expected new file mode 100644 index 00000000000..5323655a408 --- /dev/null +++ b/test/integration/targets/template/files/import_with_context.expected @@ -0,0 +1,3 @@ +hello world with context +WIBBLE +Goodbye diff --git a/test/integration/targets/template/tasks/main.yml b/test/integration/targets/template/tasks/main.yml index 2e3064306d3..4d5c4fa20e2 100644 --- a/test/integration/targets/template/tasks/main.yml +++ b/test/integration/targets/template/tasks/main.yml @@ -51,6 +51,60 @@ that: - "template_result.changed == true" +# test for import with context on jinja-2.9 See https://github.com/ansible/ansible/issues/20494 +- name: fill in a template using import with context ala issue 20494 + template: src=import_with_context.j2 dest={{output_dir}}/import_with_context.templated mode=0644 + register: template_result + +- name: copy known good import_with_context.expected into place + copy: src=import_with_context.expected dest={{output_dir}}/import_with_context.expected + +- name: compare templated file to known good import_with_context + shell: diff -uw {{output_dir}}/import_with_context.templated {{output_dir}}/import_with_context.expected + register: diff_result + +- name: verify templated import_with_context matches known good + assert: + that: + - 'diff_result.stdout == ""' + - "diff_result.rc == 0" + +# test for 'import as' on jinja-2.9 See https://github.com/ansible/ansible/issues/20494 +- name: fill in a template using import as ala fails2 case in issue 20494 + template: src=import_as.j2 dest={{output_dir}}/import_as.templated mode=0644 + register: import_as_template_result + +- name: copy known good import_as.expected into place + copy: src=import_as.expected dest={{output_dir}}/import_as.expected + +- name: compare templated file to known good import_as + shell: diff -uw {{output_dir}}/import_as.templated {{output_dir}}/import_as.expected + register: import_as_diff_result + +- name: verify templated import_as matches known good + assert: + that: + - 'import_as_diff_result.stdout == ""' + - "import_as_diff_result.rc == 0" + +# test for 'import as with context' on jinja-2.9 See https://github.com/ansible/ansible/issues/20494 +- name: fill in a template using import as with context ala fails2 case in issue 20494 + template: src=import_as_with_context.j2 dest={{output_dir}}/import_as_with_context.templated mode=0644 + register: import_as_with_context_template_result + +- name: copy known good import_as_with_context.expected into place + copy: src=import_as_with_context.expected dest={{output_dir}}/import_as_with_context.expected + +- name: compare templated file to known good import_as_with_context + shell: diff -uw {{output_dir}}/import_as_with_context.templated {{output_dir}}/import_as_with_context.expected + register: import_as_with_context_diff_result + +- name: verify templated import_as_with_context matches known good + assert: + that: + - 'import_as_with_context_diff_result.stdout == ""' + - "import_as_with_context_diff_result.rc == 0" + # VERIFY CONTENTS - name: check what python version ansible is running on diff --git a/test/integration/targets/template/templates/bar b/test/integration/targets/template/templates/bar new file mode 100644 index 00000000000..2b60207f037 --- /dev/null +++ b/test/integration/targets/template/templates/bar @@ -0,0 +1 @@ +Goodbye diff --git a/test/integration/targets/template/templates/import_as.j2 b/test/integration/targets/template/templates/import_as.j2 new file mode 100644 index 00000000000..b06f1be8497 --- /dev/null +++ b/test/integration/targets/template/templates/import_as.j2 @@ -0,0 +1,4 @@ +{% import 'qux' as qux %} +hello world import as +{{ qux.wibble }} +{% include 'bar' %} diff --git a/test/integration/targets/template/templates/import_as_with_context.j2 b/test/integration/targets/template/templates/import_as_with_context.j2 new file mode 100644 index 00000000000..3dd806a35b4 --- /dev/null +++ b/test/integration/targets/template/templates/import_as_with_context.j2 @@ -0,0 +1,3 @@ +{% import 'qux' as qux with context %} +hello world as qux with context +{{ qux.wibble }} diff --git a/test/integration/targets/template/templates/import_as_with_context.js b/test/integration/targets/template/templates/import_as_with_context.js new file mode 100644 index 00000000000..3dd806a35b4 --- /dev/null +++ b/test/integration/targets/template/templates/import_as_with_context.js @@ -0,0 +1,3 @@ +{% import 'qux' as qux with context %} +hello world as qux with context +{{ qux.wibble }} diff --git a/test/integration/targets/template/templates/import_with_context.j2 b/test/integration/targets/template/templates/import_with_context.j2 new file mode 100644 index 00000000000..104e68b3479 --- /dev/null +++ b/test/integration/targets/template/templates/import_with_context.j2 @@ -0,0 +1,4 @@ +{% import 'qux' as qux with context %} +hello world with context +{{ qux.wibble }} +{% include 'bar' %} diff --git a/test/integration/targets/template/templates/qux b/test/integration/targets/template/templates/qux new file mode 100644 index 00000000000..d8cd22e41a8 --- /dev/null +++ b/test/integration/targets/template/templates/qux @@ -0,0 +1 @@ +{% set wibble = "WIBBLE" %} diff --git a/test/units/template/test_vars.py b/test/units/template/test_vars.py index ae8ccff5952..42945ba42a4 100644 --- a/test/units/template/test_vars.py +++ b/test/units/template/test_vars.py @@ -18,3 +18,64 @@ # Make coding more python3-ish from __future__ import (absolute_import, division, print_function) __metaclass__ = type + +from ansible.compat.tests import unittest +from ansible.compat.tests.mock import MagicMock + +from ansible.template.vars import AnsibleJ2Vars + + +class TestVars(unittest.TestCase): + def setUp(self): + self.mock_templar = MagicMock(name='mock_templar') + + def test(self): + ajvars = AnsibleJ2Vars(None, None) + print(ajvars) + + def test_globals_empty_2_8(self): + ajvars = AnsibleJ2Vars(self.mock_templar, {}) + res28 = self._dict_jinja28(ajvars) + self.assertIsInstance(res28, dict) + + def test_globals_empty_2_9(self): + ajvars = AnsibleJ2Vars(self.mock_templar, {}) + res29 = self._dict_jinja29(ajvars) + self.assertIsInstance(res29, dict) + + def _assert_globals(self, res): + self.assertIsInstance(res, dict) + self.assertIn('foo', res) + self.assertEqual(res['foo'], 'bar') + + def test_globals_2_8(self): + ajvars = AnsibleJ2Vars(self.mock_templar, {'foo': 'bar', 'blip': [1, 2, 3]}) + res28 = self._dict_jinja28(ajvars) + self._assert_globals(res28) + + def test_globals_2_9(self): + ajvars = AnsibleJ2Vars(self.mock_templar, {'foo': 'bar', 'blip': [1, 2, 3]}) + res29 = self._dict_jinja29(ajvars) + self._assert_globals(res29) + + def _dicts(self, ajvars): + print(ajvars) + res28 = self._dict_jinja28(ajvars) + res29 = self._dict_jinja29(ajvars) + # res28_other = self._dict_jinja28(ajvars, {'other_key': 'other_value'}) + # other = {'other_key': 'other_value'} + # res29_other = self._dict_jinja29(ajvars, *other) + print('res28: %s' % res28) + print('res29: %s' % res29) + # print('res28_other: %s' % res28_other) + # print('res29_other: %s' % res29_other) + # return (res28, res29, res28_other, res29_other) + # assert ajvars == res28 + # assert ajvars == res29 + return (res28, res29) + + def _dict_jinja28(self, *args, **kwargs): + return dict(*args, **kwargs) + + def _dict_jinja29(self, the_vars): + return dict(the_vars)