From f4029944611ed0d18374408c8d504ab17ecde257 Mon Sep 17 00:00:00 2001 From: Matthew Luckam Date: Tue, 19 Jan 2021 19:55:48 -0500 Subject: [PATCH 1/2] corrected order of helm lint coalescing of multiple values files Signed-off-by: Matthew Luckam --- pkg/lint/rules/values.go | 7 ++++--- pkg/lint/rules/values_test.go | 19 ++++++++++++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/pkg/lint/rules/values.go b/pkg/lint/rules/values.go index c596687c5..79a294326 100644 --- a/pkg/lint/rules/values.go +++ b/pkg/lint/rules/values.go @@ -70,8 +70,9 @@ func validateValuesFile(valuesPath string, overrides map[string]interface{}) err // level values against the top-level expectations. Subchart values are not linted. // We could change that. For now, though, we retain that strategy, and thus can // coalesce tables (like reuse-values does) instead of doing the full chart - // CoalesceValues. - values = chartutil.CoalesceTables(values, overrides) + // CoalesceValues + coalescedValues := chartutil.CoalesceTables(make(map[string]interface{}, len(overrides)), overrides) + coalescedValues = chartutil.CoalesceTables(coalescedValues, values) ext := filepath.Ext(valuesPath) schemaPath := valuesPath[:len(valuesPath)-len(ext)] + ".schema.json" @@ -82,5 +83,5 @@ func validateValuesFile(valuesPath string, overrides map[string]interface{}) err if err != nil { return err } - return chartutil.ValidateAgainstSingleSchema(values, schema) + return chartutil.ValidateAgainstSingleSchema(coalescedValues, schema) } diff --git a/pkg/lint/rules/values_test.go b/pkg/lint/rules/values_test.go index 6d93d630e..eb74aee36 100644 --- a/pkg/lint/rules/values_test.go +++ b/pkg/lint/rules/values_test.go @@ -104,7 +104,7 @@ func TestValidateValuesFileSchemaFailure(t *testing.T) { } func TestValidateValuesFileSchemaOverrides(t *testing.T) { - yaml := "username: admin" + yaml := "username: admin\npassword:" overrides := map[string]interface{}{ "password": "swordfish", } @@ -118,6 +118,23 @@ func TestValidateValuesFileSchemaOverrides(t *testing.T) { } } +func TestValidateValuesFileSchemaOverridesFailure(t *testing.T) { + yaml := "username: admin\npassword:" + overrides := map[string]interface{}{ + "username": "anotherUser", + } + tmpdir := ensure.TempFile(t, "values.yaml", []byte(yaml)) + defer os.RemoveAll(tmpdir) + createTestingSchema(t, tmpdir) + + valfile := filepath.Join(tmpdir, "values.yaml") + err := validateValuesFile(valfile, overrides) + if err == nil { + t.Fatalf("expected values file to fail parsing") + } + assert.Contains(t, err.Error(), "Expected: string, given: null", "Null value for password should be caught by schema") +} + func createTestingSchema(t *testing.T, dir string) string { t.Helper() schemafile := filepath.Join(dir, "values.schema.json") From 592c338242deba41df6c684e6260daccb93acfc7 Mon Sep 17 00:00:00 2001 From: Matthew Luckam Date: Tue, 2 Mar 2021 16:33:10 -0500 Subject: [PATCH 2/2] updated unit tests to conform with helm best practices Signed-off-by: Matthew Luckam --- pkg/lint/rules/values_test.go | 56 +++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/pkg/lint/rules/values_test.go b/pkg/lint/rules/values_test.go index eb74aee36..23335cc01 100644 --- a/pkg/lint/rules/values_test.go +++ b/pkg/lint/rules/values_test.go @@ -104,7 +104,7 @@ func TestValidateValuesFileSchemaFailure(t *testing.T) { } func TestValidateValuesFileSchemaOverrides(t *testing.T) { - yaml := "username: admin\npassword:" + yaml := "username: admin" overrides := map[string]interface{}{ "password": "swordfish", } @@ -118,21 +118,51 @@ func TestValidateValuesFileSchemaOverrides(t *testing.T) { } } -func TestValidateValuesFileSchemaOverridesFailure(t *testing.T) { - yaml := "username: admin\npassword:" - overrides := map[string]interface{}{ - "username": "anotherUser", +func TestValidateValuesFile(t *testing.T) { + tests := []struct { + name string + yaml string + overrides map[string]interface{} + errorMessage string + }{ + { + name: "value added", + yaml: "username: admin", + overrides: map[string]interface{}{"password": "swordfish"}, + }, + { + name: "value not overridden", + yaml: "username: admin\npassword:", + overrides: map[string]interface{}{"username": "anotherUser"}, + errorMessage: "Expected: string, given: null", + }, + { + name: "value overridden", + yaml: "username: admin\npassword:", + overrides: map[string]interface{}{"username": "anotherUser", "password": "swordfish"}, + }, } - tmpdir := ensure.TempFile(t, "values.yaml", []byte(yaml)) - defer os.RemoveAll(tmpdir) - createTestingSchema(t, tmpdir) - valfile := filepath.Join(tmpdir, "values.yaml") - err := validateValuesFile(valfile, overrides) - if err == nil { - t.Fatalf("expected values file to fail parsing") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpdir := ensure.TempFile(t, "values.yaml", []byte(tt.yaml)) + defer os.RemoveAll(tmpdir) + createTestingSchema(t, tmpdir) + + valfile := filepath.Join(tmpdir, "values.yaml") + + err := validateValuesFile(valfile, tt.overrides) + + switch { + case err != nil && tt.errorMessage == "": + t.Errorf("Failed validation with %s", err) + case err == nil && tt.errorMessage != "": + t.Error("expected values file to fail parsing") + case err != nil && tt.errorMessage != "": + assert.Contains(t, err.Error(), tt.errorMessage, "Failed with unexpected error") + } + }) } - assert.Contains(t, err.Error(), "Expected: string, given: null", "Null value for password should be caught by schema") } func createTestingSchema(t *testing.T, dir string) string {