Merge pull request #992 from technosophos/fix/991-partials-parse-error

fix(tiller): remove partials and empty manifests

cmd/tiller/hooks.go

@@ -19,6 +19,7 @@ package main
 import (
 	"fmt"
 	"log"
+	"path"
 	"strings"
 
 	"github.com/ghodss/yaml"
@@ -79,6 +80,17 @@ func sortHooks(files map[string]string) ([]*release.Hook, map[string]string, err
 	generic := map[string]string{}
 
 	for n, c := range files {
+		// Skip partials. We could return these as a separate map, but there doesn't
+		// seem to be any need for that at this time.
+		if strings.HasPrefix(path.Base(n), "_") {
+			continue
+		}
+		// Skip empty files, and log this.
+		if len(strings.TrimSpace(c)) == 0 {
+			log.Printf("info: manifest %q is empty. Skipping.", n)
+			continue
+		}
+
 		var sh simpleHead
 		err := yaml.Unmarshal([]byte(c), &sh)
 

cmd/tiller/hooks_test.go

@@ -90,6 +90,20 @@ metadata:
   annotations:
     "helm.sh/hook": post-delete, post-install
 `,
+		}, {
+			// Regression test: files with an underscore in the base name should be skipped.
+			name:     "sixth",
+			path:     "six/_six",
+			kind:     "ReplicaSet",
+			hooks:    []release.Hook_Event{},
+			manifest: `invalid manifest`, // This will fail if partial is not skipped.
+		}, {
+			// Regression test: files with no content should be skipped.
+			name:     "seventh",
+			path:     "seven",
+			kind:     "ReplicaSet",
+			hooks:    []release.Hook_Event{},
+			manifest: "",
 		},
 	}
 
@@ -103,6 +117,7 @@ metadata:
 		t.Fatalf("Unexpected error: %s", err)
 	}
 
+	// This test will fail if 'six' or 'seven' was added.
 	if len(generic) != 1 {
 		t.Errorf("Expected 1 generic manifest, got %d", len(generic))
 	}

cmd/tiller/release_server.go

@@ -21,10 +21,8 @@ import (
 	"errors"
 	"fmt"
 	"log"
-	"path"
 	"regexp"
 	"sort"
-	"strings"
 
 	"github.com/Masterminds/semver"
 	"github.com/ghodss/yaml"
@@ -309,6 +307,10 @@ func (s *releaseServer) prepareRelease(req *services.InstallReleaseRequest) (*re
 	if err != nil {
 		return nil, err
 	}
+
+	// Sort hooks, manifests, and partials. Only hooks and manifests are returned,
+	// as partials are not used after renderer.Render. Empty manifests are also
+	// removed here.
 	hooks, manifests, err := sortHooks(files)
 	if err != nil {
 		// By catching parse errors here, we can prevent bogus releases from going
@@ -316,20 +318,11 @@ func (s *releaseServer) prepareRelease(req *services.InstallReleaseRequest) (*re
 		return nil, err
 	}
 
-	// Aggregate all non-hooks into one big doc.
+	// Aggregate all valid manifests into one big doc.
 	b := bytes.NewBuffer(nil)
 	for name, file := range manifests {
-		// Ignore templates that starts with underscore to handle them as partials
-		if strings.HasPrefix(path.Base(name), "_") {
-			continue
-		}
-
-		// Ignore empty documents because the Kubernetes library can't handle
-		// them.
-		if len(file) > 0 {
-			b.WriteString("\n---\n# Source: " + name + "\n")
-			b.WriteString(file)
-		}
+		b.WriteString("\n---\n# Source: " + name + "\n")
+		b.WriteString(file)
 	}
 
 	// Store a release.