Merge pull request #8882 from hashicorp/fix-var-file-hcl

allow to use hcl files as var files in HCL mode

command/build.go

@@ -104,7 +104,7 @@ func (c *BuildCommand) GetBuildsFromHCL(path string) ([]packer.Build, int) {
 		PostProcessorsSchemas: c.CoreConfig.Components.PostProcessorStore,
 	}
 
-	builds, diags := parser.Parse(path, c.flagVars)
+	builds, diags := parser.Parse(path, c.varFiles, c.flagVars)
 	{
 		// write HCL errors/diagnostics if any.
 		b := bytes.NewBuffer(nil)

command/meta.go

@@ -32,6 +32,7 @@ type Meta struct {
 	Version    string
 
 	// These are set by command-line flags
+	varFiles []string
 	flagVars map[string]string
 }
 
@@ -41,6 +42,17 @@ func (m *Meta) Core(tpl *template.Template) (*packer.Core, error) {
 	// Copy the config so we don't modify it
 	config := *m.CoreConfig
 	config.Template = tpl
+
+	fj := &kvflag.FlagJSON{}
+	for _, file := range m.varFiles {
+		err := fj.Set(file)
+		if err != nil {
+			return nil, err
+		}
+	}
+	for k, v := range *fj {
+		m.flagVars[k] = v
+	}
 	config.Variables = m.flagVars
 
 	// Init the core
@@ -117,7 +129,7 @@ func (m *Meta) FlagSet(n string, fs FlagSetFlags) *flag.FlagSet {
 	// FlagSetVars tells us what variables to use
 	if fs&FlagSetVars != 0 {
 		f.Var((*kvflag.Flag)(&m.flagVars), "var", "")
-		f.Var((*kvflag.FlagJSON)(&m.flagVars), "var-file", "")
+		f.Var((*kvflag.StringSlice)(&m.varFiles), "var-file", "")
 	}
 
 	// Create an io.Writer that writes to our Ui properly for errors.

hcl2template/common_test.go

@@ -37,6 +37,7 @@ func getBasicParser() *Parser {
 type parseTestArgs struct {
 	filename string
 	vars     map[string]string
+	varFiles []string
 }
 
 type parseTest struct {
@@ -58,7 +59,7 @@ func testParse(t *testing.T, tests []parseTest) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			gotCfg, gotDiags := tt.parser.parse(tt.args.filename, tt.args.vars)
+			gotCfg, gotDiags := tt.parser.parse(tt.args.filename, tt.args.varFiles, tt.args.vars)
 			if tt.parseWantDiags == (gotDiags == nil) {
 				t.Fatalf("Parser.parse() unexpected %q diagnostics.", gotDiags)
 			}
@@ -87,8 +88,11 @@ func testParse(t *testing.T, tests []parseTest) {
 				gotInputVar := gotCfg.InputVariables
 				for name, value := range tt.parseWantCfg.InputVariables {
 					if variable, ok := gotInputVar[name]; ok {
-						if variable.DefaultValue.GoString() != value.DefaultValue.GoString() {
+						if diff := cmp.Diff(variable.DefaultValue.GoString(), value.DefaultValue.GoString()); diff != "" {
-							t.Fatalf("Parser.parse() input variable %s expected '%s' but was '%s'", name, value.DefaultValue.GoString(), variable.DefaultValue.GoString())
+							t.Fatalf("Parser.parse(): unexpected default value for %s: %s", name, diff)
+						}
+						if diff := cmp.Diff(variable.VarfileValue.GoString(), value.VarfileValue.GoString()); diff != "" {
+							t.Fatalf("Parser.parse(): varfile value differs for %s: %s", name, diff)
 						}
 					} else {
 						t.Fatalf("Parser.parse() missing input variable. %s", name)

hcl2template/parser.go

@@ -52,7 +52,7 @@ const (
 	hcl2VarJsonFileExt = ".auto.pkrvars.json"
 )
 
-func (p *Parser) parse(filename string, vars map[string]string) (*PackerConfig, hcl.Diagnostics) {
+func (p *Parser) parse(filename string, varFiles []string, argVars map[string]string) (*PackerConfig, hcl.Diagnostics) {
 
 	var files []*hcl.File
 	var diags hcl.Diagnostics
@@ -60,6 +60,7 @@ func (p *Parser) parse(filename string, vars map[string]string) (*PackerConfig,
 	// parse config files
 	{
 		hclFiles, jsonFiles, moreDiags := GetHCL2Files(filename, hcl2FileExt, hcl2JsonFileExt)
+		diags = append(diags, moreDiags...)
 		if len(hclFiles)+len(jsonFiles) == 0 {
 			diags = append(moreDiags, &hcl.Diagnostic{
 				Severity: hcl.DiagError,
@@ -111,6 +112,20 @@ func (p *Parser) parse(filename string, vars map[string]string) (*PackerConfig,
 	{
 		hclVarFiles, jsonVarFiles, moreDiags := GetHCL2Files(filename, hcl2VarFileExt, hcl2VarJsonFileExt)
 		diags = append(diags, moreDiags...)
+		for _, file := range varFiles {
+			switch filepath.Ext(file) {
+			case ".hcl":
+				hclVarFiles = append(hclVarFiles, file)
+			case ".json":
+				jsonVarFiles = append(jsonVarFiles, file)
+			default:
+				diags = append(moreDiags, &hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Could not guess format of " + file,
+					Detail:   "A var file must be suffixed with `.hcl` or `.json`.",
+				})
+			}
+		}
 		var varFiles []*hcl.File
 		for _, filename := range hclVarFiles {
 			f, moreDiags := p.ParseHCLFile(filename)
@@ -123,7 +138,7 @@ func (p *Parser) parse(filename string, vars map[string]string) (*PackerConfig,
 			varFiles = append(varFiles, f)
 		}
 
-		diags = append(diags, cfg.collectInputVariableValues(os.Environ(), varFiles, vars)...)
+		diags = append(diags, cfg.collectInputVariableValues(os.Environ(), varFiles, argVars)...)
 	}
 
 	_, moreDiags := cfg.InputVariables.Values()

hcl2template/testdata/variables/foo-string.variable.pkr.hcl

@@ -0,0 +1,4 @@
+variable "foo" {
+    type = string
+    default = "bar"
+}

hcl2template/testdata/variables/set-foo-too-wee.hcl

@@ -0,0 +1 @@
+foo = "wee"

hcl2template/types.build_test.go

@@ -13,7 +13,7 @@ func TestParse_build(t *testing.T) {
 	tests := []parseTest{
 		{"basic build no src",
 			defaultParser,
-			parseTestArgs{"testdata/build/basic.pkr.hcl", nil},
+			parseTestArgs{"testdata/build/basic.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "build"),
 				Builds: Builds{
@@ -47,7 +47,7 @@ func TestParse_build(t *testing.T) {
 		},
 		{"untyped provisioner",
 			defaultParser,
-			parseTestArgs{"testdata/build/provisioner_untyped.pkr.hcl", nil},
+			parseTestArgs{"testdata/build/provisioner_untyped.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "build"),
 				Builds:  nil,
@@ -58,7 +58,7 @@ func TestParse_build(t *testing.T) {
 		},
 		{"inexistent provisioner",
 			defaultParser,
-			parseTestArgs{"testdata/build/provisioner_inexistent.pkr.hcl", nil},
+			parseTestArgs{"testdata/build/provisioner_inexistent.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "build"),
 				Builds:  nil,
@@ -69,7 +69,7 @@ func TestParse_build(t *testing.T) {
 		},
 		{"untyped post-processor",
 			defaultParser,
-			parseTestArgs{"testdata/build/post-processor_untyped.pkr.hcl", nil},
+			parseTestArgs{"testdata/build/post-processor_untyped.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "build"),
 				Builds:  nil,
@@ -80,7 +80,7 @@ func TestParse_build(t *testing.T) {
 		},
 		{"inexistent post-processor",
 			defaultParser,
-			parseTestArgs{"testdata/build/post-processor_inexistent.pkr.hcl", nil},
+			parseTestArgs{"testdata/build/post-processor_inexistent.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "build"),
 				Builds:  nil,
@@ -91,7 +91,7 @@ func TestParse_build(t *testing.T) {
 		},
 		{"invalid source",
 			defaultParser,
-			parseTestArgs{"testdata/build/invalid_source_reference.pkr.hcl", nil},
+			parseTestArgs{"testdata/build/invalid_source_reference.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "build"),
 				Builds:  nil,

hcl2template/types.packer_config.go

@@ -287,8 +287,8 @@ func (p *Parser) getBuilds(cfg *PackerConfig) ([]packer.Build, hcl.Diagnostics)
 //
 // Parse then return a slice of packer.Builds; which are what packer core uses
 // to run builds.
-func (p *Parser) Parse(path string, vars map[string]string) ([]packer.Build, hcl.Diagnostics) {
+func (p *Parser) Parse(path string, varFiles []string, argVars map[string]string) ([]packer.Build, hcl.Diagnostics) {
-	cfg, diags := p.parse(path, vars)
+	cfg, diags := p.parse(path, varFiles, argVars)
 	if diags.HasErrors() {
 		return nil, diags
 	}

hcl2template/types.packer_config_test.go

@@ -18,7 +18,7 @@ func TestParser_complete(t *testing.T) {
 	tests := []parseTest{
 		{"working build",
 			defaultParser,
-			parseTestArgs{"testdata/complete", nil},
+			parseTestArgs{"testdata/complete", nil, nil},
 			&PackerConfig{
 				Basedir: "testdata/complete",
 				InputVariables: Variables{
@@ -128,7 +128,7 @@ func TestParser_complete(t *testing.T) {
 		},
 		{"dir with no config files",
 			defaultParser,
-			parseTestArgs{"testdata/empty", nil},
+			parseTestArgs{"testdata/empty", nil, nil},
 			nil,
 			true, true,
 			nil,
@@ -136,14 +136,14 @@ func TestParser_complete(t *testing.T) {
 		},
 		{name: "inexistent dir",
 			parser:                 defaultParser,
-			args:                   parseTestArgs{"testdata/inexistent", nil},
+			args:                   parseTestArgs{"testdata/inexistent", nil, nil},
 			parseWantCfg:           nil,
 			parseWantDiags:         true,
 			parseWantDiagHasErrors: true,
 		},
 		{name: "folder named build.pkr.hcl with an unknown src",
 			parser: defaultParser,
-			args:   parseTestArgs{"testdata/build.pkr.hcl", nil},
+			args:   parseTestArgs{"testdata/build.pkr.hcl", nil, nil},
 			parseWantCfg: &PackerConfig{
 				Basedir: "testdata/build.pkr.hcl",
 				Builds: Builds{
@@ -166,7 +166,7 @@ func TestParser_complete(t *testing.T) {
 		},
 		{name: "unknown block type",
 			parser: defaultParser,
-			args:   parseTestArgs{"testdata/unknown", nil},
+			args:   parseTestArgs{"testdata/unknown", nil, nil},
 			parseWantCfg: &PackerConfig{
 				Basedir: "testdata/unknown",
 			},

hcl2template/types.source_test.go

@@ -13,7 +13,7 @@ func TestParse_source(t *testing.T) {
 	tests := []parseTest{
 		{"two basic sources",
 			defaultParser,
-			parseTestArgs{"testdata/sources/basic.pkr.hcl", nil},
+			parseTestArgs{"testdata/sources/basic.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "sources"),
 				Sources: map[SourceRef]*SourceBlock{
@@ -32,7 +32,7 @@ func TestParse_source(t *testing.T) {
 		},
 		{"untyped source",
 			defaultParser,
-			parseTestArgs{"testdata/sources/untyped.pkr.hcl", nil},
+			parseTestArgs{"testdata/sources/untyped.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "sources"),
 			},
@@ -42,7 +42,7 @@ func TestParse_source(t *testing.T) {
 		},
 		{"unnamed source",
 			defaultParser,
-			parseTestArgs{"testdata/sources/unnamed.pkr.hcl", nil},
+			parseTestArgs{"testdata/sources/unnamed.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "sources"),
 			},
@@ -52,7 +52,7 @@ func TestParse_source(t *testing.T) {
 		},
 		{"inexistent source",
 			defaultParser,
-			parseTestArgs{"testdata/sources/inexistent.pkr.hcl", nil},
+			parseTestArgs{"testdata/sources/inexistent.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "sources"),
 			},
@@ -62,7 +62,7 @@ func TestParse_source(t *testing.T) {
 		},
 		{"duplicate source",
 			defaultParser,
-			parseTestArgs{"testdata/sources/duplicate.pkr.hcl", nil},
+			parseTestArgs{"testdata/sources/duplicate.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "sources"),
 				Sources: map[SourceRef]*SourceBlock{

hcl2template/types.variables_test.go

@@ -20,7 +20,7 @@ func TestParse_variables(t *testing.T) {
 	tests := []parseTest{
 		{"basic variables",
 			defaultParser,
-			parseTestArgs{"testdata/variables/basic.pkr.hcl", nil},
+			parseTestArgs{"testdata/variables/basic.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "variables"),
 				InputVariables: Variables{
@@ -75,7 +75,7 @@ func TestParse_variables(t *testing.T) {
 		},
 		{"duplicate variable",
 			defaultParser,
-			parseTestArgs{"testdata/variables/duplicate_variable.pkr.hcl", nil},
+			parseTestArgs{"testdata/variables/duplicate_variable.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "variables"),
 				InputVariables: Variables{
@@ -90,7 +90,7 @@ func TestParse_variables(t *testing.T) {
 		},
 		{"duplicate variable in variables",
 			defaultParser,
-			parseTestArgs{"testdata/variables/duplicate_variables.pkr.hcl", nil},
+			parseTestArgs{"testdata/variables/duplicate_variables.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "variables"),
 				InputVariables: Variables{
@@ -105,7 +105,7 @@ func TestParse_variables(t *testing.T) {
 		},
 		{"invalid default type",
 			defaultParser,
-			parseTestArgs{"testdata/variables/invalid_default.pkr.hcl", nil},
+			parseTestArgs{"testdata/variables/invalid_default.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "variables"),
 				InputVariables: Variables{
@@ -121,7 +121,7 @@ func TestParse_variables(t *testing.T) {
 
 		{"unknown key",
 			defaultParser,
-			parseTestArgs{"testdata/variables/unknown_key.pkr.hcl", nil},
+			parseTestArgs{"testdata/variables/unknown_key.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "variables"),
 				InputVariables: Variables{
@@ -138,7 +138,7 @@ func TestParse_variables(t *testing.T) {
 
 		{"unset used variable",
 			defaultParser,
-			parseTestArgs{"testdata/variables/unset_used_string_variable.pkr.hcl", nil},
+			parseTestArgs{"testdata/variables/unset_used_string_variable.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "variables"),
 				InputVariables: Variables{
@@ -154,7 +154,7 @@ func TestParse_variables(t *testing.T) {
 
 		{"unset unused variable",
 			defaultParser,
-			parseTestArgs{"testdata/variables/unset_unused_string_variable.pkr.hcl", nil},
+			parseTestArgs{"testdata/variables/unset_unused_string_variable.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir: filepath.Join("testdata", "variables"),
 				InputVariables: Variables{
@@ -189,7 +189,7 @@ func TestParse_variables(t *testing.T) {
 
 		{"locals within another locals usage in different files",
 			defaultParser,
-			parseTestArgs{"testdata/variables/complicated", nil},
+			parseTestArgs{"testdata/variables/complicated", nil, nil},
 			&PackerConfig{
 				Basedir: "testdata/variables/complicated",
 				InputVariables: Variables{
@@ -231,7 +231,7 @@ func TestParse_variables(t *testing.T) {
 		},
 		{"recursive locals",
 			defaultParser,
-			parseTestArgs{"testdata/variables/recursive_locals.pkr.hcl", nil},
+			parseTestArgs{"testdata/variables/recursive_locals.pkr.hcl", nil, nil},
 			&PackerConfig{
 				Basedir:        filepath.Join("testdata", "variables"),
 				LocalVariables: Variables{},
@@ -240,6 +240,35 @@ func TestParse_variables(t *testing.T) {
 			[]packer.Build{},
 			false,
 		},
+
+		{"set variable from var-file",
+			defaultParser,
+			parseTestArgs{"testdata/variables/foo-string.variable.pkr.hcl", nil, []string{"testdata/variables/set-foo-too-wee.hcl"}},
+			&PackerConfig{
+				Basedir: filepath.Join("testdata", "variables"),
+				InputVariables: Variables{
+					"foo": &Variable{
+						DefaultValue: cty.StringVal("bar"),
+						Name:         "foo",
+						VarfileValue: cty.StringVal("wee"),
+					},
+				},
+			},
+			false, false,
+			[]packer.Build{},
+			false,
+		},
+
+		{"unknown variable from var-file",
+			defaultParser,
+			parseTestArgs{"testdata/variables/empty.pkr.hcl", nil, []string{"testdata/variables/set-foo-too-wee.hcl"}},
+			&PackerConfig{
+				Basedir: filepath.Join("testdata", "variables"),
+			},
+			true, false,
+			[]packer.Build{},
+			false,
+		},
 	}
 	testParse(t, tests)
 }

helper/flag-kv/flag_strings.go

@@ -0,0 +1,16 @@
+package kvflag
+
+import (
+	"strings"
+)
+
+type StringSlice []string
+
+func (s *StringSlice) String() string {
+	return strings.Join(*s, ", ")
+}
+
+func (s *StringSlice) Set(value string) error {
+	*s = append(*s, value)
+	return nil
+}

helper/flag-kv/flag_strings_test.go

@@ -0,0 +1,35 @@
+package kvflag
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestStringSlice_Set(t *testing.T) {
+	type args struct {
+		values []string
+	}
+	tests := []struct {
+		name            string
+		s               StringSlice
+		args            args
+		wantStringSlice StringSlice
+	}{
+		{"basic", StringSlice{"hey", "yo"}, args{[]string{"how", "are", "you"}},
+			StringSlice{"hey", "yo", "how", "are", "you"}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			for _, value := range tt.args.values {
+				err := tt.s.Set(value)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+			if diff := cmp.Diff(tt.s, tt.wantStringSlice); diff != "" {
+				t.Fatal(diff)
+			}
+		})
+	}
+}