fix(tiller): refactor template render to use chartutil.

cmd/tiller/environment/environment.go

@@ -9,6 +9,7 @@ package environment
 import (
 	"io"
 
+	"k8s.io/helm/pkg/chartutil"
 	"k8s.io/helm/pkg/engine"
 	"k8s.io/helm/pkg/kube"
 	"k8s.io/helm/pkg/proto/hapi/chart"
@@ -68,7 +69,7 @@ type Engine interface {
 	//
 	// It receives a chart, a config, and a map of overrides to the config.
 	// Overrides are assumed to be passed from the system, not the user.
-	Render(*chart.Chart, *chart.Config, map[string]interface{}) (map[string]string, error)
+	Render(*chart.Chart, chartutil.Values) (map[string]string, error)
 }
 
 // ReleaseStorage represents a storage engine for a Release.

cmd/tiller/environment/environment_test.go

@@ -5,6 +5,7 @@ import (
 	"io"
 	"testing"
 
+	"k8s.io/helm/pkg/chartutil"
 	"k8s.io/helm/pkg/proto/hapi/chart"
 	"k8s.io/helm/pkg/proto/hapi/release"
 )
@@ -13,7 +14,7 @@ type mockEngine struct {
 	out map[string]string
 }
 
-func (e *mockEngine) Render(chrt *chart.Chart, v *chart.Config, o map[string]interface{}) (map[string]string, error) {
+func (e *mockEngine) Render(chrt *chart.Chart, v chartutil.Values) (map[string]string, error) {
 	return e.out, nil
 }
 
@@ -80,7 +81,7 @@ func TestEngine(t *testing.T) {
 
 	if engine, ok := env.EngineYard.Get("test"); !ok {
 		t.Errorf("failed to get engine from EngineYard")
-	} else if out, err := engine.Render(&chart.Chart{}, &chart.Config{}, map[string]interface{}{}); err != nil {
+	} else if out, err := engine.Render(&chart.Chart{}, map[string]interface{}{}); err != nil {
 		t.Errorf("unexpected template error: %s", err)
 	} else if out["albatross"] != "test" {
 		t.Errorf("expected 'test', got %q", out["albatross"])

cmd/tiller/release_server.go

@@ -12,6 +12,7 @@ import (
 	ctx "golang.org/x/net/context"
 
 	"k8s.io/helm/cmd/tiller/environment"
+	"k8s.io/helm/pkg/chartutil"
 	"k8s.io/helm/pkg/proto/hapi/release"
 	"k8s.io/helm/pkg/proto/hapi/services"
 	"k8s.io/helm/pkg/storage"
@@ -206,7 +207,11 @@ func (s *releaseServer) InstallRelease(c ctx.Context, req *services.InstallRelea
 
 	// Render the templates
 	// TODO: Fix based on whether chart has `engine: SOMETHING` set.
-	files, err := s.env.EngineYard.Default().Render(req.Chart, req.Values, overrides)
+	vals, err := chartutil.CoalesceValues(req.Chart, req.Values, overrides)
+	if err != nil {
+		return nil, err
+	}
+	files, err := s.env.EngineYard.Default().Render(req.Chart, vals)
 	if err != nil {
 		return nil, err
 	}

pkg/chartutil/values.go

@@ -87,6 +87,14 @@ func ReadValuesFile(filename string) (Values, error) {
 // CoalesceValues coalesces all of the values in a chart (and its subcharts).
 //
 // The overrides map may be used to specifically override configuration values.
+//
+// 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 CoalesceValues(chrt *chart.Chart, vals *chart.Config, overrides map[string]interface{}) (Values, error) {
 	var cvals Values
 	// Parse values if not nil. We merge these at the top level because
@@ -152,15 +160,15 @@ func coalesceValues(c *chart.Chart, v map[string]interface{}) map[string]interfa
 		// On error, we return just the overridden values.
 		// FIXME: We should log this error. It indicates that the YAML data
 		// did not parse.
-		log.Printf("error reading default values: %s", err)
+		log.Printf("error reading default values (%s): %s", c.Values.Raw, err)
 		return v
 	}
 
 	for key, val := range nv {
 		if _, ok := v[key]; !ok {
 			v[key] = val
-		} else if dest, ok := v[key].(Values); ok {
+		} else if dest, ok := v[key].(map[string]interface{}); ok {
-			src, ok := val.(Values)
+			src, ok := val.(map[string]interface{})
 			if !ok {
 				log.Printf("warning: skipped value for %s: Not a table.", key)
 				continue

pkg/engine/engine.go

@@ -3,7 +3,6 @@ package engine
 import (
 	"bytes"
 	"fmt"
-	"log"
 	"text/template"
 
 	"github.com/Masterminds/sprig"
@@ -45,41 +44,17 @@ func New() *Engine {
 // 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 should be prepared with something like `chartutils.ReadValues`.
+//
 // 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.
-//
+func (e *Engine) Render(chrt *chart.Chart, values chartutil.Values) (map[string]string, error) {
-// 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, overrides map[string]interface{}) (map[string]string, error) {
-	var cvals chartutil.Values
-
-	// 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
-		}
-		// Override the top-level values. Overrides are NEVER merged deeply.
-		// The assumption is that an override is intended to set an explicit
-		// and exact value.
-		for k, v := range overrides {
-			evals[k] = v
-		}
-		cvals = coalesceValues(chrt, evals)
-	}
-
 	// Render the charts
-	tmap := allTemplates(chrt, cvals)
+	tmap := allTemplates(chrt, values)
 	return e.render(tmap)
 }
 
@@ -138,20 +113,20 @@ func allTemplates(c *chart.Chart, vals chartutil.Values) map[string]renderable {
 // 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
+	var cvals 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
+		cvals = 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 = tmp
 		}
 	}
-	cvals := coalesceValues(c, pvals)
+
 	//log.Printf("racAllTpls values: %v", cvals)
 	for _, child := range c.Dependencies {
 		recAllTpls(child, templates, cvals, false)
@@ -163,70 +138,3 @@ func recAllTpls(c *chart.Chart, templates map[string]renderable, parentVals char
 		}
 	}
 }
-
-// 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 YAML 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 YAML table.
-func istable(v interface{}) bool {
-	_, ok := v.(map[string]interface{})
-	return ok
-}

pkg/engine/engine_test.go

@@ -44,7 +44,11 @@ func TestRender(t *testing.T) {
 	}
 
 	e := New()
-	out, err := e.Render(c, vals, overrides)
+	v, err := chartutil.CoalesceValues(c, vals, overrides)
+	if err != nil {
+		t.Fatalf("Failed to coalesce values: %s", err)
+	}
+	out, err := e.Render(c, v)
 	if err != nil {
 		t.Errorf("Failed to render templates: %s", err)
 	}
@@ -54,7 +58,7 @@ func TestRender(t *testing.T) {
 		t.Errorf("Expected %q, got %q", expect, out["test1"])
 	}
 
-	if _, err := e.Render(c, &chart.Config{}, overrides); err != nil {
+	if _, err := e.Render(c, v); err != nil {
 		t.Errorf("Unexpected error: %s", err)
 	}
 }
@@ -163,7 +167,7 @@ func TestRenderDependency(t *testing.T) {
 		},
 	}
 
-	out, err := e.Render(ch, nil, map[string]interface{}{})
+	out, err := e.Render(ch, map[string]interface{}{})
 
 	if err != nil {
 		t.Fatalf("failed to render chart: %s", err)
@@ -192,7 +196,7 @@ func TestRenderNestedValues(t *testing.T) {
 		Templates: []*chart.Template{
 			{Name: deepestpath, Data: []byte(`And this same {{.what}} that smiles to-day`)},
 		},
-		Values: &chart.Config{Raw: `what = "milkshake"`},
+		Values: &chart.Config{Raw: `what: "milkshake"`},
 	}
 
 	inner := &chart.Chart{
@@ -200,7 +204,7 @@ func TestRenderNestedValues(t *testing.T) {
 		Templates: []*chart.Template{
 			{Name: innerpath, Data: []byte(`Old {{.who}} is still a-flyin'`)},
 		},
-		Values:       &chart.Config{Raw: `who = "Robert"`},
+		Values:       &chart.Config{Raw: `who: "Robert"`},
 		Dependencies: []*chart.Chart{deepest},
 	}
 
@@ -212,13 +216,14 @@ func TestRenderNestedValues(t *testing.T) {
 		Values: &chart.Config{
 			Raw: `
 what: stinkweed
+who: me
 herrick:
-  who: time`,
+    who: time`,
 		},
 		Dependencies: []*chart.Chart{inner},
 	}
 
-	inject := chart.Config{
+	injValues := chart.Config{
 		Raw: `
 what: rosebuds
 herrick:
@@ -226,7 +231,14 @@ herrick:
     what: flower`,
 	}
 
-	out, err := e.Render(outer, &inject, map[string]interface{}{})
+	inject, err := chartutil.CoalesceValues(outer, &injValues, map[string]interface{}{})
+	if err != nil {
+		t.Fatalf("Failed to coalesce values: %s", err)
+	}
+
+	t.Logf("Calculated values: %v", inject)
+
+	out, err := e.Render(outer, inject)
 	if err != nil {
 		t.Fatalf("failed to render templates: %s", err)
 	}
@@ -243,63 +255,3 @@ herrick:
 		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"])
-	}
-}