This is the important bit, we only support json templates at the moment, and ast.literal_eval make sure the string is a valid deserialised type, meaning a valid JSON object will turn out to be a valid dictionary, and true for other types. Without this evaluation, we would have (issue found) a rendered template containing invalid json because the boolean literal was a python literal (e.g '{ "key": True}') and not a JSON boolean literal (e.g '{ "key": true}').

Thanks for pointing out this key change here 👍

👀 This breaks in cases where a JSON template is requested but the template produces invalid JSON. It would be very helpful to have a parity test covering such a scenario.
I'm ok with doing this in a follow-up PR but it would be good to add a TODO or comment describing this limitation.

nit: It might be a bit cleaner to introduce a variable for the rendered vlt template instead of updating response._content inline.

The parity test is a good idea, but definitely on a follow-up PR. On AWS I'm sure this will end up as a 500 if the content type expected is application/json. In our case a 500 is guaranteed, the content of the exception might not be the same tho.

I understand that one could argue that Velocity templates could be considered language-independent. However, given Apache Velocity stating "Velocity is a Java-based template engine", it might still be useful to aim for parity with the Java-based reference implementation.

There, only true and false are accepted boolean values (see pointer in docs) and using True or False raises an exception (tested with https://github.com/inbravo/apache-velocity-examples). This behavior is independent of the output type (tested using text output, not json).

Hence, given the following template and variables:

template = """
{
#set( $myTrue = true )
"booleanKeyTrue": $booleanKeyTrue,
"booleanKeyFalse": $booleanKeyFalse,
"booleanLiteralTrue": true,
"booleanLiteralFalse": false,
#if( $booleanKeyTrue )
"conditionalOutcome": "PASSED"
#else
"conditionalOutcome": "FAILED"
#end
}
"""
variables = {
    "booleanKeyTrue": "true",
    "booleanKeyFalse": "false",
}

The suggested workaround with dumping JSON (json.dumps(ast.literal_eval(response._content))) produces the correct output for the content type JSON:

import json

from localstack.utils.aws.templating import VtlTemplate
rendered_template = VtlTemplate().render_vtl(template, variables)
print(rendered_template)
print(json.loads(rendered_template))

{
"booleanKeyTrue": true,
"booleanKeyFalse": false,
"booleanLiteralTrue": true,
"booleanLiteralFalse": false,
"conditionalOutcome": "PASSED"
}

Nevertheless one could still argue that the underlying airspeed templating output violates the (Java-based) Velocity protocol:

import airspeed
import json

t = airspeed.Template(template)
booleanKeyTrue = True
booleanKeyFalse = False
print(t.merge(locals()))

{
"booleanKeyTrue": True,
"booleanKeyFalse": False,
"booleanLiteralTrue": true,
"booleanLiteralFalse": false,
"conditionalOutcome": "PASSED"
}

It is a sensible assumption to expect a 500 but better write a parity test in a follow-up as I have experienced many surprises ...

Im not sure what is your point, there is a test https://github.com/localstack/localstack/pull/7557/files/826c89875cdb39a2d6593ad8d3984e30b33b7de7#diff-c1d667db96ab1afa746d61a5d2bbc4b9fb9cdf160d36110f4d41b16ec91c11abR254 that proves your point.

You can use velocity templates to render HTML, the implementation of velocity templates is Java, that's what that means. What we want is to render json in same cases, and that once deserialised is language specific, anything else is a string.

Speaking of, there is a unit test failing with valid json but fails to evaluate using ast.literal_evalwith an indentation error.

E         File "<unknown>", line 3
E           {
E       IndentationError: unexpected indent

I'll have to double check on this one.

Fixed, it doesn't support leading and trailing white-spaces.

Thanks for pointing out this key change here 👍

👀 This breaks in cases where a JSON template is requested but the template produces invalid JSON. It would be very helpful to have a parity test covering such a scenario.
I'm ok with doing this in a follow-up PR but it would be good to add a TODO or comment describing this limitation.

good json parsing addition 👍
(there is also a as_json option in render_vlt)

We should remove that option in the render_vtl template; the template should render whatever it is and make decisions outside the templating what is expected based on the content-type. There are a couple of usages, thus not being removed yet.

ok

nit: It might be a bit cleaner to introduce a variable for the rendered vlt template instead of updating response._content inline.