fix(helm): Don't crash on bad --values arg

Closes #3679

cmd/helm/init_test.go

@@ -63,10 +63,10 @@ func TestInitCmd(t *testing.T) {
 	if len(actions) != 2 {
 		t.Errorf("Expected 2 actions, got %d", len(actions))
 	}
-	if !actions[0].Matches("create", "deployments") {
+	if len(actions) >= 1 && !actions[0].Matches("create", "deployments") {
 		t.Errorf("unexpected action: %v, expected create deployment", actions[0])
 	}
-	if !actions[1].Matches("create", "services") {
+	if len(actions) >= 2 && !actions[1].Matches("create", "services") {
 		t.Errorf("unexpected action: %v, expected create service", actions[1])
 	}
 	expected := "Tiller (the Helm server-side component) has been installed into your Kubernetes Cluster."

cmd/helm/install.go

@@ -491,9 +491,12 @@ func checkDependencies(ch *chart.Chart, reqs *chartutil.Requirements) error {
 
 //readFile load a file from the local directory or a remote file with a url.
 func readFile(filePath string) ([]byte, error) {
-	u, _ := url.Parse(filePath)
+	u, err := url.Parse(filePath)
-	p := getter.All(settings)
+	if err != nil {
+		return ioutil.ReadFile(filePath)
+	}
 
+	p := getter.All(settings)
 	// FIXME: maybe someone handle other protocols like ftp.
 	getterConstructor, err := p.ByScheme(u.Scheme)
 

cmd/helm/install_test.go

@@ -17,7 +17,11 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
 	"io"
+	"net/http"
+	"net/http/httptest"
+	"os"
 	"reflect"
 	"regexp"
 	"strings"
@@ -27,7 +31,29 @@ import (
 	"k8s.io/helm/pkg/helm"
 )
 
-func TestInstall(t *testing.T) {
+func TestInstallCmd(t *testing.T) {
+	// Prepare mock server for some test cases
+	httpHandler := func(writer http.ResponseWriter, req *http.Request) {
+		switch req.URL.Path {
+		case "/extra_values.yaml":
+			fmt.Fprint(writer, "foo: bar")
+		case "/more_values.yaml":
+			fmt.Fprint(writer, "foo: baz")
+		default:
+			panic(http.ErrAbortHandler)
+		}
+	}
+	httpSrv := httptest.NewServer(http.HandlerFunc(httpHandler))
+	defer httpSrv.Close()
+
+	// Force debug flag so that results can be inspected in detail. However, suppress stdout noise.
+	wasDebug, origStdout := settings.Debug, os.Stdout
+	settings.Debug = true
+	os.Stdout, _ = os.Open(os.DevNull)
+	defer func() {
+		settings.Debug, os.Stdout = wasDebug, origStdout
+	}()
+
 	tests := []releaseCase{
 		// Install, base case
 		{
@@ -35,7 +61,6 @@ func TestInstall(t *testing.T) {
 			args:     []string{"testdata/testcharts/alpine"},
 			flags:    strings.Split("--name aeneas", " "),
 			expected: "aeneas",
-			resp:     helm.ReleaseMock(&helm.MockReleaseOptions{Name: "aeneas"}),
 		},
 		// Install, no hooks
 		{
@@ -43,39 +68,46 @@ func TestInstall(t *testing.T) {
 			args:     []string{"testdata/testcharts/alpine"},
 			flags:    strings.Split("--name aeneas --no-hooks", " "),
 			expected: "aeneas",
-			resp:     helm.ReleaseMock(&helm.MockReleaseOptions{Name: "aeneas"}),
 		},
 		// Install, values from cli
 		{
 			name:     "install with values",
 			args:     []string{"testdata/testcharts/alpine"},
 			flags:    strings.Split("--name virgil --set foo=bar", " "),
-			resp:     helm.ReleaseMock(&helm.MockReleaseOptions{Name: "virgil"}),
+			expected: `(?s)NAME:\s*virgil.+USER-SUPPLIED VALUES:\s*foo: bar`,
-			expected: "virgil",
 		},
 		// Install, values from cli via multiple --set
 		{
 			name:     "install with multiple values",
 			args:     []string{"testdata/testcharts/alpine"},
 			flags:    strings.Split("--name virgil --set foo=bar --set bar=foo", " "),
-			resp:     helm.ReleaseMock(&helm.MockReleaseOptions{Name: "virgil"}),
+			expected: `(?s)NAME:\s*virgil.+USER-SUPPLIED VALUES:\s*bar: foo\s*foo: bar`,
-			expected: "virgil",
 		},
 		// Install, values from yaml
 		{
 			name:     "install with values",
 			args:     []string{"testdata/testcharts/alpine"},
 			flags:    strings.Split("--name virgil -f testdata/testcharts/alpine/extra_values.yaml", " "),
-			resp:     helm.ReleaseMock(&helm.MockReleaseOptions{Name: "virgil"}),
+			expected: `(?s)NAME:\s*virgil.+USER-SUPPLIED VALUES:\s*foo: bar`,
-			expected: "virgil",
+		},
+		{
+			name:     "install with values",
+			args:     []string{"testdata/testcharts/alpine"},
+			flags:    strings.Split("--name virgil -f "+httpSrv.URL+"/extra_values.yaml", " "),
+			expected: `(?s)NAME:\s*virgil.+USER-SUPPLIED VALUES:\s*foo: bar`,
 		},
 		// Install, values from multiple yaml
 		{
 			name:     "install with values",
 			args:     []string{"testdata/testcharts/alpine"},
 			flags:    strings.Split("--name virgil -f testdata/testcharts/alpine/extra_values.yaml -f testdata/testcharts/alpine/more_values.yaml", " "),
-			resp:     helm.ReleaseMock(&helm.MockReleaseOptions{Name: "virgil"}),
+			expected: `(?s)NAME:\s*virgil.+USER-SUPPLIED VALUES:\s*foo: baz`,
-			expected: "virgil",
+		},
+		{
+			name:     "install with values",
+			args:     []string{"testdata/testcharts/alpine"},
+			flags:    strings.Split("--name virgil -f "+httpSrv.URL+"/extra_values.yaml --values "+httpSrv.URL+"/more_values.yaml", " "),
+			expected: `(?s)NAME:\s*virgil.+USER-SUPPLIED VALUES:\s*foo: baz`,
 		},
 		// Install, no charts
 		{
@@ -83,6 +115,19 @@ func TestInstall(t *testing.T) {
 			args: []string{},
 			err:  true,
 		},
+		// Install, bad values arg
+		{
+			name:  "install chart with bad --values #1",
+			args:  []string{"testdata/testcharts/alpine"},
+			flags: strings.Split("--name virgil -f missing", " "),
+			err:   true,
+		},
+		{
+			name:  "install chart with bad --values #2",
+			args:  []string{"testdata/testcharts/alpine"},
+			flags: strings.Split("--name virgil --values url.parsing=will:fail,for:sure", " "),
+			err:   true,
+		},
 		// Install, re-use name
 		{
 			name:     "install and replace release",

cmd/helm/testdata/testcharts/alpine/extra_values.yaml

@@ -1,2 +1 @@
-test:
+foo: bar
-  Name: extra-values

cmd/helm/testdata/testcharts/alpine/more_values.yaml

@@ -1,2 +1 @@
-test:
+foo: baz
-  Name: more-values

pkg/helm/fake.go

@@ -77,6 +77,9 @@ func (c *FakeClient) InstallReleaseFromChart(chart *chart.Chart, ns string, opts
 	}
 
 	release := ReleaseMock(&MockReleaseOptions{Name: releaseName, Namespace: ns})
+	if c.Opts.instReq.Values != nil {
+		release.Config = c.Opts.instReq.Values
+	}
 	c.Rels = append(c.Rels, release)
 
 	return &rls.InstallReleaseResponse{