From 284bd980b67f2be59b989abcc30ed2549936f2a6 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Wed, 27 Aug 2025 04:52:18 -0600 Subject: [PATCH 1/6] fix: replace pkg/engine regular expressions with parser Signed-off-by: Terry Howe --- pkg/engine/engine.go | 131 ++++++++++++++++++++++++++++--------------- 1 file changed, 85 insertions(+), 46 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 6e47a0e39..b42fa3219 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -34,18 +34,6 @@ import ( chartutil "helm.sh/helm/v4/pkg/chart/v2/util" ) -// taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=141 -// > "template: %s: executing %q at <%s>: %s" -var execErrFmt = regexp.MustCompile(`^template: (?P(?U).+): executing (?P(?U).+) at (?P(?U).+): (?P(?U).+)(?P( template:.*)?)$`) - -// taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 -// > "template: %s: %s" -var execErrFmtWithoutTemplate = regexp.MustCompile(`^template: (?P(?U).+): (?P.*)(?P( template:.*)?)$`) - -// taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=191 -// > "template: no template %q associated with template %q" -var execErrNoTemplateAssociated = regexp.MustCompile(`^template: no template (?P.*) associated with template (?P(.*)?)$`) - // Engine is an implementation of the Helm rendering implementation for templates. type Engine struct { // If strict is enabled, template rendering will fail if a template references @@ -338,7 +326,7 @@ func cleanupParseError(filename string, err error) error { 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("parse error at (%s): %s", string(location), errMsg) + return fmt.Errorf("parse error at (%s): %s", location, errMsg) } type TraceableError struct { @@ -350,25 +338,97 @@ type TraceableError struct { func (t TraceableError) String() string { var errorString strings.Builder if t.location != "" { - fmt.Fprintf(&errorString, "%s\n ", t.location) + _, _ = fmt.Fprintf(&errorString, "%s\n ", t.location) } if t.executedFunction != "" { - fmt.Fprintf(&errorString, "%s\n ", t.executedFunction) + _, _ = fmt.Fprintf(&errorString, "%s\n ", t.executedFunction) } if t.message != "" { - fmt.Fprintf(&errorString, "%s\n", t.message) + _, _ = fmt.Fprintf(&errorString, "%s\n", t.message) } return errorString.String() } +// parseTemplateExecErrorString parses a template execution error string from text/template +// without using regular expressions. It returns a TraceableError and true if parsing succeeded. +func parseTemplateExecErrorString(s string) (TraceableError, bool) { + const prefix = "template: " + if !strings.HasPrefix(s, prefix) { + return TraceableError{}, false + } + remainder := s[len(prefix):] + + // Special case: "template: no template %q associated with template %q" + // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=191 + if strings.HasPrefix(remainder, "no template ") { + return TraceableError{message: s}, true + } + + // Executing form: ": executing \"\" at <>: [ template:...]" + // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=141 + if idx := strings.Index(remainder, ": executing "); idx != -1 { + templateName := remainder[:idx] + after := remainder[idx+len(": executing "):] + if len(after) == 0 || after[0] != '"' { + return TraceableError{}, false + } + // find closing quote for function name + endQuote := strings.IndexByte(after[1:], '"') + if endQuote == -1 { + return TraceableError{}, false + } + endQuote++ // account for offset we started at 1 + functionName := after[1:endQuote] + afterFunc := after[endQuote+1:] + + // expect: " at <" then location then ">: " then message + const atPrefix = " at <" + if !strings.HasPrefix(afterFunc, atPrefix) { + return TraceableError{}, false + } + afterAt := afterFunc[len(atPrefix):] + endLoc := strings.Index(afterAt, ">: ") + if endLoc == -1 { + return TraceableError{}, false + } + locationName := afterAt[:endLoc] + errMsg := afterAt[endLoc+len(">: "):] + + // trim chained next error starting with space + "template:" if present + if cut := strings.Index(errMsg, " template:"); cut != -1 { + errMsg = errMsg[:cut] + } + return TraceableError{ + location: templateName, + message: errMsg, + executedFunction: "executing \"" + functionName + "\" at <" + locationName + ">:", + }, true + } + + // Simple form: ": " + // Use LastIndex to avoid splitting colons within line:col info. + // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 + if sep := strings.LastIndex(remainder, ": "); sep != -1 { + templateName := remainder[:sep] + errMsg := remainder[sep+2:] + if cut := strings.Index(errMsg, " template:"); cut != -1 { + errMsg = errMsg[:cut] + } + return TraceableError{location: templateName, message: errMsg}, true + } + + return TraceableError{}, false +} + // reformatExecErrorMsg takes an error message for template rendering and formats it into a formatted // multi-line error string func reformatExecErrorMsg(filename string, err error) error { - // This function matches the error message against regex's for the text/template package. - // If the regex's can parse out details from that error message such as the line number, template it failed on, + // This function parses the error message produced by text/template package. + // If it can parse out details from that error message such as the line number, template it failed on, // and error description, then it will construct a new error that displays these details in a structured way. // If there are issues with parsing the error message, the err passed into the function should return instead. - if _, isExecError := err.(template.ExecError); !isExecError { + var execError template.ExecError + if !errors.As(err, &execError) { return err } @@ -384,45 +444,24 @@ func reformatExecErrorMsg(filename string, err error) error { parts := warnRegex.FindStringSubmatch(tokens[2]) if len(parts) >= 2 { - return fmt.Errorf("execution error at (%s): %s", string(location), parts[1]) + return fmt.Errorf("execution error at (%s): %s", location, parts[1]) } current := err - fileLocations := []TraceableError{} + var fileLocations []TraceableError for current != nil { - var traceable TraceableError - if matches := execErrFmt.FindStringSubmatch(current.Error()); matches != nil { - templateName := matches[execErrFmt.SubexpIndex("templateName")] - functionName := matches[execErrFmt.SubexpIndex("functionName")] - locationName := matches[execErrFmt.SubexpIndex("location")] - errMsg := matches[execErrFmt.SubexpIndex("errMsg")] - traceable = TraceableError{ - location: templateName, - message: errMsg, - executedFunction: "executing " + functionName + " at " + locationName + ":", - } - } else if matches := execErrFmtWithoutTemplate.FindStringSubmatch(current.Error()); matches != nil { - templateName := matches[execErrFmt.SubexpIndex("templateName")] - errMsg := matches[execErrFmt.SubexpIndex("errMsg")] - traceable = TraceableError{ - location: templateName, - message: errMsg, - } - } else if matches := execErrNoTemplateAssociated.FindStringSubmatch(current.Error()); matches != nil { - traceable = TraceableError{ - message: current.Error(), + if tr, ok := parseTemplateExecErrorString(current.Error()); ok { + if len(fileLocations) == 0 || fileLocations[len(fileLocations)-1] != tr { + fileLocations = append(fileLocations, tr) } } else { return err } - if len(fileLocations) == 0 || fileLocations[len(fileLocations)-1] != traceable { - fileLocations = append(fileLocations, traceable) - } current = errors.Unwrap(current) } var finalErrorString strings.Builder for _, fileLocation := range fileLocations { - fmt.Fprintf(&finalErrorString, "%s", fileLocation.String()) + _, _ = fmt.Fprintf(&finalErrorString, "%s", fileLocation.String()) } return errors.New(strings.TrimSpace(finalErrorString.String())) From b060911075950273c8d42c005dd5928d905c0ab0 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 6 Sep 2025 11:36:39 -0400 Subject: [PATCH 2/6] refactor: break out into functions and draft tests Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 61 +++++++++++++++++++++++++++++---------- pkg/engine/engine_test.go | 47 ++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 15 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index b42fa3219..a5532f73a 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -360,12 +360,56 @@ func parseTemplateExecErrorString(s string) (TraceableError, bool) { // Special case: "template: no template %q associated with template %q" // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=191 - if strings.HasPrefix(remainder, "no template ") { - return TraceableError{message: s}, true + traceableError, done := parseTemplateNoTemplateError(s, remainder) + if done { + return traceableError, done } // Executing form: ": executing \"\" at <>: [ template:...]" // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=141 + traceableError, done = parseTemplateExecutingAtErrorType(remainder) + if done { + return traceableError, done + } + + // Simple form: ": " + // Use LastIndex to avoid splitting colons within line:col info. + // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 + traceableError, done = parseTemplateSimpleErrorString(remainder) + if done { + return traceableError, done + } + + return TraceableError{}, false +} + +// Special case: "template: no template %q associated with template %q" +// Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=191 +func parseTemplateNoTemplateError(s string, remainder string) (TraceableError, bool) { + if strings.HasPrefix(remainder, "no template ") { + return TraceableError{message: s}, true + } + return TraceableError{}, false +} + +// Simple form: ": " +// Use LastIndex to avoid splitting colons within line:col info. +// Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 +func parseTemplateSimpleErrorString(remainder string) (TraceableError, bool) { + if sep := strings.LastIndex(remainder, ": "); sep != -1 { + templateName := remainder[:sep] + errMsg := remainder[sep+2:] + if cut := strings.Index(errMsg, " template:"); cut != -1 { + errMsg = errMsg[:cut] + } + return TraceableError{location: templateName, message: errMsg}, true + } + return TraceableError{}, false +} + +// Executing form: ": executing \"\" at <>: [ template:...]" +// Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=141 +func parseTemplateExecutingAtErrorType(remainder string) (TraceableError, bool) { if idx := strings.Index(remainder, ": executing "); idx != -1 { templateName := remainder[:idx] after := remainder[idx+len(": executing "):] @@ -404,19 +448,6 @@ func parseTemplateExecErrorString(s string) (TraceableError, bool) { executedFunction: "executing \"" + functionName + "\" at <" + locationName + ">:", }, true } - - // Simple form: ": " - // Use LastIndex to avoid splitting colons within line:col info. - // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 - if sep := strings.LastIndex(remainder, ": "); sep != -1 { - templateName := remainder[:sep] - errMsg := remainder[sep+2:] - if cut := strings.Index(errMsg, " template:"); cut != -1 { - errMsg = errMsg[:cut] - } - return TraceableError{location: templateName, message: errMsg}, true - } - return TraceableError{}, false } diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index f4228fbd7..3362fa37e 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -1428,3 +1428,50 @@ func TestRenderCustomTemplateFuncs(t *testing.T) { t.Errorf("Expected %q, got %q", expected, rendered) } } + +func TestTraceableError_SimpleForm(t *testing.T) { + testStrings := []string{ + "function_not_found/templates/secret.yaml: error calling include", + } + for _, errString := range testStrings { + trace, done := parseTemplateSimpleErrorString(errString) + if !done { + t.Errorf("Expected parse to pass but did not") + } + if trace.message != "error calling include" { + t.Errorf("Expected %q, got %q", errString, trace.message) + } + } +} +func TestTraceableError_ExecutingForm(t *testing.T) { + testStrings := [][]string{ + {"template: executing \"function_not_found/templates/secret.yaml\" at : ", "function_not_found/templates/secret.yaml"}, + {"template: executing \"name\" at : ", "name"}, + } + for _, errTuple := range testStrings { + errString := errTuple[0] + expectedLocation := errTuple[1] + trace, done := parseTemplateExecutingAtErrorType(errString) + if !done { + t.Errorf("Expected parse to pass but did not") + } + if trace.location != expectedLocation { + t.Errorf("Expected %q, got %q", expectedLocation, trace.location) + } + } +} + +func TestTraceableError_NoTemplateForm(t *testing.T) { + testStrings := []string{ + "no template \"common.names.get_name\" associated with template \"gotpl\"", + } + for _, errString := range testStrings { + trace, done := parseTemplateNoTemplateError(errString, "") + if !done { + t.Errorf("Expected parse to pass but did not") + } + if trace.message != errString { + t.Errorf("Expected %q, got %q", errString, trace.message) + } + } +} From 712cde46246588beb1fdf2e2119e4faf27a612e4 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sun, 7 Sep 2025 10:42:24 -0400 Subject: [PATCH 3/6] test: passes now Signed-off-by: Jesse Simpson --- pkg/engine/engine_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 3362fa37e..8dd8f18fe 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -1445,8 +1445,8 @@ func TestTraceableError_SimpleForm(t *testing.T) { } func TestTraceableError_ExecutingForm(t *testing.T) { testStrings := [][]string{ - {"template: executing \"function_not_found/templates/secret.yaml\" at : ", "function_not_found/templates/secret.yaml"}, - {"template: executing \"name\" at : ", "name"}, + {"function_not_found/templates/secret.yaml:6:11: executing \"function_not_found/templates/secret.yaml\" at : ", "function_not_found/templates/secret.yaml:6:11"}, + {"divide_by_zero/templates/secret.yaml:6:11: executing \"divide_by_zero/templates/secret.yaml\" at : ", "divide_by_zero/templates/secret.yaml:6:11"}, } for _, errTuple := range testStrings { errString := errTuple[0] @@ -1466,7 +1466,7 @@ func TestTraceableError_NoTemplateForm(t *testing.T) { "no template \"common.names.get_name\" associated with template \"gotpl\"", } for _, errString := range testStrings { - trace, done := parseTemplateNoTemplateError(errString, "") + trace, done := parseTemplateNoTemplateError(errString, errString) if !done { t.Errorf("Expected parse to pass but did not") } From 9112687a7e0f6fc85f5736dda48c263c30496795 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Fri, 12 Sep 2025 14:12:23 -0600 Subject: [PATCH 4/6] Update pkg/engine/engine.go Co-authored-by: George Jenkins Signed-off-by: Terry Howe --- pkg/engine/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index a5532f73a..cce3c10c7 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -362,7 +362,7 @@ func parseTemplateExecErrorString(s string) (TraceableError, bool) { // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=191 traceableError, done := parseTemplateNoTemplateError(s, remainder) if done { - return traceableError, done + return traceableError, true } // Executing form: ": executing \"\" at <>: [ template:...]" From 91a65234ac52e1b693cf469a85faa78af79c2163 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Fri, 12 Sep 2025 14:26:53 -0600 Subject: [PATCH 5/6] Update pkg/engine/engine.go Co-authored-by: George Jenkins Signed-off-by: Terry Howe --- pkg/engine/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index cce3c10c7..8272889d3 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -369,7 +369,7 @@ func parseTemplateExecErrorString(s string) (TraceableError, bool) { // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=141 traceableError, done = parseTemplateExecutingAtErrorType(remainder) if done { - return traceableError, done + return traceableError, true } // Simple form: ": " From b2870379c88f7bf9a51fe0aa3c1c6442a91cda68 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Fri, 12 Sep 2025 14:27:00 -0600 Subject: [PATCH 6/6] Update pkg/engine/engine.go Co-authored-by: George Jenkins Signed-off-by: Terry Howe --- pkg/engine/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 8272889d3..9cbff272a 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -377,7 +377,7 @@ func parseTemplateExecErrorString(s string) (TraceableError, bool) { // Matches https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 traceableError, done = parseTemplateSimpleErrorString(remainder) if done { - return traceableError, done + return traceableError, true } return TraceableError{}, false