Fix for upgrade -i, add more tests

Added a fix for upgrade --install
added more tests for loader, parser and upgrade cmd

Signed-off-by: Vlad Fratila <vfratila@adobe.com>

cmd/helm/install_test.go

@@ -50,19 +50,19 @@ func TestInstall(t *testing.T) {
 		// Install, external file
 		{
 			name:   "install with external files",
-			cmd:    "install virgil testdata/testcharts/configmap --include-file external.txt=testdata/files/external.txt",
+			cmd:    "install virgil testdata/testcharts/external --include-file external.txt=testdata/files/external.txt",
 			golden: "output/install-with-external-files.txt",
 		},
 		// Install, external dir
 		{
 			name:   "install with external dir",
-			cmd:    "install virgil testdata/testcharts/configmap --set glob.enabled=true --include-dir glob=testdata/files/",
+			cmd:    "install virgil testdata/testcharts/external --set glob.enabled=true --include-dir glob=testdata/files/",
 			golden: "output/install-with-external-files.txt",
 		},
 		// Install, external glob files
 		{
 			name:   "install with external globbed files",
-			cmd:    "install virgil testdata/testcharts/configmap --set glob.enabled=true --include-dir glob=testdata/files/external.*.conf",
+			cmd:    "install virgil testdata/testcharts/external --set glob.enabled=true --include-dir glob=testdata/files/external.*.conf",
 			golden: "output/install-with-external-files.txt",
 		},
 		// Install, no hooks

cmd/helm/template_test.go

@@ -123,17 +123,17 @@ func TestTemplateCmd(t *testing.T) {
 		},
 		{
 			name:   "chart with template with external file",
-			cmd:    fmt.Sprintf("template '%s' --set external=external.txt --include-file external.txt=testdata/files/external.txt", "testdata/testcharts/configmap"),
+			cmd:    fmt.Sprintf("template '%s' --set external=external.txt --include-file external.txt=testdata/files/external.txt", "testdata/testcharts/external"),
 			golden: "output/template-with-external-file.txt",
 		},
 		{
 			name:   "chart with template with external dir",
-			cmd:    fmt.Sprintf("template '%s' --set glob.enabled=true --include-dir glob=testdata/files/", "testdata/testcharts/configmap"),
+			cmd:    fmt.Sprintf("template '%s' --set glob.enabled=true --include-dir glob=testdata/files/", "testdata/testcharts/external"),
 			golden: "output/template-with-external-dir.txt",
 		},
 		{
 			name:   "chart with template with external globbed files",
-			cmd:    fmt.Sprintf("template '%s' --set glob.enabled=true --include-dir glob=testdata/files/external.*.conf", "testdata/testcharts/configmap"),
+			cmd:    fmt.Sprintf("template '%s' --set glob.enabled=true --include-dir glob=testdata/files/external.*.conf", "testdata/testcharts/external"),
 			golden: "output/template-with-external-glob.txt",
 		},
 	}

cmd/helm/upgrade.go

@@ -100,6 +100,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 					instClient.PostRenderer = client.PostRenderer
 					instClient.DisableOpenAPIValidation = client.DisableOpenAPIValidation
 					instClient.SubNotes = client.SubNotes
+					instClient.ExternalFiles = client.ExternalFiles
 
 					rel, err := runInstall(args, instClient, valueOpts, out)
 					if err != nil {

cmd/helm/upgrade_test.go

@@ -343,6 +343,39 @@ func TestUpgradeInstallWithValuesFromStdin(t *testing.T) {
 
 }
 
+func TestUpgradeWithExternalFile(t *testing.T) {
+
+	releaseName := "funny-bunny-v7"
+
+	exFiles := []*chart.File{
+		{Name: "external.txt", Data: []byte("from-external-file")},
+	}
+
+	relMock, ch, chartPath := prepareMockReleaseWithExternal(releaseName, exFiles, t)
+
+	defer resetEnv()()
+
+	store := storageFixture()
+
+	store.Create(relMock(releaseName, 3, ch))
+
+	cmd := fmt.Sprintf("upgrade %s --set glob.enabled=false --set external=external.txt '%s'", releaseName, chartPath)
+	_, _, err := executeActionCommandC(store, cmd)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	updatedRel, err := store.Get(releaseName, 4)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	if !strings.Contains(updatedRel.Manifest, "from-external-file") {
+		t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest)
+	}
+
+}
+
 func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) {
 	tmpChart := ensure.TempDir(t)
 	configmapData, err := ioutil.ReadFile("testdata/testcharts/upgradetest/templates/configmap.yaml")
@@ -378,6 +411,43 @@ func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int,
 	return relMock, ch, chartPath
 }
 
