fix: add duplicate key detection for values.yaml

Add lint rule to detect duplicate keys in values.yaml. This addresses
the issue where Helm allows installing charts with invalid YAML that
contains duplicate keys, which silently takes the last value.

Changes:
- Add ReadValuesFileStrict and ReadValuesStrict functions in
  pkg/chart/common/values.go that use yaml.UnmarshalStrict
- Add validateValuesFileDuplicateKeys function in both v2 and v3
  lint rules to check for duplicate keys
- Add test data with duplicate keys for both v2 and v3
- Add tests to verify duplicate key detection works

Closes #31102

Signed-off-by: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com>

internal/chart/v3/lint/lint_test.go

@@ -37,6 +37,7 @@ const goodChartDir = "rules/testdata/goodone"
 const subChartValuesDir = "rules/testdata/withsubchart"
 const malformedTemplate = "rules/testdata/malformed-template"
 const invalidChartFileDir = "rules/testdata/invalidchartfile"
+const duplicateKeysDir = "rules/testdata/duplicatekeys"
 
 func TestBadChartV3(t *testing.T) {
 	var values map[string]any
@@ -241,3 +242,26 @@ func TestMalformedTemplate(t *testing.T) {
 		}
 	}
 }
+
+// TestDuplicateKeysV3 tests that values.yaml with duplicate keys is detected
+// See https://github.com/helm/helm/issues/31102
+func TestDuplicateKeysV3(t *testing.T) {
+	var values map[string]any
+	m := RunAll(duplicateKeysDir, values, namespace).Messages
+	// Expect at least 1 message (the duplicate keys error)
+	if len(m) < 1 {
+		t.Fatalf("All didn't fail with expected errors, got %#v", m)
+	}
+	// Find the duplicate keys error message
+	found := false
+	for _, msg := range m {
+		if msg.Path == "values.yaml" && msg.Severity == support.ErrorSev &&
+			strings.Contains(msg.Err.Error(), "contains duplicate keys") {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("All didn't have the error for duplicate YAML keys in values.yaml: %v", m)
+	}
+}

internal/chart/v3/lint/rules/testdata/duplicatekeys/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v3
+name: duplicatekeys
+description: testing chart with duplicate keys in values.yaml
+version: 0.1.0

internal/chart/v3/lint/rules/testdata/duplicatekeys/values.yaml

@@ -0,0 +1,4 @@
+invalid:
+  duplicate: default
+  duplicate: value-i-want
+  duplicate: last-one

internal/chart/v3/lint/rules/values.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"helm.sh/helm/v4/internal/chart/v3/lint/support"
 	"helm.sh/helm/v4/pkg/chart/common"
@@ -54,9 +55,13 @@ func validateValuesFileExistence(valuesPath string) error {
 }
 
 func validateValuesFile(valuesPath string, overrides map[string]any, skipSchemaValidation bool) error {
-	values, err := common.ReadValuesFile(valuesPath)
+	// Try strict parsing first to detect duplicate keys
-	if err != nil {
+	values, strictErr := common.ReadValuesFileStrict(valuesPath)
-		return fmt.Errorf("unable to parse YAML: %w", err)
+	if strictErr != nil {
+		if isDuplicateKeyError(strictErr) {
+			return fmt.Errorf("%s contains duplicate keys: %w", filepath.Base(valuesPath), strictErr)
+		}
+		return fmt.Errorf("unable to parse YAML: %w", strictErr)
 	}
 
 	// Helm 3.0.0 carried over the values linting from Helm 2.x, which only tests the top
@@ -83,3 +88,11 @@ func validateValuesFile(valuesPath string, overrides map[string]any, skipSchemaV
 
 	return nil
 }
+
+// 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")
+}

pkg/chart/common/values.go

@@ -118,6 +118,26 @@ func ReadValuesFile(filename string) (Values, error) {
 	return ReadValues(data)
 }
 
+// ReadValuesFileStrict will parse a YAML file into a map of values using strict unmarshaling.
+// This will detect duplicate keys in the YAML.
+func ReadValuesFileStrict(filename string) (Values, error) {
+	data, err := os.ReadFile(filename)
+	if err != nil {
+		return map[string]any{}, err
+	}
+	return ReadValuesStrict(data)
+}
+
+// ReadValuesStrict will parse YAML byte data into a Values using strict unmarshaling.
+// This will detect duplicate keys in the YAML.
+func ReadValuesStrict(data []byte) (vals Values, err error) {
+	err = yaml.UnmarshalStrict(data, &vals)
+	if len(vals) == 0 {
+		vals = Values{}
+	}
+	return vals, err
+}
+
 // ReleaseOptions represents the additional release options needed
 // for the composition of the final values struct
 type ReleaseOptions struct {

pkg/chart/common/values_test.go

@@ -19,6 +19,7 @@ package common
 import (
 	"bytes"
 	"fmt"
+	"strings"
 	"testing"
 	"text/template"
 )
@@ -203,3 +204,34 @@ chapter:
 		}
 	}
 }
