fix(engine): change template naming

Template paths were relative to the chart that contained them, which
meant that all templates were named 'template/SOMETHING'. This made it
trivially easy to hit namespace collisions as in #933.

Template path names are essentially opaque strings so this patch simply
changes them to be qualified by parent chart.

cmd/tiller/release_server_test.go

@@ -134,7 +134,7 @@ func TestInstallRelease(t *testing.T) {
 		t.Errorf("Expected manifest in %v", res)
 	}
 
-	if !strings.Contains(rel.Manifest, "---\n# Source: hello\nhello: world") {
+	if !strings.Contains(rel.Manifest, "---\n# Source: hello/hello\nhello: world") {
 		t.Errorf("unexpected output: %s", rel.Manifest)
 	}
 }
@@ -150,8 +150,8 @@ func TestInstallReleaseDryRun(t *testing.T) {
 				{Name: "hello", Data: []byte("hello: world")},
 				{Name: "goodbye", Data: []byte("goodbye: world")},
 				{Name: "empty", Data: []byte("")},
-				{Name: "with-partials", Data: []byte("hello: {{ template \"partials/_planet\" . }}")},
+				{Name: "with-partials", Data: []byte(`hello: {{ template "_planet" . }}`)},
-				{Name: "partials/_planet", Data: []byte("Earth")},
+				{Name: "partials/_planet", Data: []byte(`{{define "_planet"}}Earth{{end}}`)},
 				{Name: "hooks", Data: []byte(manifestWithHook)},
 			},
 		},
@@ -165,11 +165,11 @@ func TestInstallReleaseDryRun(t *testing.T) {
 		t.Errorf("Expected release name.")
 	}
 
