Require passing only file name without alias. E.g: helm install myrelease mychart --include-file /opt/my_file.txt

Signed-off-by: itaispiegel <itai.spiegel@gmail.com>

cmd/helm/install.go

@@ -300,36 +300,25 @@ func checkIfInstallable(ch *chart.Chart) error {
 
 func loadExternalFiles(ch *chart.Chart, exFiles files.ExternalFiles) error {
 	var errs []string
-	fs := make(map[string]string)
+	var fs []string
 
-	for _, s := range exFiles.Files {
+	fs = append(fs, exFiles.Files...)
-		if err := files.ParseIntoString(s, fs); err != nil {
+	fs = append(fs, exFiles.Globs...)
-			debug(fmt.Sprintf("error parsing include-file option %s: %v", s, err))
-			errs = append(errs, fmt.Sprintf("%s (parse error)", s))
-		}
-	}
-
-	for _, g := range exFiles.Globs {
-		if err := files.ParseIntoString(g, fs); err != nil {
-			debug(fmt.Sprintf("error parsing include-dir option %s: %v", g, err))
-			errs = append(errs, fmt.Sprintf("%s (parse error)", g))
-		}
-	}
 
-	for n, p := range fs {
+	for _, path := range fs {
-		allPaths, err := loader.ExpandLocalPath(n, p)
+		allPaths, err := loader.ExpandFilePath(path)
-		debug(fmt.Sprintf("%s expanded to: %v", p, allPaths))
+		debug(fmt.Sprintf("%s expanded to: %v", path, allPaths))
 		if err != nil {
-			debug(fmt.Sprintf("error loading external path %s: %v", p, err))
+			debug(fmt.Sprintf("error loading external path %s: %v", path, err))
-			errs = append(errs, fmt.Sprintf("%s (path not accessible)", p))
+			errs = append(errs, fmt.Sprintf("%s (path not accessible)", path))
 		}
 
-		for name, fp := range allPaths {
+		for _, name := range allPaths {
-			byt, err := ioutil.ReadFile(fp)
+			fileContentBytes, err := ioutil.ReadFile(name)
 			if err != nil {
-				errs = append(errs, fmt.Sprintf("%s (not readable)", fp))
+				errs = append(errs, fmt.Sprintf("%s (not readable)", name))
 			} else {
-				ch.Files = append(ch.Files, &chart.File{Name: name, Data: byt})
+				ch.Files = append(ch.Files, &chart.File{Name: name, Data: fileContentBytes})
 			}
 		}
 	}

cmd/helm/install_test.go

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

cmd/helm/template_test.go

@@ -133,17 +133,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/external"),
+			cmd:    fmt.Sprintf("template '%s' --set external=testdata/files/external.txt --include-file 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/external"),
+			cmd:    fmt.Sprintf("template '%s' --set glob.enabled=true --include-dir 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/external"),
+			cmd:    fmt.Sprintf("template '%s' --set glob.enabled=true --include-dir testdata/files/external.*.conf", "testdata/testcharts/external"),
 			golden: "output/template-with-external-glob.txt",
 		},
 	}

cmd/helm/testdata/testcharts/external/values.yaml

@@ -1,4 +1,4 @@
 external: false
 glob:
   enabled: false
-  path: "glob/*"
+  path: "testdata/files/*"

cmd/helm/upgrade_test.go

@@ -362,7 +362,7 @@ func TestUpgradeWithExternalFile(t *testing.T) {
 	releaseName := "funny-bunny-v7"
 
 	exFiles := []*chart.File{
-		{Name: "external.txt", Data: []byte("from-external-file")},
+		{Name: "testdata/files/external.txt", Data: []byte("from-external-file")},
 	}
 
 	relMock, ch, chartPath := prepareMockReleaseWithExternal(releaseName, exFiles, t)
@@ -373,7 +373,7 @@ func TestUpgradeWithExternalFile(t *testing.T) {
 
 	store.Create(relMock(releaseName, 3, ch))
 
-	cmd := fmt.Sprintf("upgrade %s --set glob.enabled=false --set external=external.txt '%s'", releaseName, chartPath)
+	cmd := fmt.Sprintf("upgrade %s --set glob.enabled=false --set external=testdata/files/external.txt '%s'", releaseName, chartPath)
 	_, _, err := executeActionCommandC(store, cmd)
 	if err != nil {
 		t.Errorf("unexpected error, got '%v'", err)

pkg/chart/loader/local.go

@@ -20,29 +20,28 @@ import (
 	"strings"
 )
 
-// ExpandLocalPath expands a local file, dir or glob path to a list of files
+// ExpandFilePath expands a local file, dir or glob path to a list of files
-func ExpandLocalPath(name string, path string) (map[string]string, error) {
+func ExpandFilePath(path string) ([]string, error) {
 	if strings.Contains(path, "*") {
 		// if this is a glob, we expand it and return a list of files
-		return expandGlob(name, path)
+		return expandGlob(path)
 	}
 
-	fi, err := os.Stat(path)
+	fileInfo, err := os.Stat(path)
 	if err != nil {
 		return nil, err
 	}
 
-	if fi.IsDir() {
+	if fileInfo.IsDir() {
 		// if this is a valid dir, we return all files within
-		return expandDir(name, path)
+		return expandDir(path)
 	}
 
 	// finally, this is a file, so we return it
-	return map[string]string{name: path}, nil
+	return []string{path}, nil
 }
 
-func expandGlob(name string, path string) (map[string]string, error) {
+func expandGlob(path string) ([]string, error) {
-	fmap := make(map[string]string)
 	paths, err := filepath.Glob(path)
 	if err != nil {
 		return nil, err
@@ -51,18 +50,10 @@ func expandGlob(name string, path string) (map[string]string, error) {
 		return nil, errors.New("empty glob")
 	}
 
-	namePrefix := strings.TrimRight(name, "/") + "/"
+	return paths, err
-	for _, p := range paths {
-		key := namePrefix + filepath.Base(p)
-		fmap[key] = p
-	}
-
-	return fmap, nil
 }
 
-func expandDir(name string, path string) (map[string]string, error) {
+func expandDir(path string) ([]string, error) {
-	fmap := make(map[string]string)
-
 	f, err := os.Open(path)
 
 	if err != nil {
@@ -70,16 +61,15 @@ func expandDir(name string, path string) (map[string]string, error) {
 	}
 	defer f.Close()
 
-	files, err := f.Readdir(-1)
+	filesInfos, err := f.Readdir(-1)
 	if err != nil {
 		return nil, err
 	}
 
+	var filesPaths []string
 	localDirName := strings.TrimRight(path, "/") + "/"
-	namePrefix := strings.TrimRight(name, "/") + "/"
+	for _, file := range filesInfos {
-	for _, file := range files {
+		filesPaths = append(filesPaths, localDirName+file.Name())
-		key := namePrefix + file.Name()
-		fmap[key] = localDirName + file.Name()
 	}
-	return fmap, nil
+	return filesPaths, nil
 }

pkg/chart/loader/local_test.go

@@ -16,31 +16,23 @@ package loader
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestExpandLocalPath(t *testing.T) {
-	need := require.New(t)
+	req := require.New(t)
-	is := assert.New(t)
 
-	glob, err := ExpandLocalPath("glob", "testdata/frobnitz/*.yaml")
+	glob, err := ExpandFilePath("testdata/frobnitz/*.yaml")
-	need.NoError(err)
+	req.NoError(err)
-	need.Contains(glob, "glob/Chart.yaml")
+	req.Contains(glob, "testdata/frobnitz/Chart.yaml")
-	need.Contains(glob, "glob/values.yaml")
+	req.Contains(glob, "testdata/frobnitz/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/")
+	dir, err := ExpandFilePath("testdata/albatross/")
-	need.NoError(err)
+	req.NoError(err)
-	need.Contains(dir, "dir/Chart.yaml")
+	req.Contains(dir, "testdata/albatross/Chart.yaml")
-	need.Contains(dir, "dir/values.yaml")
+	req.Contains(dir, "testdata/albatross/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"])
 
+	file, err := ExpandFilePath("testdata/albatross/Chart.yaml")
+	req.NoError(err)
+	req.Contains(file, "testdata/albatross/Chart.yaml")
 }

pkg/cli/files/parser.go

@@ -1,66 +0,0 @@
-/*
-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 files
-
-import (
-	"errors"
-	"fmt"
-	"path/filepath"
-	"strings"
-)
-
-// ParseIntoString parses a include-file line and merges the result into dest.
-func ParseIntoString(s string, dest map[string]string) error {
-	for _, val := range strings.Split(s, ",") {
-		val = strings.TrimSpace(val)
-		splt := strings.SplitN(val, "=", 2)
-
-		if len(splt) != 2 {
-			return errors.New("Could not parse line")
-		}
-
-		name := strings.TrimSpace(splt[0])
-		path := strings.TrimSpace(splt[1])
-		dest[name] = path
-	}
-
-	return nil
-}
-
-//ParseGlobIntoString parses an include-dir file line and merges all files found into dest.
-func ParseGlobIntoString(g string, dest map[string]string) error {
-	globs := make(map[string]string)
-	err := ParseIntoString(g, globs)
-	if err != nil {
-		return err
-	}
-	for k, g := range globs {
-		if !strings.Contains(g, "*") {
-			// force glob style on simple directories
-			g = strings.TrimRight(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
-		}
-	}
-
-	return nil
-}

pkg/cli/files/parser_test.go

@@ -1,76 +0,0 @@
-/*
-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 files
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-func TestParseIntoString(t *testing.T) {
-	need := require.New(t)
-	is := assert.New(t)
-
-	dest := make(map[string]string)
-	goodFlag := "foo.txt=../foo.txt"
-	anotherFlag := " bar.txt=~/bar.txt, baz.txt=/path/to/baz.txt"
-
-	err := ParseIntoString(goodFlag, dest)
-	need.NoError(err)
-
-	err = ParseIntoString(anotherFlag, dest)
-	need.NoError(err)
-
-	is.Contains(dest, "foo.txt")
-	is.Contains(dest, "bar.txt")
-	is.Contains(dest, "baz.txt")
-
-	is.Equal(dest["foo.txt"], "../foo.txt", "foo.txt not mapped properly")
-	is.Equal(dest["bar.txt"], "~/bar.txt", "bar.txt not mapped properly")
-	is.Equal(dest["baz.txt"], "/path/to/baz.txt", "baz.txt not mapped properly")
-
-	overwriteFlag := "foo.txt=../new_foo.txt"
-	err = ParseIntoString(overwriteFlag, dest)
-	need.NoError(err)
-
-	is.Equal(dest["foo.txt"], "../new_foo.txt")
-
-	badFlag := "empty.txt"
-	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

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

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

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