diff --git a/internal/lang/funcs/descriptions.go b/internal/lang/funcs/descriptions.go index cfe529b1b0..1967793ea7 100644 --- a/internal/lang/funcs/descriptions.go +++ b/internal/lang/funcs/descriptions.go @@ -420,6 +420,13 @@ var DescriptionList = map[string]descriptionEntry{ Description: "`templatefile` reads the file at the given path and renders its content as a template using a supplied set of template variables.", ParamDescription: []string{"", ""}, }, + "templatestring": { + Description: "`templatestring` takes a string from elsewhere in the module and renders its content as a template using a supplied set of template variables.", + ParamDescription: []string{ + "a simple reference to a string value containing the template source code", + "object of variables to expose in the template scope", + }, + }, "textdecodebase64": { Description: "`textdecodebase64` function decodes a string that was previously Base64-encoded, and then interprets the result as characters in a specified character encoding.", ParamDescription: []string{"", ""}, diff --git a/internal/lang/funcs/filesystem.go b/internal/lang/funcs/filesystem.go index 5447d698eb..4092cd5046 100644 --- a/internal/lang/funcs/filesystem.go +++ b/internal/lang/funcs/filesystem.go @@ -6,7 +6,7 @@ package funcs import ( "encoding/base64" "fmt" - "io/ioutil" + "io" "os" "path/filepath" "unicode/utf8" @@ -17,6 +17,8 @@ import ( homedir "github.com/mitchellh/go-homedir" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/function" + + "github.com/hashicorp/terraform/internal/collections" ) // MakeFileFunc constructs a function that takes a file path and returns the @@ -69,20 +71,7 @@ func MakeFileFunc(baseDir string, encBase64 bool) function.Function { // As a special exception, a referenced template file may not recursively call // the templatefile function, since that would risk the same file being // included into itself indefinitely. -func MakeTemplateFileFunc(baseDir string, funcsCb func() map[string]function.Function) function.Function { - - params := []function.Parameter{ - { - Name: "path", - Type: cty.String, - AllowMarked: true, - }, - { - Name: "vars", - Type: cty.DynamicPseudoType, - }, - } - +func MakeTemplateFileFunc(baseDir string, funcsCb func() (funcs map[string]function.Function, fsFuncs collections.Set[string], templateFuncs collections.Set[string])) function.Function { loadTmpl := func(fn string, marks cty.ValueMarks) (hcl.Expression, error) { // We re-use File here to ensure the same filename interpretation // as it does, along with its other safety checks. @@ -99,65 +88,20 @@ func MakeTemplateFileFunc(baseDir string, funcsCb func() map[string]function.Fun return expr, nil } - renderTmpl := func(expr hcl.Expression, varsVal cty.Value) (cty.Value, error) { - if varsTy := varsVal.Type(); !(varsTy.IsMapType() || varsTy.IsObjectType()) { - return cty.DynamicVal, function.NewArgErrorf(1, "invalid vars value: must be a map") // or an object, but we don't strongly distinguish these most of the time - } - - ctx := &hcl.EvalContext{ - Variables: varsVal.AsValueMap(), - } - - // We require all of the variables to be valid HCL identifiers, because - // otherwise there would be no way to refer to them in the template - // anyway. Rejecting this here gives better feedback to the user - // than a syntax error somewhere in the template itself. - for n := range ctx.Variables { - if !hclsyntax.ValidIdentifier(n) { - // This error message intentionally doesn't describe _all_ of - // the different permutations that are technically valid as an - // HCL identifier, but rather focuses on what we might - // consider to be an "idiomatic" variable name. - return cty.DynamicVal, function.NewArgErrorf(1, "invalid template variable name %q: must start with a letter, followed by zero or more letters, digits, and underscores", n) - } - } - - // We'll pre-check references in the template here so we can give a - // more specialized error message than HCL would by default, so it's - // clearer that this problem is coming from a templatefile call. - for _, traversal := range expr.Variables() { - root := traversal.RootName() - if _, ok := ctx.Variables[root]; !ok { - return cty.DynamicVal, function.NewArgErrorf(1, "vars map does not contain key %q, referenced at %s", root, traversal[0].SourceRange()) - } - } - - givenFuncs := funcsCb() // this callback indirection is to avoid chicken/egg problems - funcs := make(map[string]function.Function, len(givenFuncs)) - for name, fn := range givenFuncs { - if name == "templatefile" || name == "core::templatefile" { - // We stub this one out to prevent recursive calls. - funcs[name] = function.New(&function.Spec{ - Params: params, - Type: func(args []cty.Value) (cty.Type, error) { - return cty.NilType, fmt.Errorf("cannot recursively call templatefile from inside templatefile call") - }, - }) - continue - } - funcs[name] = fn - } - ctx.Functions = funcs - - val, diags := expr.Value(ctx) - if diags.HasErrors() { - return cty.DynamicVal, diags - } - return val, nil - } + renderTmpl := makeRenderTemplateFunc(funcsCb, true) return function.New(&function.Spec{ - Params: params, + Params: []function.Parameter{ + { + Name: "path", + Type: cty.String, + AllowMarked: true, + }, + { + Name: "vars", + Type: cty.DynamicPseudoType, + }, + }, Type: func(args []cty.Value) (cty.Type, error) { if !(args[0].IsKnown() && args[1].IsKnown()) { return cty.DynamicPseudoType, nil @@ -426,7 +370,7 @@ func readFileBytes(baseDir, path string, marks cty.ValueMarks) ([]byte, error) { } defer f.Close() - src, err := ioutil.ReadAll(f) + src, err := io.ReadAll(f) if err != nil { return nil, fmt.Errorf("failed to read file: %w", err) } diff --git a/internal/lang/funcs/filesystem_test.go b/internal/lang/funcs/filesystem_test.go index e54eae6ce3..cab8f27976 100644 --- a/internal/lang/funcs/filesystem_test.go +++ b/internal/lang/funcs/filesystem_test.go @@ -9,11 +9,13 @@ import ( "path/filepath" "testing" - "github.com/hashicorp/terraform/internal/lang/marks" homedir "github.com/mitchellh/go-homedir" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/function" "github.com/zclconf/go-cty/cty/function/stdlib" + + "github.com/hashicorp/terraform/internal/collections" + "github.com/hashicorp/terraform/internal/lang/marks" ) func TestFile(t *testing.T) { @@ -149,13 +151,13 @@ func TestTemplateFile(t *testing.T) { cty.StringVal("testdata/recursive.tmpl"), cty.MapValEmpty(cty.String), cty.NilVal, - `testdata/recursive.tmpl:1,3-16: Error in function call; Call to function "templatefile" failed: cannot recursively call templatefile from inside templatefile call.`, + `testdata/recursive.tmpl:1,3-16: Error in function call; Call to function "templatefile" failed: cannot recursively call templatefile from inside another template function.`, }, { cty.StringVal("testdata/recursive_namespaced.tmpl"), cty.MapValEmpty(cty.String), cty.NilVal, - `testdata/recursive_namespaced.tmpl:1,3-22: Error in function call; Call to function "core::templatefile" failed: cannot recursively call templatefile from inside templatefile call.`, + `testdata/recursive_namespaced.tmpl:1,3-22: Error in function call; Call to function "core::templatefile" failed: cannot recursively call templatefile from inside another template function.`, }, { cty.StringVal("testdata/list.tmpl"), @@ -187,14 +189,16 @@ func TestTemplateFile(t *testing.T) { }, } - templateFileFn := MakeTemplateFileFunc(".", func() map[string]function.Function { - return map[string]function.Function{ - "join": stdlib.JoinFunc, - "core::join": stdlib.JoinFunc, - "templatefile": MakeFileFunc(".", false), // just a placeholder, since templatefile itself overrides this - "core::templatefile": MakeFileFunc(".", false), // just a placeholder, since templatefile itself overrides this - } - }) + funcs := map[string]function.Function{ + "join": stdlib.JoinFunc, + "core::join": stdlib.JoinFunc, + } + funcsFunc := func() (funcTable map[string]function.Function, fsFuncs collections.Set[string], templateFuncs collections.Set[string]) { + return funcs, collections.NewSetCmp[string](), collections.NewSetCmp[string]("templatefile") + } + templateFileFn := MakeTemplateFileFunc(".", funcsFunc) + funcs["templatefile"] = templateFileFn + funcs["core::templatefile"] = templateFileFn for _, test := range tests { t.Run(fmt.Sprintf("TemplateFile(%#v, %#v)", test.Path, test.Vars), func(t *testing.T) { diff --git a/internal/lang/funcs/string.go b/internal/lang/funcs/string.go index 311ee41ed4..33142f2952 100644 --- a/internal/lang/funcs/string.go +++ b/internal/lang/funcs/string.go @@ -4,11 +4,18 @@ package funcs import ( + "fmt" "regexp" "strings" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/ext/customdecode" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" "github.com/zclconf/go-cty/cty/function" + + "github.com/hashicorp/terraform/internal/collections" ) // StartsWithFunc constructs a function that checks if a string starts with @@ -153,6 +160,231 @@ var StrContainsFunc = function.New(&function.Spec{ }, }) +// TemplateStringFunc renders a template presented either as a literal string +// or as a reference to a string from elsewhere. +func MakeTemplateStringFunc(funcsCb func() (funcs map[string]function.Function, fsFuncs collections.Set[string], templateFuncs collections.Set[string])) function.Function { + return function.New(&function.Spec{ + Params: []function.Parameter{ + { + Name: "template", + Type: customdecode.ExpressionClosureType, + }, + { + Name: "vars", + Type: cty.DynamicPseudoType, + }, + }, + Type: function.StaticReturnType(cty.String), + RefineResult: refineNotNull, + Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { + templateClosure := customdecode.ExpressionClosureFromVal(args[0]) + varsVal := args[1] + + // Our historical experience with the hashicorp/template provider's + // template_file data source tells us that situations where authors + // must write a string template that generates a string template + // cause all sorts of confusion, because the same syntax ends up + // being evaluated in two different contexts with different variables + // in scope, and new authors tend to be attracted to a function + // named "template" and so miss that the language has built-in + // support for inline template expressions. + // + // As a compromise to try to meet the (relatively unusual) use-cases + // where dynamic template fetching is needed without creating an + // attractive nuisance for those who would be better off just writing + // a plain inline template, this function imposes constraints on how + // the template argument may be provided and thus allows us + // to return slightly more helpful error messages. + // + // The only valid way to provide the template argument is as a + // simple, direct reference to some other value in scope that is + // of type string: + // templatestring(local.greeting_template, { name = "Alex" }) + // + // Those with more unusual desires, such as dynamically generating + // a template at runtime by trying to concatenate template chunks + // together, can still do such things by placing the template + // construction expression in a separate local value and then passing + // that local value to the template argument. But the restriction is + // intended to intentionally add an extra "roadbump" so that + // anyone who mistakenly thinks they need templatestring to render + // an inline template (a common mistake for new authors with + // template_file) will hopefully hit this roadblock and refer to + // the function documentation to learn about the other options that + // are probably more suitable for what they need. + switch expr := templateClosure.Expression.(type) { + case *hclsyntax.ScopeTraversalExpr: + // A standalone traversal is always acceptable. + case *hclsyntax.TemplateWrapExpr: + // This situation occurs when someone writes an interpolation-only + // expression as was required in Terraform v0.11 and earlier. + // Because older versions of Terraform required this and this + // habit has been sticky for some authors, we'll return a + // special error message. + return cty.UnknownVal(retType), function.NewArgErrorf( + 0, "invalid template expression: templatestring is only for rendering templates retrieved dynamically from elsewhere; to treat the inner expression as template syntax, write the reference expression directly without any template interpolation syntax", + ) + case *hclsyntax.TemplateExpr: + // This is the more general case of someone trying to write + // an inline template as the argument. In this case we'll + // distinguish between an entirely-literal template, which + // probably suggests someone was trying to escape their template + // for the function to consume, vs. a template with other + // sequences that suggests someone was just trying to write + // an inline template and so probably doesn't need to call + // this function at all. + literal := true + if len(expr.Parts) != 1 { + literal = false + } else if _, ok := expr.Parts[0].(*hclsyntax.LiteralValueExpr); !ok { + literal = false + } + if literal { + return cty.UnknownVal(retType), function.NewArgErrorf( + 0, "invalid template expression: templatestring is only for rendering templates retrieved dynamically from elsewhere, and so does not support providing a literal template; consider using a template string expression instead", + ) + } else { + return cty.UnknownVal(retType), function.NewArgErrorf( + 0, "invalid template expression: templatestring is only for rendering templates retrieved dynamically from elsewhere; to render an inline template, consider using a plain template string expression", + ) + } + default: + // Nothing else is allowed. + // Someone who really does want to construct a template dynamically + // can factor out that construction into a local value and refer + // to it in the templatestring call, but it's not really feasible + // to explain that clearly in a short error message so we'll deal + // with that option on the function's documentation page instead, + // where we can show a full example. + return cty.UnknownVal(retType), function.NewArgErrorf( + 0, "invalid template expression: must be a direct reference to a single string from elsewhere, containing valid Terraform template syntax", + ) + } + + templateVal, diags := templateClosure.Value() + if diags.HasErrors() { + // With the constraints we imposed above the possible errors + // here are pretty limited: it must be some kind of invalid + // traversal. As usual HCL diagnostics don't make for very + // good function errors but we've already filtered out many + // common reasons for error here, so we should get here pretty + // rarely. + return cty.UnknownVal(retType), function.NewArgErrorf( + 0, "invalid template expression: %s", + diags.Error(), + ) + } + if !templateVal.IsKnown() { + // We'll need to wait until we actually know what the template is. + return cty.UnknownVal(retType), nil + } + if templateVal.Type() != cty.String || templateVal.IsNull() { + // We're being a little stricter than usual here and requiring + // exactly a string, rather than just anything that can convert + // to one. This is because the stringification of a number or + // boolean value cannot be a useful template (it wouldn't have + // any template sequences in it) and so far more likely to be + // a mistake than actually intentional. + return cty.UnknownVal(retType), function.NewArgErrorf( + 0, "invalid template value: a string is required", + ) + } + templateVal, templateMarks := templateVal.Unmark() + templateStr := templateVal.AsString() + expr, diags := hclsyntax.ParseTemplate([]byte(templateStr), "", hcl.Pos{Line: 1, Column: 1}) + if diags.HasErrors() { + return cty.UnknownVal(retType), function.NewArgErrorf( + 0, "invalid template: %s", + diags.Error(), + ) + } + + render := makeRenderTemplateFunc(funcsCb, false) + retVal, err := render(expr, varsVal) + if err != nil { + return cty.UnknownVal(retType), err + } + retVal, err = convert.Convert(retVal, cty.String) + if err != nil { + return cty.UnknownVal(retType), fmt.Errorf("invalid template result: %s", err) + } + return retVal.WithMarks(templateMarks), nil + }, + }) +} + +func makeRenderTemplateFunc(funcsCb func() (funcs map[string]function.Function, fsFuncs collections.Set[string], templateFuncs collections.Set[string]), allowFS bool) func(expr hcl.Expression, varsVal cty.Value) (cty.Value, error) { + return func(expr hcl.Expression, varsVal cty.Value) (cty.Value, error) { + if varsTy := varsVal.Type(); !(varsTy.IsMapType() || varsTy.IsObjectType()) { + return cty.DynamicVal, function.NewArgErrorf(1, "invalid vars value: must be a map") // or an object, but we don't strongly distinguish these most of the time + } + + ctx := &hcl.EvalContext{ + Variables: varsVal.AsValueMap(), + } + + // We require all of the variables to be valid HCL identifiers, because + // otherwise there would be no way to refer to them in the template + // anyway. Rejecting this here gives better feedback to the user + // than a syntax error somewhere in the template itself. + for n := range ctx.Variables { + if !hclsyntax.ValidIdentifier(n) { + // This error message intentionally doesn't describe _all_ of + // the different permutations that are technically valid as an + // HCL identifier, but rather focuses on what we might + // consider to be an "idiomatic" variable name. + return cty.DynamicVal, function.NewArgErrorf(1, "invalid template variable name %q: must start with a letter, followed by zero or more letters, digits, and underscores", n) + } + } + + // We'll pre-check references in the template here so we can give a + // more specialized error message than HCL would by default, so it's + // clearer that this problem is coming from a templatefile call. + for _, traversal := range expr.Variables() { + root := traversal.RootName() + if _, ok := ctx.Variables[root]; !ok { + return cty.DynamicVal, function.NewArgErrorf(1, "vars map does not contain key %q, referenced at %s", root, traversal[0].SourceRange()) + } + } + + givenFuncs, fsFuncs, templateFuncs := funcsCb() // this callback indirection is to avoid chicken/egg problems + funcs := make(map[string]function.Function, len(givenFuncs)) + for name, fn := range givenFuncs { + plainName := strings.TrimPrefix(name, "core::") + switch { + case templateFuncs.Has(plainName): + funcs[name] = function.New(&function.Spec{ + Params: fn.Params(), + VarParam: fn.VarParam(), + Type: func(args []cty.Value) (cty.Type, error) { + return cty.NilType, fmt.Errorf("cannot recursively call %s from inside another template function", plainName) + }, + }) + case !allowFS && fsFuncs.Has(plainName): + // Note: for now this assumes that allowFS is false only for + // the templatestring function, and so mentions that name + // directly in the error message. + funcs[name] = function.New(&function.Spec{ + Params: fn.Params(), + VarParam: fn.VarParam(), + Type: func(args []cty.Value) (cty.Type, error) { + return cty.NilType, fmt.Errorf("cannot use filesystem access functions like %s in templatestring templates; consider passing the function result as a template variable instead", plainName) + }, + }) + default: + funcs[name] = fn + } + } + ctx.Functions = funcs + + val, diags := expr.Value(ctx) + if diags.HasErrors() { + return cty.DynamicVal, diags + } + return val, nil + } +} + // Replace searches a given string for another given substring, // and replaces all occurences with a given replacement string. func Replace(str, substr, replace cty.Value) (cty.Value, error) { diff --git a/internal/lang/funcs/string_test.go b/internal/lang/funcs/string_test.go index b66c2611d7..c0de6b3239 100644 --- a/internal/lang/funcs/string_test.go +++ b/internal/lang/funcs/string_test.go @@ -5,9 +5,16 @@ package funcs import ( "fmt" + "strings" "testing" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/ext/customdecode" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/function" + + "github.com/hashicorp/terraform/internal/collections" ) func TestReplace(t *testing.T) { @@ -253,3 +260,306 @@ func TestStartsWith(t *testing.T) { }) } } + +func TestTemplateString(t *testing.T) { + // This function has some special restrictions on what syntax is valid + // in its first argument, so we'll test this one using HCL expressions + // as the inputs, rather than direct cty values as we do for most other + // functions in this package. + tests := []struct { + templateExpr string + exprScope map[string]cty.Value + vars cty.Value + want cty.Value + wantErr string + }{ + { + `template`, + map[string]cty.Value{ + "template": cty.StringVal(`it's ${a}`), + }, + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("a value"), + }), + cty.StringVal(`it's a value`), + ``, + }, + { + `template`, + map[string]cty.Value{ + "template": cty.StringVal(`${a}`), + }, + cty.ObjectVal(map[string]cty.Value{ + "a": cty.True, + }), + // The special treatment of a template with only a single + // interpolation sequence does not apply to templatestring, because + // we're expecting to be evaluating templates fetched dynamically + // from somewhere else and want to avoid callers needing to deal + // with anything other than string results. + cty.StringVal(`true`), + ``, + }, + { + `template`, + map[string]cty.Value{ + "template": cty.StringVal(`${a}`), + }, + cty.ObjectVal(map[string]cty.Value{ + "a": cty.EmptyTupleVal, + }), + // The special treatment of a template with only a single + // interpolation sequence does not apply to templatestring, because + // we're expecting to be evaluating templates fetched dynamically + // from somewhere else and want to avoid callers needing to deal + // with anything other than string results. + cty.NilVal, + `invalid template result: string required`, + }, + { + `data.whatever.whatever["foo"].result`, + map[string]cty.Value{ + "data": cty.ObjectVal(map[string]cty.Value{ + "whatever": cty.ObjectVal(map[string]cty.Value{ + "whatever": cty.MapVal(map[string]cty.Value{ + "foo": cty.ObjectVal(map[string]cty.Value{ + "result": cty.StringVal("it's ${a}"), + }), + }), + }), + }), + }, + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("a value"), + }), + cty.StringVal(`it's a value`), + ``, + }, + { + `"can't write $${not_allowed}"`, + map[string]cty.Value{}, + cty.ObjectVal(map[string]cty.Value{ + "not_allowed": cty.StringVal("a literal template"), + }), + cty.NilVal, + `invalid template expression: templatestring is only for rendering templates retrieved dynamically from elsewhere, and so does not support providing a literal template; consider using a template string expression instead`, + }, + { + `"can't write ${not_allowed}"`, + map[string]cty.Value{}, + cty.ObjectVal(map[string]cty.Value{ + "not_allowed": cty.StringVal("a literal template"), + }), + cty.NilVal, + `invalid template expression: templatestring is only for rendering templates retrieved dynamically from elsewhere; to render an inline template, consider using a plain template string expression`, + }, + { + `"can't write %%{for x in things}a literal template%%{endfor}"`, + map[string]cty.Value{}, + cty.ObjectVal(map[string]cty.Value{ + "things": cty.ListVal([]cty.Value{cty.True}), + }), + cty.NilVal, + `invalid template expression: templatestring is only for rendering templates retrieved dynamically from elsewhere, and so does not support providing a literal template; consider using a template string expression instead`, + }, + { + `"can't write %{for x in things}a literal template%{endfor}"`, + map[string]cty.Value{}, + cty.ObjectVal(map[string]cty.Value{ + "things": cty.ListVal([]cty.Value{cty.True}), + }), + cty.NilVal, + `invalid template expression: templatestring is only for rendering templates retrieved dynamically from elsewhere; to render an inline template, consider using a plain template string expression`, + }, + { + `"${not_allowed}"`, + map[string]cty.Value{}, + cty.ObjectVal(map[string]cty.Value{ + "not allowed": cty.StringVal("an interp-only template"), + }), + cty.NilVal, + `invalid template expression: templatestring is only for rendering templates retrieved dynamically from elsewhere; to treat the inner expression as template syntax, write the reference expression directly without any template interpolation syntax`, + }, + { + `1 + 1`, + map[string]cty.Value{}, + cty.ObjectVal(map[string]cty.Value{}), + cty.NilVal, + `invalid template expression: must be a direct reference to a single string from elsewhere, containing valid Terraform template syntax`, + }, + { + `not_a_string`, + map[string]cty.Value{ + "not_a_string": cty.True, + }, + cty.ObjectVal(map[string]cty.Value{}), + cty.NilVal, + `invalid template value: a string is required`, + }, + { + `with_lower`, + map[string]cty.Value{ + "with_lower": cty.StringVal(`it's ${lower(a)}`), + }, + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("A VALUE"), + }), + cty.StringVal("it's a value"), + ``, + }, + { + `with_core_lower`, + map[string]cty.Value{ + "with_core_lower": cty.StringVal(`it's ${core::lower(a)}`), + }, + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("A VALUE"), + }), + cty.StringVal("it's a value"), + ``, + }, + { + `with_fsfunc`, + map[string]cty.Value{ + "with_fsfunc": cty.StringVal(`it's ${fsfunc()}`), + }, + cty.ObjectVal(map[string]cty.Value{}), + cty.NilVal, + `:1,8-15: Error in function call; Call to function "fsfunc" failed: cannot use filesystem access functions like fsfunc in templatestring templates; consider passing the function result as a template variable instead.`, + }, + { + `with_core_fsfunc`, + map[string]cty.Value{ + "with_core_fsfunc": cty.StringVal(`it's ${core::fsfunc()}`), + }, + cty.ObjectVal(map[string]cty.Value{}), + cty.NilVal, + `:1,8-21: Error in function call; Call to function "core::fsfunc" failed: cannot use filesystem access functions like fsfunc in templatestring templates; consider passing the function result as a template variable instead.`, + }, + { + `with_templatefunc`, + map[string]cty.Value{ + "with_templatefunc": cty.StringVal(`it's ${templatefunc()}`), + }, + cty.ObjectVal(map[string]cty.Value{}), + cty.NilVal, + `:1,8-21: Error in function call; Call to function "templatefunc" failed: cannot recursively call templatefunc from inside another template function.`, + }, + { + `with_core_templatefunc`, + map[string]cty.Value{ + "with_core_templatefunc": cty.StringVal(`it's ${core::templatefunc()}`), + }, + cty.ObjectVal(map[string]cty.Value{}), + cty.NilVal, + `:1,8-27: Error in function call; Call to function "core::templatefunc" failed: cannot recursively call templatefunc from inside another template function.`, + }, + { + `with_fstemplatefunc`, + map[string]cty.Value{ + "with_fstemplatefunc": cty.StringVal(`it's ${fstemplatefunc()}`), + }, + cty.ObjectVal(map[string]cty.Value{}), + cty.NilVal, + // The template function error takes priority over the filesystem + // function error if calling a function that's in both categories. + `:1,8-23: Error in function call; Call to function "fstemplatefunc" failed: cannot recursively call fstemplatefunc from inside another template function.`, + }, + { + `with_core_fstemplatefunc`, + map[string]cty.Value{ + "with_core_fstemplatefunc": cty.StringVal(`it's ${core::fstemplatefunc()}`), + }, + cty.ObjectVal(map[string]cty.Value{}), + cty.NilVal, + // The template function error takes priority over the filesystem + // function error if calling a function that's in both categories. + `:1,8-29: Error in function call; Call to function "core::fstemplatefunc" failed: cannot recursively call fstemplatefunc from inside another template function.`, + }, + } + + funcToTest := MakeTemplateStringFunc(func() (funcs map[string]function.Function, fsFuncs collections.Set[string], templateFuncs collections.Set[string]) { + // These are the functions available for use inside the nested template + // evaluation context. These are here only to test that we can call + // functions and that the template/filesystem functions get blocked + // with suitable error messages. This is not a realistic set of + // functions that would be available in a real call. + funcs = map[string]function.Function{ + "lower": function.New(&function.Spec{ + Params: []function.Parameter{ + { + Name: "str", + Type: cty.String, + }, + }, + Type: function.StaticReturnType(cty.String), + Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { + s := args[0].AsString() + return cty.StringVal(strings.ToLower(s)), nil + }, + }), + "fsfunc": function.New(&function.Spec{ + Type: function.StaticReturnType(cty.String), + Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { + return cty.UnknownVal(retType), fmt.Errorf("should not be able to call fsfunc") + }, + }), + "templatefunc": function.New(&function.Spec{ + Type: function.StaticReturnType(cty.String), + Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { + return cty.UnknownVal(retType), fmt.Errorf("should not be able to call templatefunc") + }, + }), + "fstemplatefunc": function.New(&function.Spec{ + Type: function.StaticReturnType(cty.String), + Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { + return cty.UnknownVal(retType), fmt.Errorf("should not be able to call fstemplatefunc") + }, + }), + } + funcs["core::lower"] = funcs["lower"] + funcs["core::fsfunc"] = funcs["fsfunc"] + funcs["core::templatefunc"] = funcs["templatefunc"] + funcs["core::fstemplatefunc"] = funcs["fstemplatefunc"] + return funcs, collections.NewSetCmp("fsfunc", "fstemplatefunc"), collections.NewSetCmp("templatefunc", "fstemplatefunc") + }) + + for _, test := range tests { + t.Run(test.templateExpr, func(t *testing.T) { + // The following mimics what HCL itself would do when preparing + // the first argument to this function, since the parameter + // uses the special "expression closure type" which causes + // HCL to delay evaluation of the expression and let the + // function handle it directly itself. + expr, diags := hclsyntax.ParseExpression([]byte(test.templateExpr), "", hcl.InitialPos) + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Error()) + } + exprClosure := &customdecode.ExpressionClosure{ + Expression: expr, + EvalContext: &hcl.EvalContext{ + Variables: test.exprScope, + }, + } + exprClosureVal := customdecode.ExpressionClosureVal(exprClosure) + + got, gotErr := funcToTest.Call([]cty.Value{exprClosureVal, test.vars}) + + if test.wantErr != "" { + if gotErr == nil { + t.Fatalf("unexpected success\ngot: %#v\nwant error: %s", got, test.wantErr) + } + if got, want := gotErr.Error(), test.wantErr; got != want { + t.Fatalf("wrong error\ngot: %s\nwant: %s", got, want) + } + return + } + if gotErr != nil { + t.Errorf("unexpected error: %s", gotErr.Error()) + } + if !test.want.RawEquals(got) { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.want) + } + }) + } +} diff --git a/internal/lang/functions.go b/internal/lang/functions.go index 10c72981a6..f811f0b503 100644 --- a/internal/lang/functions.go +++ b/internal/lang/functions.go @@ -12,6 +12,7 @@ import ( "github.com/zclconf/go-cty/cty/function" "github.com/zclconf/go-cty/cty/function/stdlib" + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/experiments" "github.com/hashicorp/terraform/internal/lang/funcs" ) @@ -22,6 +23,32 @@ var impureFunctions = []string{ "uuid", } +// filesystemFunctions are the functions that allow interacting with arbitrary +// paths in the local filesystem, and which can therefore have their results +// vary based on something other than their arguments, and might allow template +// rendering to expose details about the system where Terraform is running. +var filesystemFunctions = collections.NewSetCmp[string]( + "file", + "fileexists", + "fileset", + "filebase64", + "filebase64sha256", + "filebase64sha512", + "filemd5", + "filesha1", + "filesha256", + "filesha512", + "templatefile", +) + +// templateFunctions are functions that render nested templates. These are +// callable from module code but not from within the templates they are +// rendering. +var templateFunctions = collections.NewSetCmp[string]( + "templatefile", + "templatestring", +) + // Functions returns the set of functions that should be used to when evaluating // expressions in the receiving scope. func (s *Scope) Functions() map[string]function.Function { @@ -29,6 +56,10 @@ func (s *Scope) Functions() map[string]function.Function { if s.funcs == nil { s.funcs = baseFunctions(s.BaseDir) + // If you're adding something here, please consider whether it meets + // the criteria for either or both of the sets [filesystemFunctions] + // and [templateFunctions] and add it there if so, to ensure that + // functions relying on those classifications will behave correctly. coreFuncs := map[string]function.Function{ "abs": stdlib.AbsoluteFunc, "abspath": funcs.AbsPathFunc, @@ -148,12 +179,20 @@ func (s *Scope) Functions() map[string]function.Function { "zipmap": stdlib.ZipmapFunc, } - coreFuncs["templatefile"] = funcs.MakeTemplateFileFunc(s.BaseDir, func() map[string]function.Function { - // The templatefile function prevents recursive calls to itself - // by copying this map and overwriting the "templatefile" and - // "core:templatefile" entries. - return s.funcs - }) + // Our two template-rendering functions want to be able to call + // all of the other functions themselves, but we pass them indirectly + // via a callback to avoid chicken/egg problems while initializing + // the functions table. + funcsFunc := func() (funcs map[string]function.Function, fsFuncs collections.Set[string], templateFuncs collections.Set[string]) { + // The templatefile and templatestring functions prevent recursive + // calls to themselves and each other by copying this map and + // overwriting the relevant entries. + return s.funcs, filesystemFunctions, templateFunctions + } + coreFuncs["templatefile"] = funcs.MakeTemplateFileFunc(s.BaseDir, funcsFunc) + if s.activeExperiments.Has(experiments.TemplateStringFunc) { + coreFuncs["templatestring"] = funcs.MakeTemplateStringFunc(funcsFunc) + } if s.ConsoleMode { // The type function is only available in terraform console. @@ -228,6 +267,11 @@ func baseFunctions(baseDir string) map[string]function.Function { // in the "funcs" directory and potentially graduate to cty stdlib // later if the functionality seems to be something domain-agnostic // that would be useful to all applications using cty functions. + // + // If you're adding something here, please consider whether it meets + // the criteria for either or both of the sets [filesystemFunctions] + // and [templateFunctions] and add it there if so, to ensure that + // functions relying on those classifications will behave correctly. fs := map[string]function.Function{ "abs": stdlib.AbsoluteFunc, "abspath": funcs.AbsPathFunc, @@ -347,10 +391,10 @@ func baseFunctions(baseDir string) map[string]function.Function { "zipmap": stdlib.ZipmapFunc, } - fs["templatefile"] = funcs.MakeTemplateFileFunc(baseDir, func() map[string]function.Function { + fs["templatefile"] = funcs.MakeTemplateFileFunc(baseDir, func() (map[string]function.Function, collections.Set[string], collections.Set[string]) { // The templatefile function prevents recursive calls to itself // by copying this map and overwriting the "templatefile" entry. - return fs + return fs, filesystemFunctions, templateFunctions }) return fs