fix(templating): return json, not go structs

[b-erdem]

Motivation

When you extract a value from json using jsonpath expressions it returns values properly, only for simple values. For instance when you pass the following JSON:
{"employees": [{"name": "Baris", "age":43},{"name": "Mo", "age": 42}, {"name": "Jai", "age" :44}]} and $.employees[0].name it'll return "Baris" correctly. But let's say if you want to return an item in the array, for instance with $.employees[0] query then you'll get map[age:43 name:Baris]. This is not correct, it returns internal representation of the value when returned value is not a simple value.

We're passing some complex values to each steps and some of the values returned from a jsonpath expression could be a complex object. So we face with issues and doing some workarounds to get this correctly. Instead I thought maybe it'd be useful to fix it directly here. With this PR we'll be able to get values correctly from jsonpath expressions. For instance $.employees[0] now would return: {"age":43,"name":"Baris"}

Modifications

The returned value is now passed to json.Marshal directly without formatting it. fmt.Sprintf("%v", result)) this actually causes getting internal representation of the value.

The rest of the code is making sure we're returning a correct json value/object, mainly related to quotes. The changes I did in this PR is to cover when the returned object is a complex object and it contains strings, so we'll have quotes inside of the value. Then we need to properly encode them.

Verification

• Added some unit tests.
• Added an example with a complex return value.
• Tested locally with some of our workflows.

Please let me know if there's anything missing or something I can improve more. Thanks!

[Joibel]

LGTM.

Thanks @b-erdem for your contribution.

[b-erdem]

Hello @agilgur5 I think I need another approval to get this merged. :)

[Joibel]

This is a breaking change from previous bad but usable behaviour. I am not sure whether this should be held until 3.6. If it goes in 3.5.6 it should be highlighted in the notes.

[agilgur5]

@Joibel but did the previous behavior ever make sense / was it ever useful? From a quick glance, it sounds like it would always cause an error, so then we could straightforwardly merge this as a patch and wouldn't even necessarily need notes as the previous behavior did not work.

I feel like there might have been a use for this behavior before but I couldn't think of one off the top of my head. Using deep, nested structures without a pre-parsing step was not a use-case I had used much though.

[Joibel]

The previous behaviour didn't result in an error, was deterministic and predictable (you got the go printf %v representation).

This could have been used, it contains the values you want, just not in the representation expected. I wouldn't be surprised if someone somewhere does rely on this, and so we should point out that we are changing it.