Fix HCL2 local variables decoding to allow local usage within another local in the same locals block (#8755)

hcl2template/common_test.go

@@ -80,6 +80,31 @@ func testParse(t *testing.T, tests []parseTest) {
 			); diff != "" {
 				t.Fatalf("Parser.parse() wrong packer config. %s", diff)
 			}
+
+			if gotCfg != nil && !tt.parseWantDiagHasErrors {
+				gotInputVar := gotCfg.InputVariables
+				for name, value := range tt.parseWantCfg.InputVariables {
+					if variable, ok := gotInputVar[name]; ok {
+						if variable.DefaultValue.GoString() != value.DefaultValue.GoString() {
+							t.Fatalf("Parser.parse() input variable %s expected '%s' but was '%s'", name, value.DefaultValue.GoString(), variable.DefaultValue.GoString())
+						}
+					} else {
+						t.Fatalf("Parser.parse() missing input variable. %s", name)
+					}
+				}
+
+				gotLocalVar := gotCfg.LocalVariables
+				for name, value := range tt.parseWantCfg.LocalVariables {
+					if variable, ok := gotLocalVar[name]; ok {
+						if variable.DefaultValue.GoString() != value.DefaultValue.GoString() {
+							t.Fatalf("Parser.parse() local variable %s expected '%s' but was '%s'", name, value.DefaultValue.GoString(), variable.DefaultValue.GoString())
+						}
+					} else {
+						t.Fatalf("Parser.parse() missing local variable. %s", name)
+					}
+				}
+			}
+
 			if gotDiags.HasErrors() {
 				return
 			}

hcl2template/parser.go

@@ -95,11 +95,16 @@ func (p *Parser) parse(filename string, vars map[string]string) (*PackerConfig,
 	// can use input variables so we decode them firsthand.
 	{
 		for _, file := range files {
-			diags = append(diags, p.decodeInputVariables(file, cfg)...)
+			diags = append(diags, cfg.decodeInputVariables(file)...)
 		}
+
+		var locals []*Local
 		for _, file := range files {
-			diags = append(diags, p.decodeLocalVariables(file, cfg)...)
+			moreLocals, morediags := cfg.parseLocalVariables(file)
+			diags = append(diags, morediags...)
+			locals = append(locals, moreLocals...)
 		}
+		diags = append(diags, cfg.evaluateLocalVariables(locals)...)
 	}
 
 	// parse var files
@@ -129,49 +134,6 @@ func (p *Parser) parse(filename string, vars map[string]string) (*PackerConfig,
 	return cfg, diags
 }
 
-// decodeLocalVariables looks in the found blocks for 'variables' and
-// 'variable' blocks. It should be called firsthand so that other blocks can
-// use the variables.
-func (p *Parser) decodeInputVariables(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics {
-	var diags hcl.Diagnostics
-
-	content, moreDiags := f.Body.Content(configSchema)
-	diags = append(diags, moreDiags...)
-
-	for _, block := range content.Blocks {
-		switch block.Type {
-		case variableLabel:
-			moreDiags := cfg.InputVariables.decodeConfig(block, nil)
-			diags = append(diags, moreDiags...)
-		case variablesLabel:
-			moreDiags := cfg.InputVariables.decodeConfigMap(block, nil)
-			diags = append(diags, moreDiags...)
-		}
-	}
-
-	return diags
-}
-
-// decodeLocalVariables looks in the found blocks for 'locals' blocks. It
-// should be called after parsing input variables so that they can be
-// referenced.
-func (p *Parser) decodeLocalVariables(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics {
-	var diags hcl.Diagnostics
-
-	content, moreDiags := f.Body.Content(configSchema)
-	diags = append(diags, moreDiags...)
-
-	for _, block := range content.Blocks {
-		switch block.Type {
-		case localsLabel:
-			moreDiags := cfg.LocalVariables.decodeConfigMap(block, cfg.EvalContext())
-			diags = append(diags, moreDiags...)
-		}
-	}
-
-	return diags
-}
-
 // decodeConfig looks in the found blocks for everything that is not a variable
 // block. It should be called after parsing input variables and locals so that
 // they can be referenced.

hcl2template/testdata/variables/complicated/a.pkr.hcl

@@ -0,0 +1,8 @@
+locals {
+  for_var  = "${local.name_prefix}"
+  bar_var  = [
+    local.for_var,
+    local.foo,
+    var.name_prefix,
+  ]
+}

hcl2template/testdata/variables/complicated/b.pkr.hcl

@@ -0,0 +1,5 @@
+locals {
+  name_prefix = "${var.name_prefix}"
+  foo         = "${local.name_prefix}"
+  bar         = local.name_prefix
+}

hcl2template/testdata/variables/complicated/c.pkr.hcl

@@ -0,0 +1,3 @@
+variable "name_prefix" {
+  default = "foo"
+}

hcl2template/testdata/variables/recursive_locals.pkr.hcl

@@ -0,0 +1,4 @@
+locals {
+  first  = local.second
+  second = local.first
+}

hcl2template/types.packer_config.go

@@ -42,6 +42,127 @@ func (cfg *PackerConfig) EvalContext() *hcl.EvalContext {
 	return ectx
 }
 
+// decodeInputVariables looks in the found blocks for 'variables' and
+// 'variable' blocks. It should be called firsthand so that other blocks can
+// use the variables.
+func (c *PackerConfig) decodeInputVariables(f *hcl.File) hcl.Diagnostics {
+	var diags hcl.Diagnostics
+
+	content, moreDiags := f.Body.Content(configSchema)
+	diags = append(diags, moreDiags...)
+
+	for _, block := range content.Blocks {
+		switch block.Type {
+		case variableLabel:
+			moreDiags := c.InputVariables.decodeVariableBlock(block, nil)
+			diags = append(diags, moreDiags...)
+		case variablesLabel:
+			attrs, moreDiags := block.Body.JustAttributes()
+			diags = append(diags, moreDiags...)
+			for key, attr := range attrs {
+				moreDiags = c.InputVariables.decodeVariable(key, attr, nil)
+				diags = append(diags, moreDiags...)
+			}
+		}
+	}
+	return diags
+}
+
+// parseLocalVariables looks in the found blocks for 'locals' blocks. It
+// should be called after parsing input variables so that they can be
+// referenced.
+func (c *PackerConfig) parseLocalVariables(f *hcl.File) ([]*Local, hcl.Diagnostics) {
+	var diags hcl.Diagnostics
+
+	content, moreDiags := f.Body.Content(configSchema)
+	diags = append(diags, moreDiags...)
+	var allLocals []*Local
+
+	for _, block := range content.Blocks {
+		switch block.Type {
+		case localsLabel:
+			attrs, moreDiags := block.Body.JustAttributes()
+			diags = append(diags, moreDiags...)
+			locals := make([]*Local, 0, len(attrs))
+			for name, attr := range attrs {
+				if _, found := c.LocalVariables[name]; found {
+					diags = append(diags, &hcl.Diagnostic{
+						Severity: hcl.DiagError,
+						Summary:  "Duplicate variable",
+						Detail:   "Duplicate " + name + " variable definition found.",
+						Subject:  attr.NameRange.Ptr(),
+						Context:  block.DefRange.Ptr(),
+					})
+					return nil, diags
+				}
+				locals = append(locals, &Local{
+					Name: name,
+					Expr: attr.Expr,
+				})
+			}
+			allLocals = append(allLocals, locals...)
+		}
+	}
+
+	return allLocals, diags
+}
+
+func (c *PackerConfig) evaluateLocalVariables(locals []*Local) hcl.Diagnostics {
+	var diags hcl.Diagnostics
+
+	if len(locals) > 0 && c.LocalVariables == nil {
+		c.LocalVariables = Variables{}
+	}
+
+	var retry, previousL int
+	for len(locals) > 0 {
+		local := locals[0]
+		moreDiags := c.evaluateLocalVariable(local)
+		if moreDiags.HasErrors() {
+			if len(locals) == 1 {
+				// If this is the only local left there's no need
+				// to try evaluating again
+				return append(diags, moreDiags...)
+			}
+			if previousL == len(locals) {
+				if retry == 100 {
+					// To get to this point, locals must have a circle dependency
+					return append(diags, moreDiags...)
+				}
+				retry++
+			}
+			previousL = len(locals)
+
+			// If local uses another local that has not been evaluated yet this could be the reason of errors
+			// Push local to the end of slice to be evaluated later
+			locals = append(locals, local)
+		} else {
+			retry = 0
+			diags = append(diags, moreDiags...)
+		}
+		// Remove local from slice
+		locals = append(locals[:0], locals[1:]...)
+	}
+
+	return diags
+}
+
+func (c *PackerConfig) evaluateLocalVariable(local *Local) hcl.Diagnostics {
+	var diags hcl.Diagnostics
+
+	value, moreDiags := local.Expr.Value(c.EvalContext())
+	diags = append(diags, moreDiags...)
+	if moreDiags.HasErrors() {
+		return diags
+	}
+	c.LocalVariables[local.Name] = &Variable{
+		DefaultValue: value,
+		Type:         value.Type(),
+	}
+
+	return diags
+}
+
 // getCoreBuildProvisioners takes a list of provisioner block, starts according
 // provisioners and sends parsed HCL2 over to it.
 func (p *Parser) getCoreBuildProvisioners(blocks []*ProvisionerBlock, ectx *hcl.EvalContext, generatedVars map[string]string) ([]packer.CoreBuildProvisioner, hcl.Diagnostics) {

hcl2template/types.packer_config_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/packer/packer"
+	"github.com/zclconf/go-cty/cty"
 )
 
 var (
@@ -21,15 +22,46 @@ func TestParser_complete(t *testing.T) {
 			&PackerConfig{
 				Basedir: "testdata/complete",
 				InputVariables: Variables{
-					"foo":                     &Variable{},
-					"image_id":                &Variable{},
-					"port":                    &Variable{},
-					"availability_zone_names": &Variable{},
+					"foo": &Variable{
+						DefaultValue: cty.StringVal("value"),
+					},
+					"image_id": &Variable{
+						DefaultValue: cty.StringVal("image-id-default"),
+					},
+					"port": &Variable{
+						DefaultValue: cty.NumberIntVal(42),
+					},
+					"availability_zone_names": &Variable{
+						DefaultValue: cty.ListVal([]cty.Value{
+							cty.StringVal("A"),
+							cty.StringVal("B"),
+							cty.StringVal("C"),
+						}),
+					},
 				},
 				LocalVariables: Variables{
-					"feefoo":        &Variable{},
-					"standard_tags": &Variable{},
-					"abc_map":       &Variable{},
+					"feefoo": &Variable{
+						DefaultValue: cty.StringVal("value_image-id-default"),
+					},
+					"standard_tags": &Variable{
+						DefaultValue: cty.ObjectVal(map[string]cty.Value{
+							"Component":   cty.StringVal("user-service"),
+							"Environment": cty.StringVal("production"),
+						}),
+					},
+					"abc_map": &Variable{
+						DefaultValue: cty.TupleVal([]cty.Value{
+							cty.ObjectVal(map[string]cty.Value{
+								"id": cty.StringVal("a"),
+							}),
+							cty.ObjectVal(map[string]cty.Value{
+								"id": cty.StringVal("b"),
+							}),
+							cty.ObjectVal(map[string]cty.Value{
+								"id": cty.StringVal("c"),
+							}),
+						}),
+					},
 				},
 				Sources: map[SourceRef]*SourceBlock{
 					refVBIsoUbuntu1204: {Type: "virtualbox-iso", Name: "ubuntu-1204"},

hcl2template/types.variables.go

@@ -12,6 +12,14 @@ import (
 	"github.com/zclconf/go-cty/cty/convert"
 )
 
+// Local represents a single entry from a "locals" block in a module or file.
+// The "locals" block itself is not represented, because it serves only to
+// provide context for us to interpret its contents.
+type Local struct {
+	Name string
+	Expr hcl.Expression
+}
+
 type Variable struct {
 	// CmdValue, VarfileValue, EnvValue, DefaultValue are possible values of
 	// the variable; The first value set from these will be the one used. If
@@ -75,44 +83,40 @@ func (variables Variables) Values() map[string]cty.Value {
 	return res
 }
 
-// decodeConfig decodes a "variables" section the way packer 1 used to
-func (variables *Variables) decodeConfigMap(block *hcl.Block, ectx *hcl.EvalContext) hcl.Diagnostics {
+// decodeVariable decodes a variable key and value into Variables
+func (variables *Variables) decodeVariable(key string, attr *hcl.Attribute, ectx *hcl.EvalContext) hcl.Diagnostics {
+	var diags hcl.Diagnostics
+
 	if (*variables) == nil {
 		(*variables) = Variables{}
 	}
-	attrs, diags := block.Body.JustAttributes()
 
-	if diags.HasErrors() {
+	if _, found := (*variables)[key]; found {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Duplicate variable",
+			Detail:   "Duplicate " + key + " variable definition found.",
+			Subject:  attr.NameRange.Ptr(),
+		})
 		return diags
 	}
 
-	for key, attr := range attrs {
-		if _, found := (*variables)[key]; found {
-			diags = append(diags, &hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Duplicate variable",
-				Detail:   "Duplicate " + key + " variable definition found.",
-				Subject:  attr.NameRange.Ptr(),
-				Context:  block.DefRange.Ptr(),
-			})
-			continue
-		}
-		value, moreDiags := attr.Expr.Value(ectx)
-		diags = append(diags, moreDiags...)
-		if moreDiags.HasErrors() {
-			continue
-		}
-		(*variables)[key] = &Variable{
-			DefaultValue: value,
-			Type:         value.Type(),
-		}
+	value, moreDiags := attr.Expr.Value(ectx)
+	diags = append(diags, moreDiags...)
+	if moreDiags.HasErrors() {
+		return diags
+	}
+
+	(*variables)[key] = &Variable{
+		DefaultValue: value,
+		Type:         value.Type(),
 	}
 
 	return diags
 }
 
-// decodeConfig decodes a "variables" section the way packer 1 used to
-func (variables *Variables) decodeConfig(block *hcl.Block, ectx *hcl.EvalContext) hcl.Diagnostics {
+// decodeVariableBlock decodes a "variables" section the way packer 1 used to
+func (variables *Variables) decodeVariableBlock(block *hcl.Block, ectx *hcl.EvalContext) hcl.Diagnostics {
 	if (*variables) == nil {
 		(*variables) = Variables{}
 	}

hcl2template/types.variables_test.go

@@ -23,12 +23,25 @@ func TestParse_variables(t *testing.T) {
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "variables"),
 				InputVariables: Variables{
-					"image_name": &Variable{},
-					"key":        &Variable{},
-					"my_secret":  &Variable{},
-					"image_id":   &Variable{},
-					"port":       &Variable{},
+					"image_name": &Variable{
+						DefaultValue: cty.StringVal("foo-image-{{user `my_secret`}}"),
+					},
+					"key": &Variable{
+						DefaultValue: cty.StringVal("value"),
+					},
+					"my_secret": &Variable{
+						DefaultValue: cty.StringVal("foo"),
+					},
+					"image_id": &Variable{
+						DefaultValue: cty.StringVal("image-id-default"),
+					},
+					"port": &Variable{
+						DefaultValue: cty.NumberIntVal(42),
+					},
 					"availability_zone_names": &Variable{
+						DefaultValue: cty.ListVal([]cty.Value{
+							cty.StringVal("us-west-1a"),
+						}),
 						Description: fmt.Sprintln("Describing is awesome ;D"),
 					},
 					"super_secret_password": &Variable{
@@ -37,8 +50,12 @@ func TestParse_variables(t *testing.T) {
 					},
 				},
 				LocalVariables: Variables{
-					"owner":        &Variable{},
-					"service_name": &Variable{},
+					"owner": &Variable{
+						DefaultValue: cty.StringVal("Community Team"),
+					},
+					"service_name": &Variable{
+						DefaultValue: cty.StringVal("forum"),
+					},
 				},
 			},
 			false, false,
@@ -97,6 +114,53 @@ func TestParse_variables(t *testing.T) {
 			[]packer.Build{},
 			false,
 		},
+		{"locals within another locals usage in different files",
+			defaultParser,
+			parseTestArgs{"testdata/variables/complicated", nil},
+			&PackerConfig{
+				Basedir: "testdata/variables/complicated",
+				InputVariables: Variables{
+					"name_prefix": &Variable{
+						DefaultValue: cty.StringVal("foo"),
+					},
+				},
+				LocalVariables: Variables{
+					"name_prefix": &Variable{
+						DefaultValue: cty.StringVal("foo"),
+					},
+					"foo": &Variable{
+						DefaultValue: cty.StringVal("foo"),
+					},
+					"bar": &Variable{
+						DefaultValue: cty.StringVal("foo"),
+					},
+					"for_var": &Variable{
+						DefaultValue: cty.StringVal("foo"),
+					},
+					"bar_var": &Variable{
+						DefaultValue: cty.TupleVal([]cty.Value{
+							cty.StringVal("foo"),
+							cty.StringVal("foo"),
+							cty.StringVal("foo"),
+						}),
+					},
+				},
+			},
+			false, false,
+			[]packer.Build{},
+			false,
+		},
+		{"recursive locals",
+			defaultParser,
+			parseTestArgs{"testdata/variables/recursive_locals.pkr.hcl", nil},
+			&PackerConfig{
+				Basedir:        filepath.Join("testdata", "variables"),
+				LocalVariables: Variables{},
+			},
+			true, true,
+			[]packer.Build{},
+			false,
+		},
 	}
 	testParse(t, tests)
 }