committing local progress towards finalizing for MP

cloudinit/cmd/devel/render.py

@@ -103,15 +103,11 @@ def render_template(user_data_path, instance_data_path=None, debug=False):
         )
         return 1
     except JinjaSyntaxParsingException as e:
-        error_msg = (
-            "Failed to render user-data file '{file_path}' "
-            "due to jinja parsing error: {error}".format(
-                file_path=user_data_path,
-                error=str(e),
-            )
+        LOG.error(
+            "Failed to render templated user-data file '%s'. %s",
+            user_data_path,
+            str(e),
         )
-        LOG.error(error_msg)
-        # sys.stderr.write(error_msg + "\n")
         return 1
     if not rendered_payload:
         LOG.error("Unable to render user-data file: %s", user_data_path)

cloudinit/cmd/query.py

@@ -35,7 +35,6 @@
 NAME = "query"
 LOG = logging.getLogger(__name__)
 
-
 def get_parser(parser=None):
     """Build or extend an arg parser for query utility.
 
@@ -289,10 +288,10 @@ def handle_args(name, args):
             )
         except JinjaSyntaxParsingException as e:
             LOG.error(
-                "Failed to render templated data due to jinja parsing error: "
-                "{error}",
+                "Failed to render templated data. %s",
                 str(e),
             )
+            print("Failed to render templated data. {}".format(str(e)))
             return 1
         if rendered_payload:
             print(rendered_payload)

cloudinit/config/cc_final_message.py

@@ -96,6 +96,10 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:
             stderr=True,
             log=LOG,
         )
+    except templater.JinjaSyntaxParsingException as e:
+        util.logexc(
+            LOG, "Failed to render templated final message: %s", str(e)
+        )
     except Exception:
         util.logexc(LOG, "Failed to render final message template")
 

cloudinit/config/schema.py

@@ -815,6 +815,8 @@ def _get_config_type_and_rendered_userdata(
     :return: UserDataTypeAndDecodedContent
     :raises: SchemaValidationError when non-jinja content found but
         header declared ## template: jinja.
+    :raises JinjaSyntaxParsingException when jinja syntax error found.
+    :raises JinjaLoadError when jinja template fails to load.
     """
     from cloudinit.handlers.jinja_template import (
         JinjaLoadError,
@@ -842,8 +844,7 @@ def _get_config_type_and_rendered_userdata(
             ) from e
         except JinjaSyntaxParsingException as e:
             error(
-                "Failed to render templated cloud-config due to jinja parsing "
-                "error: " + str(e),
+                "Failed to render templated user-data. " + str(e),
                 sys_exit=True,
             )
         except JinjaLoadError as e:

cloudinit/handlers/jinja_template.py

@@ -61,7 +61,7 @@ def handle_part(self, data, ctype, filename, payload, frequency, headers):
             )
         except JinjaSyntaxParsingException as e:
             LOG.warning(
-                "Ignoring jinja template for %s. Failed to render due to jinja parsing error: %s",
+                "Ignoring jinja template for %s. Failed to render template. %s",
                 filename,
                 str(e),
             )

cloudinit/templater.py

@@ -48,6 +48,10 @@
 
 
 class JinjaSyntaxParsingException(Exception):
+    message_template = (
+        "Unable to parse Jinja template due to syntax error: "
+        "{syntax_error} on line {line_no}: {line_content}"
+    )
     pass
 
 
@@ -139,39 +143,14 @@ def jinja_render(content, params):
                 # so we just need to re-raise the original error
                 raise jtemplate_error
             except TemplateSyntaxError as template_syntax_error:
-                try:
-                    tb = "".join(
-                        traceback.format_tb(
-                            template_syntax_error.__traceback__
-                        )
+                ln = template_syntax_error.lineno
+                raise JinjaSyntaxParsingException(
+                    JinjaSyntaxParsingException.message_template.format(
+                        syntax_error=template_syntax_error.message,
+                        line_no=ln + 1,
+                        line_content=content.splitlines()[ln - 1].strip(),
                     )
-                    line_number_matches = re.findall(
-                        r'File "<unknown>", line (\d+)', tb
-                    )
-                    line_number_of_error = int(line_number_matches[0])
-                    # adjust line number to adjust for the jinja header having been
-                    # removed from the content
-                    if (
-                        len(
-                            re.findall(
-                                "##( )?template:( )?jinja",
-                                content.split("\n")[0].strip().lower(),
-                            )
-                        )
-                        == 0
-                    ):
-                        line_number_of_error += 1
-                    raise JinjaSyntaxParsingException(
-                        "{e} on line {line_no}".format(
-                            e=template_syntax_error,
-                            line_no=line_number_of_error,
-                        )
-                    ) from jtemplate_error
-
-                # if we couldn't parse the traceback for some reason,
-                # just re-raise the original error
-                except Exception:
-                    raise jtemplate_error
+                ) from jtemplate_error
             except Exception:
                 raise jtemplate_error
 

cloudinit/util.py

@@ -333,8 +333,7 @@ def read_conf(fname, *, instance_data_file=None) -> Dict:
             )
         except JinjaSyntaxParsingException as e:
             LOG.warning(
-                "Failed to render user-data file '%s' "
-                "due to jinja parsing error: %s",
+                "Failed to render templated yaml config file '%s'. %s",
                 fname,
                 str(e),
             )

tests/unittests/cmd/devel/test_render.py

@@ -8,6 +8,7 @@
 from cloudinit.helpers import Paths
 from cloudinit.util import ensure_dir, write_file
 from tests.unittests.helpers import mock, skipUnlessJinja
+from cloudinit.templater import JinjaSyntaxParsingException
 
 M_PATH = "cloudinit.cmd.devel.render."
 
@@ -157,6 +158,10 @@ def test_invalid_jinja_syntax(self, caplog, tmpdir):
         write_file(instance_data, '{"my-var": "jinja worked"}')
         assert render.render_template(user_data, instance_data, True) == 1
         assert (
-            "due to jinja parsing error: unexpected '}' on line 2"
+            JinjaSyntaxParsingException.message_template.format(
+                syntax_error="unexpected '}'",
+                line_no=2,
+                line_content="rendering: {{ my_var } }",
+            )
             in caplog.text
         )

tests/unittests/cmd/test_query.py

@@ -17,6 +17,7 @@
 from cloudinit.sources import REDACT_SENSITIVE_VALUE
 from cloudinit.util import write_file
 from tests.unittests.helpers import mock
+from cloudinit.templater import JinjaSyntaxParsingException
 
 M_PATH = "cloudinit.cmd.query."
 
@@ -33,7 +34,6 @@ def setup_mocks(mocker):
     mocker.patch("cloudinit.cmd.query.read_cfg_paths", return_value=Paths({}))
 
 
-@mock.patch(M_PATH + "add_log_handler_cli", lambda *args: "")
 class TestQuery:
     Args = namedtuple(
         "Args",
@@ -573,17 +573,23 @@ def test_handle_args_list_keys_errors_when_varname_is_not_a_dict(
             assert 1 == query.handle_args("anyname", args)
         assert expected_error in caplog.text
 
+    @pytest.mark.parametrize(
+        "header_included",
+        [ True, False ],
+    )
     def test_handle_args_formats_jinja_successfully(
-        self, caplog, tmpdir, capsys
+        self, caplog, tmpdir, capsys, header_included
     ):
-        """Raise an error when --list-keys and varname specify a non-list."""
+        """Test that rendering a jinja template works as expected."""
         instance_data = tmpdir.join("instance-data")
         instance_data.write(
             '{"v1": {"v1_1": "val1.1", "v1_2": "val1.2"}, "v2": '
             '{"v2_2": "val2.2"}, "top": "gun"}'
         )
-        format = "v1_1: {{ v1.v1_1 }}"
-        expected = "v1_1: val1.1\n"
+        header = "## template: jinja\n" if header_included else ""
+        format = header + "v1_1: {{ v1.v1_1 }}"
+        expected = header + "v1_1: val1.1\n"
+
         args = self.Args(
             debug=False,
             dump_all=False,
@@ -601,16 +607,20 @@ def test_handle_args_formats_jinja_successfully(
         assert expected == out
 
     def test_handle_args_invalid_jinja_exception(self, caplog, tmpdir, capsys):
-        """Raise an error when --list-keys and varname specify a non-list."""
+        """Raise an error when a jinja syntax error is encountered."""
         instance_data = tmpdir.join("instance-data")
         instance_data.write(
             '{"v1": {"v1_1": "val1.1", "v1_2": "val1.2"}, "v2": '
             '{"v2_2": "val2.2"}, "top": "gun"}'
         )
         format = "v1_1: {{ v1.v1_1 } }"
         expected_error = (
-            "Failed to render templated data due to jinja "
-            "parsing error: unexpected '}' on line 2\n"
+            "Failed to render templated data. " + 
+            JinjaSyntaxParsingException.message_template.format(
+                syntax_error="unexpected '}'", 
+                line_no=2,
+                line_content="v1_1: {{ v1.v1_1 } }"
+            )
         )
         args = self.Args(
             debug=False,
@@ -625,5 +635,39 @@ def test_handle_args_invalid_jinja_exception(self, caplog, tmpdir, capsys):
         with mock.patch("os.getuid") as m_getuid:
             m_getuid.return_value = 100
             assert 1 == query.handle_args("anyname", args)
-        out, _err = capsys.readouterr()
-        assert expected_error == out
+        out, err = capsys.readouterr()
+        assert expected_error in caplog.text
+        assert expected_error in err
+
+
+    # def test_handle_args_jinja_template_missing_header(self, caplog, tmpdir, capsys):
+    #     """Raise an error when --list-keys and varname specify a non-list."""
+    #     instance_data = tmpdir.join("instance-data")
+    #     instance_data.write(
+    #         '{"v1": {"v1_1": "val1.1", "v1_2": "val1.2"}, "v2": '
+    #         '{"v2_2": "val2.2"}, "top": "gun"}'
+    #     )
+    #     format = "v1_1: {{ v1.v1_1 }}"
+    #     expected_error = (
+    #         "Failed to render templated data. " + 
+    #         JinjaSyntaxParsingException.message_template.format(
+    #             syntax_error="unexpected '{'", 
+    #             line_no=2,
+    #             line_content="v1_1: {{ v1.v1_1 }}"
+    #         )
+    #     )
+    #     args = self.Args(
+    #         debug=False,
+    #         dump_all=False,
+    #         format=format,
+    #         instance_data=instance_data.strpath,
+    #         list_keys=False,
+    #         user_data="ud",
+    #         vendor_data="vd",
+    #         varname=None,
+    #     )
+    #     with mock.patch("os.getuid") as m_getuid:
+    #         m_getuid.return_value = 100
+    #         assert 1 == query.handle_args("anyname", args)
+    #     out, _err = capsys.readouterr()
+    #     assert expected_error == out

tests/unittests/config/test_schema.py

@@ -51,6 +51,8 @@
     skipUnlessJsonSchema,
     skipUnlessJsonSchemaVersionGreaterThan,
 )
+from cloudinit.templater import JinjaSyntaxParsingException
+
 
 M_PATH = "cloudinit.config.schema."
 DEPRECATED_LOG_LEVEL = 35
@@ -830,26 +832,32 @@ def test_validateconfig_file_raises_enhanced_jinja_error(
         self, annotate, tmpdir, mocker, capsys
     ):
         """ """
+        # will throw error because of space between last two }'s
+        invalid_jinja_template = "## template: jinja\na:b\nc:{{ d } }"
         mocker.patch("os.path.exists", return_value=True)
         mocker.patch(
             "cloudinit.util.load_file",
-            return_value='## template: jinja\n{"a": "{{c}}"',  # missing }
+            return_value=invalid_jinja_template,
         )
         mocker.patch(
             "cloudinit.handlers.jinja_template.load_file",
             return_value='{"c": "d"}',
         )
         config_file = tmpdir.join("my.yaml")
-        config_file.write("## template: jinja\na:b\nc:{{ d } }")
+        config_file.write(invalid_jinja_template)
         with pytest.raises(SystemExit) as context_manager:
             validate_cloudconfig_file(config_file.strpath, {}, annotate)
         assert 1 == context_manager.value.code
 
         _out, err = capsys.readouterr()
         expected = (
             "Error:\n"
-            "Failed to render templated cloud-config due to jinja parsing "
-            "error: unexpected '}' on line 3\n"
+            "Failed to render templated user-data: " + 
+            JinjaSyntaxParsingException.message_template.format(
+                syntax_error="unexpected '}'", 
+                line_no=3,
+                line_content="c:{{ d } }",
+            )
         )
         assert expected == err
 

tests/unittests/test_templating.py

@@ -168,7 +168,6 @@ def test_jinja_do_extension_render_to_string(self):
             expected_result,
         )
 
-    # give invalid jinja that has a `} }` and catch the error
     def test_jinja_invalid_syntax(self):
         """Make sure invalid jinja syntax is caught"""
         jinja_template = (
@@ -181,3 +180,7 @@ def test_jinja_invalid_syntax(self):
             self.add_header("jinja", jinja_template),
             {},
         )
+        with self.assertRaises(templater.JinjaSyntaxParsingException) as cm:
+            templater.render_string(
+                self.add_header("jinja", jinja_template), {}
+            )

tests/unittests/test_util.py

@@ -517,7 +517,10 @@ def test_read_conf_with_config_invalid_jinja_syntax(
             return_value='{"c": "d"}',
         )
         conf = util.read_conf("cfg_path", instance_data_file="vars_path")
-        assert "Failed to render user-data file" in caplog.text
+        assert (
+            "Failed to render templated yaml config file 'cfg_path'"
+            in caplog.text
+        )
         assert conf == {}
 
     @mock.patch(