diff --git a/internal/chart/v3/lint/rules/values.go b/internal/chart/v3/lint/rules/values.go index a91e70363..9ebf405c4 100644 --- a/internal/chart/v3/lint/rules/values.go +++ b/internal/chart/v3/lint/rules/values.go @@ -23,6 +23,8 @@ import ( "path/filepath" "strings" + yamlv2 "go.yaml.in/yaml/v2" + "helm.sh/helm/v4/internal/chart/v3/lint/support" "helm.sh/helm/v4/pkg/chart/common" "helm.sh/helm/v4/pkg/chart/common/util" @@ -89,10 +91,16 @@ func validateValuesFile(valuesPath string, overrides map[string]any, skipSchemaV return nil } -// isDuplicateKeyError checks if an error is related to duplicate YAML keys +// isDuplicateKeyError checks if an error is related to duplicate YAML keys. func isDuplicateKeyError(err error) bool { - errStr := err.Error() - return strings.Contains(errStr, "already set in map") || - strings.Contains(errStr, "already defined") || - strings.Contains(errStr, "duplicate") + var typeErr *yamlv2.TypeError + if !errors.As(err, &typeErr) { + return false + } + for _, e := range typeErr.Errors { + if strings.Contains(e, "already set in map") { + return true + } + } + return false } diff --git a/internal/chart/v3/lint/rules/values_test.go b/internal/chart/v3/lint/rules/values_test.go index 54c7e6457..0324930ed 100644 --- a/internal/chart/v3/lint/rules/values_test.go +++ b/internal/chart/v3/lint/rules/values_test.go @@ -17,11 +17,14 @@ limitations under the License. package rules import ( + "errors" + "fmt" "os" "path/filepath" "testing" "github.com/stretchr/testify/assert" + yamlv2 "go.yaml.in/yaml/v2" "helm.sh/helm/v4/internal/test/ensure" ) @@ -181,3 +184,58 @@ func createTestingSchema(t *testing.T, dir string) string { } return schemafile } + +func TestIsDuplicateKeyError(t *testing.T) { + tests := []struct { + name string + err error + expected bool + }{ + { + name: "duplicate key error", + err: &yamlv2.TypeError{Errors: []string{`line 2: key "key" already set in map`}}, + expected: true, + }, + { + name: "wrapped duplicate key error", + err: fmt.Errorf("error converting YAML to JSON: %w", &yamlv2.TypeError{Errors: []string{`line 2: key "key" already set in map`}}), + expected: true, + }, + { + name: "non-duplicate key error", + err: errors.New("some other error"), + expected: false, + }, + { + name: "type error but not duplicate-key related", + err: &yamlv2.TypeError{Errors: []string{"cannot unmarshal !!seq into map[string]interface {}"}}, + expected: false, + }, + { + name: "nil error", + err: nil, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isDuplicateKeyError(tt.err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestValidateValuesFileDuplicateKeys(t *testing.T) { + duplicateYaml := `key: value1 +key: value2 +` + tmpdir := ensure.TempFile(t, "values.yaml", []byte(duplicateYaml)) + valfile := filepath.Join(tmpdir, "values.yaml") + + err := validateValuesFile(valfile, map[string]any{}, false) + if err == nil { + t.Fatal("expected values file with duplicate keys to fail parsing") + } + assert.Contains(t, err.Error(), "contains duplicate keys") +}