From e9bdec1cc45bc1d6225c6a8c09f38cba7a8df336 Mon Sep 17 00:00:00 2001 From: Sergey Sannikov Date: Sun, 2 Aug 2026 14:50:10 +0400 Subject: [PATCH] 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 --- internal/chart/v3/lint/rules/valuekeys.go | 107 ++++++++++++++++++ .../chart/v3/lint/rules/valuekeys_test.go | 99 ++++++++++++++++ internal/chart/v3/lint/rules/values.go | 1 + pkg/chart/v2/lint/rules/valuekeys.go | 107 ++++++++++++++++++ pkg/chart/v2/lint/rules/valuekeys_test.go | 99 ++++++++++++++++ pkg/chart/v2/lint/rules/values.go | 1 + 6 files changed, 414 insertions(+) create mode 100644 internal/chart/v3/lint/rules/valuekeys.go create mode 100644 internal/chart/v3/lint/rules/valuekeys_test.go create mode 100644 pkg/chart/v2/lint/rules/valuekeys.go create mode 100644 pkg/chart/v2/lint/rules/valuekeys_test.go diff --git a/internal/chart/v3/lint/rules/valuekeys.go b/internal/chart/v3/lint/rules/valuekeys.go new file mode 100644 index 000000000..04c2e2d5c --- /dev/null +++ b/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") +} diff --git a/internal/chart/v3/lint/rules/valuekeys_test.go b/internal/chart/v3/lint/rules/valuekeys_test.go new file mode 100644 index 000000000..cf7740b5c --- /dev/null +++ b/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"))) +} diff --git a/internal/chart/v3/lint/rules/values.go b/internal/chart/v3/lint/rules/values.go index b4a2edb0c..d4de73e83 100644 --- a/internal/chart/v3/lint/rules/values.go +++ b/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)) } diff --git a/pkg/chart/v2/lint/rules/valuekeys.go b/pkg/chart/v2/lint/rules/valuekeys.go new file mode 100644 index 000000000..04c2e2d5c --- /dev/null +++ b/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") +} diff --git a/pkg/chart/v2/lint/rules/valuekeys_test.go b/pkg/chart/v2/lint/rules/valuekeys_test.go new file mode 100644 index 000000000..cf7740b5c --- /dev/null +++ b/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"))) +} diff --git a/pkg/chart/v2/lint/rules/values.go b/pkg/chart/v2/lint/rules/values.go index 2c766068c..b79b80c4d 100644 --- a/pkg/chart/v2/lint/rules/values.go +++ b/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)) }