From 5c70e6c11f2ce4b24cf4aff1165553f417dce232 Mon Sep 17 00:00:00 2001 From: Vlad Fratila Date: Fri, 5 Jun 2020 14:59:09 +0300 Subject: [PATCH] add glob, dir, and add more tests Signed-off-by: Vlad Fratila Signed-off-by: Matheus Hunsche --- cmd/helm/flags.go | 4 +- cmd/helm/install.go | 35 ++++++--- cmd/helm/install_test.go | 16 ++++- cmd/helm/template_test.go | 12 +++- cmd/helm/testdata/files/external.1.conf | 1 + cmd/helm/testdata/files/external.2.conf | 1 + cmd/helm/testdata/{ => files}/external.txt | 0 .../output/template-with-external-dir.txt | 22 ++++++ .../output/template-with-external-glob.txt | 21 ++++++ .../configmap/templates/config-map.yaml | 7 +- .../testdata/testcharts/configmap/values.yaml | 5 +- pkg/chart/loader/local.go | 71 ++++++++++++++++--- pkg/cli/files/parser.go | 33 +++++++++ 13 files changed, 204 insertions(+), 24 deletions(-) create mode 100644 cmd/helm/testdata/files/external.1.conf create mode 100644 cmd/helm/testdata/files/external.2.conf rename cmd/helm/testdata/{ => files}/external.txt (100%) create mode 100644 cmd/helm/testdata/output/template-with-external-dir.txt create mode 100644 cmd/helm/testdata/output/template-with-external-glob.txt diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index 5dca0bf71..b4cd9ca53 100644 --- a/cmd/helm/flags.go +++ b/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.Globs, "include-dir", []string{}, "globs 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{}, "paths or globs to local dirs to add during chart installation") } // bindOutputFlag will add the output flag to the given command and bind the diff --git a/cmd/helm/install.go b/cmd/helm/install.go index d5d58a5ac..89ecca0b8 100644 --- a/cmd/helm/install.go +++ b/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,23 +253,41 @@ 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) - errs = append(errs, s) + 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 name, path := range fs { - byt, err := loader.LoadLocalFile(path) + for n, p := range fs { + 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}) + } } - 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, ";"))) + return errors.New(fmt.Sprint("Failed to load external files: ", strings.Join(errs, "; "))) } return nil } diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go index 1a6524c63..6909265d2 100644 --- a/cmd/helm/install_test.go +++ b/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 diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go index 17caeeb45..d82aac295 100644 --- a/cmd/helm/template_test.go +++ b/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) } diff --git a/cmd/helm/testdata/files/external.1.conf b/cmd/helm/testdata/files/external.1.conf new file mode 100644 index 000000000..9e24841da --- /dev/null +++ b/cmd/helm/testdata/files/external.1.conf @@ -0,0 +1 @@ +glob-external-1 \ No newline at end of file diff --git a/cmd/helm/testdata/files/external.2.conf b/cmd/helm/testdata/files/external.2.conf new file mode 100644 index 000000000..cbe77a473 --- /dev/null +++ b/cmd/helm/testdata/files/external.2.conf @@ -0,0 +1 @@ +glob-external-2 \ No newline at end of file diff --git a/cmd/helm/testdata/external.txt b/cmd/helm/testdata/files/external.txt similarity index 100% rename from cmd/helm/testdata/external.txt rename to cmd/helm/testdata/files/external.txt diff --git a/cmd/helm/testdata/output/template-with-external-dir.txt b/cmd/helm/testdata/output/template-with-external-dir.txt new file mode 100644 index 000000000..34645fde8 --- /dev/null +++ b/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 diff --git a/cmd/helm/testdata/output/template-with-external-glob.txt b/cmd/helm/testdata/output/template-with-external-glob.txt new file mode 100644 index 000000000..7796e1c7e --- /dev/null +++ b/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 diff --git a/cmd/helm/testdata/testcharts/configmap/templates/config-map.yaml b/cmd/helm/testdata/testcharts/configmap/templates/config-map.yaml index d03a9d5a9..76052311e 100644 --- a/cmd/helm/testdata/testcharts/configmap/templates/config-map.yaml +++ b/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: -{{ (.Files.Glob .Values.external).AsConfig | indent 2 }} \ No newline at end of file +{{- if .Values.external }} +{{ (.Files.Glob .Values.external).AsConfig | indent 2 }} +{{- end }} +{{- if .Values.glob.enabled }} +{{ (.Files.Glob .Values.glob.path).AsConfig | indent 2 }} +{{- end }} \ No newline at end of file diff --git a/cmd/helm/testdata/testcharts/configmap/values.yaml b/cmd/helm/testdata/testcharts/configmap/values.yaml index 338d478d7..d795b2e7a 100644 --- a/cmd/helm/testdata/testcharts/configmap/values.yaml +++ b/cmd/helm/testdata/testcharts/configmap/values.yaml @@ -1 +1,4 @@ -external: external.txt \ No newline at end of file +external: false +glob: + enabled: false + path: "glob/*" \ No newline at end of file diff --git a/pkg/chart/loader/local.go b/pkg/chart/loader/local.go index bce94bdda..3031fb693 100644 --- a/pkg/chart/loader/local.go +++ b/pkg/chart/loader/local.go @@ -17,19 +17,72 @@ limitations under the License. package loader import ( - "io/ioutil" + "errors" "os" - - "github.com/pkg/errors" + "path/filepath" + "strings" ) -// LoadLocalFile loads a file from the local filesystem. -func LoadLocalFile(path string) ([]byte, error) { - if fi, err := os.Stat(path); err != nil { +// ExpandLocalPath expands a local file, dir or glob path to a list of files +func ExpandLocalPath(name string, path string) (map[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) + } + + 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 } diff --git a/pkg/cli/files/parser.go b/pkg/cli/files/parser.go index 6e3761ed4..3e948b739 100644 --- a/pkg/cli/files/parser.go +++ b/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 +}