From 31b940a61dea3ddc1cbd3a8b005a9273aedd9199 Mon Sep 17 00:00:00 2001 From: Ian Howell Date: Tue, 23 Jul 2019 11:33:03 -0500 Subject: [PATCH] fix(engine): Fix eating too many colons during template execution This fixes #6044, in which error parsing is greedily eating too many colons, preventing users from using colons in their warning messages to the `required` function Signed-off-by: Ian Howell --- pkg/engine/engine.go | 46 ++++++++++++++++++++++++++++++++------- pkg/engine/engine_test.go | 44 ++++++++++++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 62e6f1087..e96df79a2 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -21,6 +21,7 @@ import ( "log" "path" "path/filepath" + "regexp" "sort" "strings" "text/template" @@ -80,6 +81,12 @@ type renderable struct { basePath string } +var warnRegex = regexp.MustCompile(`HELM\[(.*)\]HELM`) + +func warnWrap(warn string) string { + return fmt.Sprintf("HELM[%s]HELM", warn) +} + // initFunMap creates the Engine's FuncMap and adds context-specific functions. func (e Engine) initFunMap(t *template.Template, referenceTpls map[string]renderable) { funcMap := funcMap() @@ -126,7 +133,7 @@ func (e Engine) initFunMap(t *template.Template, referenceTpls map[string]render log.Printf("[INFO] Missing required value: %s", warn) return "", nil } - return val, errors.Errorf(warn) + return val, errors.Errorf(warnWrap(warn)) } else if _, ok := val.(string); ok { if val == "" { if e.LintMode { @@ -134,7 +141,7 @@ func (e Engine) initFunMap(t *template.Template, referenceTpls map[string]render log.Printf("[INFO] Missing required value: %s", warn) return "", nil } - return val, errors.Errorf(warn) + return val, errors.Errorf(warnWrap(warn)) } } return val, nil @@ -181,7 +188,7 @@ func (e Engine) renderWithReferences(tpls, referenceTpls map[string]renderable) for _, filename := range keys { r := tpls[filename] if _, err := t.New(filename).Parse(r.tpl); err != nil { - return map[string]string{}, parseTemplateError(filename, err) + return map[string]string{}, cleanupParseError(filename, err) } } @@ -190,7 +197,7 @@ func (e Engine) renderWithReferences(tpls, referenceTpls map[string]renderable) for filename, r := range referenceTpls { if t.Lookup(filename) == nil { if _, err := t.New(filename).Parse(r.tpl); err != nil { - return map[string]string{}, parseTemplateError(filename, err) + return map[string]string{}, cleanupParseError(filename, err) } } } @@ -207,7 +214,7 @@ func (e Engine) renderWithReferences(tpls, referenceTpls map[string]renderable) vals["Template"] = chartutil.Values{"Name": filename, "BasePath": tpls[filename].basePath} var buf strings.Builder if err := t.ExecuteTemplate(&buf, filename, vals); err != nil { - return map[string]string{}, parseTemplateError(filename, err) + return map[string]string{}, cleanupExecError(filename, err) } // Work around the issue where Go will emit "" even if Options(missing=zero) @@ -223,18 +230,41 @@ func (e Engine) renderWithReferences(tpls, referenceTpls map[string]renderable) return rendered, nil } -func parseTemplateError(filename string, err error) error { +func cleanupParseError(filename string, err error) error { tokens := strings.Split(err.Error(), ": ") if len(tokens) == 1 { // This might happen if a non-templating error occurs - return fmt.Errorf("render error in (%s): %s", filename, err) + return fmt.Errorf("parse error in (%s): %s", filename, err) } // The first token is "template" // The second token is either "filename:lineno" or "filename:lineNo:columnNo" location := tokens[1] // The remaining tokens make up a stacktrace-like chain, ending with the relevant error errMsg := tokens[len(tokens)-1] - return fmt.Errorf("render error at (%s): %s", string(location), errMsg) + return fmt.Errorf("parse error at (%s): %s", string(location), errMsg) +} + +func cleanupExecError(filename string, err error) error { + if _, isExecError := err.(template.ExecError); !isExecError { + return err + } + + tokens := strings.SplitN(err.Error(), ": ", 3) + if len(tokens) != 3 { + // This might happen if a non-templating error occurs + return fmt.Errorf("execution error in (%s): %s", filename, err) + } + + // The first token is "template" + // The second token is either "filename:lineno" or "filename:lineNo:columnNo" + location := tokens[1] + + parts := warnRegex.FindStringSubmatch(tokens[2]) + if len(parts) >= 2 { + return fmt.Errorf("execution error at (%s): %s", string(location), parts[1]) + } + + return err } func sortTemplates(tpls map[string]renderable) []string { diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index c557d0285..d55f6ae3e 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -187,7 +187,23 @@ func TestParallelRenderInternals(t *testing.T) { wg.Wait() } -func TestRenderErrors(t *testing.T) { +func TestParseErrors(t *testing.T) { + vals := chartutil.Values{"Values": map[string]interface{}{}} + + tplsUndefinedFunction := map[string]renderable{ + "undefined_function": {tpl: `{{foo}}`, vals: vals}, + } + _, err := new(Engine).render(tplsUndefinedFunction) + if err == nil { + t.Fatalf("Expected failures while rendering: %s", err) + } + expected := `parse error at (undefined_function:1): function "foo" not defined` + if err.Error() != expected { + t.Errorf("Expected '%s', got %q", expected, err.Error()) + } +} + +func TestExecErrors(t *testing.T) { vals := chartutil.Values{"Values": map[string]interface{}{}} tplsMissingRequired := map[string]renderable{ @@ -197,23 +213,39 @@ func TestRenderErrors(t *testing.T) { if err == nil { t.Fatalf("Expected failures while rendering: %s", err) } - expected := `render error at (missing_required:1:2): foo is required` + expected := `execution error at (missing_required:1:2): foo is required` if err.Error() != expected { t.Errorf("Expected '%s', got %q", expected, err.Error()) } - tplsUndefinedFunction := map[string]renderable{ - "undefined_function": {tpl: `{{foo}}`, vals: vals}, + tplsMissingRequired = map[string]renderable{ + "missing_required_with_colons": {tpl: `{{required ":this: message: has many: colons:" .Values.foo}}`, vals: vals}, + } + _, err = new(Engine).render(tplsMissingRequired) + if err == nil { + t.Fatalf("Expected failures while rendering: %s", err) } - _, err = new(Engine).render(tplsUndefinedFunction) + expected = `execution error at (missing_required_with_colons:1:2): :this: message: has many: colons:` + if err.Error() != expected { + t.Errorf("Expected '%s', got %q", expected, err.Error()) + } + + issue6044tpl := `{{ $someEmptyValue := "" }} +{{ $myvar := "abc" }} +{{- required (printf "%s: something is missing" $myvar) $someEmptyValue | repeat 0 }}` + tplsMissingRequired = map[string]renderable{ + "issue6044": {tpl: issue6044tpl, vals: vals}, + } + _, err = new(Engine).render(tplsMissingRequired) if err == nil { t.Fatalf("Expected failures while rendering: %s", err) } - expected = `render error at (undefined_function:1): function "foo" not defined` + expected = `execution error at (issue6044:3:4): abc: something is missing` if err.Error() != expected { t.Errorf("Expected '%s', got %q", expected, err.Error()) } } + func TestAllTemplates(t *testing.T) { ch1 := &chart.Chart{ Metadata: &chart.Metadata{Name: "ch1"},