Kevin/eng 2022 investigate tpl values templates #33
Conversation
get the template dir from the file path
ISSUES: all templates in the file path are included in the rendered template,
need to update to only include the explicietly mentioned file
vekjja
requested review from
michaeljguarino,
floreks,
maciaszczykm,
zreigz and
a team
template/tpl.go
Outdated
| includePath := filepath.Join(baseDir, name) | ||
|
|
||
| // Read and parse the included template | ||
| includeContent, err := os.ReadFile(includePath) |
There was a problem hiding this comment.
is this a mirror of the implementation of include in helm? I feel like it might work differently
There was a problem hiding this comment.
My general guess is it should really just use https://cs.opensource.google/go/go/+/refs/tags/go1.22.3:src/text/template/template.go;l=188 and then call execute on that (the expectation would be the template is defined in the file itself and so would have been parsed by the text/template library and lookup-able)
There was a problem hiding this comment.
That makes sense, but I'm not sure that's the Helm definition of include
Go provides a way of including one template in another using a built-in template directive. However, the built-in function cannot be used in Go template pipelines.
To make it possible to include a template, and then perform an operation on that template's output, Helm has a special include function:
{{ include "toYaml" $value | indent 2 }}The above includes a template called toYaml, passes it $value, and then passes the output of that template to the indent function.
Because YAML ascribes significance to indentation levels and whitespace, this is one great way to include snippets of code, but handle indentation in a relevant context.
There was a problem hiding this comment.
It actually is, the confusion is because I believe helm needs to make a redefinition of that function to handle the fact they can be defined anywhere in the chart, not in-file. If you look at the implementation and how it's used, it references a named define block
There was a problem hiding this comment.
Okay, after further discussion with and these PR comments from @michaeljguarino, this Polly PR needs an update to not include the referenced template file but a template mentioned from a define directive within the current .tpl file being rendered.
Changes are coming henceforth.
…opposed to including the template file
Update
RenderTplto map the include function.This adds all values in the included .tpl file
The function was updated to accept the file path as a parameter as opposed to the byte data