diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 53da84b86..17d11c099 100644 --- a/cmd/helm/install.go +++ b/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 { - if err := files.ParseIntoString(s, fs); err != nil { - 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)) - } - } + fs = append(fs, exFiles.Files...) + fs = append(fs, exFiles.Globs...) - for n, p := range fs { - allPaths, err := loader.ExpandLocalPath(n, p) - debug(fmt.Sprintf("%s expanded to: %v", p, allPaths)) + for _, path := range fs { + allPaths, err := loader.ExpandFilePath(path) + debug(fmt.Sprintf("%s expanded to: %v", path, allPaths)) if err != nil { - debug(fmt.Sprintf("error loading external path %s: %v", p, err)) - errs = append(errs, fmt.Sprintf("%s (path not accessible)", p)) + debug(fmt.Sprintf("error loading external path %s: %v", path, err)) + errs = append(errs, fmt.Sprintf("%s (path not accessible)", path)) } - for name, fp := range allPaths { - byt, err := ioutil.ReadFile(fp) + for _, name := range allPaths { + 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}) } } } diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go index a1a97445d..d7e13dfc7 100644 --- a/cmd/helm/install_test.go +++ b/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 diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go index 91a68acec..5ffdd8bc8 100644 --- a/cmd/helm/template_test.go +++ b/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", }, } diff --git a/cmd/helm/testdata/testcharts/external/values.yaml b/cmd/helm/testdata/testcharts/external/values.yaml index ee4f44aed..422adf0ef 100644 --- a/cmd/helm/testdata/testcharts/external/values.yaml +++ b/cmd/helm/testdata/testcharts/external/values.yaml @@ -1,4 +1,4 @@ external: false glob: enabled: false - path: "glob/*" + path: "testdata/files/*" diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go index 26a459a79..29a397e2f 100644 --- a/cmd/helm/upgrade_test.go +++ b/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) diff --git a/pkg/chart/loader/local.go b/pkg/chart/loader/local.go index 9537c2742..38f080827 100644 --- a/pkg/chart/loader/local.go +++ b/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 -func ExpandLocalPath(name string, path string) (map[string]string, error) { +// ExpandFilePath expands a local file, dir or glob path to a list of files +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) { - fmap := make(map[string]string) +func expandGlob(path string) ([]string, error) { 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, "/") + "/" - for _, p := range paths { - key := namePrefix + filepath.Base(p) - fmap[key] = p - } - - return fmap, nil + return paths, err } -func expandDir(name string, path string) (map[string]string, error) { - fmap := make(map[string]string) - +func expandDir(path string) ([]string, error) { 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 files { - key := namePrefix + file.Name() - fmap[key] = localDirName + file.Name() + for _, file := range filesInfos { + filesPaths = append(filesPaths, localDirName+file.Name()) } - return fmap, nil + return filesPaths, nil } diff --git a/pkg/chart/loader/local_test.go b/pkg/chart/loader/local_test.go index 79b81f9fe..456d0dddf 100644 --- a/pkg/chart/loader/local_test.go +++ b/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) - is := assert.New(t) + req := require.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"]) + glob, err := ExpandFilePath("testdata/frobnitz/*.yaml") + req.NoError(err) + req.Contains(glob, "testdata/frobnitz/Chart.yaml") + req.Contains(glob, "testdata/frobnitz/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"]) + dir, err := ExpandFilePath("testdata/albatross/") + req.NoError(err) + req.Contains(dir, "testdata/albatross/Chart.yaml") + req.Contains(dir, "testdata/albatross/values.yaml") + file, err := ExpandFilePath("testdata/albatross/Chart.yaml") + req.NoError(err) + req.Contains(file, "testdata/albatross/Chart.yaml") } diff --git a/pkg/cli/files/parser.go b/pkg/cli/files/parser.go deleted file mode 100644 index d3dd68345..000000000 --- a/pkg/cli/files/parser.go +++ /dev/null @@ -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 -} diff --git a/pkg/cli/files/parser_test.go b/pkg/cli/files/parser_test.go deleted file mode 100644 index ac3a9d46f..000000000 --- a/pkg/cli/files/parser_test.go +++ /dev/null @@ -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"]) -} diff --git a/pkg/cli/files/testdata/foo/bar.txt b/pkg/cli/files/testdata/foo/bar.txt deleted file mode 100644 index 5716ca598..000000000 --- a/pkg/cli/files/testdata/foo/bar.txt +++ /dev/null @@ -1 +0,0 @@ -bar diff --git a/pkg/cli/files/testdata/foo/foo.txt b/pkg/cli/files/testdata/foo/foo.txt deleted file mode 100644 index 257cc5642..000000000 --- a/pkg/cli/files/testdata/foo/foo.txt +++ /dev/null @@ -1 +0,0 @@ -foo