From 79df3926f623e040ae50dd6d84a0bbd985c591af Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Tue, 10 Aug 2021 12:38:43 -0400
Subject: [PATCH] fix(engine): parse fail messages with newlines

The templating engine handles errors originating from the `required` and
`fail` template functions specially, cleaning up the error messages to
be more presentable to users. Go's text/template package unfortunately
does not make this straightforward to implement. Despite
template.ExecError implementing Unwrap, the error value returned from
the template function cannot be retrieved using errors.As. The wrapped
error in ExecError is a pre-formatted error string with the template
function's error string interpolated in with the original error value
erased. Helm works around this limitation by delimiting the
template-supplied message and extracting the message out of the
ExecError string with a regex.

Fix the parsing of `required` and `fail` error messages containing
newlines by setting the regex flag to make `.` match newline characters.

Signed-off-by: Cory Snider <csnider@mirantis.com>
---
 pkg/engine/engine.go      |  2 +-
 pkg/engine/engine_test.go | 93 ++++++++++++++++++++++++---------------
 2 files changed, 58 insertions(+), 37 deletions(-)

diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go
index 4da64f5cc..8ff42941a 100644
--- a/pkg/engine/engine.go
+++ b/pkg/engine/engine.go
@@ -97,7 +97,7 @@ const warnStartDelim = "HELM_ERR_START"
 const warnEndDelim = "HELM_ERR_END"
 const recursionMaxNums = 1000
 
-var warnRegex = regexp.MustCompile(warnStartDelim + `(.*)` + warnEndDelim)
+var warnRegex = regexp.MustCompile(warnStartDelim + `((?s).*)` + warnEndDelim)
 
 func warnWrap(warn string) string {
 	return warnStartDelim + warn + warnEndDelim
diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go
index 72ee02626..429511003 100644
--- a/pkg/engine/engine_test.go
+++ b/pkg/engine/engine_test.go
@@ -245,44 +245,65 @@ func TestParseErrors(t *testing.T) {
 
 func TestExecErrors(t *testing.T) {
 	vals := chartutil.Values{"Values": map[string]interface{}{}}
-
-	tplsMissingRequired := map[string]renderable{
-		"missing_required": {tpl: `{{required "foo is required" .Values.foo}}`, vals: vals},
-	}
-	_, err := new(Engine).render(tplsMissingRequired)
-	if err == nil {
-		t.Fatalf("Expected failures while rendering: %s", err)
-	}
-	expected := `execution error at (missing_required:1:2): foo is required`
-	if err.Error() != expected {
-		t.Errorf("Expected '%s', got %q", expected, err.Error())
-	}
-
-	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)
-	}
-	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 := "" }}
+	cases := []struct {
+		name     string
+		tpls     map[string]renderable
+		expected string
+	}{
+		{
+			name: "MissingRequired",
+			tpls: map[string]renderable{
+				"missing_required": {tpl: `{{required "foo is required" .Values.foo}}`, vals: vals},
+			},
+			expected: `execution error at (missing_required:1:2): foo is required`,
+		},
+		{
+			name: "MissingRequiredWithColons",
+			tpls: map[string]renderable{
+				"missing_required_with_colons": {tpl: `{{required ":this: message: has many: colons:" .Values.foo}}`, vals: vals},
+			},
+			expected: `execution error at (missing_required_with_colons:1:2): :this: message: has many: colons:`,
+		},
+		{
+			name: "Issue6044",
+			tpls: map[string]renderable{
+				"issue6044": {
+					vals: vals,
+					tpl: `{{ $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)
+{{- required (printf "%s: something is missing" $myvar) $someEmptyValue | repeat 0 }}`,
+				},
+			},
+			expected: `execution error at (issue6044:3:4): abc: something is missing`,
+		},
+		{
+			name: "MissingRequiredWithNewlines",
+			tpls: map[string]renderable{
+				"issue9981": {tpl: `{{required "foo is required\nmore info after the break" .Values.foo}}`, vals: vals},
+			},
+			expected: `execution error at (issue9981:1:2): foo is required
+more info after the break`,
+		},
+		{
+			name: "FailWithNewlines",
+			tpls: map[string]renderable{
+				"issue9981": {tpl: `{{fail "something is wrong\nlinebreak"}}`, vals: vals},
+			},
+			expected: `execution error at (issue9981:1:2): something is wrong
+linebreak`,
+		},
 	}
-	expected = `execution error at (issue6044:3:4): abc: something is missing`
-	if err.Error() != expected {
-		t.Errorf("Expected '%s', got %q", expected, err.Error())
+
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := new(Engine).render(tt.tpls)
+			if err == nil {
+				t.Fatalf("Expected failures while rendering: %s", err)
+			}
+			if err.Error() != tt.expected {
+				t.Errorf("Expected %q, got %q", tt.expected, err.Error())
+			}
+		})
 	}
 }