fix(2452): sort templates before parse

This sorts templates by depth before sending them to the template
parser. Deepest templates are parsed first, with umbrella templates
parsed last. Since template definition names are LIFO, that means that
the highest level templates will claim the namespace.

Or, to put it simply, you can predictably override a child's defined
template by re-defining it in a parent chart.

Closes helm#2452

Author: technosophos

pkg/engine/engine.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"fmt"
 	"path"
+	"sort"
 	"strings"
 	"text/template"
 
@@ -181,8 +182,14 @@ func (e *Engine) render(tpls map[string]renderable) (map[string]string, error) {
 
 	funcMap := e.alterFuncMap(t)
 
+	// We want to parse the templates in a predictable order. The order favors
+	// higher-level (in file system) templates over deeply nested templates.
+	keys := sortTemplates(tpls)
+
 	files := []string{}
-	for fname, r := range tpls {
+	//for fname, r := range tpls {
+	for _, fname := range keys {
+		r := tpls[fname]
 		t = t.New(fname).Funcs(funcMap)
 		if _, err := t.Parse(r.tpl); err != nil {
 			return map[string]string{}, fmt.Errorf("parse error in %q: %s", fname, err)
@@ -215,6 +222,30 @@ func (e *Engine) render(tpls map[string]renderable) (map[string]string, error) {
 	return rendered, nil
 }
 
+func sortTemplates(tpls map[string]renderable) []string {
+	keys := make([]string, len(tpls))
+	i := 0
+	for key := range tpls {
+		keys[i] = key
+		i++
+	}
+	sort.Sort(sort.Reverse(byPathLen(keys)))
+	return keys
+}
+
+type byPathLen []string
+
+func (p byPathLen) Len() int      { return len(p) }
+func (p byPathLen) Swap(i, j int) { p[j], p[i] = p[i], p[j] }
+func (p byPathLen) Less(i, j int) bool {
+	a, b := p[i], p[j]
+	ca, cb := strings.Count(a, "/"), strings.Count(b, "/")
+	if ca == cb {
+		return strings.Compare(a, b) == -1
+	}
+	return ca < cb
+}
+
 // allTemplates returns all templates for a chart and its dependencies.
 //
 // As it goes, it also prepares the values in a scope-sensitive manner.

pkg/engine/engine_test.go

@@ -27,6 +27,37 @@ import (
 	"github.com/golang/protobuf/ptypes/any"
 )
 
+func TestSortTemplates(t *testing.T) {
+	tpls := map[string]renderable{
+		"/mychart/templates/foo.tpl":                                 {},
+		"/mychart/templates/charts/foo/charts/bar/templates/foo.tpl": {},
+		"/mychart/templates/bar.tpl":                                 {},
+		"/mychart/templates/charts/foo/templates/bar.tpl":            {},
+		"/mychart/templates/_foo.tpl":                                {},
+		"/mychart/templates/charts/foo/templates/foo.tpl":            {},
+		"/mychart/templates/charts/bar/templates/foo.tpl":            {},
+	}
+	got := sortTemplates(tpls)
+	if len(got) != len(tpls) {
+		t.Fatal("Sorted results are missing templates")
+	}
+
+	expect := []string{
+		"/mychart/templates/charts/foo/charts/bar/templates/foo.tpl",
+		"/mychart/templates/charts/foo/templates/foo.tpl",
+		"/mychart/templates/charts/foo/templates/bar.tpl",
+		"/mychart/templates/charts/bar/templates/foo.tpl",
+		"/mychart/templates/foo.tpl",
+		"/mychart/templates/bar.tpl",
+		"/mychart/templates/_foo.tpl",
+	}
+	for i, e := range expect {
+		if got[i] != e {
+			t.Errorf("expected %q, got %q at index %d\n\tExp: %#v\n\tGot: %#v", e, got[i], i, expect, got)
+		}
+	}
+}
+
 func TestEngine(t *testing.T) {
 	e := New()