Skip to content

Commit

Permalink
Update messaging in value parsing to improve traceability (helm#4974)
Browse files Browse the repository at this point in the history
Closes helm#4736

Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
  • Loading branch information
hickeyma authored and Matthew Fisher committed Nov 30, 2018
1 parent 55a3385 commit 58be8e4
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 18 deletions.
6 changes: 3 additions & 3 deletions pkg/chartutil/requirements.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ func processImportValues(c *chart.Chart) error {
}
// create value map from child to be merged into parent
vm := pathToMap(nm["parent"], vv.AsMap())
b = coalesceTables(cvals, vm)
b = coalesceTables(cvals, vm, c.Metadata.Name)
case string:
nm := map[string]string{
"child": "exports." + iv,
Expand All @@ -441,14 +441,14 @@ func processImportValues(c *chart.Chart) error {
log.Printf("Warning: ImportValues missing table: %v", err)
continue
}
b = coalesceTables(b, vm.AsMap())
b = coalesceTables(b, vm.AsMap(), c.Metadata.Name)
}
}
// set our formatted import values
r.ImportValues = outiv
}
}
b = coalesceTables(b, cvals)
b = coalesceTables(b, cvals, c.Metadata.Name)
y, err := yaml.Marshal(b)
if err != nil {
return err
Expand Down
28 changes: 14 additions & 14 deletions pkg/chartutil/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func coalesceDeps(chrt *chart.Chart, dest map[string]interface{}) (map[string]in
dvmap := dv.(map[string]interface{})

// Get globals out of dest and merge them into dvmap.
coalesceGlobals(dvmap, dest)
coalesceGlobals(dvmap, dest, chrt.Metadata.Name)

var err error
// Now coalesce the rest of the values.
Expand All @@ -219,20 +219,20 @@ func coalesceDeps(chrt *chart.Chart, dest map[string]interface{}) (map[string]in
// coalesceGlobals copies the globals out of src and merges them into dest.
//
// For convenience, returns dest.
func coalesceGlobals(dest, src map[string]interface{}) map[string]interface{} {
func coalesceGlobals(dest, src map[string]interface{}, chartName string) map[string]interface{} {
var dg, sg map[string]interface{}

if destglob, ok := dest[GlobalKey]; !ok {
dg = map[string]interface{}{}
} else if dg, ok = destglob.(map[string]interface{}); !ok {
log.Printf("warning: skipping globals because destination %s is not a table.", GlobalKey)
log.Printf("Warning: Skipping globals for chart '%s' because destination '%s' is not a table.", chartName, GlobalKey)
return dg
}

if srcglob, ok := src[GlobalKey]; !ok {
sg = map[string]interface{}{}
} else if sg, ok = srcglob.(map[string]interface{}); !ok {
log.Printf("warning: skipping globals because source %s is not a table.", GlobalKey)
log.Printf("Warning: skipping globals for chart '%s' because source '%s' is not a table.", chartName, GlobalKey)
return dg
}

Expand All @@ -247,19 +247,19 @@ func coalesceGlobals(dest, src map[string]interface{}) map[string]interface{} {
if destvmap, ok := destv.(map[string]interface{}); ok {
// Basically, we reverse order of coalesce here to merge
// top-down.
coalesceTables(vv, destvmap)
coalesceTables(vv, destvmap, chartName)
dg[key] = vv
continue
} else {
log.Printf("Conflict: cannot merge map onto non-map for %q. Skipping.", key)
log.Printf("Warning: For chart '%s', cannot merge map onto non-map for key '%q'. Skipping.", chartName, key)
}
} else {
// Here there is no merge. We're just adding.
dg[key] = vv
}
} else if dv, ok := dg[key]; ok && istable(dv) {
// It's not clear if this condition can actually ever trigger.
log.Printf("key %s is table. Skipping", key)
log.Printf("Warning: For chart '%s', key '%s' is a table. Skipping.", chartName, key)
continue
}
// TODO: Do we need to do any additional checking on the value?
Expand Down Expand Up @@ -291,7 +291,7 @@ func coalesceValues(c *chart.Chart, v map[string]interface{}) (map[string]interf
// On error, we return just the overridden values.
// FIXME: We should log this error. It indicates that the YAML data
// did not parse.
return v, fmt.Errorf("error reading default values (%s): %s", c.Values.Raw, err)
return v, fmt.Errorf("Error: Reading chart '%s' default values (%s): %s", c.Metadata.Name, c.Values.Raw, err)
}

for key, val := range nv {
Expand All @@ -305,12 +305,12 @@ func coalesceValues(c *chart.Chart, v map[string]interface{}) (map[string]interf
// 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)
log.Printf("Warning: Building values map for chart '%s'. Skipped value (%+v) for '%s', as it is not a table.", c.Metadata.Name, src, key)
continue
}
// Because v has higher precedence than nv, dest values override src
// values.
coalesceTables(dest, src)
coalesceTables(dest, src, c.Metadata.Name)
}
} else {
// If the key is not in v, copy it from nv.
Expand All @@ -323,21 +323,21 @@ func coalesceValues(c *chart.Chart, v map[string]interface{}) (map[string]interf
// coalesceTables merges a source map into a destination map.
//
// dest is considered authoritative.
func coalesceTables(dst, src map[string]interface{}) map[string]interface{} {
func coalesceTables(dst, src map[string]interface{}, chartName string) map[string]interface{} {
// Because dest has higher precedence than src, dest values override src
// values.
for key, val := range src {
if istable(val) {
if innerdst, ok := dst[key]; !ok {
dst[key] = val
} else if istable(innerdst) {
coalesceTables(innerdst.(map[string]interface{}), val.(map[string]interface{}))
coalesceTables(innerdst.(map[string]interface{}), val.(map[string]interface{}), chartName)
} else {
log.Printf("warning: cannot overwrite table with non table for %s (%v)", key, val)
log.Printf("Warning: Merging destination map for chart '%s'. Cannot overwrite table item '%s', with non table value: %v", chartName, key, val)
}
continue
} else if dv, ok := dst[key]; ok && istable(dv) {
log.Printf("warning: destination for %s is a table. Ignoring non-table value %v", key, val)
log.Printf("Warning: Merging destination map for chart '%s'. The destination item '%s' is a table and ignoring the source '%s' as it has a non-table value of: %v", chartName, key, key, val)
continue
} else if !ok { // <- ok is still in scope from preceding conditional.
dst[key] = val
Expand Down
2 changes: 1 addition & 1 deletion pkg/chartutil/values_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ func TestCoalesceTables(t *testing.T) {

// What we expect is that anything in dst overrides anything in src, but that
// otherwise the values are coalesced.
coalesceTables(dst, src)
coalesceTables(dst, src, "")

if dst["name"] != "Ishmael" {
t.Errorf("Unexpected name: %s", dst["name"])
Expand Down

0 comments on commit 58be8e4

Please sign in to comment.