From d841a1b1d97fd0ac8633392c33c07a4a57b95fc5 Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Sat, 2 Mar 2019 21:09:03 -0800 Subject: [PATCH] fix(engine): make template rendering thread safe See https://github.com/helm/helm/pull/4828 Signed-off-by: Adam Reese --- pkg/engine/engine.go | 31 ++++++++++++++++++------------- pkg/engine/engine_test.go | 4 ++-- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 2d533fdff..f08d355c0 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -36,8 +36,7 @@ type Engine struct { funcMap template.FuncMap // If strict is enabled, template rendering will fail if a template references // a value that was not passed in. - Strict bool - currentTemplates map[string]renderable + Strict bool } // New creates a new Go template Engine instance. @@ -115,8 +114,7 @@ func FuncMap() template.FuncMap { func (e *Engine) Render(chrt *chart.Chart, values chartutil.Values) (map[string]string, error) { // Render the charts tmap := allTemplates(chrt, values) - e.currentTemplates = tmap - return e.render(chrt, tmap) + return e.render(tmap) } // renderable is an object that can be rendered. @@ -132,7 +130,7 @@ type renderable struct { // alterFuncMap takes the Engine's FuncMap and adds context-specific functions. // // 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. funcMap := make(template.FuncMap) for k, v := range e.funcMap { @@ -179,7 +177,7 @@ func (e *Engine) alterFuncMap(t *template.Template) template.FuncMap { templates := make(map[string]renderable) templates[templateName.(string)] = r - result, err := e.render(nil, templates) + result, err := e.renderWithReferences(templates, referenceTpls) if err != nil { return "", errors.Wrapf(err, "error during tpl function execution for %q", tpl) } @@ -190,7 +188,14 @@ func (e *Engine) alterFuncMap(t *template.Template) template.FuncMap { } // render takes a map of templates/values and renders them. -func (e *Engine) render(ch *chart.Chart, 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, referenceTpls map[string]renderable) (rendered map[string]string, err error) { + // 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 // have been parsed, we loop through again and execute every template. @@ -212,7 +217,7 @@ func (e *Engine) render(ch *chart.Chart, tpls map[string]renderable) (rendered m 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 // higher-level (in file system) templates over deeply nested templates. @@ -228,9 +233,9 @@ func (e *Engine) render(ch *chart.Chart, tpls map[string]renderable) (rendered m 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 - for fname, r := range e.currentTemplates { + for fname, r := range referenceTpls { if t.Lookup(fname) == nil { if _, err := t.New(fname).Funcs(funcMap).Parse(r.tpl); err != nil { return map[string]string{}, errors.Wrapf(err, "parse error in %q", fname) @@ -261,9 +266,9 @@ func (e *Engine) render(ch *chart.Chart, tpls map[string]renderable) (rendered m Data: []byte(strings.Replace(buf.String(), "", "", -1)), } rendered[file] = string(f.Data) - if ch != nil { - ch.Files = append(ch.Files, f) - } + // if ch != nil { + // ch.Files = append(ch.Files, f) + // } } return rendered, nil diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 0c02ef24f..20411935f 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -150,7 +150,7 @@ func TestRenderInternals(t *testing.T) { "three": {tpl: `{{template "two" dict "Value" "three"}}`, vals: vals}, } - out, err := e.render(nil, tpls) + out, err := e.render(tpls) if err != nil { t.Fatalf("Failed template rendering: %s", err) } @@ -183,7 +183,7 @@ func TestParallelRenderInternals(t *testing.T) { tt := fmt.Sprintf("expect-%d", i) v := chartutil.Values{"val": tt} tpls := map[string]renderable{fname: {tpl: `{{.val}}`, vals: v}} - out, err := e.render(nil, tpls) + out, err := e.render(tpls) if err != nil { t.Errorf("Failed to render %s: %s", tt, err) }