fix(lint): warn about values keys that YAML reads as booleans

Values are parsed with sigs.k8s.io/yaml, which follows YAML 1.1, where the
plain scalars y, Y, yes, no, on, off (and True/False) resolve to booleans.
Used as a mapping key, such a scalar becomes "true" or "false", so a chart
that declares

    gridPos:
      x: 0
      y: 0

renders a key named "true", .Values.gridPos.y reaches nothing from a
template, and two spellings of the same boolean collapse into one key,
silently dropping a value.

--set does not go through a YAML parser, so the same key stays a string
there. Passing --set gridPos.y=99 therefore adds a second key next to the
first instead of overriding it.

Changing how values are parsed would change the meaning of existing charts,
so this only reports the situation, the way lint already parses Chart.yaml
strictly. Quoting the key is the fix and it keeps working today.

Signed-off-by: Sergey Sannikov <sergey@sannikov.dev>

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

@@ -0,0 +1,107 @@
+/*
+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 rules
+
+import (
+	"fmt"
+	"os"
+	"strings"
+
+	"go.yaml.in/yaml/v3"
+)
+
+// yaml11Booleans are the plain scalars that YAML 1.1 resolves to a boolean.
+// Values are parsed with sigs.k8s.io/yaml, which speaks YAML 1.1, so such a
+// scalar used as a mapping key becomes "true" or "false".
+//
+// The values passed to --set never go through a YAML parser, so the same key
+// written there stays a string. The two ways of supplying a value therefore
+// produce two different keys.
+var yaml11Booleans = map[string]bool{
+	"y": true, "Y": true, "yes": true, "Yes": true, "YES": true,
+	"n": true, "N": true, "no": true, "No": true, "NO": true,
+	"on": true, "On": true, "ON": true, "off": true, "Off": true, "OFF": true,
+	"True": true, "False": true,
+}
+
+// booleanLikeKey describes a key that does not survive parsing under its own name.
+type booleanLikeKey struct {
+	path string
+	line int
+}
+
+func (k booleanLikeKey) String() string {
+	return fmt.Sprintf("%s (line %d)", k.path, k.line)
+}
+
+// findBooleanLikeKeys walks a values document and collects keys that YAML 1.1
+// turns into a boolean. Quoted keys are left alone: quoting is the fix, and it
+// already works.
+func findBooleanLikeKeys(data []byte) ([]booleanLikeKey, error) {
+	var doc yaml.Node
+	if err := yaml.Unmarshal(data, &doc); err != nil {
+		// Parse errors are reported by the values rule; nothing to add here.
+		return nil, nil
+	}
+	var found []booleanLikeKey
+	for _, n := range doc.Content {
+		walkKeys(n, "", &found)
+	}
+	return found, nil
+}
+
+func walkKeys(n *yaml.Node, path string, found *[]booleanLikeKey) {
+	switch n.Kind {
+	case yaml.MappingNode:
+		for i := 0; i+1 < len(n.Content); i += 2 {
+			k, v := n.Content[i], n.Content[i+1]
+			name := k.Value
+			child := name
+			if path != "" {
+				child = path + "." + name
+			}
+			if k.Style == 0 && yaml11Booleans[name] {
+				*found = append(*found, booleanLikeKey{path: child, line: k.Line})
+			}
+			walkKeys(v, child, found)
+		}
+	case yaml.SequenceNode:
+		for i, c := range n.Content {
+			walkKeys(c, fmt.Sprintf("%s[%d]", path, i), found)
+		}
+	}
+}
+
+// validateNoBooleanLikeKeys warns about keys that a template cannot reach by the
+// name the chart author wrote.
+func validateNoBooleanLikeKeys(valuesPath string) error {
+	data, err := os.ReadFile(valuesPath)
+	if err != nil {
+		return nil // absence and unreadability are reported by other rules
+	}
+	found, err := findBooleanLikeKeys(data)
+	if err != nil || len(found) == 0 {
+		return nil
+	}
+	names := make([]string, 0, len(found))
+	for _, k := range found {
+		names = append(names, k.String())
+	}
+	return fmt.Errorf("key(s) %s are read as booleans and become %q or %q in the values; "+
+		"a template cannot reach them under the original name and --set will not override them. Quote the key to keep it",
+		strings.Join(names, ", "), "true", "false")
+}

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

@@ -0,0 +1,99 @@
+/*
+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 rules
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFindBooleanLikeKeys(t *testing.T) {
+	for _, tt := range []struct {
+		name string
+		doc  string
+		want []string
+	}{
+		{
+			name: "plain keys that YAML 1.1 reads as booleans",
+			// A Grafana dashboard pasted into values is the common way to hit this.
+			doc:  "gridPos:\n  h: 8\n  w: 12\n  x: 0\n  y: 0\n",
+			want: []string{"gridPos.y"},
+		},
+		{
+			name: "inside a sequence",
+			doc:  "panels:\n  - title: cpu\n    on: true\n",
+			want: []string{"panels[0].on"},
+		},
+		{
+			name: "every affected spelling",
+			doc:  "a:\n  y: 1\n  Y: 1\n  yes: 1\n  no: 1\n  N: 1\n  off: 1\n  ON: 1\n  True: 1\n",
+			want: []string{"a.y", "a.Y", "a.yes", "a.no", "a.N", "a.off", "a.ON", "a.True"},
+		},
+		{
+			name: "quoted keys survive and are not reported",
+			doc:  "a:\n  \"y\": 1\n  'on': 2\n",
+			want: nil,
+		},
+		{
+			name: "ordinary keys",
+			doc:  "replicaCount: 1\nimage:\n  tag: latest\n",
+			want: nil,
+		},
+		{
+			name: "empty document",
+			doc:  "",
+			want: nil,
+		},
+		{
+			name: "unparsable document is left to the values rule",
+			doc:  "a:\n\t- broken\n",
+			want: nil,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			found, err := findBooleanLikeKeys([]byte(tt.doc))
+			require.NoError(t, err)
+			paths := make([]string, 0, len(found))
+			for _, k := range found {
+				paths = append(paths, k.path)
+			}
+			assert.ElementsMatch(t, tt.want, paths)
+		})
+	}
+}
+
+func TestValidateNoBooleanLikeKeys(t *testing.T) {
+	dir := t.TempDir()
+
+	affected := filepath.Join(dir, "values.yaml")
+	require.NoError(t, os.WriteFile(affected, []byte("gridPos:\n  x: 0\n  y: 0\n"), 0o644))
+	err := validateNoBooleanLikeKeys(affected)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "gridPos.y")
+	assert.Contains(t, err.Error(), "line 3")
+
+	clean := filepath.Join(dir, "clean.yaml")
+	require.NoError(t, os.WriteFile(clean, []byte("gridPos:\n  x: 0\n  \"y\": 0\n"), 0o644))
+	assert.NoError(t, validateNoBooleanLikeKeys(clean))
+
+	// A missing file is reported by validateValuesFileExistence, not here.
+	assert.NoError(t, validateNoBooleanLikeKeys(filepath.Join(dir, "nope.yaml")))
+}

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

@@ -42,6 +42,7 @@ func ValuesWithOverrides(linter *support.Linter, valueOverrides map[string]any,
 		return
 	}
 
+	linter.RunLinterRule(support.WarningSev, file, validateNoBooleanLikeKeys(vf))
 	linter.RunLinterRule(support.ErrorSev, file, validateValuesFile(vf, valueOverrides, skipSchemaValidation))
 }
 

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

@@ -0,0 +1,107 @@
+/*
+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 rules
+
+import (
+	"fmt"
+	"os"
+	"strings"
+
+	"go.yaml.in/yaml/v3"
+)
+
+// yaml11Booleans are the plain scalars that YAML 1.1 resolves to a boolean.
+// Values are parsed with sigs.k8s.io/yaml, which speaks YAML 1.1, so such a
+// scalar used as a mapping key becomes "true" or "false".
+//
+// The values passed to --set never go through a YAML parser, so the same key
+// written there stays a string. The two ways of supplying a value therefore
+// produce two different keys.
+var yaml11Booleans = map[string]bool{
+	"y": true, "Y": true, "yes": true, "Yes": true, "YES": true,
+	"n": true, "N": true, "no": true, "No": true, "NO": true,
+	"on": true, "On": true, "ON": true, "off": true, "Off": true, "OFF": true,
+	"True": true, "False": true,
+}
+
+// booleanLikeKey describes a key that does not survive parsing under its own name.
+type booleanLikeKey struct {
+	path string
+	line int
+}
+
+func (k booleanLikeKey) String() string {
+	return fmt.Sprintf("%s (line %d)", k.path, k.line)
+}
+
+// findBooleanLikeKeys walks a values document and collects keys that YAML 1.1
+// turns into a boolean. Quoted keys are left alone: quoting is the fix, and it
+// already works.
+func findBooleanLikeKeys(data []byte) ([]booleanLikeKey, error) {
+	var doc yaml.Node
+	if err := yaml.Unmarshal(data, &doc); err != nil {
+		// Parse errors are reported by the values rule; nothing to add here.
+		return nil, nil
+	}
+	var found []booleanLikeKey
+	for _, n := range doc.Content {
+		walkKeys(n, "", &found)
+	}
+	return found, nil
+}
+
+func walkKeys(n *yaml.Node, path string, found *[]booleanLikeKey) {
+	switch n.Kind {
+	case yaml.MappingNode:
+		for i := 0; i+1 < len(n.Content); i += 2 {
+			k, v := n.Content[i], n.Content[i+1]
+			name := k.Value
+			child := name
+			if path != "" {
+				child = path + "." + name
+			}
+			if k.Style == 0 && yaml11Booleans[name] {
+				*found = append(*found, booleanLikeKey{path: child, line: k.Line})
+			}
+			walkKeys(v, child, found)
+		}
+	case yaml.SequenceNode:
+		for i, c := range n.Content {
+			walkKeys(c, fmt.Sprintf("%s[%d]", path, i), found)
+		}
+	}
+}
+
+// validateNoBooleanLikeKeys warns about keys that a template cannot reach by the
+// name the chart author wrote.
+func validateNoBooleanLikeKeys(valuesPath string) error {
+	data, err := os.ReadFile(valuesPath)
+	if err != nil {
+		return nil // absence and unreadability are reported by other rules
+	}
+	found, err := findBooleanLikeKeys(data)
+	if err != nil || len(found) == 0 {
+		return nil
+	}
+	names := make([]string, 0, len(found))
+	for _, k := range found {
+		names = append(names, k.String())
+	}
+	return fmt.Errorf("key(s) %s are read as booleans and become %q or %q in the values; "+
+		"a template cannot reach them under the original name and --set will not override them. Quote the key to keep it",
+		strings.Join(names, ", "), "true", "false")
+}

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

@@ -0,0 +1,99 @@
+/*
+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 rules
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFindBooleanLikeKeys(t *testing.T) {
+	for _, tt := range []struct {
+		name string
+		doc  string
+		want []string
+	}{
+		{
+			name: "plain keys that YAML 1.1 reads as booleans",
+			// A Grafana dashboard pasted into values is the common way to hit this.
+			doc:  "gridPos:\n  h: 8\n  w: 12\n  x: 0\n  y: 0\n",
+			want: []string{"gridPos.y"},
+		},
+		{
+			name: "inside a sequence",
+			doc:  "panels:\n  - title: cpu\n    on: true\n",
+			want: []string{"panels[0].on"},
+		},
+		{
+			name: "every affected spelling",
+			doc:  "a:\n  y: 1\n  Y: 1\n  yes: 1\n  no: 1\n  N: 1\n  off: 1\n  ON: 1\n  True: 1\n",
+			want: []string{"a.y", "a.Y", "a.yes", "a.no", "a.N", "a.off", "a.ON", "a.True"},
+		},
+		{
+			name: "quoted keys survive and are not reported",
+			doc:  "a:\n  \"y\": 1\n  'on': 2\n",
+			want: nil,
+		},
+		{
+			name: "ordinary keys",
+			doc:  "replicaCount: 1\nimage:\n  tag: latest\n",
+			want: nil,
+		},
+		{
+			name: "empty document",
+			doc:  "",
+			want: nil,
+		},
+		{
+			name: "unparsable document is left to the values rule",
+			doc:  "a:\n\t- broken\n",
+			want: nil,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			found, err := findBooleanLikeKeys([]byte(tt.doc))
+			require.NoError(t, err)
+			paths := make([]string, 0, len(found))
+			for _, k := range found {
+				paths = append(paths, k.path)
+			}
+			assert.ElementsMatch(t, tt.want, paths)
+		})
+	}
+}
+
+func TestValidateNoBooleanLikeKeys(t *testing.T) {
+	dir := t.TempDir()
+
+	affected := filepath.Join(dir, "values.yaml")
+	require.NoError(t, os.WriteFile(affected, []byte("gridPos:\n  x: 0\n  y: 0\n"), 0o644))
+	err := validateNoBooleanLikeKeys(affected)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "gridPos.y")
+	assert.Contains(t, err.Error(), "line 3")
+
+	clean := filepath.Join(dir, "clean.yaml")
+	require.NoError(t, os.WriteFile(clean, []byte("gridPos:\n  x: 0\n  \"y\": 0\n"), 0o644))
+	assert.NoError(t, validateNoBooleanLikeKeys(clean))
+
+	// A missing file is reported by validateValuesFileExistence, not here.
+	assert.NoError(t, validateNoBooleanLikeKeys(filepath.Join(dir, "nope.yaml")))
+}

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

@@ -42,6 +42,7 @@ func ValuesWithOverrides(linter *support.Linter, valueOverrides map[string]any,
 		return
 	}
 
+	linter.RunLinterRule(support.WarningSev, file, validateNoBooleanLikeKeys(vf))
 	linter.RunLinterRule(support.ErrorSev, file, validateValuesFile(vf, valueOverrides, skipSchemaValidation))
 }