Sanitize var code (#11566)

* evaluateLocalVariables: modify code for readability and some (not benchmarked) perfs

* Make default input variable type the DynamicPseudoType

This should be the default, and avoids a panic. This type can represent situations where a type is not yet known. Its meaning is undefined in cty.

* do not take Empty types from default value

* Update types.variables.go

Co-authored-by: Wilken Rivera <wilken@hashicorp.com>

command/build_test.go

@@ -435,6 +435,36 @@ func TestBuild(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "hcl - recursive local using input var",
+			args: []string{
+				testFixture("hcl", "recursive_local_with_input"),
+			},
+			fileCheck: fileCheck{
+				expectedContent: map[string]string{
+					"hey.txt": "hello",
+				},
+			},
+		},
+		{
+			name: "hcl - recursive local using an unset input var",
+			args: []string{
+				testFixture("hcl", "recursive_local_with_unset_input"),
+			},
+			fileCheck:    fileCheck{},
+			expectedCode: 1,
+		},
+		{
+			name: "hcl - var with default value empty object/list can be set",
+			args: []string{
+				testFixture("hcl", "empty_object"),
+			},
+			fileCheck: fileCheck{
+				expectedContent: map[string]string{
+					"foo.txt": "yo",
+				},
+			},
+		},
 	}
 
 	for _, tt := range tc {

command/test-fixtures/hcl/empty_object/setting.auto.pkrvars.hcl

@@ -0,0 +1,4 @@
+foo = ["yo"]
+bar = { 
+  "baz": "foo",
+}

command/test-fixtures/hcl/empty_object/var.pkr.hcl

@@ -0,0 +1,18 @@
+
+variable "foo" {
+  default = []
+}
+
+variable "bar" {
+  default = {}
+}
+
+source "file" "base" {
+}
+
+build {
+  source "sources.file.base" {
+      target = "${var.bar.baz}.txt"
+      content = var.foo[0]
+  }
+}

command/test-fixtures/hcl/recursive_local_with_input/locals.pkr.hcl

@@ -0,0 +1,25 @@
+variable "vms_to_build" {
+  default = {
+    "amgroup": "hey"
+  }
+}
+
+locals {
+  vms_to_build = var.vms_to_build
+  dynamic_slice = { 
+    for vm, val in var.vms_to_build : 
+    vm => lookup(local.vms_to_build, vm, "VM NAME NOT FOUND")
+  }
+}
+
+
+source "file" "chocolate" {
+  content = "hello"
+  target = "${local.dynamic_slice.amgroup}.txt"
+}
+
+build {
+  sources = [
+    "sources.file.chocolate",
+  ]
+}

command/test-fixtures/hcl/recursive_local_with_unset_input/file.pkr.hcl

@@ -0,0 +1,12 @@
+
+variable "vms_to_build" {
+}
+
+locals {
+  vms_to_build = var.vms_to_build
+  dynamic_map = { 
+    for vm in local.vms_to_build : 
+      vm => lookup(local, vm, "VM NAME NOT FOUND")
+  }
+}
+

hcl2template/types.packer_config.go

@@ -232,30 +232,18 @@ func (c *PackerConfig) evaluateLocalVariables(locals []*LocalBlock) hcl.Diagnost
 		c.LocalVariables = Variables{}
 	}
 
-	llen := len(locals)
-	var retry int
-	// avoid to retrying more than the number of items we have in the slice
-	for i := 0; llen > 0 && retry < llen+1; i++ {
-		i := i % llen
-		local := locals[i]
-		moreDiags := c.evaluateLocalVariable(local)
-		if moreDiags.HasErrors() {
-			if llen == 1 {
-				// If this is the only local left there's no need
-				// to try evaluating again
-				return append(diags, moreDiags...)
+	for foundSomething := true; foundSomething; {
+		foundSomething = false
+		for i := 0; i < len(locals); {
+			local := locals[i]
+			moreDiags := c.evaluateLocalVariable(local)
+			if moreDiags.HasErrors() {
+				i++
+				continue
 			}
-			retry++
-			continue
+			foundSomething = true
+			locals = append(locals[:i], locals[i+1:]...)
 		}
-
-		// could evaluate
-		retry = 0
-		diags = append(diags, moreDiags...)
-
-		// Remove local from slice
-		locals = append(locals[:i], locals[i+1:]...)
-		llen--
 	}
 
 	if len(locals) != 0 {

hcl2template/types.variables.go

@@ -335,6 +335,7 @@ func (variables *Variables) decodeVariableBlock(block *hcl.Block, ectx *hcl.Eval
 	v := &Variable{
 		Name:  name,
 		Range: block.DefRange,
+		Type:  cty.DynamicPseudoType,
 	}
 
 	if attr, exists := content.Attributes["description"]; exists {
@@ -386,7 +387,9 @@ func (variables *Variables) decodeVariableBlock(block *hcl.Block, ectx *hcl.Eval
 
 		// It's possible no type attribute was assigned so lets make sure we
 		// have a valid type otherwise there could be issues parsing the value.
-		if v.Type == cty.NilType {
+		if v.Type == cty.DynamicPseudoType &&
+			!defaultValue.Type().Equals(cty.EmptyObject) &&
+			!defaultValue.Type().Equals(cty.EmptyTuple) {
 			v.Type = defaultValue.Type()
 		}
 	}
@@ -752,7 +755,7 @@ func (cfg *PackerConfig) collectInputVariableValues(env []string, files []*hcl.F
 // The specified filename is to identify the source of where value originated from in the diagnostics report, if there is an error.
 func expressionFromVariableDefinition(filename string, value string, variableType cty.Type) (hclsyntax.Expression, hcl.Diagnostics) {
 	switch variableType {
-	case cty.String, cty.Number, cty.NilType:
+	case cty.String, cty.Number, cty.NilType, cty.DynamicPseudoType:
 		// when the type is nil (not set in a variable block) we default to
 		// interpreting everything as a string literal.
 		return &hclsyntax.LiteralValueExpr{Val: cty.StringVal(value)}, nil

website/content/docs/templates/hcl_templates/functions/collection/lookup.mdx

@@ -12,10 +12,6 @@ If the given key does not exist, a the given default value is returned instead.
 lookup(map, key, default)
 ```
 
--> For historical reasons, the `default` parameter is actually optional. However,
-omitting `default` is deprecated since v0.7 because that would then be
-equivalent to the native index syntax, `map[key]`.
-
 ## Examples
 
 ```shell-session