diff --git a/hcl2template/parser.go b/hcl2template/parser.go index a1d23f640..72d5c1634 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -190,6 +190,8 @@ func (p *Parser) Parse(filename string, varFiles []string, argVars map[string]st diags = append(diags, morediags...) cfg.LocalBlocks = append(cfg.LocalBlocks, moreLocals...) } + + diags = diags.Extend(cfg.checkForDuplicateLocalDefinition()) } // parse var files @@ -296,7 +298,6 @@ func filterVarsFromLogs(inputOrLocal Variables) { func (cfg *PackerConfig) Initialize(opts packer.InitializeOptions) hcl.Diagnostics { diags := cfg.InputVariables.ValidateValues() diags = append(diags, cfg.evaluateDatasources(opts.SkipDatasourcesExecution)...) - diags = append(diags, checkForDuplicateLocalDefinition(cfg.LocalBlocks)...) diags = append(diags, cfg.evaluateLocalVariables(cfg.LocalBlocks)...) filterVarsFromLogs(cfg.InputVariables) diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 4c69c4467..51608b741 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -280,24 +280,29 @@ func (c *PackerConfig) evaluateLocalVariables(locals []*LocalBlock) hcl.Diagnost return diags } -func checkForDuplicateLocalDefinition(locals []*LocalBlock) hcl.Diagnostics { +// checkForDuplicateLocalDefinition walks through the list of defined variables +// in order to detect duplicate locals definitions. +func (c *PackerConfig) checkForDuplicateLocalDefinition() hcl.Diagnostics { var diags hcl.Diagnostics - // we could sort by name and then check contiguous names to use less memory, - // but using a map sounds good enough. - names := map[string]struct{}{} - for _, local := range locals { - if _, found := names[local.Name]; found { - diags = append(diags, &hcl.Diagnostic{ + localNames := map[string]*LocalBlock{} + + for _, block := range c.LocalBlocks { + loc, ok := localNames[block.Name] + if ok { + diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Duplicate local definition", - Detail: "Duplicate " + local.Name + " definition found.", - Subject: local.Expr.Range().Ptr(), + Detail: fmt.Sprintf("Local variable %q is defined twice in your templates. Other definition found at %q", + block.Name, loc.Expr.Range()), + Subject: block.Expr.Range().Ptr(), }) continue } - names[local.Name] = struct{}{} + + localNames[block.Name] = block } + return diags } diff --git a/packer_test/local_eval_test.go b/packer_test/local_eval_test.go index 97cecf0ef..dec5b1fdb 100644 --- a/packer_test/local_eval_test.go +++ b/packer_test/local_eval_test.go @@ -1,5 +1,7 @@ package packer_test +import "fmt" + func (ts *PackerTestSuite) TestEvalLocalsOrder() { ts.SkipNoAcc() @@ -12,3 +14,18 @@ func (ts *PackerTestSuite) TestEvalLocalsOrder() { SetArgs("console", "./templates/locals_no_order.pkr.hcl"). Assert(MustSucceed(), Grep("\\[\\]", grepStdout, grepInvert)) } + +func (ts *PackerTestSuite) TestLocalDuplicates() { + pluginDir, cleanup := ts.MakePluginDir() + defer cleanup() + + for _, cmd := range []string{"console", "validate", "build"} { + ts.Run(fmt.Sprintf("duplicate local detection with %s command - expect error", cmd), func() { + ts.PackerCommand().UsePluginDir(pluginDir). + SetArgs(cmd, "./templates/locals_duplicate.pkr.hcl"). + Assert(MustFail(), + Grep("Duplicate local definition"), + Grep("Local variable \"test\" is defined twice")) + }) + } +} diff --git a/packer_test/templates/locals_duplicate.pkr.hcl b/packer_test/templates/locals_duplicate.pkr.hcl new file mode 100644 index 000000000..fe91e3b77 --- /dev/null +++ b/packer_test/templates/locals_duplicate.pkr.hcl @@ -0,0 +1,18 @@ +local "test" { + expression = "two" + sensitive = true +} + +locals { + test = local.test +} + +variable "test" { + type = string + default = "home" +} +source "null" "example" {} + +build { + sources = ["source.null.example"] +}