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)) }