+func prepareMockReleaseWithExternal(releaseName string, exFiles []*chart.File, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) {
+	tmpChart := ensure.TempDir(t)
+	configmapData, err := ioutil.ReadFile("testdata/testcharts/external/templates/config-map.yaml")
+
+	if err != nil {
+		t.Fatalf("Error loading template yaml %v", err)
+	}
+	cfile := &chart.Chart{
+		Metadata: &chart.Metadata{
+			APIVersion:  chart.APIVersionV1,
+			Name:        "testUpgradeChart",
+			Description: "A Helm chart for Kubernetes",
+			Version:     "0.1.0",
+		},
+		Templates: []*chart.File{{Name: "templates/configmap.yaml", Data: configmapData}},
+		Files:     exFiles,
+	}
+	chartPath := filepath.Join(tmpChart, cfile.Metadata.Name)
+	if err := chartutil.SaveDir(cfile, tmpChart); err != nil {
+		t.Fatalf("Error creating chart for upgrade: %v", err)
+	}
+	ch, err := loader.Load(chartPath)
+	if err != nil {
+		t.Fatalf("Error loading chart: %v", err)
+	}
+	_ = release.Mock(&release.MockReleaseOptions{
+		Name:  releaseName,
+		Chart: ch,
+	})
+
+	relMock := func(n string, v int, ch *chart.Chart) *release.Release {
+		return release.Mock(&release.MockReleaseOptions{Name: n, Version: v, Chart: ch})
+	}
+
+	return relMock, ch, chartPath
+}
+
 func TestUpgradeOutputCompletion(t *testing.T) {
 	outputFlagCompletionTest(t, "upgrade")
 }

pkg/chart/loader/local_test.go

@@ -0,0 +1,49 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package loader
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestExpandLocalPath(t *testing.T) {
+	need := require.New(t)
+	is := assert.New(t)
+
+	glob, err := ExpandLocalPath("glob", "testdata/frobnitz/*.yaml")
+	need.NoError(err)
+	need.Contains(glob, "glob/Chart.yaml")
+	need.Contains(glob, "glob/values.yaml")
+	is.Equal("testdata/frobnitz/Chart.yaml", glob["glob/Chart.yaml"])
+	is.Equal("testdata/frobnitz/values.yaml", glob["glob/values.yaml"])
+
+	dir, err := ExpandLocalPath("dir", "testdata/albatross/")
+	need.NoError(err)
+	need.Contains(dir, "dir/Chart.yaml")
+	need.Contains(dir, "dir/values.yaml")
+	is.Equal("testdata/albatross/Chart.yaml", dir["dir/Chart.yaml"])
+	is.Equal("testdata/albatross/values.yaml", dir["dir/values.yaml"])
+
+	file, err := ExpandLocalPath("file", "testdata/albatross/Chart.yaml")
+	need.NoError(err)
+	need.Contains(file, "file")
+	is.Equal("testdata/albatross/Chart.yaml", file["file"])
+
+}

pkg/cli/files/parser.go

@@ -19,7 +19,6 @@ package files
 import (
 	"errors"
 	"fmt"
-	"os"
 	"path/filepath"
 	"strings"
 )
@@ -51,19 +50,16 @@ func ParseGlobIntoString(g string, dest map[string]string) error {
 	}
 	for k, g := range globs {
 		if !strings.Contains(g, "*") {
-			if _, err := os.Stat(g); os.IsNotExist(err) {
-				return err
-			}
-
 			// force glob style on simple directories
 			g = strings.TrimRight(g, "/") + "/*"
 		}
-		fmt.Println(g)
 
 		paths, err := filepath.Glob(g)
 		if err != nil {
 			return err
 		}
+
+		k = strings.TrimRight(k, "/")
 		for _, path := range paths {
 			dest[fmt.Sprintf("%s/%s", k, filepath.Base(path))] = path
 		}

pkg/cli/files/parser_test.go

@@ -55,3 +55,24 @@ func TestParseIntoString(t *testing.T) {
 	err = ParseIntoString(badFlag, dest)
 	is.NotNil(err)
 }
+
+func TestParseGlobIntoString(t *testing.T) {
+	need := require.New(t)
+	is := assert.New(t)
+
+	dest := make(map[string]string)
+	globFlagSlash := "glob/=testdata/foo/foo.*"
+	dirFlagNoSlash := "dir=testdata/foo/"
+
+	err := ParseGlobIntoString(globFlagSlash, dest)
+	need.NoError(err)
+	need.Contains(dest, "glob/foo.txt")
+	is.Equal("testdata/foo/foo.txt", dest["glob/foo.txt"])
+
+	err = ParseGlobIntoString(dirFlagNoSlash, dest)
+	need.NoError(err)
+	need.Contains(dest, "dir/foo.txt")
+	need.Contains(dest, "dir/bar.txt")
+	is.Equal("testdata/foo/foo.txt", dest["dir/foo.txt"])
+	is.Equal("testdata/foo/bar.txt", dest["dir/bar.txt"])
+}

pkg/cli/files/testdata/foo/bar.txt

@@ -0,0 +1 @@
+bar

pkg/cli/files/testdata/foo/foo.txt

@@ -0,0 +1 @@
+foo