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 <ian.howell0@gmail.com>

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 "<no value>" 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 {

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{
+	tplsMissingRequired = map[string]renderable{
-		"undefined_function": {tpl: `{{foo}}`, vals: vals},
+		"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"},