hcl2template: recursively evaluate local variables

The logic for evaluating local variables used to rely on their
definition order, with some edge cases. Typically `locals` blocks define
multiple local variables, which don't necessarily appear in the same
order at evaluation as within the template, leading to inconsistent
behaviour, as the order in which those are added to the list of local
variables is non-deterministic.

To avoid this problem, we change how local variables are evaluated, and
we're adopting a workflow similar to datasources, where the local
variables first build a list of direct dependencies. Then when
evaluation happens, we evaluate all the dependencies recursively for
each local variable, which takes care of this issue.

As with Datasources, we add a cap to the recursion: 10. I.e. if the
evaluation of a single variable provokes an infinite recursion, we stop
at that point and return an error to the user, telling them to fix their
template.

hcl2template/types.packer_config.go

@@ -217,14 +217,16 @@ func parseLocalVariableBlocks(f *hcl.File) ([]*LocalBlock, hcl.Diagnostics) {
 	return locals, diags
 }
 
-func (c *PackerConfig) evaluateAllLocalVariables(locals []*LocalBlock) hcl.Diagnostics {
-	var diags hcl.Diagnostics
+func (c *PackerConfig) localByName(local string) (*LocalBlock, error) {
+	for _, loc := range c.LocalBlocks {
+		if loc.Name != local {
+			continue
+		}
 
-	for _, local := range locals {
-		diags = append(diags, c.evaluateLocalVariable(local)...)
+		return loc, nil
 	}
 
-	return diags
+	return nil, fmt.Errorf("local %s not found", local)
 }
 
 func (c *PackerConfig) evaluateLocalVariables(locals []*LocalBlock) hcl.Diagnostics {
@@ -238,23 +240,41 @@ func (c *PackerConfig) evaluateLocalVariables(locals []*LocalBlock) hcl.Diagnost
 		c.LocalVariables = Variables{}
 	}
 
-	for foundSomething := true; foundSomething; {
-		foundSomething = false
-		for i := 0; i < len(locals); {
-			local := locals[i]
-			moreDiags := c.evaluateLocalVariable(local)
-			if moreDiags.HasErrors() {
-				i++
+	for _, local := range c.LocalBlocks {
+		// Note: when looking at the expressions, we only need to care about
+		// attributes, as HCL2 expressions are not allowed in a block's labels.
+		vars := FilterTraversalsByType(local.Expr.Variables(), "local")
+
+		var localDeps []*LocalBlock
+		for _, v := range vars {
+			// Some local variables may be locally aliased as
+			// `local`, which
+			if len(v) < 2 {
 				continue
 			}
-			foundSomething = true
-			locals = append(locals[:i], locals[i+1:]...)
+			varName := v[1].(hcl.TraverseAttr).Name
+			block, err := c.localByName(varName)
+			if err != nil {
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Missing variable dependency",
+					Detail: fmt.Sprintf("The expression for variable %q depends on local.%s, which is not defined.",
+						local.Name, varName),
+				})
+				continue
+			}
+			localDeps = append(localDeps, block)
 		}
+		local.dependencies = localDeps
 	}
 
-	if len(locals) != 0 {
-		// get errors from remaining variables
-		return c.evaluateAllLocalVariables(locals)
+	// Immediately return in case the dependencies couldn't be figured out.
+	if diags.HasErrors() {
+		return diags
+	}
+
+	for _, local := range c.LocalBlocks {
+		diags = diags.Extend(c.evaluateLocalVariable(local, 0))
 	}
 
 	return diags
@@ -281,10 +301,33 @@ func checkForDuplicateLocalDefinition(locals []*LocalBlock) hcl.Diagnostics {
 	return diags
 }
 
-func (c *PackerConfig) evaluateLocalVariable(local *LocalBlock) hcl.Diagnostics {
+func (c *PackerConfig) evaluateLocalVariable(local *LocalBlock, depth int) hcl.Diagnostics {
+	// If the variable already was evaluated, we can return immediately
+	if local.evaluated {
+		return nil
+	}
+
+	if depth >= 10 {
+		return hcl.Diagnostics{&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Max local recursion depth exceeded.",
+			Detail: "An error occured while recursively evaluating locals." +
+				"Your local variables likely have a cyclic dependency. " +
+				"Please simplify your config to continue. ",
+		}}
+	}
+
 	var diags hcl.Diagnostics
 
+	for _, dep := range local.dependencies {
+		localDiags := c.evaluateLocalVariable(dep, depth+1)
+		diags = diags.Extend(localDiags)
+	}
+
 	value, moreDiags := local.Expr.Value(c.EvalContext(LocalContext, nil))
+
+	local.evaluated = true
+
 	diags = append(diags, moreDiags...)
 	if moreDiags.HasErrors() {
 		return diags

hcl2template/types.variables.go

@@ -30,6 +30,17 @@ type LocalBlock struct {
 	// When Sensitive is set to true Packer will try its best to hide/obfuscate
 	// the variable from the output stream. By replacing the text.
 	Sensitive bool
+
+	// dependsOn lists the dependencies for being able to evaluate this local
+	//
+	// Only `local`/`locals` will be referenced here as we execute all the
+	// same component types at once.
+	dependencies []*LocalBlock
+	// evaluated toggles to true if it has been evaluated.
+	//
+	// We use this to determine if we're ready to get the value of the
+	// expression.
+	evaluated bool
 }
 
 // VariableAssignment represents a way a variable was set: the expression

packer_test/local_eval_test.go

@@ -0,0 +1,14 @@
+package packer_test
+
+func (ts *PackerTestSuite) TestEvalLocalsOrder() {
+	ts.SkipNoAcc()
+
+	pluginDir, cleanup := ts.MakePluginDir()
+	defer cleanup()
+
+	ts.PackerCommand().UsePluginDir(pluginDir).
+		Runs(10).
+		Stdin("local.test_local\n").
+		SetArgs("console", "./templates/locals_no_order.pkr.hcl").
+		Assert(MustSucceed(), Grep("\\[\\]", grepStdout, grepInvert))
+}

packer_test/templates/locals_no_order.pkr.hcl

@@ -0,0 +1,7 @@
+locals {
+  test_local = can(local.test_data) ? local.test_data : []
+
+  test_data = [
+    { key = "value" }
+  ]
+}