Merge pull request #964 from technosophos/fix/957-template-name-on-failure

fix(tiller): provide filename when YAML is bad

cmd/tiller/hooks.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"log"
 	"strings"
 
@@ -57,16 +58,17 @@ type simpleHead struct {
 //
 // Files that do not parse into the expected format are simply placed into a map and
 // returned.
-func sortHooks(files map[string]string) (hs []*release.Hook, generic map[string]string) {
-	hs = []*release.Hook{}
-	generic = map[string]string{}
+func sortHooks(files map[string]string) ([]*release.Hook, map[string]string, error) {
+	hs := []*release.Hook{}
+	generic := map[string]string{}
 
 	for n, c := range files {
 		var sh simpleHead
 		err := yaml.Unmarshal([]byte(c), &sh)
 
 		if err != nil {
-			log.Printf("YAML parse error on %s: %s (skipping)", n, err)
+			e := fmt.Errorf("YAML parse error on %s: %s", n, err)
+			return hs, generic, e
 		}
 
 		if sh.Metadata == nil || sh.Metadata.Annotations == nil || len(sh.Metadata.Annotations) == 0 {
@@ -103,5 +105,5 @@ func sortHooks(files map[string]string) (hs []*release.Hook, generic map[string]
 		}
 		hs = append(hs, h)
 	}
-	return
+	return hs, generic, nil
 }

cmd/tiller/hooks_test.go

@@ -82,7 +82,10 @@ metadata:
 		manifests[o.path] = o.manifest
 	}
 
-	hs, generic := sortHooks(manifests)
+	hs, generic, err := sortHooks(manifests)
+	if err != nil {
+		t.Fatalf("Unexpected error: %s", err)
+	}
 
 	if len(generic) != 1 {
 		t.Errorf("Expected 1 generic manifest, got %d", len(generic))

cmd/tiller/release_server.go

@@ -26,6 +26,7 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/ghodss/yaml"
 	"github.com/technosophos/moniker"
 	ctx "golang.org/x/net/context"
 
@@ -217,10 +218,15 @@ func (s *releaseServer) engine(ch *chart.Chart) environment.Engine {
 func (s *releaseServer) InstallRelease(c ctx.Context, req *services.InstallReleaseRequest) (*services.InstallReleaseResponse, error) {
 	rel, err := s.prepareRelease(req)
 	if err != nil {
+		log.Printf("Failed install prepare step: %s", err)
 		return nil, err
 	}
 
-	return s.performRelease(rel, req)
+	res, err := s.performRelease(rel, req)
+	if err != nil {
+		log.Printf("Failed install perform step: %s", err)
+	}
+	return res, err
 }
 
 // prepareRelease builds a release for an install operation.
@@ -246,7 +252,12 @@ func (s *releaseServer) prepareRelease(req *services.InstallReleaseRequest) (*re
 	if err != nil {
 		return nil, err
 	}
-	hooks, manifests := sortHooks(files)
+	hooks, manifests, err := sortHooks(files)
+	if err != nil {
+		// By catching parse errors here, we can prevent bogus releases from going
+		// to Kubernetes.
+		return nil, err
+	}
 
 	// Aggregate all non-hooks into one big doc.
 	b := bytes.NewBuffer(nil)
@@ -282,6 +293,12 @@ func (s *releaseServer) prepareRelease(req *services.InstallReleaseRequest) (*re
 	return rel, nil
 }
 
+// validateYAML checks to see if YAML is well-formed.
+func validateYAML(data string) error {
+	b := map[string]interface{}{}
+	return yaml.Unmarshal([]byte(data), b)
+}
+
 // performRelease runs a release.
 func (s *releaseServer) performRelease(r *release.Release, req *services.InstallReleaseRequest) (*services.InstallReleaseResponse, error) {
 	res := &services.InstallReleaseResponse{Release: r}