pass without exception if jinja parameter isn't found

[guzzijones]

If a jinja parameter doesn't exist as an input parameter then do not throw an exception and leave the value as a string parameter.
We have some input paramters like:

{ "test": "{{some_text_here}}"}

These cause stackstorm to attempt to resolve some_text_here as a variable since it has matching {{ and }}.

This pr would allow those through as normal strings if they cannot be resolved.

[CLAassistant]

All committers have signed the CLA.

[cognifloyd]

Maybe we could:

• collect a list of nodes (vars) that aren't valid as template vars
• if ANY of the vars DO exist, throw an error for the missing nodes.
• if NONE of the vars exist, then assume it is a string and ignore.

Otherwise, I'm concerned this could hide misspellings.

[guzzijones]

I don't understand:

collect a list of nodes (vars) that aren't valid as template vars

[guzzijones]

Most, if not all, my actions already have a jinja template parameter for {{config_context.var}}. So I don't think check if ALL are parameters would work in my case.

We have a lot of log files with {{...}} in the json as a string. These are unable to be processed by Stackstorm currently.
I actually don't understand the use case for putting {{...}} templating parameter evaluation inside stackstorm except for the case of config context or secrets from the KV store.

One can always just utilize a parameter in their python script.

[nzlosh]

I'd rather see a more deterministic method to processing (or not) template strings. For example, adding a new input data type raw_string, which will not attempt to process the input variable and treat it at as raw string until it is handed off to the workflow/action. string would remain unchanged and process strings against template detection.

[guzzijones]

It would be nice to get this merged in. We have a lot of logs with {{ strings in them that we feed into st2.

[guzzijones]

Can we get some eyes on this?

[cognifloyd]

Tree traversal logic is confusing for me. 😅 Here's a few more questions as I try to grok this logic.

[cognifloyd]

I can follow the logic in this PR, and it seems like it would be functional. So, I give a technical approval of the implementation.

I'm still hesitant on this change. I'm worried about backwards incompatibility and silently ignoring actual errors. I also see how st2 is currently raising false positive errors for strings that are not Jinja.

If other members of the TSC agree with this change, then I'll approve it.

[nzlosh]

I'd still prefer a deterministic solution to applying expression rendering to a parameter because I think this change is going to produce more confusion and loss of time for the general use case of typos or missing variables that silently pass through the input validation process.

Feeding raw logs into an action seems like an edge use case and comes with less control than human input. My initial thought was to suggest log entries are sanitised before calling the action but this suffers from altering logs which would prevent or hinder further processing.

Since a new data type raw_string is an insufficient solution, I suggest we should add a new parameter flag that enables/disables the expression renderer for all data types. The renderer flag would control input processing behaviour in a similar way to immutable or secret and gives users explicit control of rendering for any input variable data type. The flag would be of type boolean and default to true to maintain backward compatibility.

This would allow ingest of raw logs without matching jinja expressions and still provide correct input validation processing with jinja expression rendering for the general use case. The only use case that would not be handled by this would be when someone attempts to send a mixture of rendered and non-rendered jinja expressions in the same input. In the case of mixed expression patterns, post-input processing would be required to tease out the parts needed to be rendered verses non-rendered.

Thoughts?

[guzzijones]

I actually like the idea of the render flag.

…

On Wed, Nov 20, 2024 at 03:35 Carlos ***@***.***> wrote: I'd still prefer a deterministic solution to applying expression rendering to a parameter because I think this change is going to produce more confusion and loss of time for the general use case of typos or missing variables that silently pass through the input validation process. Feeding raw logs into an action seems like an edge use case and comes with less control than human input. My initial thought was to suggest log entries are sanitised before calling the action but this suffers from altering logs which would prevent or hinder further processing. Since a new data type raw_string is an insufficient solution, I suggest we should add a new parameter flag that enables/disables the expression renderer for all data types. The renderer flag would control input processing behaviour in a similar way to immutable or secret and gives users explicit control of rendering for any input variable data type. The flag would be of type boolean and default to true to maintain backward compatibility. This would allow ingest of raw logs without matching jinja expressions and still provide correct input validation processing with jinja expression rendering for the general use case. The only use case that would not be handled by this would be when someone attempts to send a mixture of rendered and non-rendered jinja expressions in the same input. In the case of mixed expression patterns, post-input processing would be required to tease out the parts needed to be rendered verses non-rendered. Thoughts? — Reply to this email directly, view it on GitHub <#6101 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACZ5TIITTNKHLST4RCQM3JL2BRCWVAVCNFSM6AAAAABA62FCUSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBXHA4DSOBRHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[guzzijones]

This PR is basically dead. I am going to close this and do a 'render flag' PR. @nzlosh @cognifloyd sound good?