[Actions UI] Enable adding non quoted alert type variables in JSON body of action type

[YulNaumenko]

When user are adding a document for Index Action type, she should be able to has a valid JSON:

{
  "AlertInstanceId": "{{alertInstanceId}}",
  "CurrentLocation": {{context.currentLocation}},
  "EntriesNumber":  {{context.previousLocation}},
}

which after mustache transformation will be:

{
  "AlertInstanceId": "test-id",
  "CurrentLocation": 2344.23,
  "EntriesNumber":  2,
}

or for a Webhook:

Currently it is not possible because validation require to add a quotas around the variable.

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[pmuellr]

I had a slightly crazy thought on doing the JSON validation for these:

• get the text content as a string
• replace all the {{...}} and {{{...}}} with the string 0 (or maybe true)
• during that replacement, pad to the right so the replacement string is the exact same length as the thing being replaced
• do a JSON.parse() on that
• if it fails, the line/column numbers (if provided) should match the original source

During the template replacement, we'd want to avoid replacing the section/partial/etc directives, just replace the simple variables.

This seems like it might work to me, devil is in the details ...

[gmmorris]

I've been investigating what would be needed to support this.

On the UI side I found that it is pretty straight forward to allow an AlertType to hint to the framework what format a param might be (basically, any JSON valid type, so: 'boolean' | 'number' | 'string' | 'object'). Using this we can stub out the templating tokens with hard coded values (for example, replace {{alertId}} with any string, or replace a param with a number hint with some number) and try parsing the JSON.
This works fine and didn't require much of a change as we could default all existing params to be string and so we can incrementally add support for other types.

The problem is on the server side.
We currently store the documents to index as:

const ParamsSchema = schema.object({
  documents: schema.arrayOf(schema.recordOf(schema.string(), schema.any())),
});

This won't work for us anymore.
Say the user's input is:

{
  "someNumber": {{params.someNumber}},
  "key": "{{alertInstanceId}}"
}

We cannot parse this as actual JSON, so storing the esIndex's documents params a JSON object is no longer possible.
We can migrate the esIndex actions to stringify their documents, but that will require a major migration in 7.11.
Another option is to add support for either a JSON document or a string which we then parse in the action executor. This avoids the need for a migration but means we'll have some actions with objects and some with strings... not ideal either.

I believe we can address the requirement in the issue, but it's becoming a bit of a larger change than I originally thoughts, so thought I'd check in before continuing.

[pmuellr]

I agree we may need to consider making the es index documents parameter a string - we will actually have to make it something like the following, to allow "legacy" uses of a JSON structure, if we can't migrate them (migrating params in this way seems "hard", but perhaps it's possible).

documents: schema.oneOf(schema.string(), schema.arrayOf(schema.recordOf(schema.string(), schema.any()))),

We'd then change the UI to always send the string version.

We could try to enhance the params validator when it gets a string to do the "replace mustache vars with literals", but that might be hard - what parser would we use?

Alternatively, we can do the validation at execution time. At execution time, we'll still have the string, but the mustache variables will have been replaced with their values, so we can do a simple JSON.parse() of it to create the object to send to ES, and raise JSON validation errors at that point. Not great, but probably good enough.

[pmuellr]

On the UI side I found that it is pretty straight forward to allow an AlertType to hint to the framework what format a param might be (basically, any JSON valid type, so: 'boolean' | 'number' | 'string' | 'object'). Using this we can stub out the templating tokens with hard coded values (for example, replace {{alertId}} with any string, or replace a param with a number hint with some number) and try parsing the JSON.

We'd need typing on context variables as well, I think, to do this kind of replacement, or at least to do "type checking" of context variables as mustache variables. And then consider we have to support sections like the following, assuming person is falsy (imagine some falsy-ish context variable)

Shown.{{#person}}Never shown!{{/person}}

[pmuellr]

We should keep in mind this list of other mustache / action parameter issues: #79786

I just starting noodling around on this one: #79371 - it's going to get pretty hairy, as I think we're going to need to make (or perhaps "allow") action types to do per-parameter rendering of the mustache templates. But I think in terms of changing documents to a string, it doesn't make this problem any easier or harder.

[pmuellr]

And note we still have to figure out how to handle these things in the UI. Are we just going to turn off the JSON parsing / checking?

Once we have "in alert" action testing, I think actions are going to have to provide some "default context variables" to use for these tests. If we have that, then we could do the mustache replacement with those, and then do the JSON verification of that. Any resulting syntax errors might be confusing to look at, with default data provided, but guessing it won't be horribly confusing.

[pmuellr]

Also, what does watcher do?

[gmmorris]

I agree we may need to consider making the es index documents parameter a string - we will actually have to make it something like the following, to allow "legacy" uses of a JSON structure, if we can't migrate them (migrating params in this way seems "hard", but perhaps it's possible).

documents: schema.oneOf(schema.string(), schema.arrayOf(schema.recordOf(schema.string(), schema.any()))),

Yup, thats what I've done...

We'd then change the UI to always send the string version.

Yeah, it's means all new esIndex will be string while old ones will be objects, but we can handle that.

We could try to enhance the params validator when it gets a string to do the "replace mustache vars with literals", but that might be hard - what parser would we use?

Yeah, we have no way of knowing the potential vars there, that's the problem :/
I've been considering requiring the list of potential variables as part of creation, but that would be a breaking change... that said, we haven't commited to no breaking api changes until GA, so 🤷

Alternatively, we can do the validation at execution time. At execution time, we'll still have the string, but the mustache variables will have been replaced with their values, so we can do a simple JSON.parse() of it to create the object to send to ES, and raise JSON validation errors at that point. Not great, but probably good enough.

Exactly, that's been the direction I've been investigating... it works even if not the best,.. as you said - probably good enough.

[gmmorris]

And note we still have to figure out how to handle these things in the UI. Are we just going to turn off the JSON parsing / checking?

It's annoying... we can validate the JSON by replacing the tokens, but the UI will complain because it wants a valid JSON.
I'm hoping we can keep the JSON validation somehow but haven't found a way yet.

[gmmorris]

Also, what does watcher do?

I've looked at the "Simulate" section and they have the exact same limitation we currently have.
What they do allow though is a string input and the usage of {{toJSON}} to convert a string to JSON.

For example:

"actions" : {
  "create_github_issue" : {
    "webhook" : {
      "method" : "POST",
      "url" : "https://api.github.com/repos/<owner>/<repo>/issues",
      "body": "{{#toJson}}ctx.payload{{/toJson}}",
    }
  }
}

As we have a want a full UI experience we son't want to fall back to JSON input like this...

[pmuellr]

That {{#toJson}} stuff is ... interesting. I'm not sure our mustache implementation has support for extracting embedded bits like ctx.payload there or not - it's certainly an interesting approach. The implementation of mustache we're using is this one: https://github.com/janl/mustache.js

That support was added to watcher in PR elastic/elasticsearch#18856 , along with a similar join "function", meant to join stringified array elements.

[gmmorris]

During the Alerting team sync we decided to put this back in the backlog for the following reasons:

1. The reason we prioritised this in the first place for GA was to enable this for Webhooks, but turns out - as we don't validate Webhooks, this actually works as is.
2. Realising this same limitation exists in Watcher as well suggests this isn't as urgent as we might have thought and no customer has expressed the opposite. This can also be handled at ingestion if a customer really needs it.
3. The complexity revealed in this change comes at a cost of other higher priority GA related issues.

[mikecote]

Moving from 7.x - Candidates to 8.x - Candidates (Backlog) after the latest 7.x planning session.

[mikecote]

Closing due to lack of activity.