From 57fecb273566c5dcffa3c53d089a33a26e65d739 Mon Sep 17 00:00:00 2001
From: Matt Butcher <mbutcher@engineyard.com>
Date: Wed, 20 Jul 2016 06:48:30 -0600
Subject: [PATCH] fix(tiller): provide filename when YAML is bad

This makes the template system less tolerant in the name of detecting
YAML parse errors before things are sent to Kubernetes. It returns a
more detailed error message when a template creates a manifest which is
not valid YAML.

Closes #957
---
 cmd/tiller/hooks.go          | 12 +++++++-----
 cmd/tiller/hooks_test.go     |  5 ++++-
 cmd/tiller/release_server.go | 21 +++++++++++++++++++--
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/cmd/tiller/hooks.go b/cmd/tiller/hooks.go
index 762272986..8678bb193 100644
--- a/cmd/tiller/hooks.go
+++ b/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
 }
diff --git a/cmd/tiller/hooks_test.go b/cmd/tiller/hooks_test.go
index a33880525..c2ea57fa2 100644
--- a/cmd/tiller/hooks_test.go
+++ b/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))
diff --git a/cmd/tiller/release_server.go b/cmd/tiller/release_server.go
index df9eda87e..077848fc1 100644
--- a/cmd/tiller/release_server.go
+++ b/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"
 
@@ -218,10 +219,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.
@@ -247,7 +253,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}