Commit 6635bff3 authored by Sean Eagan's avatar Sean Eagan Committed by Matthew Fisher
Browse files

fix(engine): Fix template rendering thread safety issue (#4828)


Remove the engine `currentTemplates` field which was shared state
across threads and thus not thread safe, and instead just pass these
reference templates as parameters down recursively.

Closes #4819

Signed-off-by: default avatarSean Eagan <sean.eagan@att.com>
Showing with 12 additions and 8 deletions
+12 -8
...@@ -38,8 +38,7 @@ type Engine struct { ...@@ -38,8 +38,7 @@ type Engine struct {
FuncMap template.FuncMap FuncMap template.FuncMap
// If strict is enabled, template rendering will fail if a template references // If strict is enabled, template rendering will fail if a template references
// a value that was not passed in. // a value that was not passed in.
Strict bool Strict bool
CurrentTemplates map[string]renderable
// In LintMode, some 'required' template values may be missing, so don't fail // In LintMode, some 'required' template values may be missing, so don't fail
LintMode bool LintMode bool
} }
...@@ -122,7 +121,6 @@ func FuncMap() template.FuncMap { ...@@ -122,7 +121,6 @@ func FuncMap() template.FuncMap {
func (e *Engine) Render(chrt *chart.Chart, values chartutil.Values) (map[string]string, error) { func (e *Engine) Render(chrt *chart.Chart, values chartutil.Values) (map[string]string, error) {
// Render the charts // Render the charts
tmap := allTemplates(chrt, values) tmap := allTemplates(chrt, values)
e.CurrentTemplates = tmap
return e.render(tmap) return e.render(tmap)
} }
...@@ -139,7 +137,7 @@ type renderable struct { ...@@ -139,7 +137,7 @@ type renderable struct {
// alterFuncMap takes the Engine's FuncMap and adds context-specific functions. // alterFuncMap takes the Engine's FuncMap and adds context-specific functions.
// //
// The resulting FuncMap is only valid for the passed-in template. // The resulting FuncMap is only valid for the passed-in template.
func (e *Engine) alterFuncMap(t *template.Template) template.FuncMap { func (e *Engine) alterFuncMap(t *template.Template, referenceTpls map[string]renderable) template.FuncMap {
// Clone the func map because we are adding context-specific functions. // Clone the func map because we are adding context-specific functions.
var funcMap template.FuncMap = map[string]interface{}{} var funcMap template.FuncMap = map[string]interface{}{}
for k, v := range e.FuncMap { for k, v := range e.FuncMap {
...@@ -198,7 +196,7 @@ func (e *Engine) alterFuncMap(t *template.Template) template.FuncMap { ...@@ -198,7 +196,7 @@ func (e *Engine) alterFuncMap(t *template.Template) template.FuncMap {
templates[templateName.(string)] = r templates[templateName.(string)] = r
result, err := e.render(templates) result, err := e.renderWithReferences(templates, referenceTpls)
if err != nil { if err != nil {
return "", fmt.Errorf("Error during tpl function execution for %q: %s", tpl, err.Error()) return "", fmt.Errorf("Error during tpl function execution for %q: %s", tpl, err.Error())
} }
...@@ -210,6 +208,12 @@ func (e *Engine) alterFuncMap(t *template.Template) template.FuncMap { ...@@ -210,6 +208,12 @@ func (e *Engine) alterFuncMap(t *template.Template) template.FuncMap {
// render takes a map of templates/values and renders them. // render takes a map of templates/values and renders them.
func (e *Engine) render(tpls map[string]renderable) (rendered map[string]string, err error) { func (e *Engine) render(tpls map[string]renderable) (rendered map[string]string, err error) {
return e.renderWithReferences(tpls, tpls)
}
// renderWithReferences takes a map of templates/values to render, and a map of
// templates which can be referenced within them.
func (e *Engine) renderWithReferences(tpls map[string]renderable, referenceTpls map[string]renderable) (rendered map[string]string, err error) {
// Basically, what we do here is start with an empty parent template and then // Basically, what we do here is start with an empty parent template and then
// build up a list of templates -- one for each file. Once all of the templates // build up a list of templates -- one for each file. Once all of the templates
// have been parsed, we loop through again and execute every template. // have been parsed, we loop through again and execute every template.
...@@ -231,7 +235,7 @@ func (e *Engine) render(tpls map[string]renderable) (rendered map[string]string, ...@@ -231,7 +235,7 @@ func (e *Engine) render(tpls map[string]renderable) (rendered map[string]string,
t.Option("missingkey=zero") t.Option("missingkey=zero")
} }
funcMap := e.alterFuncMap(t) funcMap := e.alterFuncMap(t, referenceTpls)
// We want to parse the templates in a predictable order. The order favors // We want to parse the templates in a predictable order. The order favors
// higher-level (in file system) templates over deeply nested templates. // higher-level (in file system) templates over deeply nested templates.
...@@ -248,9 +252,9 @@ func (e *Engine) render(tpls map[string]renderable) (rendered map[string]string, ...@@ -248,9 +252,9 @@ func (e *Engine) render(tpls map[string]renderable) (rendered map[string]string,
files = append(files, fname) files = append(files, fname)
} }
// Adding the engine's currentTemplates to the template context // Adding the reference templates to the template context
// so they can be referenced in the tpl function // so they can be referenced in the tpl function
for fname, r := range e.CurrentTemplates { for fname, r := range referenceTpls {
if t.Lookup(fname) == nil { if t.Lookup(fname) == nil {
t = t.New(fname).Funcs(funcMap) t = t.New(fname).Funcs(funcMap)
if _, err := t.Parse(r.tpl); err != nil { if _, err := t.Parse(r.tpl); err != nil {
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment