Merge 3f45ceed40 into 53dfa521e0

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

@@ -0,0 +1,109 @@
+/*
+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)
+		}
+	default:
+		// Scalar, alias and document nodes carry no mapping keys to check.
+	}
+}
+
+// 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,109 @@
+/*
+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)
+		}
+	default:
+		// Scalar, alias and document nodes carry no mapping keys to check.
+	}
+}
+
+// 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))
 }