[Ingest] Support yaml variables in datasource

[nchaulet]

Summary

Resolve #64152

Support yaml value in stream template. (Needed for redis package for example)

This PR allow agent stream to contain yaml variables, and variables to contains ..

Added more tests to cover these scenarios.

The way yaml variables are processed is done in two step:

1. process handlebar template and replace yaml value by a placeholder
2. traverse the generated yaml and replace placeholders by yaml values.

Example redis key metrics

[elasticmachine]

Pinging @elastic/ingest-management (Team:Ingest Management)

[ruflin]

Can you elaborate on what you exactly mean by "traverse the generated yaml and replace placeholders by yaml values."

[nchaulet]

@ruflin Yes

let's say we have the following template

host: {{host}}
{{#if key.patterns}}
key.patterns: {{key.patterns}}
{{/if}}

and the following values

{
 host: { value: "http://127.0.0.1"},
  "key.patterns": { value: "\n- limit: 20\n  pattern: .*\n", type: "yaml"}
}

1. we are going to compile it with handlebar to

host: http://127.0.0.1
key.patterns: "{{key.patterns}}"

load this yaml as json

{ 
  "host": "http://127.0.0.1"
  "key.patterns": "{{key.patterns}}"
}

2. I am looking for each yaml variables ({{yaml variables key }} ) and replace it by the parsed yaml value

{ 
  "host": "http://127.0.0.1"
  "key.patterns": [
    { limit: 20, pattern: ".*" }
  ]
}

[ruflin]

@nchaulet I'm still missing on why in step 1 not all of the handlebar parts are replaced? Is the second step because of the . in the name?

[jfsiii]

@nchaulet We chose Handlebars to avoid all the safety/performance/etc issues with rolling our own templating. I'd like to find a way to work within Handlebars if possible. I think we can change either/both a) the object/keys we supply to handlebars b) adding/registering a yaml or other custom helper https://handlebarsjs.com/guide/#custom-helpers

[jfsiii]

This function went from returning a YAML string out to returning JS object. I'd like to keep some function that accepts a template string & variables and returns YAML. It doesn't need to this createStream function, but I'd like to keep a function which accepts the template string & variables and outputs the YAML. That way we don't lose the YAML output step and can do safeLoad if/when we want to convert it to JS.

[jfsiii]

If we changed createStream to accept a map of string -> any or unknown (vs current string -> string) I believe we could do this test like

test('Test nested', () => {
  const streamTemplate = `
host: {{host}}
{{#if key.patterns}}
key.patterns:{{key.patterns}}
{{/if}}
`;
  const vars = {
    host: 'http://127.0.0.1',
    key: {
      patterns: '\n  - limit: 20\n    pattern: .*',
    },
  };

  const output = createStream(vars, streamTemplate);

  expect(output).toBe(`
host: http://127.0.0.1
key.patterns:
  - limit: 20
    pattern: .*
`);
});

[nchaulet]

I could retranslate to a yaml after, but it's going to be stored as a JSON and send to the agent as a JSON, so if we can avoid one other round of conversion to yaml -> to json

[ruflin]

@jfsiii @nchaulet Good discussion in this PR, please keep it going. One thing I wonder is if there is an option to get this PR in quickly in some form to unblock the package development and follow up with the proper implementation?

[jfsiii]

@ruflin @nchaulet agreed on iterating. I think the example I posted works and will allow us to keep progressing without the safety/other issues.

If we adopt this we really need to test and/or get feedback from security before releasing. We're processing user input into JS. That's a risky operation and something I'd like to run through a system which has guards and tests instead of rolling our own.

[nchaulet]

@jfsiii Yes I would like to avoid to write logic that looks like a template engine too, but I think handlebar is very limited here and replacing a yaml value is more complex than just a string interpolation, you need to have the right indentation, (for some value we could consider escaping them too)

I now it's frequent to do it this way in Helm charts for example but I think this make template really hard to write.

- name: xxxxx-volume
   resources:
      {{- .Values.xxxxx.resources | toYaml | nindent 6 }}

[jfsiii]

@nchaulet handlebars is intentionally limited but extensible and iiuc still suitable for what we're doing here.

If you can supply some more examples/tests of inputs/outputs I am happy to look into it.

To @ruflin's request I guess we can merge this as-is, but I would like to open a ticket for security to review before any release.

[ruflin]

@jfsiii My hope is that we have a follow up PR directly after this one fixing it.

[nchaulet]

@jfsiii @ruflin if we go with a follow up PR we probably need to update the templates after, so it's not really unlocking us, I will prefer to have this figured before

[nchaulet]

@jfsiii for the security perspective, we are already loading yaml coming from user using safeLoad at multiple place.

[ruflin]

@nchaulet One comment on the indentation: My assumption would be that the indentation already has to be correct in the package. If it is not, the package has to be fixed. @mtojek Now wondering if the yaml syntax really allows this or magically removes some spaces ...

[nchaulet]

@ruflin @jfsiii if we stay with a solution that is purely handlebar the template will change from

host: {{host}}
{{#if key.patterns}}
key.patterns: {{key.patterns}}
{{/if}}

to

{{#if key.patterns}}
key.patterns: 
  {{ toYaml key.patterns 2 }}
{{/if}}

and we need to introduce a custom handlebar helper something like to toYaml(value, indent)

[jfsiii]

@nchaulet did you see #64459 (comment)?

Doesn't that keep the same template and use handlebars w/o a custom helper?

[nchaulet]

@jfsiii in you example you need to know the place where the variable is going to use to already have the indentation in the variable

[ruflin]

Couldn't it also be:

host: {{host}}
{{#if key.patterns}}
key.patterns:
{{key.patterns}}
{{/if}}

And now it is purely up to the package to make sure the variable in key.patterns has the correct indentation?

[jfsiii]

@nchaulet I took that from the example you posted

"​key.patterns​"​:​ { value​:​ ​"​\n​- limit: 20​\n​  pattern: .*​\n​"​, type​:​ ​"​yaml​"​}

If you have other examples, I can adapt.

Let's try to get together soon and talk about this.

[nchaulet]

@ruflin in you example this mean the variable can be used only one time as the variable will contains the indentation, and we need to add new config to the yaml variable to have the indentation inside

[ruflin]

Not sure I follow the part that it can be used only one time. But yes, as the user will provide some input here, he would also have to use the correct indentation in the editor.

[jfsiii]

🚀 Thanks for the token changes and restoring the escaping. That addresses my biggest concerns.

Any changes I can think of wouldn't affect the templates or ability to move the input to arbitrary positions in the map. We can do that in a later PR if it comes up.

Thanks again

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 4836c11

History

• 💔 Build #43851 failed d3933c0
• 💚 Build #43431 succeeded 792bf79
• 💔 Build #43191 failed 41ba84b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[jen-huang]

Not a blocker for this PR but future improvement: I think these services belong in /services/datasources.ts too rather than EPM as EPM services are registry & asset interactions. Might be good to make a separate directory for data source services as that single file is getting rather long.

[nchaulet]

Trying to resume discussions we had offline:

One confusion was coming from the fact that even if we dispaly a YAML to the user, we do not store in saved object or send the agent configuration to the agent as a YAML but as a JSON.

This solution allow to have this template working, and we do not need to change existing templates, and because we load yaml using safeLoad it should not be so insecure.

title: "example"
keys.patterns: {{key.patterns}}
key:
  description: "yaml value one level deep"
{{#if keys.patterns}}
  patterns: {{key.patterns}}
{{/if}}