From 6077968341b10a8929c243a8ae6f604ea5b5fbf0 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Thu, 21 Apr 2016 14:50:16 -0600 Subject: [PATCH 1/3] feat(engine): add recursive template resolution --- pkg/engine/engine.go | 19 +++++++++++--- pkg/engine/engine_test.go | 54 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index c19caab58..77b25140d 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -66,10 +66,7 @@ func (e *Engine) Render(chrt *chart.Chart, vals *chart.Config) (map[string]strin } // Render the charts - tmap := make(map[string]string, len(chrt.Templates)) - for _, tpl := range chrt.Templates { - tmap[tpl.Name] = string(tpl.Data) - } + tmap := allTemplates(chrt) return e.render(tmap, cvals) } @@ -103,3 +100,17 @@ func (e *Engine) render(tpls map[string]string, v interface{}) (map[string]strin return rendered, nil } + +// allTemplates returns all templates for a chart and its dependencies. +func allTemplates(c *chart.Chart) map[string]string { + templates := map[string]string{} + for _, child := range c.Dependencies { + for _, t := range child.Templates { + templates[t.Name] = string(t.Data) + } + } + for _, t := range c.Templates { + templates[t.Name] = string(t.Data) + } + return templates +} diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index dba3b2fe3..006a91bcd 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -4,6 +4,8 @@ import ( "fmt" "sync" "testing" + + "github.com/deis/tiller/pkg/proto/hapi/chart" ) func TestEngine(t *testing.T) { @@ -80,3 +82,55 @@ func TestParallelRenderInternals(t *testing.T) { } wg.Wait() } + +func TestAllTemplates(t *testing.T) { + ch1 := &chart.Chart{ + Templates: []*chart.Template{ + {Name: "foo", Data: []byte("foo")}, + {Name: "bar", Data: []byte("bar")}, + }, + Dependencies: []*chart.Chart{ + {Templates: []*chart.Template{ + {Name: "pinky", Data: []byte("pinky")}, + {Name: "brain", Data: []byte("brain")}, + }}, + }, + } + + tpls := allTemplates(ch1) + if len(tpls) != 4 { + t.Errorf("Expected 4 charts, got %d", len(tpls)) + } +} + +func TestRenderDependency(t *testing.T) { + e := New() + deptpl := `{{define "myblock"}}World{{end}}` + toptpl := `Hello {{template "myblock"}}` + ch := &chart.Chart{ + Templates: []*chart.Template{ + {Name: "outer", Data: []byte(toptpl)}, + }, + Dependencies: []*chart.Chart{ + {Templates: []*chart.Template{ + {Name: "inner", Data: []byte(deptpl)}, + }}, + }, + } + + out, err := e.Render(ch, nil) + + if err != nil { + t.Fatalf("failed to render chart: %s", err) + } + + if len(out) != 2 { + t.Errorf("Expected 2, got %d", len(out)) + } + + expect := "Hello World" + if out["outer"] != expect { + t.Errorf("Expected %q, got %q", expect, out["outer"]) + } + +} From 07312c68e917854ce2f767c06c8b502ea7861c36 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Thu, 21 Apr 2016 15:52:16 -0600 Subject: [PATCH 2/3] fix(engine): support chart template recursion --- pkg/engine/engine.go | 11 +++++++---- pkg/engine/engine_test.go | 27 ++++++++++++++++++--------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 77b25140d..4d3c8b315 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -104,13 +104,16 @@ 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) + return templates +} + +func recAllTpls(c *chart.Chart, templates map[string]string) { for _, child := range c.Dependencies { - for _, t := range child.Templates { - templates[t.Name] = string(t.Data) - } + recAllTpls(child, templates) } for _, t := range c.Templates { templates[t.Name] = string(t.Data) } - return templates + } diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 006a91bcd..36992a1f2 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -90,16 +90,23 @@ func TestAllTemplates(t *testing.T) { {Name: "bar", Data: []byte("bar")}, }, Dependencies: []*chart.Chart{ - {Templates: []*chart.Template{ - {Name: "pinky", Data: []byte("pinky")}, - {Name: "brain", Data: []byte("brain")}, - }}, + { + Templates: []*chart.Template{ + {Name: "pinky", Data: []byte("pinky")}, + {Name: "brain", Data: []byte("brain")}, + }, + Dependencies: []*chart.Chart{ + {Templates: []*chart.Template{ + {Name: "innermost", Data: []byte("innermost")}, + }}, + }, + }, }, } tpls := allTemplates(ch1) - if len(tpls) != 4 { - t.Errorf("Expected 4 charts, got %d", len(tpls)) + if len(tpls) != 5 { + t.Errorf("Expected 5 charts, got %d", len(tpls)) } } @@ -112,9 +119,11 @@ func TestRenderDependency(t *testing.T) { {Name: "outer", Data: []byte(toptpl)}, }, Dependencies: []*chart.Chart{ - {Templates: []*chart.Template{ - {Name: "inner", Data: []byte(deptpl)}, - }}, + { + Templates: []*chart.Template{ + {Name: "inner", Data: []byte(deptpl)}, + }, + }, }, } From ba22a18fadbe22db80bc7fd3ba794456255da5c2 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Thu, 21 Apr 2016 16:45:42 -0600 Subject: [PATCH 3/3] fix(engine): coalesce values in templates --- pkg/engine/engine.go | 163 +++++++++++++++++++++++++++++++------- pkg/engine/engine_test.go | 144 ++++++++++++++++++++++++++++++--- 2 files changed, 269 insertions(+), 38 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 4d3c8b315..76af245a9 100644 --- a/pkg/engine/engine.go +++ b/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 } diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 36992a1f2..6a88fe2bd 100644 --- a/pkg/engine/engine_test.go +++ b/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"]) + } +}