From af7f6ef441a99102afd9a46e423e445c4aa3e629 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 28 Aug 2019 11:54:04 -0400 Subject: [PATCH] lang/funcs: Update fileset() function to include path as separate first argument, automatically trim the path argument from results, and ensure results are always canonical with forward slash path separators Reference: https://github.com/hashicorp/terraform/pull/22523#pullrequestreview-279694703 These changes center around better function usability and consistency with other functions. The function has not yet been released, so these breaking changes can be applied safely. --- lang/funcs/filesystem.go | 32 ++++-- lang/funcs/filesystem_test.go | 104 ++++++++++++++---- lang/functions_test.go | 27 ++++- .../functions-test/subdirectory/hello.tmpl | 1 + .../functions-test/subdirectory/hello.txt | 1 + .../configuration/functions/fileset.html.md | 24 +++- 6 files changed, 150 insertions(+), 39 deletions(-) create mode 100644 lang/testdata/functions-test/subdirectory/hello.tmpl create mode 100644 lang/testdata/functions-test/subdirectory/hello.txt diff --git a/lang/funcs/filesystem.go b/lang/funcs/filesystem.go index 12ed96af8..5a8341a7e 100644 --- a/lang/funcs/filesystem.go +++ b/lang/funcs/filesystem.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "unicode/utf8" "github.com/hashicorp/hcl2/hcl" @@ -212,6 +213,10 @@ func MakeFileExistsFunc(baseDir string) function.Function { func MakeFileSetFunc(baseDir string) function.Function { return function.New(&function.Spec{ Params: []function.Parameter{ + { + Name: "path", + Type: cty.String, + }, { Name: "pattern", Type: cty.String, @@ -219,18 +224,22 @@ func MakeFileSetFunc(baseDir string) function.Function { }, Type: function.StaticReturnType(cty.Set(cty.String)), Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { - pattern := args[0].AsString() - pattern, err := homedir.Expand(pattern) + path := args[0].AsString() + pattern := args[1].AsString() + + path, err := homedir.Expand(path) if err != nil { return cty.UnknownVal(cty.Set(cty.String)), fmt.Errorf("failed to expand ~: %s", err) } - if !filepath.IsAbs(pattern) { - pattern = filepath.Join(baseDir, pattern) + if !filepath.IsAbs(path) { + path = filepath.Join(baseDir, path) } - // Ensure that the path is canonical for the host OS - pattern = filepath.Clean(pattern) + // Join the path to the glob pattern, while ensuring the full + // pattern is canonical for the host OS. The joined path is + // automatically cleaned during this operation. + pattern = filepath.Join(path, pattern) matches, err := filepath.Glob(pattern) if err != nil { @@ -249,6 +258,13 @@ func MakeFileSetFunc(baseDir string) function.Function { continue } + // Remove the path and file separator from matches. + match = strings.TrimPrefix(match, path+string(filepath.Separator)) + + // Return matches with the Terraform canonical pattern + // of forward slashes for cross-system compatibility. + match = filepath.ToSlash(match) + matchVals = append(matchVals, cty.StringVal(match)) } @@ -375,9 +391,9 @@ func FileExists(baseDir string, path cty.Value) (cty.Value, error) { // The underlying function implementation works relative to a particular base // directory, so this wrapper takes a base directory string and uses it to // construct the underlying function before calling it. -func FileSet(baseDir string, pattern cty.Value) (cty.Value, error) { +func FileSet(baseDir string, path, pattern cty.Value) (cty.Value, error) { fn := MakeFileSetFunc(baseDir) - return fn.Call([]cty.Value{pattern}) + return fn.Call([]cty.Value{path, pattern}) } // FileBase64 reads the contents of the file at the given path. diff --git a/lang/funcs/filesystem_test.go b/lang/funcs/filesystem_test.go index 03d5ba75a..b331de2d6 100644 --- a/lang/funcs/filesystem_test.go +++ b/lang/funcs/filesystem_test.go @@ -226,36 +226,43 @@ func TestFileExists(t *testing.T) { func TestFileSet(t *testing.T) { tests := []struct { + Path cty.Value Pattern cty.Value Want cty.Value Err bool }{ { - cty.StringVal("testdata/missing"), - cty.SetValEmpty(cty.String), - false, - }, - { - cty.StringVal("testdata/missing*"), - cty.SetValEmpty(cty.String), - false, - }, - { - cty.StringVal("*/missing"), - cty.SetValEmpty(cty.String), - false, - }, - { - cty.StringVal("testdata"), - cty.SetValEmpty(cty.String), - false, - }, - { + cty.StringVal("."), cty.StringVal("testdata*"), cty.SetValEmpty(cty.String), false, }, { + cty.StringVal("."), + cty.StringVal("testdata"), + cty.SetValEmpty(cty.String), + false, + }, + { + cty.StringVal("."), + cty.StringVal("testdata/missing"), + cty.SetValEmpty(cty.String), + false, + }, + { + cty.StringVal("."), + cty.StringVal("testdata/missing*"), + cty.SetValEmpty(cty.String), + false, + }, + { + cty.StringVal("."), + cty.StringVal("*/missing"), + cty.SetValEmpty(cty.String), + false, + }, + { + cty.StringVal("."), cty.StringVal("testdata/*.txt"), cty.SetVal([]cty.Value{ cty.StringVal("testdata/hello.txt"), @@ -263,6 +270,7 @@ func TestFileSet(t *testing.T) { false, }, { + cty.StringVal("."), cty.StringVal("testdata/hello.txt"), cty.SetVal([]cty.Value{ cty.StringVal("testdata/hello.txt"), @@ -270,6 +278,7 @@ func TestFileSet(t *testing.T) { false, }, { + cty.StringVal("."), cty.StringVal("testdata/hello.???"), cty.SetVal([]cty.Value{ cty.StringVal("testdata/hello.txt"), @@ -277,6 +286,7 @@ func TestFileSet(t *testing.T) { false, }, { + cty.StringVal("."), cty.StringVal("testdata/hello*"), cty.SetVal([]cty.Value{ cty.StringVal("testdata/hello.tmpl"), @@ -285,6 +295,7 @@ func TestFileSet(t *testing.T) { false, }, { + cty.StringVal("."), cty.StringVal("*/hello.txt"), cty.SetVal([]cty.Value{ cty.StringVal("testdata/hello.txt"), @@ -292,6 +303,7 @@ func TestFileSet(t *testing.T) { false, }, { + cty.StringVal("."), cty.StringVal("*/*.txt"), cty.SetVal([]cty.Value{ cty.StringVal("testdata/hello.txt"), @@ -299,6 +311,7 @@ func TestFileSet(t *testing.T) { false, }, { + cty.StringVal("."), cty.StringVal("*/hello*"), cty.SetVal([]cty.Value{ cty.StringVal("testdata/hello.tmpl"), @@ -307,20 +320,67 @@ func TestFileSet(t *testing.T) { false, }, { + cty.StringVal("."), cty.StringVal("["), cty.SetValEmpty(cty.String), true, }, { + cty.StringVal("."), cty.StringVal("\\"), cty.SetValEmpty(cty.String), true, }, + { + cty.StringVal("testdata"), + cty.StringVal("missing"), + cty.SetValEmpty(cty.String), + false, + }, + { + cty.StringVal("testdata"), + cty.StringVal("missing*"), + cty.SetValEmpty(cty.String), + false, + }, + { + cty.StringVal("testdata"), + cty.StringVal("*.txt"), + cty.SetVal([]cty.Value{ + cty.StringVal("hello.txt"), + }), + false, + }, + { + cty.StringVal("testdata"), + cty.StringVal("hello.txt"), + cty.SetVal([]cty.Value{ + cty.StringVal("hello.txt"), + }), + false, + }, + { + cty.StringVal("testdata"), + cty.StringVal("hello.???"), + cty.SetVal([]cty.Value{ + cty.StringVal("hello.txt"), + }), + false, + }, + { + cty.StringVal("testdata"), + cty.StringVal("hello*"), + cty.SetVal([]cty.Value{ + cty.StringVal("hello.tmpl"), + cty.StringVal("hello.txt"), + }), + false, + }, } for _, test := range tests { - t.Run(fmt.Sprintf("FileSet(\".\", %#v)", test.Pattern), func(t *testing.T) { - got, err := FileSet(".", test.Pattern) + t.Run(fmt.Sprintf("FileSet(\".\", %#v, %#v)", test.Path, test.Pattern), func(t *testing.T) { + got, err := FileSet(".", test.Path, test.Pattern) if test.Err { if err == nil { diff --git a/lang/functions_test.go b/lang/functions_test.go index 046c7b940..ac0ff91d9 100644 --- a/lang/functions_test.go +++ b/lang/functions_test.go @@ -281,10 +281,31 @@ func TestFunctions(t *testing.T) { "fileset": { { - `fileset("hello.*")`, + `fileset(".", "*/hello.*")`, cty.SetVal([]cty.Value{ - cty.StringVal("testdata/functions-test/hello.tmpl"), - cty.StringVal("testdata/functions-test/hello.txt"), + cty.StringVal("subdirectory/hello.tmpl"), + cty.StringVal("subdirectory/hello.txt"), + }), + }, + { + `fileset(".", "subdirectory/hello.*")`, + cty.SetVal([]cty.Value{ + cty.StringVal("subdirectory/hello.tmpl"), + cty.StringVal("subdirectory/hello.txt"), + }), + }, + { + `fileset(".", "hello.*")`, + cty.SetVal([]cty.Value{ + cty.StringVal("hello.tmpl"), + cty.StringVal("hello.txt"), + }), + }, + { + `fileset("subdirectory", "hello.*")`, + cty.SetVal([]cty.Value{ + cty.StringVal("hello.tmpl"), + cty.StringVal("hello.txt"), }), }, }, diff --git a/lang/testdata/functions-test/subdirectory/hello.tmpl b/lang/testdata/functions-test/subdirectory/hello.tmpl new file mode 100644 index 000000000..f112ef899 --- /dev/null +++ b/lang/testdata/functions-test/subdirectory/hello.tmpl @@ -0,0 +1 @@ +Hello, ${name}! \ No newline at end of file diff --git a/lang/testdata/functions-test/subdirectory/hello.txt b/lang/testdata/functions-test/subdirectory/hello.txt new file mode 100644 index 000000000..3462721fd --- /dev/null +++ b/lang/testdata/functions-test/subdirectory/hello.txt @@ -0,0 +1 @@ +hello! \ No newline at end of file diff --git a/website/docs/configuration/functions/fileset.html.md b/website/docs/configuration/functions/fileset.html.md index 35753dcad..749183469 100644 --- a/website/docs/configuration/functions/fileset.html.md +++ b/website/docs/configuration/functions/fileset.html.md @@ -12,10 +12,13 @@ description: |- earlier, see [0.11 Configuration Language: Interpolation Syntax](../../configuration-0-11/interpolation.html). -`fileset` enumerates a set of regular file names given a pattern. +`fileset` enumerates a set of regular file names given a path and pattern. +The path is automatically removed from the resulting set of file names and any +result still containing path separators always returns forward slash (`/`) as +the path separator for cross-system compatibility. ```hcl -fileset(pattern) +fileset(path, pattern) ``` Supported pattern matches: @@ -32,16 +35,25 @@ before Terraform takes any actions. ## Examples ``` -> fileset("${path.module}/*.txt") +> fileset(path.module, "files/*.txt") [ - "path/to/module/hello.txt", - "path/to/module/world.txt", + "files/hello.txt", + "files/world.txt", +] + +> fileset("${path.module}/files", "*.txt") +[ + "hello.txt", + "world.txt", ] ``` +A common use of `fileset` is to create one resource instance per matched file, using +[the `for_each` meta-argument](/docs/configuration/resources.html#for_each-multiple-resource-instances-defined-by-a-map-or-set-of-strings): + ```hcl resource "example_thing" "example" { - for_each = fileset("${path.module}/files/*") + for_each = fileset(path.module, "files/*") # other configuration using each.value }