+
+func TestReadValuesStrict(t *testing.T) {
+	doc := `# Test YAML parse
+poet: "Coleridge"
+title: "Rime of the Ancient Mariner"
+`
+
+	data, err := ReadValuesStrict([]byte(doc))
+	if err != nil {
+		t.Fatalf("Error parsing bytes: %s", err)
+	}
+	if data["poet"] != "Coleridge" {
+		t.Errorf("Unexpected poet: %v", data["poet"])
+	}
+}
+
+func TestReadValuesStrictDuplicateKeys(t *testing.T) {
+	doc := `invalid:
+  duplicate: default
+  duplicate: value-i-want
+  duplicate: last-one
+`
+
+	_, err := ReadValuesStrict([]byte(doc))
+	if err == nil {
+		t.Fatal("Expected error for duplicate keys, got nil")
+	}
+	if !strings.Contains(err.Error(), "already") && !strings.Contains(err.Error(), "duplicate") {
+		t.Fatalf("Expected duplicate-key error, got: %s", err)
+	}
+}

pkg/chart/v2/lint/lint_test.go

@@ -37,6 +37,7 @@ const goodChartDir = "rules/testdata/goodone"
 const subChartValuesDir = "rules/testdata/withsubchart"
 const malformedTemplate = "rules/testdata/malformed-template"
 const invalidChartFileDir = "rules/testdata/invalidchartfile"
+const duplicateKeysDir = "rules/testdata/duplicatekeys"
 
 func TestBadChart(t *testing.T) {
 	var values map[string]any
@@ -245,3 +246,26 @@ func TestMalformedTemplate(t *testing.T) {
 		}
 	}
 }
+
+// TestDuplicateKeys tests that values.yaml with duplicate keys is detected
+// See https://github.com/helm/helm/issues/31102
+func TestDuplicateKeys(t *testing.T) {
+	var values map[string]any
+	m := RunAll(duplicateKeysDir, values, namespace).Messages
+	// Expect at least 1 message (the duplicate keys error)
+	if len(m) < 1 {
+		t.Fatalf("All didn't fail with expected errors, got %#v", m)
+	}
+	// Find the duplicate keys error message
+	found := false
+	for _, msg := range m {
+		if msg.Path == "values.yaml" && msg.Severity == support.ErrorSev &&
+			strings.Contains(msg.Err.Error(), "contains duplicate keys") {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("All didn't have the error for duplicate YAML keys in values.yaml: %v", m)
+	}
+}

pkg/chart/v2/lint/rules/testdata/duplicatekeys/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v2
+name: duplicatekeys
+description: testing chart with duplicate keys in values.yaml
+version: 0.1.0

pkg/chart/v2/lint/rules/testdata/duplicatekeys/values.yaml

@@ -0,0 +1,4 @@
+invalid:
+  duplicate: default
+  duplicate: value-i-want
+  duplicate: last-one

pkg/chart/v2/lint/rules/values.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"helm.sh/helm/v4/pkg/chart/common"
 	"helm.sh/helm/v4/pkg/chart/common/util"
@@ -54,9 +55,13 @@ func validateValuesFileExistence(valuesPath string) error {
 }
 
 func validateValuesFile(valuesPath string, overrides map[string]any, skipSchemaValidation bool) error {
-	values, err := common.ReadValuesFile(valuesPath)
+	// Try strict parsing first to detect duplicate keys
-	if err != nil {
+	values, strictErr := common.ReadValuesFileStrict(valuesPath)
-		return fmt.Errorf("unable to parse YAML: %w", err)
+	if strictErr != nil {
+		if isDuplicateKeyError(strictErr) {
+			return fmt.Errorf("%s contains duplicate keys: %w", filepath.Base(valuesPath), strictErr)
+		}
+		return fmt.Errorf("unable to parse YAML: %w", strictErr)
 	}
 
 	// Helm 3.0.0 carried over the values linting from Helm 2.x, which only tests the top
@@ -83,3 +88,11 @@ func validateValuesFile(valuesPath string, overrides map[string]any, skipSchemaV
 
 	return nil
 }
+
+// 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")
+}