SC-1603-jinja-enhancements

[a-dubs]

Proposed Commit Message

feat(jinja): better jinja feedback and error catching

When an error is encountered while trying to parse a jinja template, the traceback is automatically parsed to get the specific error and the line number that caused this error, and then this information is turned into a user-friendly single-line error message. 

The cloud-init runtime will log these error messages when jinja syntax errors are encountered in the cloud-config file. For the query, render, and schema commands, the nasty traceback is caught and a single line error message is given to the user instead.

Fixes GH-4360

PR Checklist:

• My code follows the process laid out in the documentation
• I have updated or added any unit tests accordingly
• I have updated or added any documentation accordingly

[aciba90]

This PR follows up the work from #4510.

[blackboxsw]

@a-dubs I think we determined in our discussion today that this unittest shouldn't be checking stderr setup because the query module no longer invokes setup_basic_logging directly to add a stderr stream handler to logging. This is now done up in cloudinit.cmd.main before calling this submodule gets invoked

Since the logging setup is outside of this scope of this unit, I'd suggest we have a supplemental test that asserts that main does the proper setup_basic_logging on all subcommands not in ('modules', 'init'). We already have separate unittest coverage that setup_basic_logging properly sets up a stderr handler here. So no need to add extra stderr validation if we minimally assert when setup_basic_logging is called below.

--- a/tests/unittests/test_cli.py
+++ b/tests/unittests/test_cli.py
@@ -2,6 +2,7 @@
 
 import contextlib
 import io
+import logging
 import os
 from collections import namedtuple
 
@@ -172,6 +173,45 @@ class TestCLI:
         for subcommand in expected_subcommands:
             assert subcommand in err
 
+    @pytest.mark.parametrize(
+        "subcommand,log_to_stderr,mocks",
+        (
+            ("init", False, [mock.patch("cloudinit.cmd.main.status_wrapper")]),
+            (
+                "modules",
+                False,
+                [mock.patch("cloudinit.cmd.main.status_wrapper")],
+            ),
+            (
+                "schema",
+                True,
+                [
+                    mock.patch(
+                        "cloudinit.stages.Init._read_cfg", return_value={}
+                    ),
+                    mock.patch("cloudinit.config.schema.handle_schema_args"),
+                ],
+            ),
+        ),
+    )
+    @mock.patch("cloudinit.cmd.main.setup_basic_logging")
+    def test_subcommands_log_to_stderr_via_setup_basic_logging(
+        self, setup_basic_logging, subcommand, log_to_stderr, mocks
+    ):
+        """setup_basic_logging is called for modules to use stderr
+
+        Subcommands with exception of 'init'  and 'modules' use
+        setup_basic_logging to direct logged errors to stderr.
+        """
+        with contextlib.ExitStack() as mockstack:
+            for mymock in mocks:
+                mockstack.enter_context(mymock)
+            self._call_main(["cloud-init", subcommand])
+        if log_to_stderr:
+            setup_basic_logging.assert_called_once_with(logging.WARNING)
+        else:
+            setup_basic_logging.assert_not_called()
+
     @pytest.mark.parametrize("subcommand", ["init", "modules"])
     @mock.patch("cloudinit.cmd.main.status_wrapper")
     def test_modules_subcommand_parser(self, m_status_wrapper, subcommand):

[a-dubs]

Patch has been applied. Thank you!

[blackboxsw]

@a-dubs thanks for this confirmed much better error messages on CLI tooling for bogus jinja template formatting instead of Tracebacks on the CLI. I have a couple of suggestions throughout for simpler error handling, more clear unittest value validation and possibly a better way to declare JinjaSyntaxParsingException.

Let me know what you think.

$ cat > bogus-jinja-fmt.tmpl <<EOF
{% if  platform  == 'gce' -%}
YEP I'm Gce
{% else -%}
NOPE I AM: {{ platform }}
{% endif %-}
EOF

$ PYTHONPATH=. python3 cloudinit/cmd/main.py query --format "$(cat bogus-jinja-fmt.tmpl)" -i md.json
2023-12-06 20:19:43,919 - query.py[ERROR]: Failed to render templated data. Unable to parse Jinja template due to syntax error: expected token 'end of statement block', got '%' on line 6: {% endif %-}

What I thnk is missing

[a-dubs]

This is ready for a re-review now.

@blackboxsw I did my best to take your suggested changes and modify them as I saw fit and not just blindly patch them in. Let me know if the changes I made are inline with what you were hoping to see implemented.

[blackboxsw]

A couple of other nits and questions. Thanks @a-dubs . I'll wrap up the review today.

[blackboxsw]

Thanks @a-dubs for the cleanups here. This looks good and shows simplier errors for all of our subcommands when invalid jinja templates are provided.