lang/funcs: templatefile requires valid variable names

lang/funcs/filesystem.go

@@ -100,6 +100,20 @@ func MakeTemplateFileFunc(baseDir string, funcsCb func() map[string]function.Fun
 			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.

lang/funcs/filesystem_test.go

@@ -58,47 +58,55 @@ func TestTemplateFile(t *testing.T) {
 		Path cty.Value
 		Vars cty.Value
 		Want cty.Value
-		Err  bool
+		Err  string
 	}{
 		{
 			cty.StringVal("testdata/hello.txt"),
 			cty.EmptyObjectVal,
 			cty.StringVal("Hello World"),
-			false,
+			``,
 		},
 		{
 			cty.StringVal("testdata/icon.png"),
 			cty.EmptyObjectVal,
 			cty.NilVal,
-			true, // Not valid UTF-8
+			`contents of testdata/icon.png are not valid UTF-8; use the filebase64 function to obtain the Base64 encoded contents or the other file functions (e.g. filemd5, filesha256) to obtain file hashing results instead`,
 		},
 		{
 			cty.StringVal("testdata/missing"),
 			cty.EmptyObjectVal,
 			cty.NilVal,
-			true, // no file exists
+			`no file exists at testdata/missing`,
 		},
 		{
 			cty.StringVal("testdata/hello.tmpl"),
 			cty.MapVal(map[string]cty.Value{
 				"name": cty.StringVal("Jodie"),
 			}),
 			cty.StringVal("Hello, Jodie!"),
-			false,
+			``,
+		},
+		{
+			cty.StringVal("testdata/hello.tmpl"),
+			cty.MapVal(map[string]cty.Value{
+				"name!": cty.StringVal("Jodie"),
+			}),
+			cty.NilVal,
+			`invalid template variable name "name!": must start with a letter, followed by zero or more letters, digits, and underscores`,
 		},
 		{
 			cty.StringVal("testdata/hello.tmpl"),
 			cty.ObjectVal(map[string]cty.Value{
 				"name": cty.StringVal("Jimbo"),
 			}),
 			cty.StringVal("Hello, Jimbo!"),
-			false,
+			``,
 		},
 		{
 			cty.StringVal("testdata/hello.tmpl"),
 			cty.EmptyObjectVal,
 			cty.NilVal,
-			true, // "name" is missing from the vars map
+			`vars map does not contain key "name", referenced at testdata/hello.tmpl:1,10-14`,
 		},
 		{
 			cty.StringVal("testdata/func.tmpl"),
@@ -110,13 +118,13 @@ func TestTemplateFile(t *testing.T) {
 				}),
 			}),
 			cty.StringVal("The items are a, b, c"),
-			false,
+			``,
 		},
 		{
 			cty.StringVal("testdata/recursive.tmpl"),
 			cty.MapValEmpty(cty.String),
 			cty.NilVal,
-			true, // recursive templatefile call not allowed
+			`testdata/recursive.tmpl:1,3-16: Error in function call; Call to function "templatefile" failed: cannot recursively call templatefile from inside templatefile call.`,
 		},
 		{
 			cty.StringVal("testdata/list.tmpl"),
@@ -128,23 +136,23 @@ func TestTemplateFile(t *testing.T) {
 				}),
 			}),
 			cty.StringVal("- a\n- b\n- c\n"),
-			false,
+			``,
 		},
 		{
 			cty.StringVal("testdata/list.tmpl"),
 			cty.ObjectVal(map[string]cty.Value{
 				"list": cty.True,
 			}),
 			cty.NilVal,
-			true, // iteration over non-iterable value
+			`testdata/list.tmpl:1,13-17: Iteration over non-iterable value; A value of type bool cannot be used as the collection in a 'for' expression.`,
 		},
 		{
 			cty.StringVal("testdata/bare.tmpl"),
 			cty.ObjectVal(map[string]cty.Value{
 				"val": cty.True,
 			}),
 			cty.True, // since this template contains only an interpolation, its true value shines through
-			false,
+			``,
 		},
 	}
 
@@ -165,10 +173,13 @@ func TestTemplateFile(t *testing.T) {
 				}
 			}
 
-			if test.Err {
+			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)

website/docs/configuration/functions/templatefile.html.md

@@ -29,7 +29,9 @@ into a separate file for readability.
 The "vars" argument must be a map. 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 Terraform language, except that
-recursive calls to `templatefile` are not permitted.
+recursive calls to `templatefile` are not permitted. Variable names must
+each start with a letter, followed by zero or more letters, digits, or
+underscores.
 
 Strings in the Terraform language are sequences of Unicode characters, so
 this function will interpret the file contents as UTF-8 encoded text and