From 14f902c2455ee29436a4d23532db5ee234f3fc7e Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 2 Dec 2024 21:52:41 -0500 Subject: [PATCH 01/46] wip: draft at making cleaner stacktraces Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 50 ++++++++++++++++++++++++++++++++++++++- pkg/engine/engine_test.go | 21 ++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 839ad4a31..00b3d917a 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -330,6 +330,11 @@ func cleanupParseError(filename string, err error) error { return fmt.Errorf("parse error at (%s): %s", string(location), errMsg) } +type TraceableError struct { + location string + message string +} + func cleanupExecError(filename string, err error) error { if _, isExecError := err.(template.ExecError); !isExecError { return err @@ -349,8 +354,51 @@ func cleanupExecError(filename string, err error) error { if len(parts) >= 2 { return fmt.Errorf("execution error at (%s): %s", string(location), parts[1]) } + current := err + fileLocations := []TraceableError{} + for { + if current == nil { + break + } + tokens = strings.SplitN(current.Error(), ": ", 3) + location = tokens[1] + traceable := TraceableError{ + location: location, + message: current.Error(), + } + fileLocations = append(fileLocations, traceable) + current = errors.Unwrap(current) + } + + prevMessage := "" + for i := len(fileLocations) - 1; i >= 0; i-- { + currentMsg := fileLocations[i].message + if i == len(fileLocations)-1 { + prevMessage = currentMsg + continue + } + + if strings.Contains(currentMsg, prevMessage) { + fileLocations[i].message = strings.ReplaceAll(fileLocations[i].message, prevMessage, "") + } + prevMessage = currentMsg + } + + for i := len(fileLocations) - 1; i >= 0; i-- { + if strings.Contains(fileLocations[i].message, fileLocations[i].location) { + fileLocations[i].message = strings.ReplaceAll(fileLocations[i].message, fileLocations[i].location, "") + } + } + + finalErrorString := "" + for _, i := range fileLocations { + if i.message == "" { + continue + } + finalErrorString = finalErrorString + "\n" + i.location + " " + i.message + } - return err + return fmt.Errorf("%s\n\n\n\nError: %s", finalErrorString, err.Error()) } func sortTemplates(tpls map[string]renderable) []string { diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 68e0158fa..8445abf83 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -18,6 +18,8 @@ package engine import ( "fmt" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/pkg/chart/v2/loader" "path" "strings" "sync" @@ -1301,6 +1303,25 @@ func TestRenderTplMissingKeyString(t *testing.T) { } } +func TestSometimesJesseJustBe(t *testing.T) { + c, _ := loader.Load("/home/jesse/code/camunda-platform-helm/charts/camunda-platform-8.5") + + v, _ := chartutil.ReadValuesFile("/home/jesse/code/helm/values.yaml") + val, _ := chartutil.CoalesceValues(c, v) + vals := map[string]interface{}{ + "Values": val.AsMap(), + } + out, err := Render(c, vals) + + if err != nil { + t.Errorf("Failed to render templates: %s", err) + } + assert.NotNil(t, out) + data := strings.TrimSpace(out["jesse-subchart-values-hacktest/charts/keycloak/templates/ingress.yaml"]) + fmt.Println(data) + assert.NotEmpty(t, data) +} + func TestRenderCustomTemplateFuncs(t *testing.T) { // Create a chart with two templates that use custom functions c := &chart.Chart{ From cc477e9f7919ad6a6962e5f1076acde9db0e37db Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Tue, 3 Dec 2024 18:32:56 -0500 Subject: [PATCH 02/46] fix: adjust test to not require external chart Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 2 +- pkg/engine/engine_test.go | 28 +++++++++++++++++++--------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 00b3d917a..65e633f89 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -398,7 +398,7 @@ func cleanupExecError(filename string, err error) error { finalErrorString = finalErrorString + "\n" + i.location + " " + i.message } - return fmt.Errorf("%s\n\n\n\nError: %s", finalErrorString, err.Error()) + return fmt.Errorf("NEW ERROR FORMAT: \n%s\n\n\nORIGINAL ERROR:\n%s", finalErrorString, err.Error()) } func sortTemplates(tpls map[string]renderable) []string { diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 8445abf83..fc2a1a3f3 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -19,7 +19,6 @@ package engine import ( "fmt" "github.com/stretchr/testify/assert" - "helm.sh/helm/v4/pkg/chart/v2/loader" "path" "strings" "sync" @@ -1303,23 +1302,34 @@ func TestRenderTplMissingKeyString(t *testing.T) { } } -func TestSometimesJesseJustBe(t *testing.T) { - c, _ := loader.Load("/home/jesse/code/camunda-platform-helm/charts/camunda-platform-8.5") +func TestNestedHelpersProducesMultilineStacktrace(t *testing.T) { + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "NestedHelperFunctions"}, + Templates: []*chart.File{ + {Name: "templates/svc.yaml", Data: []byte( + `name: {{ include "nested_helper.name" . }}`, + )}, + {Name: "templates/_helpers_1.tpl", Data: []byte( + `{{- define "nested_helper.name" -}}{{- include "common.names.get_name" . -}}{{- end -}}`, + )}, + {Name: "charts/common/templates/_helpers_2.tpl", Data: []byte( + `{{- define "common.names.get_name" -}}{{- .Release.Name | trunc 63 | trimSuffix "-" -}}{{- end -}}`, + )}, + }, + } + + v := chartutil.Values{} - v, _ := chartutil.ReadValuesFile("/home/jesse/code/helm/values.yaml") val, _ := chartutil.CoalesceValues(c, v) vals := map[string]interface{}{ "Values": val.AsMap(), } - out, err := Render(c, vals) + _, err := Render(c, vals) + assert.NotNil(t, err) if err != nil { t.Errorf("Failed to render templates: %s", err) } - assert.NotNil(t, out) - data := strings.TrimSpace(out["jesse-subchart-values-hacktest/charts/keycloak/templates/ingress.yaml"]) - fmt.Println(data) - assert.NotEmpty(t, data) } func TestRenderCustomTemplateFuncs(t *testing.T) { From 6cd0c0082a4f0c996adc7d71328b5d0d8953f1b8 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 23 Dec 2024 14:44:37 -0500 Subject: [PATCH 03/46] fix: split up the multiline errors to be more vertical Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 65e633f89..0336801c6 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -331,8 +331,9 @@ func cleanupParseError(filename string, err error) error { } type TraceableError struct { - location string - message string + location string + message string + executedFunction string } func cleanupExecError(filename string, err error) error { @@ -390,12 +391,35 @@ func cleanupExecError(filename string, err error) error { } } + for i := len(fileLocations) - 1; i >= 0; i-- { + if strings.Contains(fileLocations[i].message, "template:") { + fileLocations[i].message = strings.TrimSpace(strings.ReplaceAll(fileLocations[i].message, "template:", "")) + } + if strings.HasPrefix(fileLocations[i].message, ": ") { + fileLocations[i].message = strings.TrimSpace(strings.TrimPrefix(fileLocations[i].message, ": ")) + } + } + + for i := len(fileLocations) - 1; i >= 0; i-- { + if fileLocations[i].message == "" { + continue + } + executionLocationRegex, regexFindErr := regexp.Compile(`executing "[^\"]*" at <[^\<\>]*>:?\s*`) + if regexFindErr != nil { + continue + } + byteArrayMsg := []byte(fileLocations[i].message) + executionLocations := executionLocationRegex.FindAll(byteArrayMsg, -1) + fileLocations[i].executedFunction = string(executionLocations[0]) + fileLocations[i].message = strings.ReplaceAll(fileLocations[i].message, fileLocations[i].executedFunction, "") + } + finalErrorString := "" for _, i := range fileLocations { if i.message == "" { continue } - finalErrorString = finalErrorString + "\n" + i.location + " " + i.message + finalErrorString = finalErrorString + "\n" + i.location + "\n " + i.executedFunction + "\n " + i.message } return fmt.Errorf("NEW ERROR FORMAT: \n%s\n\n\nORIGINAL ERROR:\n%s", finalErrorString, err.Error()) From 87f9e2dc45532bb94421c9c817e12c5b37c789f8 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 23 Dec 2024 15:35:39 -0500 Subject: [PATCH 04/46] style: create String function for TraceableError representing one chunk of a stacktrace Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 0336801c6..5234ce293 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -336,6 +336,10 @@ type TraceableError struct { executedFunction string } +func (t TraceableError) String() string { + return t.location + "\n " + t.executedFunction + "\n " + t.message + "\n" +} + func cleanupExecError(filename string, err error) error { if _, isExecError := err.(template.ExecError); !isExecError { return err @@ -419,7 +423,7 @@ func cleanupExecError(filename string, err error) error { if i.message == "" { continue } - finalErrorString = finalErrorString + "\n" + i.location + "\n " + i.executedFunction + "\n " + i.message + finalErrorString = finalErrorString + i.String() } return fmt.Errorf("NEW ERROR FORMAT: \n%s\n\n\nORIGINAL ERROR:\n%s", finalErrorString, err.Error()) From a56daca82bb4b741bf59a6c1e2e7c8772d520859 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 23 Dec 2024 15:53:20 -0500 Subject: [PATCH 05/46] style: use interface functions instead of inline logic Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 58 +++++++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 5234ce293..273560212 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -340,6 +340,35 @@ func (t TraceableError) String() string { return t.location + "\n " + t.executedFunction + "\n " + t.message + "\n" } +func (t TraceableError) ExtractExecutedFunction() (TraceableError, error) { + executionLocationRegex, regexFindErr := regexp.Compile(`executing "[^\"]*" at <[^\<\>]*>:?\s*`) + if regexFindErr != nil { + return t, regexFindErr + } + byteArrayMsg := []byte(t.message) + executionLocations := executionLocationRegex.FindAll(byteArrayMsg, -1) + t.executedFunction = string(executionLocations[0]) + t.message = strings.ReplaceAll(t.message, t.executedFunction, "") + return t, nil +} + +func (t TraceableError) FilterLocation() TraceableError { + if strings.Contains(t.message, t.location) { + t.message = strings.ReplaceAll(t.message, t.location, "") + } + return t +} + +func (t TraceableError) FilterUnnecessaryWords() TraceableError { + if strings.Contains(t.message, "template:") { + t.message = strings.TrimSpace(strings.ReplaceAll(t.message, "template:", "")) + } + if strings.HasPrefix(t.message, ": ") { + t.message = strings.TrimSpace(strings.TrimPrefix(t.message, ": ")) + } + return t +} + func cleanupExecError(filename string, err error) error { if _, isExecError := err.(template.ExecError); !isExecError { return err @@ -389,33 +418,24 @@ func cleanupExecError(filename string, err error) error { prevMessage = currentMsg } - for i := len(fileLocations) - 1; i >= 0; i-- { - if strings.Contains(fileLocations[i].message, fileLocations[i].location) { - fileLocations[i].message = strings.ReplaceAll(fileLocations[i].message, fileLocations[i].location, "") - } + for i, fileLocation := range fileLocations { + t := fileLocation.FilterLocation() + fileLocations[i] = t } - for i := len(fileLocations) - 1; i >= 0; i-- { - if strings.Contains(fileLocations[i].message, "template:") { - fileLocations[i].message = strings.TrimSpace(strings.ReplaceAll(fileLocations[i].message, "template:", "")) - } - if strings.HasPrefix(fileLocations[i].message, ": ") { - fileLocations[i].message = strings.TrimSpace(strings.TrimPrefix(fileLocations[i].message, ": ")) - } + for i, fileLocation := range fileLocations { + fileLocations[i] = fileLocation.FilterUnnecessaryWords() } - for i := len(fileLocations) - 1; i >= 0; i-- { - if fileLocations[i].message == "" { + for i, fileLocation := range fileLocations { + if fileLocation.message == "" { continue } - executionLocationRegex, regexFindErr := regexp.Compile(`executing "[^\"]*" at <[^\<\>]*>:?\s*`) - if regexFindErr != nil { + t, extractionErr := fileLocation.ExtractExecutedFunction() + if extractionErr != nil { continue } - byteArrayMsg := []byte(fileLocations[i].message) - executionLocations := executionLocationRegex.FindAll(byteArrayMsg, -1) - fileLocations[i].executedFunction = string(executionLocations[0]) - fileLocations[i].message = strings.ReplaceAll(fileLocations[i].message, fileLocations[i].executedFunction, "") + fileLocations[i] = t } finalErrorString := "" From 1357db4db133a486d69f00072bf23754765e9fbf Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 23 Dec 2024 15:54:13 -0500 Subject: [PATCH 06/46] style: removed variable only used once Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 273560212..156aafcf0 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -419,8 +419,7 @@ func cleanupExecError(filename string, err error) error { } for i, fileLocation := range fileLocations { - t := fileLocation.FilterLocation() - fileLocations[i] = t + fileLocations[i] = fileLocation.FilterLocation() } for i, fileLocation := range fileLocations { From f09bbb8ab8b0a034bfc4cfbca35c395ed332ba2d Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 23 Dec 2024 16:19:16 -0500 Subject: [PATCH 07/46] style: consolidate for loops Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 156aafcf0..6a9b49a3d 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -419,14 +419,7 @@ func cleanupExecError(filename string, err error) error { } for i, fileLocation := range fileLocations { - fileLocations[i] = fileLocation.FilterLocation() - } - - for i, fileLocation := range fileLocations { - fileLocations[i] = fileLocation.FilterUnnecessaryWords() - } - - for i, fileLocation := range fileLocations { + fileLocations[i] = fileLocation.FilterLocation().FilterUnnecessaryWords() if fileLocation.message == "" { continue } From 2e3f6dce28b1c204bf99d6ba660365a0ca23adfe Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 23 Dec 2024 16:25:18 -0500 Subject: [PATCH 08/46] fix: save to fileLocation rather than fileLocations[i] which gets overwritten Signed-off-by: Jesse Simpson --- 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 6a9b49a3d..a533e3241 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -419,7 +419,7 @@ func cleanupExecError(filename string, err error) error { } for i, fileLocation := range fileLocations { - fileLocations[i] = fileLocation.FilterLocation().FilterUnnecessaryWords() + fileLocation = fileLocation.FilterLocation().FilterUnnecessaryWords() if fileLocation.message == "" { continue } From c48147098534e935ea5f9725fbfd04f39431791b Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 23 Dec 2024 16:27:31 -0500 Subject: [PATCH 09/46] style: renamed i variable for consistency Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index a533e3241..21d3fd2c8 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -431,11 +431,11 @@ func cleanupExecError(filename string, err error) error { } finalErrorString := "" - for _, i := range fileLocations { - if i.message == "" { + for _, fileLocation := range fileLocations { + if fileLocation.message == "" { continue } - finalErrorString = finalErrorString + i.String() + finalErrorString = finalErrorString + fileLocation.String() } return fmt.Errorf("NEW ERROR FORMAT: \n%s\n\n\nORIGINAL ERROR:\n%s", finalErrorString, err.Error()) From d8bec4e30f6f2aa068d0b90e68266d6cba7c8253 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 23 Dec 2024 16:33:03 -0500 Subject: [PATCH 10/46] fix: remove comparison from old error message to new Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 21d3fd2c8..4c7021872 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -437,8 +437,12 @@ func cleanupExecError(filename string, err error) error { } finalErrorString = finalErrorString + fileLocation.String() } + if strings.TrimSpace(finalErrorString) == "" { + // Fallback to original error message if nothing was extracted + return fmt.Errorf("%s", err.Error()) + } - return fmt.Errorf("NEW ERROR FORMAT: \n%s\n\n\nORIGINAL ERROR:\n%s", finalErrorString, err.Error()) + return fmt.Errorf("%s", finalErrorString) } func sortTemplates(tpls map[string]renderable) []string { From 383a758aecd00e6c1bd7043f0b6b9c8151d850eb Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Tue, 31 Dec 2024 18:47:09 -0500 Subject: [PATCH 11/46] fix: add quality checks to ensure formatted error doesnt remove important info Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 4c7021872..9d81a90db 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -369,6 +369,33 @@ func (t TraceableError) FilterUnnecessaryWords() TraceableError { return t } +// In the process of formatting the error, we want to ensure that the formatted version of the error +// is not losing any necessary information. This function will tokenize and compare the two strings +// and if the formatted error doesn't meet the threshold, it will fallback to the originalErr +func determineIfFormattedErrorIsAcceptable(formattedErr error, originalErr error) error { + formattedErrTokens := strings.Fields(formattedErr.Error()) + originalErrTokens := strings.Fields(originalErr.Error()) + + tokenSet := make(map[string]struct{}) + for _, token := range originalErrTokens { + tokenSet[token] = struct{}{} + } + + matchCount := 0 + for _, token := range formattedErrTokens { + if _, exists := tokenSet[token]; exists { + matchCount++ + } + } + + equivalenceRating := (float64(matchCount) / float64(len(formattedErrTokens))) * 100 + fmt.Printf("Rating: %f\n", equivalenceRating) + if equivalenceRating >= 80 { + return formattedErr + } + return originalErr +} + func cleanupExecError(filename string, err error) error { if _, isExecError := err.(template.ExecError); !isExecError { return err @@ -442,7 +469,7 @@ func cleanupExecError(filename string, err error) error { return fmt.Errorf("%s", err.Error()) } - return fmt.Errorf("%s", finalErrorString) + return determineIfFormattedErrorIsAcceptable(fmt.Errorf("%s", finalErrorString), err) } func sortTemplates(tpls map[string]renderable) []string { From 6bb836374b1b27841f2ed1db93028e7502cb8148 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Tue, 31 Dec 2024 18:54:34 -0500 Subject: [PATCH 12/46] test: adjusted to make it more meaningful and to pass Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 1 - pkg/engine/engine_test.go | 15 ++++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 9d81a90db..c8e50eed5 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -389,7 +389,6 @@ func determineIfFormattedErrorIsAcceptable(formattedErr error, originalErr error } equivalenceRating := (float64(matchCount) / float64(len(formattedErrTokens))) * 100 - fmt.Printf("Rating: %f\n", equivalenceRating) if equivalenceRating >= 80 { return formattedErr } diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index fc2a1a3f3..55fb0087c 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -1318,6 +1318,17 @@ func TestNestedHelpersProducesMultilineStacktrace(t *testing.T) { }, } + expectedErrorMessage := `NestedHelperFunctions/templates/svc.yaml:1:9 + executing "NestedHelperFunctions/templates/svc.yaml" at : + error calling include: +NestedHelperFunctions/templates/_helpers_1.tpl:1:39 + executing "nested_helper.name" at : + error calling include: +NestedHelperFunctions/charts/common/templates/_helpers_2.tpl:1:50 + executing "common.names.get_name" at <.Release.Name>: + nil pointer evaluating interface {}.Name +` + v := chartutil.Values{} val, _ := chartutil.CoalesceValues(c, v) @@ -1327,9 +1338,7 @@ func TestNestedHelpersProducesMultilineStacktrace(t *testing.T) { _, err := Render(c, vals) assert.NotNil(t, err) - if err != nil { - t.Errorf("Failed to render templates: %s", err) - } + assert.Equal(t, expectedErrorMessage, err.Error()) } func TestRenderCustomTemplateFuncs(t *testing.T) { From 487f72b822655185c279e87a4bef51c7b51792c6 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Wed, 1 Jan 2025 12:26:39 -0500 Subject: [PATCH 13/46] fix: added protection against while-true condition in unbounded for loop Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index c8e50eed5..e06c18f34 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -392,7 +392,7 @@ func determineIfFormattedErrorIsAcceptable(formattedErr error, originalErr error if equivalenceRating >= 80 { return formattedErr } - return originalErr + return fmt.Errorf("%s", originalErr.Error()) } func cleanupExecError(filename string, err error) error { @@ -416,7 +416,8 @@ func cleanupExecError(filename string, err error) error { } current := err fileLocations := []TraceableError{} - for { + maxIterations := 100 + for i := 0; i < maxIterations && current != nil; i++ { if current == nil { break } @@ -429,6 +430,9 @@ func cleanupExecError(filename string, err error) error { fileLocations = append(fileLocations, traceable) current = errors.Unwrap(current) } + if current != nil { + return fmt.Errorf("%s", err.Error()) + } prevMessage := "" for i := len(fileLocations) - 1; i >= 0; i-- { From edf0f7be592c3909ece2735f93278ce9d859b813 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 25 Jan 2025 09:04:21 -0500 Subject: [PATCH 14/46] test: adjust formatting for error in test Signed-off-by: Jesse Simpson --- pkg/action/install_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index e39674c80..6a028bd85 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -446,7 +446,9 @@ func TestInstallReleaseIncorrectTemplate_DryRun(t *testing.T) { instAction.DryRun = true vals := map[string]interface{}{} _, err := instAction.Run(buildChart(withSampleIncludingIncorrectTemplates()), vals) - expectedErr := "\"hello/templates/incorrect\" at <.Values.bad.doh>: nil pointer evaluating interface {}.doh" + expectedErr := `hello/templates/incorrect:1:10 + executing "hello/templates/incorrect" at <.Values.bad.doh>: + nil pointer evaluating interface {}.doh` if err == nil { t.Fatalf("Install should fail containing error: %s", expectedErr) } From 80d7a1b33f556c03bbf3b39fea488ca34731a2d2 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 25 Jan 2025 09:26:02 -0500 Subject: [PATCH 15/46] style: make format Signed-off-by: Jesse Simpson --- pkg/engine/engine_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 55fb0087c..79560bc75 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -18,13 +18,14 @@ package engine import ( "fmt" - "github.com/stretchr/testify/assert" "path" "strings" "sync" "testing" "text/template" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" From 98da3e28b666b04664a787e515d2a3adc1fb0289 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 25 Jan 2025 10:06:06 -0500 Subject: [PATCH 16/46] fix: add some index checking and fixed a test that relied on type-checking Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 10 +++++++++- pkg/engine/engine_test.go | 14 ++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index e06c18f34..0c7d72dba 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -347,6 +347,9 @@ func (t TraceableError) ExtractExecutedFunction() (TraceableError, error) { } byteArrayMsg := []byte(t.message) executionLocations := executionLocationRegex.FindAll(byteArrayMsg, -1) + if len(executionLocations) == 0 { + return t, nil + } t.executedFunction = string(executionLocations[0]) t.message = strings.ReplaceAll(t.message, t.executedFunction, "") return t, nil @@ -422,7 +425,12 @@ func cleanupExecError(filename string, err error) error { break } tokens = strings.SplitN(current.Error(), ": ", 3) - location = tokens[1] + if len(tokens) == 1 { + // For cases where the error message doesn't contain a colon + location = tokens[0] + } else { + location = tokens[1] + } traceable := TraceableError{ location: location, message: current.Error(), diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 79560bc75..a34082e5f 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -22,7 +22,6 @@ import ( "strings" "sync" "testing" - "text/template" "github.com/stretchr/testify/assert" @@ -1291,16 +1290,11 @@ func TestRenderTplMissingKeyString(t *testing.T) { t.Errorf("Expected error, got %v", out) return } - switch err.(type) { - case (template.ExecError): - errTxt := fmt.Sprint(err) - if !strings.Contains(errTxt, "noSuchKey") { - t.Errorf("Expected error to contain 'noSuchKey', got %s", errTxt) - } - default: - // Some unexpected error. - t.Fatal(err) + errTxt := fmt.Sprint(err) + if !strings.Contains(errTxt, "noSuchKey") { + t.Errorf("Expected error to contain 'noSuchKey', got %s", errTxt) } + } func TestNestedHelpersProducesMultilineStacktrace(t *testing.T) { From daf4c348791a81607dc5ced88fff3b18ede847a4 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 22 Feb 2025 09:46:21 -0500 Subject: [PATCH 17/46] fix: use originalErr instead of formatting original error Signed-off-by: Jesse Simpson --- 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 0c7d72dba..d70e9fdad 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -395,7 +395,7 @@ func determineIfFormattedErrorIsAcceptable(formattedErr error, originalErr error if equivalenceRating >= 80 { return formattedErr } - return fmt.Errorf("%s", originalErr.Error()) + return originalErr } func cleanupExecError(filename string, err error) error { From 13b232e061f0d33d365beb9dc28ca72656051552 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 22 Feb 2025 11:58:11 -0500 Subject: [PATCH 18/46] refactor: make use of regexs for err parsing Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 132 ++++++++++++++------------------------ pkg/engine/engine_test.go | 6 +- 2 files changed, 50 insertions(+), 88 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index d70e9fdad..48020a521 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -339,67 +339,22 @@ type TraceableError struct { func (t TraceableError) String() string { return t.location + "\n " + t.executedFunction + "\n " + t.message + "\n" } - -func (t TraceableError) ExtractExecutedFunction() (TraceableError, error) { - executionLocationRegex, regexFindErr := regexp.Compile(`executing "[^\"]*" at <[^\<\>]*>:?\s*`) - if regexFindErr != nil { - return t, regexFindErr - } - byteArrayMsg := []byte(t.message) - executionLocations := executionLocationRegex.FindAll(byteArrayMsg, -1) - if len(executionLocations) == 0 { - return t, nil - } - t.executedFunction = string(executionLocations[0]) - t.message = strings.ReplaceAll(t.message, t.executedFunction, "") - return t, nil -} - -func (t TraceableError) FilterLocation() TraceableError { - if strings.Contains(t.message, t.location) { - t.message = strings.ReplaceAll(t.message, t.location, "") - } - return t -} - -func (t TraceableError) FilterUnnecessaryWords() TraceableError { - if strings.Contains(t.message, "template:") { - t.message = strings.TrimSpace(strings.ReplaceAll(t.message, "template:", "")) - } - if strings.HasPrefix(t.message, ": ") { - t.message = strings.TrimSpace(strings.TrimPrefix(t.message, ": ")) - } - return t -} - -// In the process of formatting the error, we want to ensure that the formatted version of the error -// is not losing any necessary information. This function will tokenize and compare the two strings -// and if the formatted error doesn't meet the threshold, it will fallback to the originalErr -func determineIfFormattedErrorIsAcceptable(formattedErr error, originalErr error) error { - formattedErrTokens := strings.Fields(formattedErr.Error()) - originalErrTokens := strings.Fields(originalErr.Error()) - - tokenSet := make(map[string]struct{}) - for _, token := range originalErrTokens { - tokenSet[token] = struct{}{} +func cleanupExecError(filename string, err error) error { + if _, isExecError := err.(template.ExecError); !isExecError { + return err } - matchCount := 0 - for _, token := range formattedErrTokens { - if _, exists := tokenSet[token]; exists { - matchCount++ - } - } + // taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 + // > "template: %s: %s" + // 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" - equivalenceRating := (float64(matchCount) / float64(len(formattedErrTokens))) * 100 - if equivalenceRating >= 80 { - return formattedErr + execErrFmt, compileErr := regexp.Compile(`^template: (?P(?U).+): executing (?P(?U).+) at (?P(?U).+): (?P(?U).+)(?P( template:.*)?)$`) + if compileErr != nil { + return err } - return originalErr -} - -func cleanupExecError(filename string, err error) error { - if _, isExecError := err.(template.ExecError); !isExecError { + execErrFmtWithoutTemplate, compileErr := regexp.Compile(`^template: (?P(?U).+): (?P.*)(?P( template:.*)?)$`) + if compileErr != nil { return err } @@ -424,16 +379,35 @@ func cleanupExecError(filename string, err error) error { if current == nil { break } - tokens = strings.SplitN(current.Error(), ": ", 3) - if len(tokens) == 1 { - // For cases where the error message doesn't contain a colon - location = tokens[0] + + var traceable TraceableError + if execErrFmt.MatchString(current.Error()) { + matches := execErrFmt.FindStringSubmatch(current.Error()) + templateIndex := execErrFmt.SubexpIndex("templateName") + templateName := matches[templateIndex] + functionNameIndex := execErrFmt.SubexpIndex("functionName") + functionName := matches[functionNameIndex] + locationNameIndex := execErrFmt.SubexpIndex("location") + locationName := matches[locationNameIndex] + errMsgIndex := execErrFmt.SubexpIndex("errMsg") + errMsg := matches[errMsgIndex] + traceable = TraceableError{ + location: templateName, + message: errMsg, + executedFunction: "executing " + functionName + " at " + locationName + ":", + } + } else if execErrFmtWithoutTemplate.MatchString(current.Error()) { + matches := execErrFmt.FindStringSubmatch(current.Error()) + templateIndex := execErrFmt.SubexpIndex("templateName") + templateName := matches[templateIndex] + errMsgIndex := execErrFmt.SubexpIndex("errMsg") + errMsg := matches[errMsgIndex] + traceable = TraceableError{ + location: templateName, + message: errMsg, + } } else { - location = tokens[1] - } - traceable := TraceableError{ - location: location, - message: current.Error(), + return err } fileLocations = append(fileLocations, traceable) current = errors.Unwrap(current) @@ -442,30 +416,18 @@ func cleanupExecError(filename string, err error) error { return fmt.Errorf("%s", err.Error()) } - prevMessage := "" + var prev TraceableError for i := len(fileLocations) - 1; i >= 0; i-- { - currentMsg := fileLocations[i].message + current := fileLocations[i] if i == len(fileLocations)-1 { - prevMessage = currentMsg + prev = current continue } - if strings.Contains(currentMsg, prevMessage) { - fileLocations[i].message = strings.ReplaceAll(fileLocations[i].message, prevMessage, "") - } - prevMessage = currentMsg - } - - for i, fileLocation := range fileLocations { - fileLocation = fileLocation.FilterLocation().FilterUnnecessaryWords() - if fileLocation.message == "" { - continue - } - t, extractionErr := fileLocation.ExtractExecutedFunction() - if extractionErr != nil { - continue + if current.message == prev.message && current.location == prev.location && current.executedFunction == prev.executedFunction { + fileLocations[i].message = "" } - fileLocations[i] = t + prev = current } finalErrorString := "" @@ -480,7 +442,7 @@ func cleanupExecError(filename string, err error) error { return fmt.Errorf("%s", err.Error()) } - return determineIfFormattedErrorIsAcceptable(fmt.Errorf("%s", finalErrorString), err) + return fmt.Errorf("%s", finalErrorString) } func sortTemplates(tpls map[string]renderable) []string { diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index a34082e5f..58d8588ab 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -1314,13 +1314,13 @@ func TestNestedHelpersProducesMultilineStacktrace(t *testing.T) { } expectedErrorMessage := `NestedHelperFunctions/templates/svc.yaml:1:9 - executing "NestedHelperFunctions/templates/svc.yaml" at : + executing "NestedHelperFunctions/templates/svc.yaml" at : error calling include: NestedHelperFunctions/templates/_helpers_1.tpl:1:39 - executing "nested_helper.name" at : + executing "nested_helper.name" at : error calling include: NestedHelperFunctions/charts/common/templates/_helpers_2.tpl:1:50 - executing "common.names.get_name" at <.Release.Name>: + executing "common.names.get_name" at <.Release.Name>: nil pointer evaluating interface {}.Name ` From 0a7dd4b269da6b8f739c7189b06227e16c3dafe7 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 22 Feb 2025 12:15:49 -0500 Subject: [PATCH 19/46] test: adjust failing test on extra whitespace Signed-off-by: Jesse Simpson --- pkg/action/install_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 6a028bd85..61cc2946f 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -447,7 +447,7 @@ func TestInstallReleaseIncorrectTemplate_DryRun(t *testing.T) { vals := map[string]interface{}{} _, err := instAction.Run(buildChart(withSampleIncludingIncorrectTemplates()), vals) expectedErr := `hello/templates/incorrect:1:10 - executing "hello/templates/incorrect" at <.Values.bad.doh>: + executing "hello/templates/incorrect" at <.Values.bad.doh>: nil pointer evaluating interface {}.doh` if err == nil { t.Fatalf("Install should fail containing error: %s", expectedErr) From deea4a0d0e91f7d25761a07713fbcb362cc46f49 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 22 Feb 2025 12:34:05 -0500 Subject: [PATCH 20/46] refactor: remove more unnecessary format calls Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 48020a521..74cc899ef 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -413,7 +413,7 @@ func cleanupExecError(filename string, err error) error { current = errors.Unwrap(current) } if current != nil { - return fmt.Errorf("%s", err.Error()) + return err } var prev TraceableError @@ -439,7 +439,7 @@ func cleanupExecError(filename string, err error) error { } if strings.TrimSpace(finalErrorString) == "" { // Fallback to original error message if nothing was extracted - return fmt.Errorf("%s", err.Error()) + return err } return fmt.Errorf("%s", finalErrorString) From 3cc4cb60ba3316e25c630d263a0aad9225441af0 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 22 Feb 2025 13:10:07 -0500 Subject: [PATCH 21/46] refactor: prevent duplicates being inserted rather than post-filtering Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 74cc899ef..d5c507eab 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -379,6 +379,9 @@ func cleanupExecError(filename string, err error) error { if current == nil { break } + if i == maxIterations-1 { + return err + } var traceable TraceableError if execErrFmt.MatchString(current.Error()) { @@ -409,34 +412,24 @@ func cleanupExecError(filename string, err error) error { } else { return err } + if len(fileLocations) > 0 { + lastErr := fileLocations[len(fileLocations)-1] + if lastErr.message == traceable.message && + lastErr.location == traceable.location && + lastErr.executedFunction == traceable.executedFunction { + current = errors.Unwrap(current) + continue + } + } fileLocations = append(fileLocations, traceable) current = errors.Unwrap(current) } - if current != nil { - return err - } - - var prev TraceableError - for i := len(fileLocations) - 1; i >= 0; i-- { - current := fileLocations[i] - if i == len(fileLocations)-1 { - prev = current - continue - } - - if current.message == prev.message && current.location == prev.location && current.executedFunction == prev.executedFunction { - fileLocations[i].message = "" - } - prev = current - } finalErrorString := "" for _, fileLocation := range fileLocations { - if fileLocation.message == "" { - continue - } finalErrorString = finalErrorString + fileLocation.String() } + if strings.TrimSpace(finalErrorString) == "" { // Fallback to original error message if nothing was extracted return err From cdcf1bc6016593126b92c0eab09adaa55ed5e0e8 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sun, 16 Mar 2025 20:45:11 -0400 Subject: [PATCH 22/46] style: remove unnecessary break Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index d5c507eab..c1356e7c2 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -376,9 +376,6 @@ func cleanupExecError(filename string, err error) error { fileLocations := []TraceableError{} maxIterations := 100 for i := 0; i < maxIterations && current != nil; i++ { - if current == nil { - break - } if i == maxIterations-1 { return err } From cbdc22128eb5e01041311e30267beb0dbbea9c3b Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sun, 16 Mar 2025 20:45:31 -0400 Subject: [PATCH 23/46] refactor: use strings.Builder instead of string concatenation Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index c1356e7c2..46e75a1f3 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -422,17 +422,17 @@ func cleanupExecError(filename string, err error) error { current = errors.Unwrap(current) } - finalErrorString := "" + var finalErrorString strings.Builder for _, fileLocation := range fileLocations { - finalErrorString = finalErrorString + fileLocation.String() + fmt.Fprintf(&finalErrorString, "%s", fileLocation.String()) } - if strings.TrimSpace(finalErrorString) == "" { + if strings.TrimSpace(finalErrorString.String()) == "" { // Fallback to original error message if nothing was extracted return err } - return fmt.Errorf("%s", finalErrorString) + return fmt.Errorf("%s", finalErrorString.String()) } func sortTemplates(tpls map[string]renderable) []string { From 5202820f2f9a0c04514f2c15621491d398623ac4 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sun, 16 Mar 2025 20:53:18 -0400 Subject: [PATCH 24/46] refactor: define regexs at package scope Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 46e75a1f3..0dea01dd6 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -33,6 +33,14 @@ 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:.*)?)$`) + // 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 @@ -344,20 +352,6 @@ func cleanupExecError(filename string, err error) error { return err } - // taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 - // > "template: %s: %s" - // 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" - - execErrFmt, compileErr := regexp.Compile(`^template: (?P(?U).+): executing (?P(?U).+) at (?P(?U).+): (?P(?U).+)(?P( template:.*)?)$`) - if compileErr != nil { - return err - } - execErrFmtWithoutTemplate, compileErr := regexp.Compile(`^template: (?P(?U).+): (?P.*)(?P( template:.*)?)$`) - if compileErr != nil { - return err - } - tokens := strings.SplitN(err.Error(), ": ", 3) if len(tokens) != 3 { // This might happen if a non-templating error occurs From 4f63c7335306f9197f6d560fbf3ccf3c5416e248 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sun, 16 Mar 2025 21:07:52 -0400 Subject: [PATCH 25/46] refactor: remove impractical safety check Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 0dea01dd6..3be69f1ab 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -421,11 +421,6 @@ func cleanupExecError(filename string, err error) error { fmt.Fprintf(&finalErrorString, "%s", fileLocation.String()) } - if strings.TrimSpace(finalErrorString.String()) == "" { - // Fallback to original error message if nothing was extracted - return err - } - return fmt.Errorf("%s", finalErrorString.String()) } From 4b9a9ecaf64acddea4f8a2cc5d22a8ad04d8b727 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sun, 16 Mar 2025 21:21:04 -0400 Subject: [PATCH 26/46] refactor: rename function and add doc-string Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 3be69f1ab..cc1951af7 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -312,7 +312,7 @@ func (e Engine) render(tpls map[string]renderable) (rendered map[string]string, 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{}, cleanupExecError(filename, err) + return map[string]string{}, reformatExecErrorMsg(filename, err) } // Work around the issue where Go will emit "" even if Options(missing=zero) @@ -347,7 +347,14 @@ type TraceableError struct { func (t TraceableError) String() string { return t.location + "\n " + t.executedFunction + "\n " + t.message + "\n" } -func cleanupExecError(filename string, err error) error { + +// 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, + // 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 { return err } From 782f6c640987f3df24b5e925e3bece4e7fdc17df Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 17 Mar 2025 12:37:18 -0400 Subject: [PATCH 27/46] refactor: shorten regex subexp syntax Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index cc1951af7..ad2c1679d 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -384,14 +384,10 @@ func reformatExecErrorMsg(filename string, err error) error { var traceable TraceableError if execErrFmt.MatchString(current.Error()) { matches := execErrFmt.FindStringSubmatch(current.Error()) - templateIndex := execErrFmt.SubexpIndex("templateName") - templateName := matches[templateIndex] - functionNameIndex := execErrFmt.SubexpIndex("functionName") - functionName := matches[functionNameIndex] - locationNameIndex := execErrFmt.SubexpIndex("location") - locationName := matches[locationNameIndex] - errMsgIndex := execErrFmt.SubexpIndex("errMsg") - errMsg := matches[errMsgIndex] + 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, @@ -399,10 +395,8 @@ func reformatExecErrorMsg(filename string, err error) error { } } else if execErrFmtWithoutTemplate.MatchString(current.Error()) { matches := execErrFmt.FindStringSubmatch(current.Error()) - templateIndex := execErrFmt.SubexpIndex("templateName") - templateName := matches[templateIndex] - errMsgIndex := execErrFmt.SubexpIndex("errMsg") - errMsg := matches[errMsgIndex] + templateName := matches[execErrFmt.SubexpIndex("templateName")] + errMsg := matches[execErrFmt.SubexpIndex("errMsg")] traceable = TraceableError{ location: templateName, message: errMsg, From 65084371c9a774c6c4c6afadd5f6fa2152b31b8a Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Mon, 17 Mar 2025 12:39:55 -0400 Subject: [PATCH 28/46] refactor: replace if MatchString with FindStringSubMatch Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index ad2c1679d..26f80a2f0 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -382,8 +382,7 @@ func reformatExecErrorMsg(filename string, err error) error { } var traceable TraceableError - if execErrFmt.MatchString(current.Error()) { - matches := execErrFmt.FindStringSubmatch(current.Error()) + if matches := execErrFmt.FindStringSubmatch(current.Error()); matches != nil { templateName := matches[execErrFmt.SubexpIndex("templateName")] functionName := matches[execErrFmt.SubexpIndex("functionName")] locationName := matches[execErrFmt.SubexpIndex("location")] @@ -393,8 +392,7 @@ func reformatExecErrorMsg(filename string, err error) error { message: errMsg, executedFunction: "executing " + functionName + " at " + locationName + ":", } - } else if execErrFmtWithoutTemplate.MatchString(current.Error()) { - matches := execErrFmt.FindStringSubmatch(current.Error()) + } else if matches := execErrFmtWithoutTemplate.FindStringSubmatch(current.Error()); matches != nil { templateName := matches[execErrFmt.SubexpIndex("templateName")] errMsg := matches[execErrFmt.SubexpIndex("errMsg")] traceable = TraceableError{ From ac98e977c392885c702515f4899b8f2a2c873870 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Tue, 18 Mar 2025 20:30:19 -0400 Subject: [PATCH 29/46] refactor: switch to while loop instead of for to reduce unnecessary variables Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 26f80a2f0..fee26490f 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -375,12 +375,7 @@ func reformatExecErrorMsg(filename string, err error) error { } current := err fileLocations := []TraceableError{} - maxIterations := 100 - for i := 0; i < maxIterations && current != nil; i++ { - if i == maxIterations-1 { - return err - } - + for current != nil { var traceable TraceableError if matches := execErrFmt.FindStringSubmatch(current.Error()); matches != nil { templateName := matches[execErrFmt.SubexpIndex("templateName")] From 48922e21d1e1647957ea7570d2b4c1fbf81283f5 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Tue, 18 Mar 2025 20:39:06 -0400 Subject: [PATCH 30/46] refactor: use struct equality instead of comparing each composition Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index fee26490f..7b0f87feb 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -399,9 +399,7 @@ func reformatExecErrorMsg(filename string, err error) error { } if len(fileLocations) > 0 { lastErr := fileLocations[len(fileLocations)-1] - if lastErr.message == traceable.message && - lastErr.location == traceable.location && - lastErr.executedFunction == traceable.executedFunction { + if lastErr == traceable { current = errors.Unwrap(current) continue } From 868cdc261ff645052c00b917b43e83187bb91a7f Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Tue, 18 Mar 2025 21:11:25 -0400 Subject: [PATCH 31/46] refactor: reduce flow-control operations Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 7b0f87feb..5b6999021 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -397,14 +397,9 @@ func reformatExecErrorMsg(filename string, err error) error { } else { return err } - if len(fileLocations) > 0 { - lastErr := fileLocations[len(fileLocations)-1] - if lastErr == traceable { - current = errors.Unwrap(current) - continue - } + if len(fileLocations) == 0 || fileLocations[len(fileLocations)-1] != traceable { + fileLocations = append(fileLocations, traceable) } - fileLocations = append(fileLocations, traceable) current = errors.Unwrap(current) } From 013f27c2947362a79f1c93a10add188e67c280fc Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sun, 20 Apr 2025 17:45:59 -0400 Subject: [PATCH 32/46] fix: use errors.New instead of fmt.Errorf Signed-off-by: Jesse Simpson --- 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 5b6999021..04feb43fe 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -408,7 +408,7 @@ func reformatExecErrorMsg(filename string, err error) error { fmt.Fprintf(&finalErrorString, "%s", fileLocation.String()) } - return fmt.Errorf("%s", finalErrorString.String()) + return errors.New(finalErrorString.String()) } func sortTemplates(tpls map[string]renderable) []string { From 0e0a8cc76534177a1c9809804232ac334d7f0f0e Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Wed, 23 Apr 2025 21:56:30 -0400 Subject: [PATCH 33/46] fix: address no-template-associated type of error Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 20 +++++++++++++++++++- pkg/engine/engine_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 04feb43fe..7c92b4505 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -41,6 +41,10 @@ var execErrFmt = regexp.MustCompile(`^template: (?P(?U).+): execut // > "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 @@ -345,7 +349,17 @@ type TraceableError struct { } func (t TraceableError) String() string { - return t.location + "\n " + t.executedFunction + "\n " + t.message + "\n" + var errorString strings.Builder + if t.location != "" { + fmt.Fprintf(&errorString, "%s\n ", t.location) + } + if t.executedFunction != "" { + fmt.Fprintf(&errorString, "%s\n ", t.executedFunction) + } + if t.message != "" { + fmt.Fprintf(&errorString, "%s\n", t.message) + } + return errorString.String() } // reformatExecErrorMsg takes an error message for template rendering and formats it into a formatted @@ -394,6 +408,10 @@ func reformatExecErrorMsg(filename string, err error) error { location: templateName, message: errMsg, } + } else if matches := execErrNoTemplateAssociated.FindStringSubmatch(current.Error()); matches != nil { + traceable = TraceableError{ + message: current.Error(), + } } else { return err } diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 58d8588ab..5359465d4 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -22,6 +22,7 @@ import ( "strings" "sync" "testing" + "text/template" "github.com/stretchr/testify/assert" @@ -1336,6 +1337,40 @@ NestedHelperFunctions/charts/common/templates/_helpers_2.tpl:1:50 assert.Equal(t, expectedErrorMessage, err.Error()) } +func TestMultilineNoTemplateAssociatedError(t *testing.T) { + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "multiline"}, + Templates: []*chart.File{ + {Name: "templates/svc.yaml", Data: []byte( + `name: {{ include "nested_helper.name" . }}`, + )}, + {Name: "templates/test.yaml", Data: []byte( + `{{ toYaml .Values }}`, + )}, + {Name: "charts/common/templates/_helpers_2.tpl", Data: []byte( + `{{ toYaml .Values }}`, + )}, + }, + } + + expectedErrorMessage := `multiline/templates/svc.yaml:1:9 + executing "multiline/templates/svc.yaml" at : + error calling include: +template: no template "nested_helper.name" associated with template "gotpl" +` + + v := chartutil.Values{} + + val, _ := chartutil.CoalesceValues(c, v) + vals := map[string]interface{}{ + "Values": val.AsMap(), + } + _, err := Render(c, vals) + + assert.NotNil(t, err) + assert.Equal(t, expectedErrorMessage, err.Error()) +} + func TestRenderCustomTemplateFuncs(t *testing.T) { // Create a chart with two templates that use custom functions c := &chart.Chart{ From d10c5f642901946d9bf89e01e29a85ffdd97c690 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Wed, 23 Apr 2025 22:07:21 -0400 Subject: [PATCH 34/46] style: trim space from formatted error messages Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 2 +- pkg/engine/engine_test.go | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 7c92b4505..009b5e432 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -426,7 +426,7 @@ func reformatExecErrorMsg(filename string, err error) error { fmt.Fprintf(&finalErrorString, "%s", fileLocation.String()) } - return errors.New(finalErrorString.String()) + return errors.New(strings.TrimSpace(finalErrorString.String())) } func sortTemplates(tpls map[string]renderable) []string { diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 5359465d4..e99fac2e8 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -1322,8 +1322,7 @@ NestedHelperFunctions/templates/_helpers_1.tpl:1:39 error calling include: NestedHelperFunctions/charts/common/templates/_helpers_2.tpl:1:50 executing "common.names.get_name" at <.Release.Name>: - nil pointer evaluating interface {}.Name -` + nil pointer evaluating interface {}.Name` v := chartutil.Values{} @@ -1356,8 +1355,7 @@ func TestMultilineNoTemplateAssociatedError(t *testing.T) { expectedErrorMessage := `multiline/templates/svc.yaml:1:9 executing "multiline/templates/svc.yaml" at : error calling include: -template: no template "nested_helper.name" associated with template "gotpl" -` +template: no template "nested_helper.name" associated with template "gotpl"` v := chartutil.Values{} From e0a67b1028dc175aafa2116eb3a17ce786cbc8cd Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Thu, 24 Apr 2025 13:08:30 -0400 Subject: [PATCH 35/46] test: use more realistic unit-test scenario by not relying on Release.Name Signed-off-by: Jesse Simpson --- pkg/engine/engine_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index e99fac2e8..f4228fbd7 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -1309,7 +1309,7 @@ func TestNestedHelpersProducesMultilineStacktrace(t *testing.T) { `{{- define "nested_helper.name" -}}{{- include "common.names.get_name" . -}}{{- end -}}`, )}, {Name: "charts/common/templates/_helpers_2.tpl", Data: []byte( - `{{- define "common.names.get_name" -}}{{- .Release.Name | trunc 63 | trimSuffix "-" -}}{{- end -}}`, + `{{- define "common.names.get_name" -}}{{- .Values.nonexistant.key | trunc 63 | trimSuffix "-" -}}{{- end -}}`, )}, }, } @@ -1320,9 +1320,9 @@ func TestNestedHelpersProducesMultilineStacktrace(t *testing.T) { NestedHelperFunctions/templates/_helpers_1.tpl:1:39 executing "nested_helper.name" at : error calling include: -NestedHelperFunctions/charts/common/templates/_helpers_2.tpl:1:50 - executing "common.names.get_name" at <.Release.Name>: - nil pointer evaluating interface {}.Name` +NestedHelperFunctions/charts/common/templates/_helpers_2.tpl:1:49 + executing "common.names.get_name" at <.Values.nonexistant.key>: + nil pointer evaluating interface {}.key` v := chartutil.Values{} From 9a2ac850776f0b5ac0987d5e1a685baa6ee76d48 Mon Sep 17 00:00:00 2001 From: MichaelMorris Date: Mon, 20 Nov 2023 17:26:27 +0000 Subject: [PATCH 36/46] Consider GroupVersionKind when matching resources This change shall take Group, Version and Kind from GroupVersionKind into consideration instead of the current behavior of only considering the Kind Closes: #12578 Signed-off-by: MichaelMorris --- pkg/kube/resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kube/resource.go b/pkg/kube/resource.go index 600f256b3..d88b171f0 100644 --- a/pkg/kube/resource.go +++ b/pkg/kube/resource.go @@ -81,5 +81,5 @@ func (r ResourceList) Intersect(rs ResourceList) ResourceList { // isMatchingInfo returns true if infos match on Name and GroupVersionKind. func isMatchingInfo(a, b *resource.Info) bool { - return a.Name == b.Name && a.Namespace == b.Namespace && a.Mapping.GroupVersionKind.Kind == b.Mapping.GroupVersionKind.Kind && a.Mapping.GroupVersionKind.Group == b.Mapping.GroupVersionKind.Group + return a.Name == b.Name && a.Namespace == b.Namespace && a.Mapping.GroupVersionKind == b.Mapping.GroupVersionKind } From 1460ebd14a864e89ad8040af24fa180558702701 Mon Sep 17 00:00:00 2001 From: MichaelMorris Date: Tue, 13 May 2025 23:11:22 +0100 Subject: [PATCH 37/46] Added test case to resource_test.go Signed-off-by: MichaelMorris --- pkg/kube/resource_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/pkg/kube/resource_test.go b/pkg/kube/resource_test.go index c405ca382..ccc613c1b 100644 --- a/pkg/kube/resource_test.go +++ b/pkg/kube/resource_test.go @@ -59,3 +59,42 @@ func TestResourceList(t *testing.T) { t.Error("expected intersect to return bar") } } + +func TestIsMatchingInfo(t *testing.T) { + gvk := schema.GroupVersionKind{Group: "group1", Version: "version1", Kind: "pod"} + resourceInfo := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvk}} + + gvkDiffGroup := schema.GroupVersionKind{Group: "diff", Version: "version1", Kind: "pod"} + resourceInfoDiffGroup := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvkDiffGroup}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffGroup) { + t.Error("expected resources not equal") + } + + gvkDiffVersion := schema.GroupVersionKind{Group: "group1", Version: "diff", Kind: "pod"} + resourceInfoDiffVersion := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvkDiffVersion}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffVersion) { + t.Error("expected resources not equal") + } + + gvkDiffKind := schema.GroupVersionKind{Group: "group1", Version: "version1", Kind: "deployment"} + resourceInfoDiffKind := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvkDiffKind}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffKind) { + t.Error("expected resources not equal") + } + + resourceInfoDiffName := resource.Info{Name: "diff", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvk}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffName) { + t.Error("expected resources not equal") + } + + resourceInfoDiffNamespace := resource.Info{Name: "name1", Namespace: "diff", Mapping: &meta.RESTMapping{GroupVersionKind: gvk}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffNamespace) { + t.Error("expected resources not equal") + } + + gvkEqual := schema.GroupVersionKind{Group: "group1", Version: "version1", Kind: "pod"} + resourceInfoEqual := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvkEqual}} + if !isMatchingInfo(&resourceInfo, &resourceInfoEqual) { + t.Error("expected resources to be equal") + } +} From 6f8e9e09a4e8db22667afef34878f4226cddb030 Mon Sep 17 00:00:00 2001 From: jinjiadu Date: Sat, 24 May 2025 13:23:03 +0800 Subject: [PATCH 38/46] refactor: replace HasPrefix+TrimPrefix with CutPrefix Signed-off-by: jinjiadu --- pkg/ignore/rules.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/ignore/rules.go b/pkg/ignore/rules.go index 5281c3d59..3511c2d40 100644 --- a/pkg/ignore/rules.go +++ b/pkg/ignore/rules.go @@ -170,10 +170,10 @@ func (r *Rules) parseRule(rule string) error { rule = strings.TrimSuffix(rule, "/") } - if strings.HasPrefix(rule, "/") { + if after, ok := strings.CutPrefix(rule, "/"); ok { // Require path matches the root path. p.match = func(n string, _ os.FileInfo) bool { - rule = strings.TrimPrefix(rule, "/") + rule = after ok, err := filepath.Match(rule, n) if err != nil { slog.Error("failed to compile", "rule", rule, slog.Any("error", err)) From 5fe7a87138a3fb4903575fb20b5bb9b98c87a56b Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Mon, 26 May 2025 11:48:24 -0600 Subject: [PATCH 39/46] fix: add debug logging to oci transport Signed-off-by: Terry Howe Co-authored-by: Billy Zha --- pkg/registry/client.go | 26 +-- pkg/registry/transport.go | 175 +++++++++++++++ pkg/registry/transport_test.go | 399 +++++++++++++++++++++++++++++++++ 3 files changed, 580 insertions(+), 20 deletions(-) create mode 100644 pkg/registry/transport.go create mode 100644 pkg/registry/transport_test.go diff --git a/pkg/registry/client.go b/pkg/registry/client.go index c5ab0b4ba..b8235927b 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -100,27 +100,8 @@ func NewClient(options ...ClientOption) (*Client, error) { client.credentialsFile = helmpath.ConfigPath(CredentialsFileBasename) } if client.httpClient == nil { - type cloner[T any] interface { - Clone() T - } - - // try to copy (clone) the http.DefaultTransport so any mutations we - // perform on it (e.g. TLS config) are not reflected globally - // follow https://github.com/golang/go/issues/39299 for a more elegant - // solution in the future - transport := http.DefaultTransport - if t, ok := transport.(cloner[*http.Transport]); ok { - transport = t.Clone() - } else if t, ok := transport.(cloner[http.RoundTripper]); ok { - // this branch will not be used with go 1.20, it was added - // optimistically to try to clone if the http.DefaultTransport - // implementation changes, still the Clone method in that case - // might not return http.RoundTripper... - transport = t.Clone() - } - client.httpClient = &http.Client{ - Transport: retry.NewTransport(transport), + Transport: NewTransport(client.debug), } } @@ -296,6 +277,11 @@ func ensureTLSConfig(client *auth.Client) (*tls.Config, error) { switch t := t.Base.(type) { case *http.Transport: transport = t + case *LoggingTransport: + switch t := t.RoundTripper.(type) { + case *http.Transport: + transport = t + } } } diff --git a/pkg/registry/transport.go b/pkg/registry/transport.go new file mode 100644 index 000000000..7b9c6744b --- /dev/null +++ b/pkg/registry/transport.go @@ -0,0 +1,175 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package registry + +import ( + "bytes" + "fmt" + "io" + "log/slog" + "mime" + "net/http" + "strings" + "sync/atomic" + + "oras.land/oras-go/v2/registry/remote/retry" +) + +var ( + // requestCount records the number of logged request-response pairs and will + // be used as the unique id for the next pair. + requestCount uint64 + + // toScrub is a set of headers that should be scrubbed from the log. + toScrub = []string{ + "Authorization", + "Set-Cookie", + } +) + +// payloadSizeLimit limits the maximum size of the response body to be printed. +const payloadSizeLimit int64 = 16 * 1024 // 16 KiB + +// LoggingTransport is an http.RoundTripper that keeps track of the in-flight +// request and add hooks to report HTTP tracing events. +type LoggingTransport struct { + http.RoundTripper +} + +// NewTransport creates and returns a new instance of LoggingTransport +func NewTransport(debug bool) *retry.Transport { + type cloner[T any] interface { + Clone() T + } + + // try to copy (clone) the http.DefaultTransport so any mutations we + // perform on it (e.g. TLS config) are not reflected globally + // follow https://github.com/golang/go/issues/39299 for a more elegant + // solution in the future + transport := http.DefaultTransport + if t, ok := transport.(cloner[*http.Transport]); ok { + transport = t.Clone() + } else if t, ok := transport.(cloner[http.RoundTripper]); ok { + // this branch will not be used with go 1.20, it was added + // optimistically to try to clone if the http.DefaultTransport + // implementation changes, still the Clone method in that case + // might not return http.RoundTripper... + transport = t.Clone() + } + if debug { + transport = &LoggingTransport{RoundTripper: transport} + } + + return retry.NewTransport(transport) +} + +// RoundTrip calls base round trip while keeping track of the current request. +func (t *LoggingTransport) RoundTrip(req *http.Request) (resp *http.Response, err error) { + id := atomic.AddUint64(&requestCount, 1) - 1 + + slog.Debug("Request", "id", id, "url", req.URL, "method", req.Method, "header", logHeader(req.Header)) + resp, err = t.RoundTripper.RoundTrip(req) + if err != nil { + slog.Debug("Response", "id", id, "error", err) + } else if resp != nil { + slog.Debug("Response", "id", id, "status", resp.Status, "header", logHeader(resp.Header), "body", logResponseBody(resp)) + } else { + slog.Debug("Response", "id", id, "response", "nil") + } + + return resp, err +} + +// logHeader prints out the provided header keys and values, with auth header scrubbed. +func logHeader(header http.Header) string { + if len(header) > 0 { + headers := []string{} + for k, v := range header { + for _, h := range toScrub { + if strings.EqualFold(k, h) { + v = []string{"*****"} + } + } + headers = append(headers, fmt.Sprintf(" %q: %q", k, strings.Join(v, ", "))) + } + return strings.Join(headers, "\n") + } + return " Empty header" +} + +// logResponseBody prints out the response body if it is printable and within size limit. +func logResponseBody(resp *http.Response) string { + if resp.Body == nil || resp.Body == http.NoBody { + return " No response body to print" + } + + // non-applicable body is not printed and remains untouched for subsequent processing + contentType := resp.Header.Get("Content-Type") + if contentType == "" { + return " Response body without a content type is not printed" + } + if !isPrintableContentType(contentType) { + return fmt.Sprintf(" Response body of content type %q is not printed", contentType) + } + + buf := bytes.NewBuffer(nil) + body := resp.Body + // restore the body by concatenating the read body with the remaining body + resp.Body = struct { + io.Reader + io.Closer + }{ + Reader: io.MultiReader(buf, body), + Closer: body, + } + // read the body up to limit+1 to check if the body exceeds the limit + if _, err := io.CopyN(buf, body, payloadSizeLimit+1); err != nil && err != io.EOF { + return fmt.Sprintf(" Error reading response body: %v", err) + } + + readBody := buf.String() + if len(readBody) == 0 { + return " Response body is empty" + } + if containsCredentials(readBody) { + return " Response body redacted due to potential credentials" + } + if len(readBody) > int(payloadSizeLimit) { + return readBody[:payloadSizeLimit] + "\n...(truncated)" + } + return readBody +} + +// isPrintableContentType returns true if the contentType is printable. +func isPrintableContentType(contentType string) bool { + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil { + return false + } + + switch mediaType { + case "application/json", // JSON types + "text/plain", "text/html": // text types + return true + } + return strings.HasSuffix(mediaType, "+json") +} + +// containsCredentials returns true if the body contains potential credentials. +func containsCredentials(body string) bool { + return strings.Contains(body, `"token"`) || strings.Contains(body, `"access_token"`) +} diff --git a/pkg/registry/transport_test.go b/pkg/registry/transport_test.go new file mode 100644 index 000000000..b4990c526 --- /dev/null +++ b/pkg/registry/transport_test.go @@ -0,0 +1,399 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package registry + +import ( + "bytes" + "errors" + "io" + "net/http" + "testing" +) + +var errMockRead = errors.New("mock read error") + +type errorReader struct{} + +func (e *errorReader) Read(_ []byte) (n int, err error) { + return 0, errMockRead +} + +func Test_isPrintableContentType(t *testing.T) { + tests := []struct { + name string + contentType string + want bool + }{ + { + name: "Empty content type", + contentType: "", + want: false, + }, + { + name: "General JSON type", + contentType: "application/json", + want: true, + }, + { + name: "General JSON type with charset", + contentType: "application/json; charset=utf-8", + want: true, + }, + { + name: "Random type with application/json prefix", + contentType: "application/jsonwhatever", + want: false, + }, + { + name: "Manifest type in JSON", + contentType: "application/vnd.oci.image.manifest.v1+json", + want: true, + }, + { + name: "Manifest type in JSON with charset", + contentType: "application/vnd.oci.image.manifest.v1+json; charset=utf-8", + want: true, + }, + { + name: "Random content type in JSON", + contentType: "application/whatever+json", + want: true, + }, + { + name: "Plain text type", + contentType: "text/plain", + want: true, + }, + { + name: "Plain text type with charset", + contentType: "text/plain; charset=utf-8", + want: true, + }, + { + name: "Random type with text/plain prefix", + contentType: "text/plainnnnn", + want: false, + }, + { + name: "HTML type", + contentType: "text/html", + want: true, + }, + { + name: "Plain text type with charset", + contentType: "text/html; charset=utf-8", + want: true, + }, + { + name: "Random type with text/html prefix", + contentType: "text/htmlllll", + want: false, + }, + { + name: "Binary type", + contentType: "application/octet-stream", + want: false, + }, + { + name: "Unknown type", + contentType: "unknown/unknown", + want: false, + }, + { + name: "Invalid type", + contentType: "text/", + want: false, + }, + { + name: "Random string", + contentType: "random123!@#", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isPrintableContentType(tt.contentType); got != tt.want { + t.Errorf("isPrintableContentType() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_logResponseBody(t *testing.T) { + tests := []struct { + name string + resp *http.Response + want string + wantData []byte + }{ + { + name: "Nil body", + resp: &http.Response{ + Body: nil, + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, + want: " No response body to print", + }, + { + name: "No body", + wantData: nil, + resp: &http.Response{ + Body: http.NoBody, + ContentLength: 100, // in case of HEAD response, the content length is set but the body is empty + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, + want: " No response body to print", + }, + { + name: "Empty body", + wantData: []byte(""), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte(""))), + ContentLength: 0, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: " Response body is empty", + }, + { + name: "Unknown content length", + wantData: []byte("whatever"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("whatever"))), + ContentLength: -1, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: "whatever", + }, + { + name: "Missing content type header", + wantData: []byte("whatever"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("whatever"))), + ContentLength: 8, + }, + want: " Response body without a content type is not printed", + }, + { + name: "Empty content type header", + wantData: []byte("whatever"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("whatever"))), + ContentLength: 8, + Header: http.Header{"Content-Type": []string{""}}, + }, + want: " Response body without a content type is not printed", + }, + { + name: "Non-printable content type", + wantData: []byte("binary data"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("binary data"))), + ContentLength: 11, + Header: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }, + want: " Response body of content type \"application/octet-stream\" is not printed", + }, + { + name: "Body at the limit", + wantData: bytes.Repeat([]byte("a"), int(payloadSizeLimit)), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), int(payloadSizeLimit)))), + ContentLength: payloadSizeLimit, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: string(bytes.Repeat([]byte("a"), int(payloadSizeLimit))), + }, + { + name: "Body larger than limit", + wantData: bytes.Repeat([]byte("a"), int(payloadSizeLimit+1)), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), int(payloadSizeLimit+1)))), // 1 byte larger than limit + ContentLength: payloadSizeLimit + 1, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: string(bytes.Repeat([]byte("a"), int(payloadSizeLimit))) + "\n...(truncated)", + }, + { + name: "Printable content type within limit", + wantData: []byte("data"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("data"))), + ContentLength: 4, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: "data", + }, + { + name: "Actual body size is larger than content length", + wantData: []byte("data"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("data"))), + ContentLength: 3, // mismatched content length + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: "data", + }, + { + name: "Actual body size is larger than content length and exceeds limit", + wantData: bytes.Repeat([]byte("a"), int(payloadSizeLimit+1)), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), int(payloadSizeLimit+1)))), // 1 byte larger than limit + ContentLength: 1, // mismatched content length + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: string(bytes.Repeat([]byte("a"), int(payloadSizeLimit))) + "\n...(truncated)", + }, + { + name: "Actual body size is smaller than content length", + wantData: []byte("data"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("data"))), + ContentLength: 5, // mismatched content length + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: "data", + }, + { + name: "Body contains token", + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte(`{"token":"12345"}`))), + ContentLength: 17, + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, + wantData: []byte(`{"token":"12345"}`), + want: " Response body redacted due to potential credentials", + }, + { + name: "Body contains access_token", + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte(`{"access_token":"12345"}`))), + ContentLength: 17, + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, + wantData: []byte(`{"access_token":"12345"}`), + want: " Response body redacted due to potential credentials", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := logResponseBody(tt.resp); got != tt.want { + t.Errorf("logResponseBody() = %v, want %v", got, tt.want) + } + // validate the response body + if tt.resp.Body != nil { + readBytes, err := io.ReadAll(tt.resp.Body) + if err != nil { + t.Errorf("failed to read body after logResponseBody(), err= %v", err) + } + if !bytes.Equal(readBytes, tt.wantData) { + t.Errorf("resp.Body after logResponseBody() = %v, want %v", readBytes, tt.wantData) + } + if closeErr := tt.resp.Body.Close(); closeErr != nil { + t.Errorf("failed to close body after logResponseBody(), err= %v", closeErr) + } + } + }) + } +} + +func Test_logResponseBody_error(t *testing.T) { + tests := []struct { + name string + resp *http.Response + want string + }{ + { + name: "Error reading body", + resp: &http.Response{ + Body: io.NopCloser(&errorReader{}), + ContentLength: 10, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: " Error reading response body: mock read error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := logResponseBody(tt.resp); got != tt.want { + t.Errorf("logResponseBody() = %v, want %v", got, tt.want) + } + if closeErr := tt.resp.Body.Close(); closeErr != nil { + t.Errorf("failed to close body after logResponseBody(), err= %v", closeErr) + } + }) + } +} + +func Test_containsCredentials(t *testing.T) { + tests := []struct { + name string + body string + want bool + }{ + { + name: "Contains token keyword", + body: `{"token": "12345"}`, + want: true, + }, + { + name: "Contains quoted token keyword", + body: `whatever "token" blah`, + want: true, + }, + { + name: "Contains unquoted token keyword", + body: `whatever token blah`, + want: false, + }, + { + name: "Contains access_token keyword", + body: `{"access_token": "12345"}`, + want: true, + }, + { + name: "Contains quoted access_token keyword", + body: `whatever "access_token" blah`, + want: true, + }, + { + name: "Contains unquoted access_token keyword", + body: `whatever access_token blah`, + want: false, + }, + { + name: "Does not contain credentials", + body: `{"key": "value"}`, + want: false, + }, + { + name: "Empty body", + body: ``, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := containsCredentials(tt.body); got != tt.want { + t.Errorf("containsCredentials() = %v, want %v", got, tt.want) + } + }) + } +} From 56a2bb4188dea25710e01ce8eb5321665b171069 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Sun, 27 Apr 2025 22:22:57 +0200 Subject: [PATCH 40/46] chore: enable usetesting linter Signed-off-by: Matthieu MOREL --- .golangci.yml | 1 + internal/test/ensure/ensure.go | 14 +++---- pkg/action/install_test.go | 6 +-- pkg/action/upgrade_test.go | 19 ++++------ pkg/cli/environment_test.go | 8 +--- pkg/cmd/create_test.go | 19 +++++----- pkg/cmd/helpers_test.go | 12 ------ pkg/cmd/package_test.go | 4 +- pkg/cmd/plugin_test.go | 5 +-- pkg/cmd/repo_add_test.go | 6 +-- pkg/cmd/root_test.go | 2 +- pkg/gates/gates_test.go | 3 +- pkg/helmpath/home_unix_test.go | 9 ++--- pkg/helmpath/lazypath_unix_test.go | 13 ++----- pkg/kube/ready_test.go | 60 +++++++++++++++--------------- pkg/plugin/installer/base_test.go | 4 +- pkg/postrender/exec_test.go | 8 +--- pkg/provenance/sign_test.go | 2 +- pkg/registry/client_test.go | 3 +- pkg/repo/chartrepo_test.go | 2 +- pkg/repo/repo_test.go | 2 +- pkg/repo/repotest/server.go | 16 +++----- 22 files changed, 87 insertions(+), 131 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 156dd0509..a9b13c35f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -30,6 +30,7 @@ linters: - thelper - unused - usestdlibvars + - usetesting exclusions: generated: lax diff --git a/internal/test/ensure/ensure.go b/internal/test/ensure/ensure.go index c131e6da5..a72f48c2d 100644 --- a/internal/test/ensure/ensure.go +++ b/internal/test/ensure/ensure.go @@ -29,12 +29,12 @@ import ( func HelmHome(t *testing.T) { t.Helper() base := t.TempDir() - os.Setenv(xdg.CacheHomeEnvVar, base) - os.Setenv(xdg.ConfigHomeEnvVar, base) - os.Setenv(xdg.DataHomeEnvVar, base) - os.Setenv(helmpath.CacheHomeEnvVar, "") - os.Setenv(helmpath.ConfigHomeEnvVar, "") - os.Setenv(helmpath.DataHomeEnvVar, "") + t.Setenv(xdg.CacheHomeEnvVar, base) + t.Setenv(xdg.ConfigHomeEnvVar, base) + t.Setenv(xdg.DataHomeEnvVar, base) + t.Setenv(helmpath.CacheHomeEnvVar, "") + t.Setenv(helmpath.ConfigHomeEnvVar, "") + t.Setenv(helmpath.DataHomeEnvVar, "") } // TempFile ensures a temp file for unit testing purposes. @@ -49,7 +49,7 @@ func TempFile(t *testing.T, name string, data []byte) string { t.Helper() path := t.TempDir() filename := filepath.Join(path, name) - if err := os.WriteFile(filename, data, 0755); err != nil { + if err := os.WriteFile(filename, data, 0o755); err != nil { t.Fatal(err) } return path diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index dabd57b22..7a88d82b5 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -131,7 +131,7 @@ func TestInstallRelease(t *testing.T) { instAction := installAction(t) vals := map[string]interface{}{} - ctx, done := context.WithCancel(context.Background()) + ctx, done := context.WithCancel(t.Context()) res, err := instAction.RunWithContext(ctx, buildChart(), vals) if err != nil { t.Fatalf("Failed install: %s", err) @@ -557,7 +557,7 @@ func TestInstallRelease_Wait_Interrupted(t *testing.T) { instAction.WaitStrategy = kube.StatusWatcherStrategy vals := map[string]interface{}{} - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(t.Context()) time.AfterFunc(time.Second, cancel) goroutines := runtime.NumGoroutine() @@ -641,7 +641,7 @@ func TestInstallRelease_Atomic_Interrupted(t *testing.T) { instAction.Atomic = true vals := map[string]interface{}{} - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(t.Context()) time.AfterFunc(time.Second, cancel) goroutines := runtime.NumGoroutine() diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 4476bc44d..e20955560 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -57,7 +57,7 @@ func TestUpgradeRelease_Success(t *testing.T) { upAction.WaitStrategy = kube.StatusWatcherStrategy vals := map[string]interface{}{} - ctx, done := context.WithCancel(context.Background()) + ctx, done := context.WithCancel(t.Context()) res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) done() req.NoError(err) @@ -384,7 +384,6 @@ func TestUpgradeRelease_Pending(t *testing.T) { } func TestUpgradeRelease_Interrupted_Wait(t *testing.T) { - is := assert.New(t) req := require.New(t) @@ -400,8 +399,7 @@ func TestUpgradeRelease_Interrupted_Wait(t *testing.T) { upAction.WaitStrategy = kube.StatusWatcherStrategy vals := map[string]interface{}{} - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) + ctx, cancel := context.WithCancel(t.Context()) time.AfterFunc(time.Second, cancel) res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) @@ -409,11 +407,9 @@ func TestUpgradeRelease_Interrupted_Wait(t *testing.T) { req.Error(err) is.Contains(res.Info.Description, "Upgrade \"interrupted-release\" failed: context canceled") is.Equal(res.Info.Status, release.StatusFailed) - } func TestUpgradeRelease_Interrupted_Atomic(t *testing.T) { - is := assert.New(t) req := require.New(t) @@ -429,8 +425,7 @@ func TestUpgradeRelease_Interrupted_Atomic(t *testing.T) { upAction.Atomic = true vals := map[string]interface{}{} - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) + ctx, cancel := context.WithCancel(t.Context()) time.AfterFunc(time.Second, cancel) res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) @@ -446,7 +441,7 @@ func TestUpgradeRelease_Interrupted_Atomic(t *testing.T) { } func TestMergeCustomLabels(t *testing.T) { - var tests = [][3]map[string]string{ + tests := [][3]map[string]string{ {nil, nil, map[string]string{}}, {map[string]string{}, map[string]string{}, map[string]string{}}, {map[string]string{"k1": "v1", "k2": "v2"}, nil, map[string]string{"k1": "v1", "k2": "v2"}}, @@ -551,7 +546,7 @@ func TestUpgradeRelease_DryRun(t *testing.T) { upAction.DryRun = true vals := map[string]interface{}{} - ctx, done := context.WithCancel(context.Background()) + ctx, done := context.WithCancel(t.Context()) res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(withSampleSecret()), vals) done() req.NoError(err) @@ -567,7 +562,7 @@ func TestUpgradeRelease_DryRun(t *testing.T) { upAction.HideSecret = true vals = map[string]interface{}{} - ctx, done = context.WithCancel(context.Background()) + ctx, done = context.WithCancel(t.Context()) res, err = upAction.RunWithContext(ctx, rel.Name, buildChart(withSampleSecret()), vals) done() req.NoError(err) @@ -583,7 +578,7 @@ func TestUpgradeRelease_DryRun(t *testing.T) { upAction.DryRun = false vals = map[string]interface{}{} - ctx, done = context.WithCancel(context.Background()) + ctx, done = context.WithCancel(t.Context()) _, err = upAction.RunWithContext(ctx, rel.Name, buildChart(withSampleSecret()), vals) done() req.Error(err) diff --git a/pkg/cli/environment_test.go b/pkg/cli/environment_test.go index 8a3b87936..52326eeff 100644 --- a/pkg/cli/environment_test.go +++ b/pkg/cli/environment_test.go @@ -38,7 +38,6 @@ func TestSetNamespace(t *testing.T) { if settings.namespace != "testns" { t.Errorf("Expected namespace testns, got %s", settings.namespace) } - } func TestEnvSettings(t *testing.T) { @@ -126,7 +125,7 @@ func TestEnvSettings(t *testing.T) { defer resetEnv()() for k, v := range tt.envvars { - os.Setenv(k, v) + t.Setenv(k, v) } flags := pflag.NewFlagSet("testing", pflag.ContinueOnError) @@ -233,10 +232,7 @@ func TestEnvOrBool(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if tt.env != "" { - t.Cleanup(func() { - os.Unsetenv(tt.env) - }) - os.Setenv(tt.env, tt.val) + t.Setenv(tt.env, tt.val) } actual := envBoolOr(tt.env, tt.def) if actual != tt.expected { diff --git a/pkg/cmd/create_test.go b/pkg/cmd/create_test.go index 26eabbfc3..103cd3bc0 100644 --- a/pkg/cmd/create_test.go +++ b/pkg/cmd/create_test.go @@ -33,7 +33,7 @@ func TestCreateCmd(t *testing.T) { ensure.HelmHome(t) cname := "testchart" dir := t.TempDir() - defer testChdir(t, dir)() + defer t.Chdir(dir) // Run a create if _, _, err := executeActionCommand("create " + cname); err != nil { @@ -64,19 +64,19 @@ func TestCreateStarterCmd(t *testing.T) { ensure.HelmHome(t) cname := "testchart" defer resetEnv()() - os.MkdirAll(helmpath.CachePath(), 0755) - defer testChdir(t, helmpath.CachePath())() + os.MkdirAll(helmpath.CachePath(), 0o755) + defer t.Chdir(helmpath.CachePath()) // Create a starter. starterchart := helmpath.DataPath("starters") - os.MkdirAll(starterchart, 0755) + os.MkdirAll(starterchart, 0o755) if dest, err := chartutil.Create("starterchart", starterchart); err != nil { t.Fatalf("Could not create chart: %s", err) } else { t.Logf("Created %s", dest) } tplpath := filepath.Join(starterchart, "starterchart", "templates", "foo.tpl") - if err := os.WriteFile(tplpath, []byte("test"), 0644); err != nil { + if err := os.WriteFile(tplpath, []byte("test"), 0o644); err != nil { t.Fatalf("Could not write template: %s", err) } @@ -122,7 +122,6 @@ func TestCreateStarterCmd(t *testing.T) { if !found { t.Error("Did not find foo.tpl") } - } func TestCreateStarterAbsoluteCmd(t *testing.T) { @@ -132,19 +131,19 @@ func TestCreateStarterAbsoluteCmd(t *testing.T) { // Create a starter. starterchart := helmpath.DataPath("starters") - os.MkdirAll(starterchart, 0755) + os.MkdirAll(starterchart, 0o755) if dest, err := chartutil.Create("starterchart", starterchart); err != nil { t.Fatalf("Could not create chart: %s", err) } else { t.Logf("Created %s", dest) } tplpath := filepath.Join(starterchart, "starterchart", "templates", "foo.tpl") - if err := os.WriteFile(tplpath, []byte("test"), 0644); err != nil { + if err := os.WriteFile(tplpath, []byte("test"), 0o644); err != nil { t.Fatalf("Could not write template: %s", err) } - os.MkdirAll(helmpath.CachePath(), 0755) - defer testChdir(t, helmpath.CachePath())() + os.MkdirAll(helmpath.CachePath(), 0o755) + defer t.Chdir(helmpath.CachePath()) starterChartPath := filepath.Join(starterchart, "starterchart") diff --git a/pkg/cmd/helpers_test.go b/pkg/cmd/helpers_test.go index b48f802b5..5d71fecad 100644 --- a/pkg/cmd/helpers_test.go +++ b/pkg/cmd/helpers_test.go @@ -149,15 +149,3 @@ func resetEnv() func() { settings = cli.New() } } - -func testChdir(t *testing.T, dir string) func() { - t.Helper() - old, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - if err := os.Chdir(dir); err != nil { - t.Fatal(err) - } - return func() { os.Chdir(old) } -} diff --git a/pkg/cmd/package_test.go b/pkg/cmd/package_test.go index 54358fc12..b17684aa6 100644 --- a/pkg/cmd/package_test.go +++ b/pkg/cmd/package_test.go @@ -111,9 +111,9 @@ func TestPackage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cachePath := t.TempDir() - defer testChdir(t, cachePath)() + defer t.Chdir(cachePath) - if err := os.MkdirAll("toot", 0777); err != nil { + if err := os.MkdirAll("toot", 0o777); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/plugin_test.go b/pkg/cmd/plugin_test.go index bc0f7de48..74f7a276a 100644 --- a/pkg/cmd/plugin_test.go +++ b/pkg/cmd/plugin_test.go @@ -79,7 +79,6 @@ func TestManuallyProcessArgs(t *testing.T) { t.Errorf("expected unknown flag %d to be %q, got %q", i, expectUnknown[i], k) } } - } func TestLoadPlugins(t *testing.T) { @@ -327,7 +326,6 @@ func checkCommand(t *testing.T, plugins []*cobra.Command, tests []staticCompleti } func TestPluginDynamicCompletion(t *testing.T) { - tests := []cmdTestCase{{ name: "completion for plugin", cmd: "__complete args ''", @@ -364,7 +362,7 @@ func TestLoadPlugins_HelmNoPlugins(t *testing.T) { settings.PluginsDirectory = "testdata/helmhome/helm/plugins" settings.RepositoryConfig = "testdata/helmhome/helm/repository" - os.Setenv("HELM_NO_PLUGINS", "1") + t.Setenv("HELM_NO_PLUGINS", "1") out := bytes.NewBuffer(nil) cmd := &cobra.Command{} @@ -377,7 +375,6 @@ func TestLoadPlugins_HelmNoPlugins(t *testing.T) { } func TestPluginCmdsCompletion(t *testing.T) { - tests := []cmdTestCase{{ name: "completion for plugin update", cmd: "__complete plugin update ''", diff --git a/pkg/cmd/repo_add_test.go b/pkg/cmd/repo_add_test.go index cfa610611..aa6c4eaad 100644 --- a/pkg/cmd/repo_add_test.go +++ b/pkg/cmd/repo_add_test.go @@ -50,7 +50,7 @@ func TestRepoAddCmd(t *testing.T) { defer srv2.Stop() tmpdir := filepath.Join(t.TempDir(), "path-component.yaml/data") - if err := os.MkdirAll(tmpdir, 0777); err != nil { + if err := os.MkdirAll(tmpdir, 0o777); err != nil { t.Fatal(err) } repoFile := filepath.Join(tmpdir, "repositories.yaml") @@ -99,7 +99,7 @@ func TestRepoAdd(t *testing.T) { forceUpdate: false, repoFile: repoFile, } - os.Setenv(xdg.CacheHomeEnvVar, rootDir) + t.Setenv(xdg.CacheHomeEnvVar, rootDir) if err := o.run(io.Discard); err != nil { t.Error(err) @@ -153,7 +153,7 @@ func TestRepoAddCheckLegalName(t *testing.T) { forceUpdate: false, repoFile: repoFile, } - os.Setenv(xdg.CacheHomeEnvVar, rootDir) + t.Setenv(xdg.CacheHomeEnvVar, rootDir) wantErrorMsg := fmt.Sprintf("repository name (%s) contains '/', please specify a different name without '/'", testRepoName) diff --git a/pkg/cmd/root_test.go b/pkg/cmd/root_test.go index 9521a5aa2..84e3d9ed2 100644 --- a/pkg/cmd/root_test.go +++ b/pkg/cmd/root_test.go @@ -80,7 +80,7 @@ func TestRootCmd(t *testing.T) { ensure.HelmHome(t) for k, v := range tt.envvars { - os.Setenv(k, v) + t.Setenv(k, v) } if _, _, err := executeActionCommand(tt.args); err != nil { diff --git a/pkg/gates/gates_test.go b/pkg/gates/gates_test.go index 6bdd17ed6..4d77199e6 100644 --- a/pkg/gates/gates_test.go +++ b/pkg/gates/gates_test.go @@ -23,14 +23,13 @@ import ( const name string = "HELM_EXPERIMENTAL_FEATURE" func TestIsEnabled(t *testing.T) { - os.Unsetenv(name) g := Gate(name) if g.IsEnabled() { t.Errorf("feature gate shows as available, but the environment variable %s was not set", name) } - os.Setenv(name, "1") + t.Setenv(name, "1") if !g.IsEnabled() { t.Errorf("feature gate shows as disabled, but the environment variable %s was set", name) diff --git a/pkg/helmpath/home_unix_test.go b/pkg/helmpath/home_unix_test.go index 6e4189bc9..a64c9bcd6 100644 --- a/pkg/helmpath/home_unix_test.go +++ b/pkg/helmpath/home_unix_test.go @@ -16,7 +16,6 @@ package helmpath import ( - "os" "runtime" "testing" @@ -24,9 +23,9 @@ import ( ) func TestHelmHome(t *testing.T) { - os.Setenv(xdg.CacheHomeEnvVar, "/cache") - os.Setenv(xdg.ConfigHomeEnvVar, "/config") - os.Setenv(xdg.DataHomeEnvVar, "/data") + t.Setenv(xdg.CacheHomeEnvVar, "/cache") + t.Setenv(xdg.ConfigHomeEnvVar, "/config") + t.Setenv(xdg.DataHomeEnvVar, "/data") isEq := func(t *testing.T, got, expected string) { t.Helper() if expected != got { @@ -40,7 +39,7 @@ func TestHelmHome(t *testing.T) { isEq(t, DataPath(), "/data/helm") // test to see if lazy-loading environment variables at runtime works - os.Setenv(xdg.CacheHomeEnvVar, "/cache2") + t.Setenv(xdg.CacheHomeEnvVar, "/cache2") isEq(t, CachePath(), "/cache2/helm") } diff --git a/pkg/helmpath/lazypath_unix_test.go b/pkg/helmpath/lazypath_unix_test.go index 534735d10..4b0f2429b 100644 --- a/pkg/helmpath/lazypath_unix_test.go +++ b/pkg/helmpath/lazypath_unix_test.go @@ -16,7 +16,6 @@ package helmpath import ( - "os" "path/filepath" "testing" @@ -32,15 +31,13 @@ const ( ) func TestDataPath(t *testing.T) { - os.Unsetenv(xdg.DataHomeEnvVar) - expected := filepath.Join(homedir.HomeDir(), ".local", "share", appName, testFile) if lazy.dataPath(testFile) != expected { t.Errorf("expected '%s', got '%s'", expected, lazy.dataPath(testFile)) } - os.Setenv(xdg.DataHomeEnvVar, "/tmp") + t.Setenv(xdg.DataHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) @@ -50,15 +47,13 @@ func TestDataPath(t *testing.T) { } func TestConfigPath(t *testing.T) { - os.Unsetenv(xdg.ConfigHomeEnvVar) - expected := filepath.Join(homedir.HomeDir(), ".config", appName, testFile) if lazy.configPath(testFile) != expected { t.Errorf("expected '%s', got '%s'", expected, lazy.configPath(testFile)) } - os.Setenv(xdg.ConfigHomeEnvVar, "/tmp") + t.Setenv(xdg.ConfigHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) @@ -68,15 +63,13 @@ func TestConfigPath(t *testing.T) { } func TestCachePath(t *testing.T) { - os.Unsetenv(xdg.CacheHomeEnvVar) - expected := filepath.Join(homedir.HomeDir(), ".cache", appName, testFile) if lazy.cachePath(testFile) != expected { t.Errorf("expected '%s', got '%s'", expected, lazy.cachePath(testFile)) } - os.Setenv(xdg.CacheHomeEnvVar, "/tmp") + t.Setenv(xdg.CacheHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) diff --git a/pkg/kube/ready_test.go b/pkg/kube/ready_test.go index 9d1dfd272..db0d02cbe 100644 --- a/pkg/kube/ready_test.go +++ b/pkg/kube/ready_test.go @@ -60,7 +60,7 @@ func Test_ReadyChecker_IsReady_Pod(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.Pod{}, Name: "foo", Namespace: defaultNamespace}, }, pod: newPodWithCondition("foo", corev1.ConditionTrue), @@ -75,7 +75,7 @@ func Test_ReadyChecker_IsReady_Pod(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.Pod{}, Name: "foo", Namespace: defaultNamespace}, }, pod: newPodWithCondition("bar", corev1.ConditionTrue), @@ -90,7 +90,7 @@ func Test_ReadyChecker_IsReady_Pod(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.CoreV1().Pods(defaultNamespace).Create(context.TODO(), tt.pod, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().Pods(defaultNamespace).Create(t.Context(), tt.pod, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Pod error: %v", err) return } @@ -132,7 +132,7 @@ func Test_ReadyChecker_IsReady_Job(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &batchv1.Job{}, Name: "foo", Namespace: defaultNamespace}, }, job: newJob("bar", 1, intToInt32(1), 1, 0), @@ -147,7 +147,7 @@ func Test_ReadyChecker_IsReady_Job(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &batchv1.Job{}, Name: "foo", Namespace: defaultNamespace}, }, job: newJob("foo", 1, intToInt32(1), 1, 0), @@ -162,7 +162,7 @@ func Test_ReadyChecker_IsReady_Job(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.BatchV1().Jobs(defaultNamespace).Create(context.TODO(), tt.job, metav1.CreateOptions{}); err != nil { + if _, err := c.client.BatchV1().Jobs(defaultNamespace).Create(t.Context(), tt.job, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Job error: %v", err) return } @@ -204,7 +204,7 @@ func Test_ReadyChecker_IsReady_Deployment(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.Deployment{}, Name: "foo", Namespace: defaultNamespace}, }, replicaSet: newReplicaSet("foo", 0, 0, true), @@ -220,7 +220,7 @@ func Test_ReadyChecker_IsReady_Deployment(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.Deployment{}, Name: "foo", Namespace: defaultNamespace}, }, replicaSet: newReplicaSet("foo", 0, 0, true), @@ -236,11 +236,11 @@ func Test_ReadyChecker_IsReady_Deployment(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.AppsV1().Deployments(defaultNamespace).Create(context.TODO(), tt.deployment, metav1.CreateOptions{}); err != nil { + if _, err := c.client.AppsV1().Deployments(defaultNamespace).Create(t.Context(), tt.deployment, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Deployment error: %v", err) return } - if _, err := c.client.AppsV1().ReplicaSets(defaultNamespace).Create(context.TODO(), tt.replicaSet, metav1.CreateOptions{}); err != nil { + if _, err := c.client.AppsV1().ReplicaSets(defaultNamespace).Create(t.Context(), tt.replicaSet, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create ReplicaSet error: %v", err) return } @@ -281,7 +281,7 @@ func Test_ReadyChecker_IsReady_PersistentVolumeClaim(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.PersistentVolumeClaim{}, Name: "foo", Namespace: defaultNamespace}, }, pvc: newPersistentVolumeClaim("foo", corev1.ClaimPending), @@ -296,7 +296,7 @@ func Test_ReadyChecker_IsReady_PersistentVolumeClaim(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.PersistentVolumeClaim{}, Name: "foo", Namespace: defaultNamespace}, }, pvc: newPersistentVolumeClaim("bar", corev1.ClaimPending), @@ -311,7 +311,7 @@ func Test_ReadyChecker_IsReady_PersistentVolumeClaim(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.CoreV1().PersistentVolumeClaims(defaultNamespace).Create(context.TODO(), tt.pvc, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().PersistentVolumeClaims(defaultNamespace).Create(t.Context(), tt.pvc, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create PersistentVolumeClaim error: %v", err) return } @@ -352,7 +352,7 @@ func Test_ReadyChecker_IsReady_Service(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.Service{}, Name: "foo", Namespace: defaultNamespace}, }, svc: newService("foo", corev1.ServiceSpec{Type: corev1.ServiceTypeLoadBalancer, ClusterIP: ""}), @@ -367,7 +367,7 @@ func Test_ReadyChecker_IsReady_Service(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.Service{}, Name: "foo", Namespace: defaultNamespace}, }, svc: newService("bar", corev1.ServiceSpec{Type: corev1.ServiceTypeExternalName, ClusterIP: ""}), @@ -382,7 +382,7 @@ func Test_ReadyChecker_IsReady_Service(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.CoreV1().Services(defaultNamespace).Create(context.TODO(), tt.svc, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().Services(defaultNamespace).Create(t.Context(), tt.svc, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Service error: %v", err) return } @@ -423,7 +423,7 @@ func Test_ReadyChecker_IsReady_DaemonSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.DaemonSet{}, Name: "foo", Namespace: defaultNamespace}, }, ds: newDaemonSet("foo", 0, 0, 1, 0, true), @@ -438,7 +438,7 @@ func Test_ReadyChecker_IsReady_DaemonSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.DaemonSet{}, Name: "foo", Namespace: defaultNamespace}, }, ds: newDaemonSet("bar", 0, 1, 1, 1, true), @@ -453,7 +453,7 @@ func Test_ReadyChecker_IsReady_DaemonSet(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.AppsV1().DaemonSets(defaultNamespace).Create(context.TODO(), tt.ds, metav1.CreateOptions{}); err != nil { + if _, err := c.client.AppsV1().DaemonSets(defaultNamespace).Create(t.Context(), tt.ds, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create DaemonSet error: %v", err) return } @@ -494,7 +494,7 @@ func Test_ReadyChecker_IsReady_StatefulSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.StatefulSet{}, Name: "foo", Namespace: defaultNamespace}, }, ss: newStatefulSet("foo", 1, 0, 0, 1, true), @@ -509,7 +509,7 @@ func Test_ReadyChecker_IsReady_StatefulSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.StatefulSet{}, Name: "foo", Namespace: defaultNamespace}, }, ss: newStatefulSet("bar", 1, 0, 1, 1, true), @@ -524,7 +524,7 @@ func Test_ReadyChecker_IsReady_StatefulSet(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.AppsV1().StatefulSets(defaultNamespace).Create(context.TODO(), tt.ss, metav1.CreateOptions{}); err != nil { + if _, err := c.client.AppsV1().StatefulSets(defaultNamespace).Create(t.Context(), tt.ss, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create StatefulSet error: %v", err) return } @@ -565,7 +565,7 @@ func Test_ReadyChecker_IsReady_ReplicationController(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.ReplicationController{}, Name: "foo", Namespace: defaultNamespace}, }, rc: newReplicationController("foo", false), @@ -580,7 +580,7 @@ func Test_ReadyChecker_IsReady_ReplicationController(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.ReplicationController{}, Name: "foo", Namespace: defaultNamespace}, }, rc: newReplicationController("bar", false), @@ -595,7 +595,7 @@ func Test_ReadyChecker_IsReady_ReplicationController(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.ReplicationController{}, Name: "foo", Namespace: defaultNamespace}, }, rc: newReplicationController("foo", true), @@ -610,7 +610,7 @@ func Test_ReadyChecker_IsReady_ReplicationController(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.CoreV1().ReplicationControllers(defaultNamespace).Create(context.TODO(), tt.rc, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().ReplicationControllers(defaultNamespace).Create(t.Context(), tt.rc, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create ReplicationController error: %v", err) return } @@ -651,7 +651,7 @@ func Test_ReadyChecker_IsReady_ReplicaSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.ReplicaSet{}, Name: "foo", Namespace: defaultNamespace}, }, rs: newReplicaSet("foo", 1, 1, true), @@ -666,7 +666,7 @@ func Test_ReadyChecker_IsReady_ReplicaSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.ReplicaSet{}, Name: "foo", Namespace: defaultNamespace}, }, rs: newReplicaSet("bar", 1, 1, false), @@ -1014,12 +1014,12 @@ func Test_ReadyChecker_podsReadyForObject(t *testing.T) { t.Run(tt.name, func(t *testing.T) { c := NewReadyChecker(fake.NewClientset()) for _, pod := range tt.existPods { - if _, err := c.client.CoreV1().Pods(defaultNamespace).Create(context.TODO(), &pod, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().Pods(defaultNamespace).Create(t.Context(), &pod, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Pod error: %v", err) return } } - got, err := c.podsReadyForObject(context.TODO(), tt.args.namespace, tt.args.obj) + got, err := c.podsReadyForObject(t.Context(), tt.args.namespace, tt.args.obj) if (err != nil) != tt.wantErr { t.Errorf("podsReadyForObject() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/plugin/installer/base_test.go b/pkg/plugin/installer/base_test.go index f4dd6d6be..732ac7927 100644 --- a/pkg/plugin/installer/base_test.go +++ b/pkg/plugin/installer/base_test.go @@ -14,7 +14,6 @@ limitations under the License. package installer // import "helm.sh/helm/v4/pkg/plugin/installer" import ( - "os" "testing" ) @@ -37,12 +36,11 @@ func TestPath(t *testing.T) { for _, tt := range tests { - os.Setenv("HELM_PLUGINS", tt.helmPluginsDir) + t.Setenv("HELM_PLUGINS", tt.helmPluginsDir) baseIns := newBase(tt.source) baseInsPath := baseIns.Path() if baseInsPath != tt.expectPath { t.Errorf("expected name %s, got %s", tt.expectPath, baseInsPath) } - os.Unsetenv("HELM_PLUGINS") } } diff --git a/pkg/postrender/exec_test.go b/pkg/postrender/exec_test.go index 2b091cc12..a10ad2cc4 100644 --- a/pkg/postrender/exec_test.go +++ b/pkg/postrender/exec_test.go @@ -60,11 +60,7 @@ func TestGetFullPath(t *testing.T) { t.Run("binary in PATH resolves correctly", func(t *testing.T) { testpath := setupTestingScript(t) - realPath := os.Getenv("PATH") - os.Setenv("PATH", filepath.Dir(testpath)) - defer func() { - os.Setenv("PATH", realPath) - }() + t.Setenv("PATH", filepath.Dir(testpath)) fullPath, err := getFullPath(filepath.Base(testpath)) is.NoError(err) @@ -183,7 +179,7 @@ func setupTestingScript(t *testing.T) (filepath string) { t.Fatalf("unable to write tempfile for testing: %s", err) } - err = f.Chmod(0755) + err = f.Chmod(0o755) if err != nil { t.Fatalf("unable to make tempfile executable for testing: %s", err) } diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index 69a6dad5b..9a60fd19c 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -276,7 +276,7 @@ func TestDecodeSignature(t *testing.T) { t.Fatal(err) } - f, err := os.CreateTemp("", "helm-test-sig-") + f, err := os.CreateTemp(t.TempDir(), "helm-test-sig-") if err != nil { t.Fatal(err) } diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 8fc392336..2ffd691c2 100644 --- a/pkg/registry/client_test.go +++ b/pkg/registry/client_test.go @@ -17,7 +17,6 @@ limitations under the License. package registry import ( - "context" "io" "testing" @@ -31,7 +30,7 @@ import ( func TestTagManifestTransformsReferences(t *testing.T) { memStore := memory.New() client := &Client{out: io.Discard} - ctx := context.Background() + ctx := t.Context() refWithPlus := "test-registry.io/charts/test:1.0.0+metadata" expectedRef := "test-registry.io/charts/test:1.0.0_metadata" // + becomes _ diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index bc15560c9..05e034dd8 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -70,7 +70,7 @@ func TestIndexCustomSchemeDownload(t *testing.T) { } repo.CachePath = t.TempDir() - tempIndexFile, err := os.CreateTemp("", "test-repo") + tempIndexFile, err := os.CreateTemp(t.TempDir(), "test-repo") if err != nil { t.Fatalf("Failed to create temp index file: %v", err) } diff --git a/pkg/repo/repo_test.go b/pkg/repo/repo_test.go index c2087ebbe..bdaa61eda 100644 --- a/pkg/repo/repo_test.go +++ b/pkg/repo/repo_test.go @@ -197,7 +197,7 @@ func TestWriteFile(t *testing.T) { }, ) - file, err := os.CreateTemp("", "helm-repo") + file, err := os.CreateTemp(t.TempDir(), "helm-repo") if err != nil { t.Errorf("failed to create test-file (%v)", err) } diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index b366572d8..ea9d5290c 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -16,7 +16,6 @@ limitations under the License. package repotest import ( - "context" "crypto/tls" "fmt" "net/http" @@ -91,10 +90,7 @@ type Server struct { // The temp dir will be removed by testing package automatically when test finished. func NewTempServer(t *testing.T, options ...ServerOption) *Server { t.Helper() - docrootTempDir, err := os.MkdirTemp("", "helm-repotest-") - if err != nil { - t.Fatal(err) - } + docrootTempDir := t.TempDir() srv := newServer(t, docrootTempDir, options...) @@ -173,7 +169,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { t.Fatal("error generating bcrypt password for test htpasswd file") } htpasswdPath := filepath.Join(dir, testHtpasswdFileBasename) - err = os.WriteFile(htpasswdPath, []byte(fmt.Sprintf("%s:%s\n", testUsername, string(pwBytes))), 0644) + err = os.WriteFile(htpasswdPath, []byte(fmt.Sprintf("%s:%s\n", testUsername, string(pwBytes))), 0o644) if err != nil { t.Fatalf("error creating test htpasswd file") } @@ -197,7 +193,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { registryURL := fmt.Sprintf("localhost:%d", port) - r, err := registry.NewRegistry(context.Background(), config) + r, err := registry.NewRegistry(t.Context(), config) if err != nil { t.Fatal(err) } @@ -331,7 +327,7 @@ func (s *Server) CopyCharts(origin string) ([]string, error) { if err != nil { return []string{}, err } - if err := os.WriteFile(newname, data, 0644); err != nil { + if err := os.WriteFile(newname, data, 0o644); err != nil { return []string{}, err } copied[i] = newname @@ -355,7 +351,7 @@ func (s *Server) CreateIndex() error { } ifile := filepath.Join(s.docroot, "index.yaml") - return os.WriteFile(ifile, d, 0644) + return os.WriteFile(ifile, d, 0o644) } func (s *Server) start() { @@ -407,5 +403,5 @@ func setTestingRepository(url, fname string) error { Name: "test", URL: url, }) - return r.WriteFile(fname, 0640) + return r.WriteFile(fname, 0o640) } From a8cbf3aa51d98bf75e3f437e84eb084d5370bf16 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 27 May 2025 11:40:46 -0600 Subject: [PATCH 41/46] fix: action hooks delete policy mutex Signed-off-by: Terry Howe --- pkg/action/action.go | 3 +++ pkg/action/hooks.go | 28 +++++++++++++++++++--------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/pkg/action/action.go b/pkg/action/action.go index 6905f3f44..40194dfd7 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -26,6 +26,7 @@ import ( "path" "path/filepath" "strings" + "sync" "text/template" "k8s.io/apimachinery/pkg/api/meta" @@ -86,6 +87,8 @@ type Configuration struct { // HookOutputFunc called with container name and returns and expects writer that will receive the log output. HookOutputFunc func(namespace, pod, container string) io.Writer + + mutex sync.Mutex } // renderResources renders the templates in a chart diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 591371e44..7f265797b 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -49,13 +49,7 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, for i, h := range executingHooks { // Set default delete policy to before-hook-creation - if len(h.DeletePolicies) == 0 { - // TODO(jlegrone): Only apply before-hook-creation delete policy to run to completion - // resources. For all other resource types update in place if a - // resource with the same name already exists and is owned by the - // current release. - h.DeletePolicies = []release.HookDeletePolicy{release.HookBeforeHookCreation} - } + cfg.hookSetDeletePolicy(h) if err := cfg.deleteHookByPolicy(h, release.HookBeforeHookCreation, waitStrategy, timeout); err != nil { return err @@ -154,7 +148,7 @@ func (cfg *Configuration) deleteHookByPolicy(h *release.Hook, policy release.Hoo if h.Kind == "CustomResourceDefinition" { return nil } - if hookHasDeletePolicy(h, policy) { + if cfg.hookHasDeletePolicy(h, policy) { resources, err := cfg.KubeClient.Build(bytes.NewBufferString(h.Manifest), false) if err != nil { return fmt.Errorf("unable to build kubernetes object for deleting hook %s: %w", h.Path, err) @@ -188,10 +182,26 @@ func (cfg *Configuration) deleteHooksByPolicy(hooks []*release.Hook, policy rele // hookHasDeletePolicy determines whether the defined hook deletion policy matches the hook deletion polices // supported by helm. If so, mark the hook as one should be deleted. -func hookHasDeletePolicy(h *release.Hook, policy release.HookDeletePolicy) bool { +func (cfg *Configuration) hookHasDeletePolicy(h *release.Hook, policy release.HookDeletePolicy) bool { + cfg.mutex.Lock() + defer cfg.mutex.Unlock() return slices.Contains(h.DeletePolicies, policy) } +// hookClearDeletePolicy determines whether the defined hook deletion policy matches the hook deletion polices +// supported by helm. If so, mark the hook as one should be deleted. +func (cfg *Configuration) hookSetDeletePolicy(h *release.Hook) { + cfg.mutex.Lock() + defer cfg.mutex.Unlock() + if len(h.DeletePolicies) == 0 { + // TODO(jlegrone): Only apply before-hook-creation delete policy to run to completion + // resources. For all other resource types update in place if a + // resource with the same name already exists and is owned by the + // current release. + h.DeletePolicies = []release.HookDeletePolicy{release.HookBeforeHookCreation} + } +} + // outputLogsByPolicy outputs a pods logs if the hook policy instructs it to func (cfg *Configuration) outputLogsByPolicy(h *release.Hook, releaseNamespace string, policy release.HookOutputLogPolicy) error { if !hookHasOutputLogPolicy(h, policy) { From f8204031f1b557bc2372f498487de654b7f544a4 Mon Sep 17 00:00:00 2001 From: Carlos Lima Date: Wed, 4 Jun 2025 08:40:05 +0800 Subject: [PATCH 42/46] Fix tests deleting XDG_DATA_HOME That includes ~/.local/share/keyrings which was the most immediatelly visible effect. Signed-off-by: Carlos Lima --- pkg/plugin/installer/local_installer_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/plugin/installer/local_installer_test.go b/pkg/plugin/installer/local_installer_test.go index b28920af4..9effcd2c4 100644 --- a/pkg/plugin/installer/local_installer_test.go +++ b/pkg/plugin/installer/local_installer_test.go @@ -20,12 +20,14 @@ import ( "path/filepath" "testing" + "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/helmpath" ) var _ Installer = new(LocalInstaller) func TestLocalInstaller(t *testing.T) { + ensure.HelmHome(t) // Make a temp dir tdir := t.TempDir() if err := os.WriteFile(filepath.Join(tdir, "plugin.yaml"), []byte{}, 0644); err != nil { From 9623fb80f1fe8ec782cd873902913d32b7daab36 Mon Sep 17 00:00:00 2001 From: acceptacross Date: Wed, 4 Jun 2025 23:54:30 +0800 Subject: [PATCH 43/46] chore: fix some function names in comment Signed-off-by: acceptacross --- pkg/action/hooks.go | 2 +- pkg/chart/v2/util/chartfile.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 7f265797b..1213e87e2 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -188,7 +188,7 @@ func (cfg *Configuration) hookHasDeletePolicy(h *release.Hook, policy release.Ho return slices.Contains(h.DeletePolicies, policy) } -// hookClearDeletePolicy determines whether the defined hook deletion policy matches the hook deletion polices +// hookSetDeletePolicy determines whether the defined hook deletion policy matches the hook deletion polices // supported by helm. If so, mark the hook as one should be deleted. func (cfg *Configuration) hookSetDeletePolicy(h *release.Hook) { cfg.mutex.Lock() diff --git a/pkg/chart/v2/util/chartfile.go b/pkg/chart/v2/util/chartfile.go index 6748c6a91..1f9c712b2 100644 --- a/pkg/chart/v2/util/chartfile.go +++ b/pkg/chart/v2/util/chartfile.go @@ -39,7 +39,7 @@ func LoadChartfile(filename string) (*chart.Metadata, error) { return y, err } -// StrictLoadChartFile loads a Chart.yaml into a *chart.Metadata using a strict unmarshaling +// StrictLoadChartfile loads a Chart.yaml into a *chart.Metadata using a strict unmarshaling func StrictLoadChartfile(filename string) (*chart.Metadata, error) { b, err := os.ReadFile(filename) if err != nil { From fee9907a801e1ab904f447f955b94b558c895d8d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 5 Jun 2025 21:58:39 +0000 Subject: [PATCH 44/46] build(deps): bump golang.org/x/text from 0.25.0 to 0.26.0 Bumps [golang.org/x/text](https://github.com/golang/text) from 0.25.0 to 0.26.0. - [Release notes](https://github.com/golang/text/releases) - [Commits](https://github.com/golang/text/compare/v0.25.0...v0.26.0) --- updated-dependencies: - dependency-name: golang.org/x/text dependency-version: 0.26.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- go.mod | 10 +++++----- go.sum | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index f08dfc7a1..eab1612e9 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/stretchr/testify v1.10.0 golang.org/x/crypto v0.38.0 golang.org/x/term v0.32.0 - golang.org/x/text v0.25.0 + golang.org/x/text v0.26.0 gopkg.in/yaml.v3 v3.0.1 k8s.io/api v0.33.1 k8s.io/apiextensions-apiserver v0.33.1 @@ -154,13 +154,13 @@ require ( go.opentelemetry.io/otel/sdk/metric v1.32.0 // indirect go.opentelemetry.io/otel/trace v1.34.0 // indirect go.opentelemetry.io/proto/otlp v1.4.0 // indirect - golang.org/x/mod v0.24.0 // indirect - golang.org/x/net v0.39.0 // indirect + golang.org/x/mod v0.25.0 // indirect + golang.org/x/net v0.40.0 // indirect golang.org/x/oauth2 v0.29.0 // indirect - golang.org/x/sync v0.14.0 // indirect + golang.org/x/sync v0.15.0 // indirect golang.org/x/sys v0.33.0 // indirect golang.org/x/time v0.11.0 // indirect - golang.org/x/tools v0.32.0 // indirect + golang.org/x/tools v0.33.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20241209162323-e6fa225c2576 // indirect google.golang.org/grpc v1.68.1 // indirect diff --git a/go.sum b/go.sum index 7fd86290d..1101b2582 100644 --- a/go.sum +++ b/go.sum @@ -392,8 +392,8 @@ golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91 golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= -golang.org/x/mod v0.24.0 h1:ZfthKaKaT4NrhGVZHO1/WDTwGES4De8KtWO0SIbNJMU= -golang.org/x/mod v0.24.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww= +golang.org/x/mod v0.25.0 h1:n7a+ZbQKQA/Ysbyb0/6IbB1H/X41mKgbhfv7AfG/44w= +golang.org/x/mod v0.25.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= @@ -407,8 +407,8 @@ golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/net v0.18.0/go.mod h1:/czyP5RqHAH4odGYxBJ1qz0+CE5WZ+2j1YgoEo8F2jQ= -golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY= -golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E= +golang.org/x/net v0.40.0 h1:79Xs7wF06Gbdcg4kdCCIQArK11Z1hr5POQ6+fIYHNuY= +golang.org/x/net v0.40.0/go.mod h1:y0hY0exeL2Pku80/zKK7tpntoX23cqL3Oa6njdgRtds= golang.org/x/oauth2 v0.29.0 h1:WdYw2tdTK1S8olAzWHdgeqfy+Mtm9XNhv/xJsY65d98= golang.org/x/oauth2 v0.29.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -421,8 +421,8 @@ golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/sync v0.14.0 h1:woo0S4Yywslg6hp4eUFjTVOyKt0RookbpAHG4c1HmhQ= -golang.org/x/sync v0.14.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8= +golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -462,8 +462,8 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.25.0 h1:qVyWApTSYLk/drJRO5mDlNYskwQznZmkpV2c8q9zls4= -golang.org/x/text v0.25.0/go.mod h1:WEdwpYrmk1qmdHvhkSTNPm3app7v4rsT8F2UD6+VHIA= +golang.org/x/text v0.26.0 h1:P42AVeLghgTYr4+xUnTRKDMqpar+PtX7KWuNQL21L8M= +golang.org/x/text v0.26.0/go.mod h1:QK15LZJUUQVJxhz7wXgxSy/CJaTFjd0G+YLonydOVQA= golang.org/x/time v0.11.0 h1:/bpjEDfN9tkoN/ryeYHnv5hcMlc8ncjMcM4XBk5NWV0= golang.org/x/time v0.11.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -474,8 +474,8 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= golang.org/x/tools v0.15.0/go.mod h1:hpksKq4dtpQWS1uQ61JkdqWM3LscIS6Slf+VVkm+wQk= -golang.org/x/tools v0.32.0 h1:Q7N1vhpkQv7ybVzLFtTjvQya2ewbwNDZzUgfXGqtMWU= -golang.org/x/tools v0.32.0/go.mod h1:ZxrU41P/wAbZD8EDa6dDCa6XfpkhJ7HFMjHJXfBDu8s= +golang.org/x/tools v0.33.0 h1:4qz2S3zmRxbGIhDIAgjxvFutSvH5EfnsYrRBj0UI0bc= +golang.org/x/tools v0.33.0/go.mod h1:CIJMaWEY88juyUfo7UbgPqbC8rU2OqfAV1h2Qp0oMYI= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= From 93ec064640867f2642a293732ffb9b5f4602eb18 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 27 May 2025 11:39:35 -0600 Subject: [PATCH 45/46] fix: repo update cmd mutex Signed-off-by: Terry Howe --- pkg/cmd/repo_update.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/cmd/repo_update.go b/pkg/cmd/repo_update.go index 9f4a603ae..6547446f0 100644 --- a/pkg/cmd/repo_update.go +++ b/pkg/cmd/repo_update.go @@ -113,14 +113,19 @@ func updateCharts(repos []*repo.ChartRepository, out io.Writer) error { var wg sync.WaitGroup failRepoURLChan := make(chan string, len(repos)) + writeMutex := sync.Mutex{} for _, re := range repos { wg.Add(1) go func(re *repo.ChartRepository) { defer wg.Done() if _, err := re.DownloadIndexFile(); err != nil { + writeMutex.Lock() + defer writeMutex.Unlock() fmt.Fprintf(out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", re.Config.Name, re.Config.URL, err) failRepoURLChan <- re.Config.URL } else { + writeMutex.Lock() + defer writeMutex.Unlock() fmt.Fprintf(out, "...Successfully got an update from the %q chart repository\n", re.Config.Name) } }(re) From b250b1de82e4d0381eabb1ddacb559d9c75a0d7a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 6 Jun 2025 17:37:49 +0000 Subject: [PATCH 46/46] build(deps): bump golang.org/x/crypto from 0.38.0 to 0.39.0 Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.38.0 to 0.39.0. - [Commits](https://github.com/golang/crypto/compare/v0.38.0...v0.39.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-version: 0.39.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index eab1612e9..66de5f821 100644 --- a/go.mod +++ b/go.mod @@ -31,7 +31,7 @@ require ( github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.6 github.com/stretchr/testify v1.10.0 - golang.org/x/crypto v0.38.0 + golang.org/x/crypto v0.39.0 golang.org/x/term v0.32.0 golang.org/x/text v0.26.0 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 1101b2582..29ce847c9 100644 --- a/go.sum +++ b/go.sum @@ -384,8 +384,8 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72g= -golang.org/x/crypto v0.38.0 h1:jt+WWG8IZlBnVbomuhg2Mdq0+BBQaHbtqHEFEigjUV8= -golang.org/x/crypto v0.38.0/go.mod h1:MvrbAqul58NNYPKnOra203SB9vpuZW0e+RRZV+Ggqjw= +golang.org/x/crypto v0.39.0 h1:SHs+kF4LP+f+p14esP5jAoDpHU8Gu/v9lFRK6IT5imM= +golang.org/x/crypto v0.39.0/go.mod h1:L+Xg3Wf6HoL4Bn4238Z6ft6KfEpN0tJGo53AAPC632U= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=