add glob, dir, and add more tests

Signed-off-by: Vlad Fratila <vfratila@adobe.com>
Signed-off-by: Matheus Hunsche <matheus.hunsche@ifood.com.br>

cmd/helm/flags.go

@@ -54,8 +54,8 @@ func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) {
 }
 
 func addExternalFilesFlags(f *pflag.FlagSet, v *files.ExternalFiles) {
-	f.StringArrayVar(&v.Files, "include-file", []string{}, "paths to local external files to use during chart installation")
+	f.StringArrayVar(&v.Files, "include-file", []string{}, "paths to local files to add during chart installation")
-	f.StringArrayVar(&v.Globs, "include-dir", []string{}, "globs to local external files to use during chart installation")
+	f.StringArrayVar(&v.Globs, "include-dir", []string{}, "paths or globs to local dirs to add during chart installation")
 }
 
 // bindOutputFlag will add the output flag to the given command and bind the

cmd/helm/install.go

@@ -19,6 +19,7 @@ package main
 import (
 	"fmt"
 	"io"
+	"io/ioutil"
 	"strings"
 	"time"
 
@@ -231,7 +232,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options
 
 	err = loadExternalFiles(chartRequested, client.ExternalFiles)
 	if err != nil {
-		fmt.Fprintln(out, err)
+		return nil, err
 	}
 
 	client.Namespace = settings.Namespace()
@@ -252,20 +253,38 @@ func isChartInstallable(ch *chart.Chart) (bool, error) {
 func loadExternalFiles(ch *chart.Chart, exFiles files.ExternalFiles) error {
 	var errs []string
 	fs := make(map[string]string)
+
 	for _, s := range exFiles.Files {
 		if err := files.ParseIntoString(s, fs); err != nil {
-			debug("error parsing file option", s, err)
+			debug(fmt.Sprintf("error parsing include-file option %s: %v", s, err))
-			errs = append(errs, s)
+			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 name, path := range fs {
+	for n, p := range fs {
-		byt, err := loader.LoadLocalFile(path)
+		allPaths, err := loader.ExpandLocalPath(n, p)
+		debug(fmt.Sprintf("%s expanded to: %v", p, allPaths))
 		if err != nil {
-			errs = append(errs, path)
+			debug(fmt.Sprintf("error loading external path %s: %v", p, err))
+			errs = append(errs, fmt.Sprintf("%s (path not accessible)", p))
 		}
+
+		for name, fp := range allPaths {
+			byt, err := ioutil.ReadFile(fp)
+			if err != nil {
+				errs = append(errs, fmt.Sprintf("%s (not readable)", fp))
+			} else {
 				ch.Files = append(ch.Files, &chart.File{Name: name, Data: byt})
 			}
+		}
+	}
 
 	if len(errs) > 0 {
 		return errors.New(fmt.Sprint("Failed to load external files: ", strings.Join(errs, "; ")))

cmd/helm/install_test.go

@@ -47,10 +47,22 @@ func TestInstall(t *testing.T) {
 			cmd:    "install virgil testdata/testcharts/alpine -f testdata/testcharts/alpine/extra_values.yaml",
 			golden: "output/install-with-values-file.txt",
 		},
-		// Install, external files
+		// Install, external file
 		{
 			name:   "install with external files",
-			cmd:    "install virgil testdata/testcharts/configmap --include-file external.txt=testdata/external.txt",
+			cmd:    "install virgil testdata/testcharts/configmap --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/",
+			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",
 			golden: "output/install-with-external-files.txt",
 		},
 		// Install, no hooks

cmd/helm/template_test.go

@@ -123,9 +123,19 @@ func TestTemplateCmd(t *testing.T) {
 		},
 		{
 			name:   "chart with template with external file",
-			cmd:    fmt.Sprintf("template '%s' --include-file external.txt=testdata/external.txt", "testdata/testcharts/configmap"),
+			cmd:    fmt.Sprintf("template '%s' --set external=external.txt --include-file external.txt=testdata/files/external.txt", "testdata/testcharts/configmap"),
 			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"),
+			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"),
+			golden: "output/template-with-external-glob.txt",
+		},
 	}
 	runTestCmd(t, tests)
 }

cmd/helm/testdata/files/external.1.conf

@@ -0,0 +1 @@
+glob-external-1

cmd/helm/testdata/files/external.2.conf

@@ -0,0 +1 @@
+glob-external-2

cmd/helm/testdata/output/template-with-external-dir.txt

@@ -0,0 +1,22 @@
+---
+# Source: configmap/templates/config-map.yaml
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: "RELEASE-NAME-"
+  labels:
+    # The "app.kubernetes.io/managed-by" label is used to track which tool
+    # deployed a given chart. It is useful for admins who want to see what
+    # releases a particular tool is responsible for.
+    app.kubernetes.io/managed-by: "Helm"
+    # The "app.kubernetes.io/instance" convention makes it easy to tie a release
+    # to all of the Kubernetes resources that were created as part of that
+    # release.
+    app.kubernetes.io/instance: "RELEASE-NAME"
+    app.kubernetes.io/version: 1.0
+    # This makes it easy to audit chart usage.
+    helm.sh/chart: "configmap-0.1.0"
+data:
+  external.1.conf: glob-external-1
+  external.2.conf: glob-external-2
+  external.txt: out-of-chart-dir

cmd/helm/testdata/output/template-with-external-glob.txt

@@ -0,0 +1,21 @@
+---
+# Source: configmap/templates/config-map.yaml
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: "RELEASE-NAME-"
+  labels:
+    # The "app.kubernetes.io/managed-by" label is used to track which tool
+    # deployed a given chart. It is useful for admins who want to see what
+    # releases a particular tool is responsible for.
+    app.kubernetes.io/managed-by: "Helm"
+    # The "app.kubernetes.io/instance" convention makes it easy to tie a release
+    # to all of the Kubernetes resources that were created as part of that
+    # release.
+    app.kubernetes.io/instance: "RELEASE-NAME"
+    app.kubernetes.io/version: 1.0
+    # This makes it easy to audit chart usage.
+    helm.sh/chart: "configmap-0.1.0"
+data:
+  external.1.conf: glob-external-1
+  external.2.conf: glob-external-2

cmd/helm/testdata/testcharts/configmap/templates/config-map.yaml

@@ -15,4 +15,9 @@ metadata:
     # This makes it easy to audit chart usage.
     helm.sh/chart: "{{.Chart.Name}}-{{.Chart.Version}}"
 data:
+{{- if .Values.external }}
 {{ (.Files.Glob .Values.external).AsConfig | indent 2 }}
+{{- end }}
+{{- if .Values.glob.enabled }}
+{{ (.Files.Glob .Values.glob.path).AsConfig | indent 2 }}
+{{- end }}

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

@@ -1 +1,4 @@
-external: external.txt
+external: false
+glob:
+  enabled: false
+  path: "glob/*"

pkg/chart/loader/local.go

@@ -17,19 +17,72 @@ limitations under the License.
 package loader
 
 import (
-	"io/ioutil"
+	"errors"
 	"os"
-
+	"path/filepath"
-	"github.com/pkg/errors"
+	"strings"
 )
 
-// LoadLocalFile loads a file from the local filesystem.
+// ExpandLocalPath expands a local file, dir or glob path to a list of files
-func LoadLocalFile(path string) ([]byte, error) {
+func ExpandLocalPath(name string, path string) (map[string]string, error) {
-	if fi, err := os.Stat(path); err != nil {
+	if strings.Contains(path, "*") {
+		// if this is a glob, we expand it and return a list of files
+		return expandGlob(name, path)
+	}
+
+	fi, err := os.Stat(path)
+	if err != nil {
+		return nil, err
+	}
+
+	if fi.IsDir() {
+		// if this is a valid dir, we return all files within
+		return expandDir(name, path)
+	}
+
+	// finally, this is a file, so we return it
+	return map[string]string{name: path}, nil
+}
+
+func expandGlob(name string, path string) (map[string]string, error) {
+	fmap := make(map[string]string)
+	paths, err := filepath.Glob(path)
+	if err != nil {
+		return nil, err
+	}
+	if len(paths) == 0 {
+		return nil, errors.New("empty glob")
+	}
+
+	namePrefix := strings.TrimRight(name, "/") + "/"
+	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) {
+	fmap := make(map[string]string)
+
+	f, err := os.Open(path)
+	defer f.Close()
+
+	if err != nil {
+		return nil, err
+	}
+
+	files, err := f.Readdir(-1)
+	if err != nil {
 		return nil, err
-	} else if fi.IsDir() {
-		return nil, errors.New("cannot load a directory")
 	}
 
-	return ioutil.ReadFile(path)
+	localDirName := strings.TrimRight(path, "/") + "/"
+	namePrefix := strings.TrimRight(name, "/") + "/"
+	for _, file := range files {
+		key := namePrefix + file.Name()
+		fmap[key] = localDirName + file.Name()
+	}
+	return fmap, nil
 }

pkg/cli/files/parser.go

@@ -18,6 +18,9 @@ package files
 
 import (
 	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
 	"strings"
 )
 
@@ -38,3 +41,33 @@ func ParseIntoString(s string, dest map[string]string) error {
 
 	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, "*") {
+			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
+		}
+		for _, path := range paths {
+			dest[fmt.Sprintf("%s/%s", k, filepath.Base(path))] = path
+		}
+	}
+
+	return nil
+}