From 8937c775a9799387d5e1b76222d197919a992ae7 Mon Sep 17 00:00:00 2001
From: Matt Butcher <technosophos@gmail.com>
Date: Fri, 26 May 2017 17:06:41 -0600
Subject: [PATCH] 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 #2452
---
 pkg/engine/engine.go      | 33 ++++++++++++++++++++++++++++++++-
 pkg/engine/engine_test.go | 31 +++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go
index 85b6bd30d..3d348fb41 100644
--- a/pkg/engine/engine.go
+++ b/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.
diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go
index f821b2de5..88118c420 100644
--- a/pkg/engine/engine_test.go
+++ b/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()