fix(engine): coalesce values in templates

pkg/engine/engine.go

@@ -3,6 +3,7 @@ package engine
 import (
 	"bytes"
 	"fmt"
+	"log"
 	"text/template"
 
 	"github.com/Masterminds/sprig"
@@ -39,38 +40,53 @@ func New() *Engine {
 //
 // This will look in the chart's 'templates' data (e.g. the 'templates/' directory)
 // and attempt to render the templates there using the values passed in.
+//
+// Values are scoped to their templates. A dependency template will not have
+// access to the values set for its parent. If chart "foo" includes chart "bar",
+// "bar" will not have access to the values for "foo".
+//
+// Values are passed through the templates according to scope. If the top layer
+// chart includes the chart foo, which includes the chart bar, the values map
+// will be examined for a table called "foo". If "foo" is found in vals,
+// that section of the values will be passed into the "foo" chart. And if that
+// section contains a value named "bar", that value will be passed on to the
+// bar chart during render time.
+//
+// Values are coalesced together using the fillowing rules:
+//
+//	- Values in a higher level chart always override values in a lower-level
+//		dependency chart
+//	- Scalar values and arrays are replaced, maps are merged
+//	- A chart has access to all of the variables for it, as well as all of
+//		the values destined for its dependencies.
 func (e *Engine) Render(chrt *chart.Chart, vals *chart.Config) (map[string]string, error) {
 	var cvals chartutil.Values
-	if chrt.Values == nil {
-		cvals = map[string]interface{}{}
-	} else {
-		var err error
-		cvals, err = chartutil.ReadValues([]byte(chrt.Values.Raw))
-		if err != nil {
-			return map[string]string{}, err
-		}
-	}
 
-	// Parse values if not nil
+	// Parse values if not nil. We merge these at the top level because
+	// the passed-in values are in the same namespace as the parent chart.
 	if vals != nil {
 		evals, err := chartutil.ReadValues([]byte(vals.Raw))
 		if err != nil {
 			return map[string]string{}, err
 		}
-		// Coalesce chart default values and values
-		for k, v := range evals {
-			// FIXME: This needs to merge tables. Ideally, this feature should
-			// be part of the Values type.
-			cvals[k] = v
-		}
+		cvals = coalesceValues(chrt, evals)
 	}
 
 	// Render the charts
-	tmap := allTemplates(chrt)
-	return e.render(tmap, cvals)
+	tmap := allTemplates(chrt, cvals)
+	return e.render(tmap)
+}
+
+// renderable is an object that can be rendered.
+type renderable struct {
+	// tpl is the current template.
+	tpl string
+	// vals are the values to be supplied to the template.
+	vals chartutil.Values
 }
 
-func (e *Engine) render(tpls map[string]string, v interface{}) (map[string]string, error) {
+// render takes a map of templates/values and renders them.
+func (e *Engine) render(tpls map[string]renderable) (map[string]string, error) {
 	// Basically, what we do here is start with an empty parent template and then
 	// build up a list of templates -- one for each file. Once all of the templates
 	// have been parsed, we loop through again and execute every template.
@@ -80,9 +96,9 @@ func (e *Engine) render(tpls map[string]string, v interface{}) (map[string]strin
 	// template engine.
 	t := template.New("gotpl")
 	files := []string{}
-	for fname, tpl := range tpls {
+	for fname, r := range tpls {
 		t = t.New(fname).Funcs(e.FuncMap)
-		if _, err := t.Parse(tpl); err != nil {
+		if _, err := t.Parse(r.tpl); err != nil {
 			return map[string]string{}, fmt.Errorf("parse error in %q: %s", fname, err)
 		}
 		files = append(files, fname)
@@ -91,7 +107,8 @@ func (e *Engine) render(tpls map[string]string, v interface{}) (map[string]strin
 	rendered := make(map[string]string, len(files))
 	var buf bytes.Buffer
 	for _, file := range files {
-		if err := t.ExecuteTemplate(&buf, file, v); err != nil {
+		//	log.Printf("Exec %s with %v (%s)", file, tpls[file].vals, tpls[file].tpl)
+		if err := t.ExecuteTemplate(&buf, file, tpls[file].vals); err != nil {
 			return map[string]string{}, fmt.Errorf("render error in %q: %s", file, err)
 		}
 		rendered[file] = buf.String()
@@ -102,18 +119,108 @@ func (e *Engine) render(tpls map[string]string, v interface{}) (map[string]strin
 }
 
 // allTemplates returns all templates for a chart and its dependencies.
-func allTemplates(c *chart.Chart) map[string]string {
-	templates := map[string]string{}
-	recAllTpls(c, templates)
+//
+// As it goes, it also prepares the values in a scope-sensitive manner.
+func allTemplates(c *chart.Chart, vals chartutil.Values) map[string]renderable {
+	templates := map[string]renderable{}
+	recAllTpls(c, templates, vals, true)
 	return templates
 }
 
-func recAllTpls(c *chart.Chart, templates map[string]string) {
+// recAllTpls recurses through the templates in a chart.
+//
+// As it recurses, it also sets the values to be appropriate for the template
+// scope.
+func recAllTpls(c *chart.Chart, templates map[string]renderable, parentVals chartutil.Values, top bool) {
+	var pvals chartutil.Values
+	if top {
+		// If this is the top of the rendering tree, assume that parentVals
+		// is already resolved to the authoritative values.
+		pvals = parentVals
+	} else if c.Metadata != nil && c.Metadata.Name != "" {
+		// An error indicates that the table doesn't exist. So we leave it as
+		// an empty map.
+		tmp, err := parentVals.Table(c.Metadata.Name)
+		if err == nil {
+			pvals = tmp
+		}
+	}
+	cvals := coalesceValues(c, pvals)
+	//log.Printf("racAllTpls values: %v", cvals)
 	for _, child := range c.Dependencies {
-		recAllTpls(child, templates)
+		recAllTpls(child, templates, cvals, false)
 	}
 	for _, t := range c.Templates {
-		templates[t.Name] = string(t.Data)
+		templates[t.Name] = renderable{
+			tpl:  string(t.Data),
+			vals: cvals,
+		}
+	}
+}
+
+// coalesceValues builds up a values map for a particular chart.
+//
+// Values in v will override the values in the chart.
+func coalesceValues(c *chart.Chart, v chartutil.Values) chartutil.Values {
+	// If there are no values in the chart, we just return the given values
+	if c.Values == nil {
+		return v
+	}
+
+	nv, err := chartutil.ReadValues([]byte(c.Values.Raw))
+	if err != nil {
+		// On error, we return just the overridden values.
+		// FIXME: We should log this error. It indicates that the TOML data
+		// did not parse.
+		log.Printf("error reading default values: %s", err)
+		return v
 	}
 
+	for k, val := range v {
+		// NOTE: We could block coalesce on cases where nv does not explicitly
+		// declare a value. But that forces the chart author to explicitly
+		// set a default for every template param. We want to preserve the
+		// possibility of "hidden" parameters.
+		if istable(val) {
+			if inmap, ok := nv[k]; ok && istable(inmap) {
+				coalesceTables(inmap.(map[string]interface{}), val.(map[string]interface{}))
+			} else if ok {
+				log.Printf("Cannot copy table into non-table value for %s (%v)", k, inmap)
+			} else {
+				// The parent table does not have a key entry for this item,
+				// so we can safely set it. This is necessary for nested charts.
+				log.Printf("Copying %s into map %v", k, nv)
+				nv[k] = val
+			}
+		} else {
+			nv[k] = val
+		}
+	}
+	return nv
+}
+
+// coalesceTables merges a source map into a destination map.
+func coalesceTables(dst, src map[string]interface{}) {
+	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{}))
+			} else {
+				log.Printf("Cannot overwrite table with non table for %s (%v)", key, val)
+			}
+			continue
+		} else if dv, ok := dst[key]; ok && istable(dv) {
+			log.Printf("Destination for %s is a table. Ignoring non-table value %v", key, val)
+			continue
+		}
+		dst[key] = val
+	}
+}
+
+// istable is a special-purpose function to see if the present thing matches the definition of a TOML table.
+func istable(v interface{}) bool {
+	_, ok := v.(map[string]interface{})
+	return ok
 }

pkg/engine/engine_test.go

@@ -5,6 +5,7 @@ import (
 	"sync"
 	"testing"
 
+	chartutil "github.com/deis/tiller/pkg/chart"
 	"github.com/deis/tiller/pkg/proto/hapi/chart"
 )
 
@@ -28,16 +29,16 @@ func TestRenderInternals(t *testing.T) {
 	// Test the internals of the rendering tool.
 	e := New()
 
-	tpls := map[string]string{
-		"one": `Hello {{title .Name}}`,
-		"two": `Goodbye {{upper .Value}}`,
+	vals := chartutil.Values{"Name": "one", "Value": "two"}
+	tpls := map[string]renderable{
+		"one": {tpl: `Hello {{title .Name}}`, vals: vals},
+		"two": {tpl: `Goodbye {{upper .Value}}`, vals: vals},
 		// Test whether a template can reliably reference another template
 		// without regard for ordering.
-		"three": `{{template "two" dict "Value" "three"}}`,
+		"three": {tpl: `{{template "two" dict "Value" "three"}}`, vals: vals},
 	}
-	vals := map[string]string{"Name": "one", "Value": "two"}
 
-	out, err := e.render(tpls, vals)
+	out, err := e.render(tpls)
 	if err != nil {
 		t.Fatalf("Failed template rendering: %s", err)
 	}
@@ -68,9 +69,9 @@ func TestParallelRenderInternals(t *testing.T) {
 		go func(i int) {
 			fname := "my/file/name"
 			tt := fmt.Sprintf("expect-%d", i)
-			tpls := map[string]string{fname: `{{.val}}`}
-			v := map[string]string{"val": tt}
-			out, err := e.render(tpls, v)
+			v := chartutil.Values{"val": tt}
+			tpls := map[string]renderable{fname: {tpl: `{{.val}}`, vals: v}}
+			out, err := e.render(tpls)
 			if err != nil {
 				t.Errorf("Failed to render %s: %s", tt, err)
 			}
@@ -104,7 +105,8 @@ func TestAllTemplates(t *testing.T) {
 		},
 	}
 
-	tpls := allTemplates(ch1)
+	var v chartutil.Values
+	tpls := allTemplates(ch1, v)
 	if len(tpls) != 5 {
 		t.Errorf("Expected 5 charts, got %d", len(tpls))
 	}
@@ -143,3 +145,125 @@ func TestRenderDependency(t *testing.T) {
 	}
 
 }
+
+func TestRenderNestedValues(t *testing.T) {
+	e := New()
+
+	innerpath := "charts/inner/templates/inner.tpl"
+	outerpath := "templates/outer.tpl"
+	deepestpath := "charts/inner/charts/deepest/templates/deepest.tpl"
+
+	deepest := &chart.Chart{
+		Metadata: &chart.Metadata{Name: "deepest"},
+		Templates: []*chart.Template{
+			{Name: deepestpath, Data: []byte(`And this same {{.what}} that smiles to-day`)},
+		},
+		Values: &chart.Config{Raw: `what = "milkshake"`},
+	}
+
+	inner := &chart.Chart{
+		Metadata: &chart.Metadata{Name: "herrick"},
+		Templates: []*chart.Template{
+			{Name: innerpath, Data: []byte(`Old {{.who}} is still a-flyin'`)},
+		},
+		Values:       &chart.Config{Raw: `who = "Robert"`},
+		Dependencies: []*chart.Chart{deepest},
+	}
+
+	outer := &chart.Chart{
+		Metadata: &chart.Metadata{Name: "top"},
+		Templates: []*chart.Template{
+			{Name: outerpath, Data: []byte(`Gather ye {{.what}} while ye may`)},
+		},
+		Values: &chart.Config{
+			Raw: `what = "stinkweed"
+	[herrick]
+	who = "time"
+	`},
+		Dependencies: []*chart.Chart{inner},
+	}
+
+	inject := chart.Config{
+		Raw: `
+		what = "rosebuds"
+		[herrick.deepest]
+		what = "flower"`,
+	}
+
+	out, err := e.Render(outer, &inject)
+	if err != nil {
+		t.Fatalf("failed to render templates: %s", err)
+	}
+
+	if out[outerpath] != "Gather ye rosebuds while ye may" {
+		t.Errorf("Unexpected outer: %q", out[outerpath])
+	}
+
+	if out[innerpath] != "Old time is still a-flyin'" {
+		t.Errorf("Unexpected inner: %q", out[innerpath])
+	}
+
+	if out[deepestpath] != "And this same flower that smiles to-day" {
+		t.Errorf("Unexpected deepest: %q", out[deepestpath])
+	}
+}
+
+func TestCoalesceTables(t *testing.T) {
+	dst := map[string]interface{}{
+		"name": "Ishmael",
+		"address": map[string]interface{}{
+			"street": "123 Spouter Inn Ct.",
+			"city":   "Nantucket",
+		},
+		"details": map[string]interface{}{
+			"friends": []string{"Tashtego"},
+		},
+		"boat": "pequod",
+	}
+	src := map[string]interface{}{
+		"occupation": "whaler",
+		"address": map[string]interface{}{
+			"state":  "MA",
+			"street": "234 Spouter Inn Ct.",
+		},
+		"details": "empty",
+		"boat": map[string]interface{}{
+			"mast": true,
+		},
+	}
+	coalesceTables(dst, src)
+
+	if dst["name"] != "Ishmael" {
+		t.Errorf("Unexpected name: %s", dst["name"])
+	}
+	if dst["occupation"] != "whaler" {
+		t.Errorf("Unexpected occupation: %s", dst["occupation"])
+	}
+
+	addr, ok := dst["address"].(map[string]interface{})
+	if !ok {
+		t.Fatal("Address went away.")
+	}
+
+	if addr["street"].(string) != "234 Spouter Inn Ct." {
+		t.Errorf("Unexpected address: %v", addr["street"])
+	}
+
+	if addr["city"].(string) != "Nantucket" {
+		t.Errorf("Unexpected city: %v", addr["city"])
+	}
+
+	if addr["state"].(string) != "MA" {
+		t.Errorf("Unexpected state: %v", addr["state"])
+	}
+
+	if det, ok := dst["details"].(map[string]interface{}); !ok {
+		t.Fatalf("Details is the wrong type: %v", dst["details"])
+	} else if _, ok := det["friends"]; !ok {
+		t.Error("Could not find your friends. Maybe you don't have any. :-(")
+	}
+
+	if dst["boat"].(string) != "pequod" {
+		t.Errorf("Expected boat string, got %v", dst["boat"])
+	}
+}