From e32d60bbcd44ccc7761268c6d6f00db036db962b Mon Sep 17 00:00:00 2001
From: Matt Martz <matt@sivel.net>
Date: Tue, 25 Jun 2019 10:54:37 -0500
Subject: [PATCH] Add back _contains_vars method as maybe_template (#58290)

* Add back _contains_vars method as maybe_template. Fixes #58282

* Remove template guard in a few places

* maybe_template sounds like it might template something, rename to is_possibly_template

* Add tests for is_possibly_template
---
 lib/ansible/playbook/helpers.py         |  3 +--
 lib/ansible/playbook/role/definition.py |  3 +--
 lib/ansible/template/__init__.py        | 25 ++++++++++++++---
 test/units/template/test_templar.py     | 36 +++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py
index 881939f9188..85e17ba3e2a 100644
--- a/lib/ansible/playbook/helpers.py
+++ b/lib/ansible/playbook/helpers.py
@@ -351,8 +351,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
                     # template the role name now, if needed
                     all_vars = variable_manager.get_vars(play=play, task=ir)
                     templar = Templar(loader=loader, variables=all_vars)
-                    if templar.is_template(ir._role_name):
-                        ir._role_name = templar.template(ir._role_name)
+                    ir._role_name = templar.template(ir._role_name)
 
                     # uses compiled list from object
                     blocks, _ = ir.get_block_list(variable_manager=variable_manager, loader=loader)
diff --git a/lib/ansible/playbook/role/definition.py b/lib/ansible/playbook/role/definition.py
index fa4a1464e5e..02b15e49c2a 100644
--- a/lib/ansible/playbook/role/definition.py
+++ b/lib/ansible/playbook/role/definition.py
@@ -129,8 +129,7 @@ class RoleDefinition(Base, Conditional, Taggable, CollectionSearch):
         if self._variable_manager:
             all_vars = self._variable_manager.get_vars(play=self._play)
             templar = Templar(loader=self._loader, variables=all_vars)
-            if templar.is_template(role_name):
-                role_name = templar.template(role_name)
+            role_name = templar.template(role_name)
 
         return role_name
 
diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py
index 857d950f28c..c113075ab75 100644
--- a/lib/ansible/template/__init__.py
+++ b/lib/ansible/template/__init__.py
@@ -535,7 +535,7 @@ class Templar:
             if isinstance(variable, string_types):
                 result = variable
 
-                if self.is_template(variable):
+                if self.is_possibly_template(variable):
                     # Check to see if the string we are trying to render is just referencing a single
                     # var.  In this case we don't want to accidentally change the type of the variable
                     # to a string by using the jinja template renderer. We just want to pass it.
@@ -631,7 +631,7 @@ class Templar:
                 return variable
 
     def is_template(self, data):
-        ''' lets us know if data has a template'''
+        '''lets us know if data has a template'''
         if isinstance(data, string_types):
             return is_template(data, self.environment)
         elif isinstance(data, (list, tuple)):
@@ -644,7 +644,26 @@ class Templar:
                     return True
         return False
 
-    templatable = _contains_vars = is_template
+    templatable = is_template
+
+    def is_possibly_template(self, data):
+        '''Determines if a string looks like a template, by seeing if it
+        contains a jinja2 start delimiter. Does not guarantee that the string
+        is actually a template.
+
+        This is different than ``is_template`` which is more strict.
+        This method may return ``True`` on a string that is not templatable.
+
+        Useful when guarding passing a string for templating, but when
+        you want to allow the templating engine to make the final
+        assessment which may result in ``TemplateSyntaxError``.
+        '''
+        env = self.environment
+        if isinstance(data, string_types):
+            for marker in (env.block_start_string, env.variable_start_string, env.comment_start_string):
+                if marker in data:
+                    return True
+        return False
 
     def _convert_bare_variable(self, variable):
         '''
diff --git a/test/units/template/test_templar.py b/test/units/template/test_templar.py
index 8c74853942c..d96e43b7c16 100644
--- a/test/units/template/test_templar.py
+++ b/test/units/template/test_templar.py
@@ -104,6 +104,42 @@ class TestTemplarTemplate(BaseTemplar, unittest.TestCase):
         # self.assertEqual(res['{{ a_keyword }}'], "blip")
         print(res)
 
+    def test_is_possibly_template_true(self):
+        tests = [
+            '{{ foo }}',
+            '{% foo %}',
+            '{# foo #}',
+            '{# {{ foo }} #}',
+            '{# {{ nothing }} {# #}',
+            '{# {{ nothing }} {# #} #}',
+            '{% raw %}{{ foo }}{% endraw %}',
+            '{{',
+            '{%',
+            '{#',
+            '{% raw',
+        ]
+        for test in tests:
+            self.assertTrue(self.templar.is_possibly_template(test))
+
+    def test_is_possibly_template_false(self):
+        tests = [
+            '{',
+            '%',
+            '#',
+            'foo',
+            '}}',
+            '%}',
+            'raw %}',
+            '#}',
+        ]
+        for test in tests:
+            self.assertFalse(self.templar.is_possibly_template(test))
+
+    def test_is_possible_template(self):
+        """This test ensures that a broken template still gets templated"""
+        # Purposefully invalid jinja
+        self.assertRaises(AnsibleError, self.templar.template, '{{ foo|default(False)) }}')
+
     def test_is_template_true(self):
         tests = [
             '{{ foo }}',