-	if !strings.Contains(res.Release.Manifest, "---\n# Source: hello\nhello: world") {
+	if !strings.Contains(res.Release.Manifest, "---\n# Source: hello/hello\nhello: world") {
 		t.Errorf("unexpected output: %s", res.Release.Manifest)
 	}
 
-	if !strings.Contains(res.Release.Manifest, "---\n# Source: goodbye\ngoodbye: world") {
+	if !strings.Contains(res.Release.Manifest, "---\n# Source: hello/goodbye\ngoodbye: world") {
 		t.Errorf("unexpected output: %s", res.Release.Manifest)
 	}
 
@@ -177,7 +177,7 @@ func TestInstallReleaseDryRun(t *testing.T) {
 		t.Errorf("Should contain partial content. %s", res.Release.Manifest)
 	}
 
-	if strings.Contains(res.Release.Manifest, "hello: {{ template \"partials/_planet\" . }}") {
+	if strings.Contains(res.Release.Manifest, "hello: {{ template \"_planet\" . }}") {
 		t.Errorf("Should not contain partial templates itself. %s", res.Release.Manifest)
 	}
 

pkg/engine/engine.go

@@ -19,6 +19,8 @@ package engine
 import (
 	"bytes"
 	"fmt"
+	"log"
+	"path"
 	"strings"
 	"text/template"
 
@@ -105,6 +107,7 @@ func (e *Engine) render(tpls map[string]renderable) (map[string]string, error) {
 	}
 	files := []string{}
 	for fname, r := range tpls {
+		log.Printf("Preparing template %s", fname)
 		t = t.New(fname).Funcs(e.FuncMap)
 		if _, err := t.Parse(r.tpl); err != nil {
 			return map[string]string{}, fmt.Errorf("parse error in %q: %s", fname, err)
@@ -137,7 +140,7 @@ func (e *Engine) render(tpls map[string]renderable) (map[string]string, error) {
 // 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)
+	recAllTpls(c, templates, vals, true, "")
 	return templates
 }
 
@@ -145,7 +148,7 @@ 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) {
+func recAllTpls(c *chart.Chart, templates map[string]renderable, parentVals chartutil.Values, top bool, parentID string) {
 	// This should never evaluate to a nil map. That will cause problems when
 	// values are appended later.
 	cvals := chartutil.Values{}
@@ -170,11 +173,19 @@ func recAllTpls(c *chart.Chart, templates map[string]renderable, parentVals char
 		}
 	}
 
+	newParentID := c.Metadata.Name
+	if parentID != "" {
+		// We artificially reconstruct the chart path to child templates. This
+		// creates a namespaced filename that can be used to track down the source
+		// of a particular template declaration.
+		newParentID = path.Join(parentID, "charts", newParentID)
+	}
+
 	for _, child := range c.Dependencies {
-		recAllTpls(child, templates, cvals, false)
+		recAllTpls(child, templates, cvals, false, newParentID)
 	}
 	for _, t := range c.Templates {
-		templates[t.Name] = renderable{
+		templates[path.Join(newParentID, t.Name)] = renderable{
 			tpl:  string(t.Data),
 			vals: cvals,
 		}

pkg/engine/engine_test.go

@@ -75,16 +75,16 @@ func TestRender(t *testing.T) {
 	}
 
 	expect := "Spouter Inn"
-	if out["test1"] != expect {
+	if out["moby/test1"] != expect {
 		t.Errorf("Expected %q, got %q", expect, out["test1"])
 	}
 
 	expect = "ishmael"
-	if out["test2"] != expect {
+	if out["moby/test2"] != expect {
 		t.Errorf("Expected %q, got %q", expect, out["test2"])
 	}
 	expect = ""
-	if out["test3"] != expect {
+	if out["moby/test3"] != expect {
 		t.Errorf("Expected %q, got %q", expect, out["test3"])
 	}
 
@@ -188,11 +188,13 @@ func TestRenderDependency(t *testing.T) {
 	deptpl := `{{define "myblock"}}World{{end}}`
 	toptpl := `Hello {{template "myblock"}}`
 	ch := &chart.Chart{
+		Metadata: &chart.Metadata{Name: "outerchart"},
 		Templates: []*chart.Template{
 			{Name: "outer", Data: []byte(toptpl)},
 		},
 		Dependencies: []*chart.Chart{
 			{
+				Metadata: &chart.Metadata{Name: "innerchart"},
 				Templates: []*chart.Template{
 					{Name: "inner", Data: []byte(deptpl)},
 				},
@@ -211,7 +213,7 @@ func TestRenderDependency(t *testing.T) {
 	}
 
 	expect := "Hello World"
-	if out["outer"] != expect {
+	if out["outerchart/outer"] != expect {
 		t.Errorf("Expected %q, got %q", expect, out["outer"])
 	}
 
@@ -220,10 +222,11 @@ func TestRenderDependency(t *testing.T) {
 func TestRenderNestedValues(t *testing.T) {
 	e := New()
 
-	innerpath := "charts/inner/templates/inner.tpl"
+	innerpath := "templates/inner.tpl"
 	outerpath := "templates/outer.tpl"
-	deepestpath := "charts/inner/charts/deepest/templates/deepest.tpl"
+	// Ensure namespacing rules are working.
-	checkrelease := "charts/inner/charts/deepest/templates/release.tpl"
+	deepestpath := "templates/inner.tpl"
+	checkrelease := "templates/release.tpl"
 
 	deepest := &chart.Chart{
 		Metadata: &chart.Metadata{Name: "deepest"},
@@ -288,19 +291,19 @@ global:
 		t.Fatalf("failed to render templates: %s", err)
 	}
 
-	if out[outerpath] != "Gather ye rosebuds while ye may" {
+	if out["top/"+outerpath] != "Gather ye rosebuds while ye may" {
 		t.Errorf("Unexpected outer: %q", out[outerpath])
 	}
 
-	if out[innerpath] != "Old time is still a-flyin'" {
+	if out["top/charts/herrick/"+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" {
+	if out["top/charts/herrick/charts/deepest/"+deepestpath] != "And this same flower that smiles to-day" {
 		t.Errorf("Unexpected deepest: %q", out[deepestpath])
 	}
 
-	if out[checkrelease] != "Tomorrow will be dyin" {
+	if out["top/charts/herrick/charts/deepest/"+checkrelease] != "Tomorrow will be dyin" {
 		t.Errorf("Unexpected release: %q", out[checkrelease])
 	}
 }
@@ -340,8 +343,8 @@ func TestRenderBuiltinValues(t *testing.T) {
 	}
 
 	expects := map[string]string{
-		"Lavinia": "LaviniaLatiumAeneid",
+		"Troy/charts/Latium/Lavinia": "Troy/charts/Latium/LaviniaLatiumAeneid",
-		"Aeneas":  "AeneasTroyAeneid",
+		"Troy/Aeneas":                "Troy/AeneasTroyAeneid",
 	}
 	for file, expect := range expects {
 		if out[file] != expect {