Allow for templatefile recursion (up to 1024 depth default) (#1250)

Signed-off-by: Christian Mesh <christianmesh1@gmail.com>
Co-authored-by: Janos <86970079+janosdebugs@users.noreply.github.com>

.github/workflows/compare-snapshots.yml

@@ -18,9 +18,10 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v3
 
+
       - name: Install Go
         uses: actions/setup-go@v2
-        with: { go-version: '1.20' }
+        with: { go-version: '1.21.3' }
 
       - name: Get the equivalence test binary
         run: |

CHANGELOG.md

@@ -23,6 +23,7 @@ ENHANCEMENTS:
 * Support the XDG Base Directory Specification ([#1200](https://github.com/opentofu/opentofu/pull/1200))
 * Allow referencing the output from a test run in the local variables block of another run (tofu test). ([#1254](https://github.com/opentofu/opentofu/pull/1254))
 * Add documentation for the `removed` block. ([#1332](https://github.com/opentofu/opentofu/pull/1332))
+* Allow for templatefile function recursion (up to 1024 call depth default). ([#1250](https://github.com/opentofu/opentofu/pull/1250))
 
 BUG FIXES:
 * Fix view hooks unit test flakiness by deterministically waiting for heartbeats to execute ([$1153](https://github.com/opentofu/opentofu/issues/1153))

internal/lang/funcs/filesystem.go

@@ -9,8 +9,11 @@ import (
 	"encoding/base64"
 	"fmt"
 	"io"
+	"log"
 	"os"
 	"path/filepath"
+	"strconv"
+	"strings"
 	"unicode/utf8"
 
 	"github.com/bmatcuk/doublestar/v4"
@@ -58,6 +61,50 @@ func MakeFileFunc(baseDir string, encBase64 bool) function.Function {
 	})
 }
 
+func templateMaxRecursionDepth() (int, error) {
+	envkey := "TF_TEMPLATE_RECURSION_DEPTH"
+	val := os.Getenv(envkey)
+	if val != "" {
+		i, err := strconv.Atoi(val)
+		if err != nil {
+			return -1, fmt.Errorf("invalid value for %s: %w", envkey, err)
+		}
+		return i, nil
+	}
+	return 1024, nil // Sane Default
+}
+
+type ErrorTemplateRecursionLimit struct {
+	sources []string
+}
+
+func (err ErrorTemplateRecursionLimit) Error() string {
+	trace := make([]string, 0)
+	maxTrace := 16
+
+	// Look for repetition in the first N sources
+	for _, source := range err.sources[:min(maxTrace, len(err.sources))] {
+		looped := false
+		for _, st := range trace {
+			if st == source {
+				// Repeated source, probably a loop.  TF_LOG=debug will contain the full trace.
+				looped = true
+				break
+			}
+		}
+
+		trace = append(trace, source)
+
+		if looped {
+			break
+		}
+	}
+
+	log.Printf("[DEBUG] Template Stack (%d): %s", len(err.sources)-1, err.sources[len(err.sources)-1])
+
+	return fmt.Sprintf("maximum recursion depth %d reached in %s ... ", len(err.sources)-1, strings.Join(trace, ", "))
+}
+
 // MakeTemplateFileFunc constructs a function that takes a file path and
 // an arbitrary object of named values and attempts to render the referenced
 // file as a template using HCL template syntax.
@@ -68,10 +115,12 @@ func MakeFileFunc(baseDir string, encBase64 bool) function.Function {
 // those variables provided in the second function argument, to ensure that all
 // dependencies on other graph nodes can be seen before executing this function.
 //
-// As a special exception, a referenced template file may not recursively call
+// As a special exception, a referenced template file may call the templatefile
-// the templatefile function, since that would risk the same file being
+// function, with a recursion depth limit providing an error when reached
-// included into itself indefinitely.
 func MakeTemplateFileFunc(baseDir string, funcsCb func() map[string]function.Function) function.Function {
+	return makeTemplateFileFuncImpl(baseDir, funcsCb, 0)
+}
+func makeTemplateFileFuncImpl(baseDir string, funcsCb func() map[string]function.Function, depth int) function.Function {
 
 	params := []function.Parameter{
 		{
@@ -86,6 +135,15 @@ func MakeTemplateFileFunc(baseDir string, funcsCb func() map[string]function.Fun
 	}
 
 	loadTmpl := func(fn string, marks cty.ValueMarks) (hcl.Expression, error) {
+		maxDepth, err := templateMaxRecursionDepth()
+		if err != nil {
+			return nil, err
+		}
+		if depth > maxDepth {
+			// Sources will unwind up the stack
+			return nil, ErrorTemplateRecursionLimit{}
+		}
+
 		// We re-use File here to ensure the same filename interpretation
 		// as it does, along with its other safety checks.
 		tmplVal, err := File(baseDir, cty.StringVal(fn).WithMarks(marks))
@@ -101,6 +159,20 @@ func MakeTemplateFileFunc(baseDir string, funcsCb func() map[string]function.Fun
 		return expr, nil
 	}
 
+	funcsCbDepth := func() map[string]function.Function {
+		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" {
+				// Increment the recursion depth counter
+				funcs[name] = makeTemplateFileFuncImpl(baseDir, funcsCb, depth+1)
+				continue
+			}
+			funcs[name] = fn
+		}
+		return funcs
+	}
+
 	return function.New(&function.Spec{
 		Params: params,
 		Type: func(args []cty.Value) (cty.Type, error) {
@@ -120,7 +192,7 @@ func MakeTemplateFileFunc(baseDir string, funcsCb func() map[string]function.Fun
 
 			// This is safe even if args[1] contains unknowns because the HCL
 			// template renderer itself knows how to short-circuit those.
-			val, err := renderTemplate(expr, args[1], funcsCb)
+			val, err := renderTemplate(expr, args[1], funcsCbDepth())
 			return val.Type(), err
 		},
 		Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
@@ -129,7 +201,8 @@ func MakeTemplateFileFunc(baseDir string, funcsCb func() map[string]function.Fun
 			if err != nil {
 				return cty.DynamicVal, err
 			}
-			result, err := renderTemplate(expr, args[1], funcsCb)
+
+			result, err := renderTemplate(expr, args[1], funcsCbDepth())
 			return result.WithMarks(pathMarks), err
 		},
 	})

internal/lang/funcs/filesystem_test.go

@@ -151,7 +151,7 @@ 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 or templatestring.`,
+			`maximum recursion depth 1024 reached in testdata/recursive.tmpl:1,3-16, testdata/recursive.tmpl:1,3-16 ... `,
 		},
 		{
 			cty.StringVal("testdata/list.tmpl"),
@@ -219,6 +219,50 @@ func TestTemplateFile(t *testing.T) {
 	}
 }
 
+func Test_templateMaxRecursionDepth(t *testing.T) {
+	tests := []struct {
+		Input string
+		Want  int
+		Err   string
+	}{
+		{
+			"",
+			1024,
+			``,
+		}, {
+			"4096",
+			4096,
+			``,
+		}, {
+			"apple",
+			-1,
+			`invalid value for TF_TEMPLATE_RECURSION_DEPTH: strconv.Atoi: parsing "apple": invalid syntax`,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("templateMaxRecursion(%s)", test.Input), func(t *testing.T) {
+			os.Setenv("TF_TEMPLATE_RECURSION_DEPTH", test.Input)
+			got, err := templateMaxRecursionDepth()
+			if test.Err != "" {
+				if err == nil {
+					t.Fatal("succeeded; want error")
+				}
+				if got, want := err.Error(), test.Err; got != want {
+					t.Errorf("wrong error\ngot:  %s\nwant: %s", got, want)
+				}
+				return
+			} else if err != nil {
+				t.Fatalf("unexpected error: %s", err)
+			}
+
+			if got != test.Want {
+				t.Errorf("wrong result\ngot:  %#v\nwant: %#v", got, test.Want)
+			}
+		})
+	}
+}
+
 func TestFileExists(t *testing.T) {
 	tests := []struct {
 		Path cty.Value

internal/lang/funcs/render_template.go

@@ -14,13 +14,14 @@ import (
 	"github.com/zclconf/go-cty/cty/function"
 )
 
-func renderTemplate(expr hcl.Expression, varsVal cty.Value, funcsCb func() map[string]function.Function) (cty.Value, error) {
+func renderTemplate(expr hcl.Expression, varsVal cty.Value, funcs map[string]function.Function) (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(),
+		Functions: funcs,
 	}
 
 	// We require all of the variables to be valid HCL identifiers, because
@@ -54,36 +55,21 @@ func renderTemplate(expr hcl.Expression, varsVal cty.Value, funcsCb func() map[s
 		}
 	}
 
-	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" {
-			// We stub this one out to prevent recursive calls.
-			funcs[name] = function.New(&function.Spec{
-				Params: []function.Parameter{
-					{
-						Name:        "path",
-						Type:        cty.String,
-						AllowMarked: true,
-					},
-					{
-						Name: "vars",
-						Type: cty.DynamicPseudoType,
-					},
-				},
-				Type: func(args []cty.Value) (cty.Type, error) {
-					return cty.NilType, fmt.Errorf("cannot recursively call templatefile from inside templatefile or templatestring")
-				},
-			})
-			continue
-		}
-		funcs[name] = fn
-	}
-	ctx.Functions = funcs
-
 	val, diags := expr.Value(ctx)
 	if diags.HasErrors() {
+		for _, diag := range diags {
+			// Roll up recursive errors
+			if extra, ok := diag.Extra.(hclsyntax.FunctionCallDiagExtra); ok {
+				if extra.CalledFunctionName() == "templatefile" {
+					err := extra.FunctionCallError()
+					if err, ok := err.(ErrorTemplateRecursionLimit); ok {
+						return cty.DynamicVal, ErrorTemplateRecursionLimit{sources: append(err.sources, diag.Subject.String())}
+					}
+				}
+			}
+		}
 		return cty.DynamicVal, diags
 	}
+
 	return val, nil
 }

internal/lang/funcs/render_template_test.go

@@ -88,9 +88,7 @@ func TestRenderTemplate(t *testing.T) {
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
 
-			got, err := renderTemplate(test.Expr, test.Vars, func() map[string]function.Function {
+			got, err := renderTemplate(test.Expr, test.Vars, map[string]function.Function{})
-				return map[string]function.Function{}
-			})
 
 			if err != nil {
 				if test.Err == "" {

internal/lang/funcs/string.go

@@ -218,7 +218,7 @@ func MakeTemplateStringFunc(content string, funcsCb func() map[string]function.F
 
 			// This is safe even if args[1] contains unknowns because the HCL
 			// template renderer itself knows how to short-circuit those.
-			val, err := renderTemplate(expr, args[1], funcsCb)
+			val, err := renderTemplate(expr, args[1], funcsCb())
 			return val.Type(), err
 		},
 		Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
@@ -227,7 +227,7 @@ func MakeTemplateStringFunc(content string, funcsCb func() map[string]function.F
 			if err != nil {
 				return cty.DynamicVal, err
 			}
-			result, err := renderTemplate(expr, args[1], funcsCb)
+			result, err := renderTemplate(expr, args[1], funcsCb())
 			return result.WithMarks(dataMarks), err
 		},
 	})

internal/lang/funcs/testdata/recursive.tmpl

@@ -1 +1 @@
-${templatefile("recursive.tmpl", {})}
+${templatefile("testdata/recursive.tmpl", {})}

website/docs/language/functions/templatefile.mdx

@@ -22,10 +22,9 @@ out into a separate file for readability.
 
 The "vars" argument must be an object. Within the template file, each of the
 keys in the map is available as a variable for interpolation. The template may
-also use any other function available in the OpenTofu language, except that
+also use any other function available in the OpenTofu language. Variable
-recursive calls to `templatefile` are not permitted. Variable names must
+names must each start with a letter, followed by zero or more
-each start with a letter, followed by zero or more letters, digits, or
+letters, digits, or underscores.
-underscores.
 
 Strings in the OpenTofu language are sequences of Unicode characters, so
 this function will interpret the file contents as UTF-8 encoded text and
@@ -42,6 +41,29 @@ OpenTofu will not prevent you from using other names, but following this
 convention will help your editor understand the content and likely provide
 better editing experience as a result.
 
+## Recursion
+
+There are a few limitations to be aware of if recursion is used with templatefile.
+
+Any recursive calls to `templatefile` will have a limited call depth (1024 by default).
+This is to prevent crashes due to unintential infinite recursive calls and limit the chance
+of Out Of Memory crashes.
+
+As tail-recursion is not supported, all documents in a call stack must be loaded
+into memory before the stack can unwind. On most modern systems and configurations
+this will likely not be an issue, but it is worth being mindful of.
+
+If the maximum recursion depth is hit during execution, a concise error will be provided
+which describes the first few steps of the call stack to help you diagnose the issue.
+If you need the full call stack, setting `TF_LOG=debug` will cause the full templatefile
+callstack to be printed to the console.
+
+If your configuration requires a larger maximum recursion depth, you can override the
+default using the `TF_TEMPLATE_RECURSION_DEPTH` environment variable. This is not
+recommended and is only provided as an escape hatch. Additionally, setting it lower
+than the 1024 default has the potential to cause problems with modules that use
+the templatefile function.
+
 ## Examples
 
 ### Lists
@@ -148,3 +170,5 @@ For more information, see the main documentation for
 
 * [`file`](/docs/language/functions/file) reads a file from disk and returns its literal contents
   without any template interpretation.
+* [`templatestring`](/docs/language/functions/templatestring) takes a string and renders it as a
+  template using a supplied set of template variables.