From 51b59497eed3537340c80750680e6626145520fc Mon Sep 17 00:00:00 2001 From: Sam Leavens Date: Wed, 19 Jul 2017 17:47:17 -0700 Subject: [PATCH 1/2] ref(pkg/chartutil): decrease map lookups Decrease map lookups in `chartutil.coalesceValues` and move comment to unit test. Closes #2663 --- pkg/chartutil/values.go | 33 +++++++++++++++------------------ pkg/chartutil/values_test.go | 4 ++++ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go index a2343555e..696f3ec95 100644 --- a/pkg/chartutil/values.go +++ b/pkg/chartutil/values.go @@ -278,26 +278,23 @@ func coalesceValues(c *chart.Chart, v map[string]interface{}) (map[string]interf } for key, val := range nv { - if _, ok := v[key]; !ok { + if value, ok := v[key]; ok { + if value == nil { + delete(v, key) + } else if dest, ok := value.(map[string]interface{}); ok { + // if v[key] is a table, merge nv's val table into v[key]. + src, ok := val.(map[string]interface{}) + if !ok { + log.Printf("warning: skipped value for %s: Not a table.", key) + continue + } + // Because v has higher precedence than nv, dest values override src + // values. + coalesceTables(dest, src) + } + } else { // If the key is not in v, copy it from nv. v[key] = val - } else if ok && v[key] == nil { - // When the YAML value is null, we remove the value's key. - // This allows Helm's various sources of values (value files or --set) to - // remove incompatible keys from any previous chart, file, or set values. - // ref: http://www.yaml.org/spec/1.2/spec.html#id2803362 - delete(v, key) - continue - } else if dest, ok := v[key].(map[string]interface{}); ok { - // if v[key] is a table, merge nv's val table into v[key]. - src, ok := val.(map[string]interface{}) - if !ok { - log.Printf("warning: skipped value for %s: Not a table.", key) - continue - } - // Because v has higher precedence than nv, dest values override src - // values. - coalesceTables(dest, src) } } return v, nil diff --git a/pkg/chartutil/values_test.go b/pkg/chartutil/values_test.go index 7d5750bd5..eb240452a 100644 --- a/pkg/chartutil/values_test.go +++ b/pkg/chartutil/values_test.go @@ -350,6 +350,10 @@ func TestCoalesceValues(t *testing.T) { } } + // When the YAML value is null, we remove the value's key. + // This allows Helm's various sources of values (value files or --set) to + // remove incompatible keys from any previous chart, file, or set values. + // ref: http://www.yaml.org/spec/1.2/spec.html#id2803362 nullKeys := []string{"bottom", "right", "left", "front"} for _, nullKey := range nullKeys { if _, ok := v[nullKey]; ok { From 8df52b6501f0ba2166e85077c697e739db4dd17d Mon Sep 17 00:00:00 2001 From: Sam Leavens Date: Thu, 20 Jul 2017 13:10:59 -0700 Subject: [PATCH 2/2] ref(pkg/chartutil): move comment Move comment out of test. Move ref location in test. --- pkg/chartutil/values.go | 3 +++ pkg/chartutil/values_test.go | 5 +---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go index 696f3ec95..d2f2b3485 100644 --- a/pkg/chartutil/values.go +++ b/pkg/chartutil/values.go @@ -280,6 +280,9 @@ func coalesceValues(c *chart.Chart, v map[string]interface{}) (map[string]interf for key, val := range nv { if value, ok := v[key]; ok { if value == nil { + // When the YAML value is null, we remove the value's key. + // This allows Helm's various sources of values (value files or --set) to + // remove incompatible keys from any previous chart, file, or set values. delete(v, key) } else if dest, ok := value.(map[string]interface{}); ok { // if v[key] is a table, merge nv's val table into v[key]. diff --git a/pkg/chartutil/values_test.go b/pkg/chartutil/values_test.go index eb240452a..51c9b673e 100644 --- a/pkg/chartutil/values_test.go +++ b/pkg/chartutil/values_test.go @@ -275,6 +275,7 @@ func ttpl(tpl string, v map[string]interface{}) (string, error) { return b.String(), nil } +// ref: http://www.yaml.org/spec/1.2/spec.html#id2803362 var testCoalesceValuesYaml = ` top: yup bottom: null @@ -350,10 +351,6 @@ func TestCoalesceValues(t *testing.T) { } } - // When the YAML value is null, we remove the value's key. - // This allows Helm's various sources of values (value files or --set) to - // remove incompatible keys from any previous chart, file, or set values. - // ref: http://www.yaml.org/spec/1.2/spec.html#id2803362 nullKeys := []string{"bottom", "right", "left", "front"} for _, nullKey := range nullKeys { if _, ok := v[nullKey]; ok {