From e8e58f4c04f5505c5b3b8ca2712423e01f594073 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 13 Jan 2020 15:22:41 +0100 Subject: [PATCH] allow to set pkr variables from env, from local files and from arguments --- command/build.go | 2 +- hcl2template/common_test.go | 3 +- hcl2template/parser.go | 89 ++++++++--- hcl2template/types.build_test.go | 12 +- hcl2template/types.packer_config.go | 4 +- hcl2template/types.packer_config_test.go | 10 +- hcl2template/types.source_test.go | 10 +- hcl2template/types.variables.go | 191 +++++++++++++++++++++-- hcl2template/types.variables_test.go | 7 +- hcl2template/utils.go | 19 +-- 10 files changed, 274 insertions(+), 73 deletions(-) diff --git a/command/build.go b/command/build.go index 74b21b17f3b..0339909efe3 100644 --- a/command/build.go +++ b/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) + builds, diags := parser.Parse(path, c.flagVars) { // write HCL errors/diagnostics if any. b := bytes.NewBuffer(nil) diff --git a/hcl2template/common_test.go b/hcl2template/common_test.go index 87ca16d3db0..c65bb5b9da3 100644 --- a/hcl2template/common_test.go +++ b/hcl2template/common_test.go @@ -33,6 +33,7 @@ func getBasicParser() *Parser { type parseTestArgs struct { filename string + vars map[string]string } type parseTest struct { @@ -54,7 +55,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) + gotCfg, gotDiags := tt.parser.parse(tt.args.filename, tt.args.vars) if tt.parseWantDiags == (gotDiags == nil) { t.Fatalf("Parser.parse() unexpected diagnostics. %s", gotDiags) } diff --git a/hcl2template/parser.go b/hcl2template/parser.go index ccc12f6a011..5237d56b702 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -2,6 +2,7 @@ package hcl2template import ( "fmt" + "os" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclparse" @@ -39,44 +40,82 @@ type Parser struct { } const ( - hcl2FileExt = ".pkr.hcl" - hcl2JsonFileExt = ".pkr.json" + hcl2FileExt = ".pkr.hcl" + hcl2JsonFileExt = ".pkr.json" + hcl2VarFileExt = ".auto.pkrvars.hcl" + hcl2VarJsonFileExt = ".auto.pkrvars.json" ) -func (p *Parser) parse(filename string) (*PackerConfig, hcl.Diagnostics) { - - hclFiles, jsonFiles, diags := GetHCL2Files(filename) +func (p *Parser) parse(filename string, vars map[string]string) (*PackerConfig, hcl.Diagnostics) { var files []*hcl.File - for _, filename := range hclFiles { - f, moreDiags := p.ParseHCLFile(filename) - diags = append(diags, moreDiags...) - files = append(files, f) - } - for _, filename := range jsonFiles { - f, moreDiags := p.ParseJSONFile(filename) - diags = append(diags, moreDiags...) - files = append(files, f) - } - if diags.HasErrors() { - return nil, diags + var diags hcl.Diagnostics + + // parse config files + { + hclFiles, jsonFiles, moreDiags := GetHCL2Files(filename, hcl2FileExt, hcl2JsonFileExt) + if len(hclFiles)+len(jsonFiles) == 0 { + diags = append(moreDiags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Could not find any config file in " + filename, + Detail: "A config file must be suffixed with `.pkr.hcl` or " + + "`.pkr.json`. A folder can be referenced.", + }) + } + for _, filename := range hclFiles { + f, moreDiags := p.ParseHCLFile(filename) + diags = append(diags, moreDiags...) + files = append(files, f) + } + for _, filename := range jsonFiles { + f, moreDiags := p.ParseJSONFile(filename) + diags = append(diags, moreDiags...) + files = append(files, f) + } + if diags.HasErrors() { + return nil, diags + } } + // decode variable blocks cfg := &PackerConfig{} - for _, file := range files { - diags = append(diags, p.parseInputVariables(file, cfg)...) + { + for _, file := range files { + diags = append(diags, p.decodeInputVariables(file, cfg)...) + } + for _, file := range files { + diags = append(diags, p.decodeLocalVariables(file, cfg)...) + } } - for _, file := range files { - diags = append(diags, p.parseLocalVariables(file, cfg)...) + + // parse var files + { + hclVarFiles, jsonVarFiles, moreDiags := GetHCL2Files(filename, hcl2VarFileExt, hcl2VarJsonFileExt) + diags = append(diags, moreDiags...) + var varFiles []*hcl.File + for _, filename := range hclVarFiles { + f, moreDiags := p.ParseHCLFile(filename) + diags = append(diags, moreDiags...) + varFiles = append(varFiles, f) + } + for _, filename := range jsonVarFiles { + f, moreDiags := p.ParseJSONFile(filename) + diags = append(diags, moreDiags...) + varFiles = append(varFiles, f) + } + + diags = append(diags, cfg.InputVariables.collectVariableValues(os.Environ(), varFiles, vars)...) } + + // decode the actual content for _, file := range files { - diags = append(diags, p.parseConfig(file, cfg)...) + diags = append(diags, p.decodeConfig(file, cfg)...) } return cfg, diags } -func (p *Parser) parseInputVariables(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics { +func (p *Parser) decodeInputVariables(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics { var diags hcl.Diagnostics content, moreDiags := f.Body.Content(configSchema) @@ -96,7 +135,7 @@ func (p *Parser) parseInputVariables(f *hcl.File, cfg *PackerConfig) hcl.Diagnos return diags } -func (p *Parser) parseLocalVariables(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics { +func (p *Parser) decodeLocalVariables(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics { var diags hcl.Diagnostics content, moreDiags := f.Body.Content(configSchema) @@ -113,7 +152,7 @@ func (p *Parser) parseLocalVariables(f *hcl.File, cfg *PackerConfig) hcl.Diagnos return diags } -func (p *Parser) parseConfig(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics { +func (p *Parser) decodeConfig(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics { var diags hcl.Diagnostics content, moreDiags := f.Body.Content(configSchema) diff --git a/hcl2template/types.build_test.go b/hcl2template/types.build_test.go index 2a346c84c4d..d9ecf3f95fd 100644 --- a/hcl2template/types.build_test.go +++ b/hcl2template/types.build_test.go @@ -12,7 +12,7 @@ func TestParse_build(t *testing.T) { tests := []parseTest{ {"basic build no src", defaultParser, - parseTestArgs{"testdata/build/basic.pkr.hcl"}, + parseTestArgs{"testdata/build/basic.pkr.hcl", nil}, &PackerConfig{ Builds: Builds{ &BuildBlock{ @@ -45,7 +45,7 @@ func TestParse_build(t *testing.T) { }, {"untyped provisioner", defaultParser, - parseTestArgs{"testdata/build/provisioner_untyped.pkr.hcl"}, + parseTestArgs{"testdata/build/provisioner_untyped.pkr.hcl", nil}, &PackerConfig{ Builds: nil, }, @@ -55,7 +55,7 @@ func TestParse_build(t *testing.T) { }, {"inexistent provisioner", defaultParser, - parseTestArgs{"testdata/build/provisioner_inexistent.pkr.hcl"}, + parseTestArgs{"testdata/build/provisioner_inexistent.pkr.hcl", nil}, &PackerConfig{ Builds: nil, }, @@ -65,7 +65,7 @@ func TestParse_build(t *testing.T) { }, {"untyped post-processor", defaultParser, - parseTestArgs{"testdata/build/post-processor_untyped.pkr.hcl"}, + parseTestArgs{"testdata/build/post-processor_untyped.pkr.hcl", nil}, &PackerConfig{ Builds: nil, }, @@ -75,7 +75,7 @@ func TestParse_build(t *testing.T) { }, {"inexistent post-processor", defaultParser, - parseTestArgs{"testdata/build/post-processor_inexistent.pkr.hcl"}, + parseTestArgs{"testdata/build/post-processor_inexistent.pkr.hcl", nil}, &PackerConfig{ Builds: nil, }, @@ -85,7 +85,7 @@ func TestParse_build(t *testing.T) { }, {"invalid source", defaultParser, - parseTestArgs{"testdata/build/invalid_source_reference.pkr.hcl"}, + parseTestArgs{"testdata/build/invalid_source_reference.pkr.hcl", nil}, &PackerConfig{ Builds: nil, }, diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 49cbe53de22..db69d1363ca 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -115,8 +115,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) ([]packer.Build, hcl.Diagnostics) { - cfg, diags := p.parse(path) +func (p *Parser) Parse(path string, vars map[string]string) ([]packer.Build, hcl.Diagnostics) { + cfg, diags := p.parse(path, vars) if diags.HasErrors() { return nil, diags } diff --git a/hcl2template/types.packer_config_test.go b/hcl2template/types.packer_config_test.go index c763406fe14..31cb01db3db 100644 --- a/hcl2template/types.packer_config_test.go +++ b/hcl2template/types.packer_config_test.go @@ -17,7 +17,7 @@ func TestParser_complete(t *testing.T) { tests := []parseTest{ {"working build", defaultParser, - parseTestArgs{"testdata/complete"}, + parseTestArgs{"testdata/complete", nil}, &PackerConfig{ InputVariables: Variables{ "foo": Variable{}, @@ -61,7 +61,7 @@ func TestParser_complete(t *testing.T) { }, {"dir with no config files", defaultParser, - parseTestArgs{"testdata/empty"}, + parseTestArgs{"testdata/empty", nil}, nil, true, true, nil, @@ -69,14 +69,14 @@ func TestParser_complete(t *testing.T) { }, {name: "inexistent dir", parser: defaultParser, - args: parseTestArgs{"testdata/inexistent"}, + args: parseTestArgs{"testdata/inexistent", 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"}, + args: parseTestArgs{"testdata/build.pkr.hcl", nil}, parseWantCfg: &PackerConfig{ Builds: Builds{ &BuildBlock{ @@ -98,7 +98,7 @@ func TestParser_complete(t *testing.T) { }, {name: "unknown block type", parser: defaultParser, - args: parseTestArgs{"testdata/unknown"}, + args: parseTestArgs{"testdata/unknown", nil}, parseWantCfg: &PackerConfig{}, parseWantDiags: true, parseWantDiagHasErrors: true, diff --git a/hcl2template/types.source_test.go b/hcl2template/types.source_test.go index 66d38860a5d..98da4c42d1e 100644 --- a/hcl2template/types.source_test.go +++ b/hcl2template/types.source_test.go @@ -12,7 +12,7 @@ func TestParse_source(t *testing.T) { tests := []parseTest{ {"two basic sources", defaultParser, - parseTestArgs{"testdata/sources/basic.pkr.hcl"}, + parseTestArgs{"testdata/sources/basic.pkr.hcl", nil}, &PackerConfig{ Sources: map[SourceRef]*Source{ SourceRef{ @@ -30,7 +30,7 @@ func TestParse_source(t *testing.T) { }, {"untyped source", defaultParser, - parseTestArgs{"testdata/sources/untyped.pkr.hcl"}, + parseTestArgs{"testdata/sources/untyped.pkr.hcl", nil}, &PackerConfig{}, true, true, nil, @@ -38,7 +38,7 @@ func TestParse_source(t *testing.T) { }, {"unnamed source", defaultParser, - parseTestArgs{"testdata/sources/unnamed.pkr.hcl"}, + parseTestArgs{"testdata/sources/unnamed.pkr.hcl", nil}, &PackerConfig{}, true, true, nil, @@ -46,7 +46,7 @@ func TestParse_source(t *testing.T) { }, {"inexistent source", defaultParser, - parseTestArgs{"testdata/sources/inexistent.pkr.hcl"}, + parseTestArgs{"testdata/sources/inexistent.pkr.hcl", nil}, &PackerConfig{}, true, true, nil, @@ -54,7 +54,7 @@ func TestParse_source(t *testing.T) { }, {"duplicate source", defaultParser, - parseTestArgs{"testdata/sources/duplicate.pkr.hcl"}, + parseTestArgs{"testdata/sources/duplicate.pkr.hcl", nil}, &PackerConfig{ Sources: map[SourceRef]*Source{ SourceRef{ diff --git a/hcl2template/types.variables.go b/hcl2template/types.variables.go index 33c14bb7e84..fbb2da95fdd 100644 --- a/hcl2template/types.variables.go +++ b/hcl2template/types.variables.go @@ -1,31 +1,63 @@ package hcl2template import ( + "fmt" + "strings" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/ext/typeexpr" "github.com/hashicorp/hcl/v2/gohcl" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" ) type Variable struct { - Default cty.Value - Type cty.Type + // CmdValue, VarfileValue, EnvValue, DefaultValue are possible values of + // the variable; The first value set from these will be the one used. If + // none is set; an error will be returned if a user tries to use the + // Variable. + CmdValue cty.Value + VarfileValue cty.Value + EnvValue cty.Value + DefaultValue cty.Value + + // Cty Type of the variable + Type cty.Type + // Description of the variable Description string - Sensible bool + // When Sensible is set to true Packer will try it best to hide/obfuscate + // the variable from the output stream. By replacing the text. + Sensible bool block *hcl.Block } -func (v *Variable) Value() cty.Value { - return v.Default +func (v *Variable) Value() (cty.Value, *hcl.Diagnostic) { + for _, value := range []cty.Value{ + v.CmdValue, + v.VarfileValue, + v.EnvValue, + v.DefaultValue, + } { + if !value.IsNull() { + return value, nil + } + } + return cty.NilVal, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unset variable", + Detail: "A used variable must be set; see " + + "https://packer.io/docs/configuration/from-1.5/syntax.html for details.", + Context: v.block.DefRange.Ptr(), + } } -type Variables map[string]Variable +type Variables map[string]*Variable func (variables Variables) Values() map[string]cty.Value { res := map[string]cty.Value{} for k, v := range variables { - res[k] = v.Value() + res[k], _ = v.Value() } return res } @@ -57,9 +89,9 @@ func (variables *Variables) decodeConfigMap(block *hcl.Block, ectx *hcl.EvalCont if moreDiags.HasErrors() { continue } - (*variables)[key] = Variable{ - Default: value, - Type: value.Type(), + (*variables)[key] = &Variable{ + DefaultValue: value, + Type: value.Type(), } } @@ -83,7 +115,7 @@ func (variables *Variables) decodeConfig(block *hcl.Block, ectx *hcl.EvalContext return diags } - res := Variable{ + res := &Variable{ Description: b.Description, Sensible: b.Sensible, block: block, @@ -98,8 +130,9 @@ func (variables *Variables) decodeConfig(block *hcl.Block, ectx *hcl.EvalContext if moreDiags.HasErrors() { return diags } - res.Default = defaultValue + res.DefaultValue = defaultValue res.Type = defaultValue.Type() + delete(attrs, "default") } if t, ok := attrs["type"]; ok { tp, moreDiags := typeexpr.Type(t.Expr) @@ -109,9 +142,143 @@ func (variables *Variables) decodeConfig(block *hcl.Block, ectx *hcl.EvalContext } res.Type = tp + delete(attrs, "type") + } + if len(attrs) > 0 { + keys := []string{} + for k := range attrs { + keys = append(keys, k) + } + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Unknown keys", + Detail: fmt.Sprintf("unknown variable keys: %s", keys), + Context: block.DefRange.Ptr(), + }) } (*variables)[block.Labels[0]] = res return diags } + +const VarEnvPrefix = "PKR_VAR_" + +func (variables Variables) collectVariableValues(env []string, files []*hcl.File, argv map[string]string) hcl.Diagnostics { + var diags hcl.Diagnostics + + for _, raw := range env { + if !strings.HasPrefix(raw, VarEnvPrefix) { + continue + } + raw = raw[len(VarEnvPrefix):] // trim the prefix + + eq := strings.Index(raw, "=") + if eq == -1 { + // Seems invalid, so we'll ignore it. + continue + } + + name := raw[:eq] + rawVal := raw[eq+1:] + + // this variable was not defined in the hcl files, let's skip it ! + v, found := variables[name] + if !found { + continue + } + + value := cty.StringVal(rawVal) + + v.EnvValue = value + } + + // files will contain files found in the folder then files passed as + // arguments. + for _, file := range files { + // Before we do our real decode, we'll probe to see if there are any blocks + // of type "variable" in this body, since it's a common mistake for new + // users to put variable declarations in tfvars rather than variable value + // definitions, and otherwise our error message for that case is not so + // helpful. + { + content, _, _ := file.Body.PartialContent(&hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{ + { + Type: "variable", + LabelNames: []string{"name"}, + }, + }, + }) + for _, block := range content.Blocks { + name := block.Labels[0] + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Variable declaration in a .pkrvar file", + Detail: fmt.Sprintf("A .pkrvar file is used to assign "+ + "values to variables that have already been declared "+ + "in .pkr files, not to declare new variables. To "+ + "declare variable %q, place this block in one of your"+ + " .pkr files,such as variables.pkr.hcl\n\nTo set a "+ + "value for this variable in %s, use the definition "+ + "syntax instead:\n %s = ", + name, block.TypeRange.Filename, name), + Subject: &block.TypeRange, + }) + } + if diags.HasErrors() { + // If we already found problems then JustAttributes below will find + // the same problems with less-helpful messages, so we'll bail for + // now to let the user focus on the immediate problem. + return diags + } + } + + attrs, moreDiags := file.Body.JustAttributes() + diags = append(diags, moreDiags...) + + for name, attr := range attrs { + variable, found := variables[name] + if !found { + // No file defines this variable; let's skip it + continue + } + + val, moreDiags := attr.Expr.Value(nil) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + continue + } + variable.VarfileValue = val + } + } + + // Finally we process values given explicitly on the command line. + for name, value := range argv { + variable, found := variables[name] + if !found { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Unknown -var variable", + Detail: fmt.Sprintf("A %q variable was passed in the command "+ + "line but was not found in known variables."+ + "To declare variable %q, place this block in one of your"+ + " .pkr files,such as variables.pkr.hcl", + name, name), + }) + continue + } + + fakeFilename := fmt.Sprintf("", name) + expr, moreDiags := hclsyntax.ParseExpression([]byte(value), fakeFilename, hcl.Pos{Line: 1, Column: 1}) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + continue + } + val, valDiags := expr.Value(nil) + diags = append(diags, valDiags...) + variable.CmdValue = val + } + + return diags +} diff --git a/hcl2template/types.variables_test.go b/hcl2template/types.variables_test.go index 0f8b347a0f4..f09e528380f 100644 --- a/hcl2template/types.variables_test.go +++ b/hcl2template/types.variables_test.go @@ -1,6 +1,7 @@ package hcl2template import ( + "fmt" "testing" "github.com/hashicorp/packer/packer" @@ -12,7 +13,7 @@ func TestParse_variables(t *testing.T) { tests := []parseTest{ {"basic variables", defaultParser, - parseTestArgs{"testdata/variables/basic.pkr.hcl"}, + parseTestArgs{"testdata/variables/basic.pkr.hcl", nil}, &PackerConfig{ InputVariables: Variables{ "image_name": Variable{}, @@ -21,11 +22,11 @@ func TestParse_variables(t *testing.T) { "image_id": Variable{}, "port": Variable{}, "availability_zone_names": Variable{ - Description: "Describing is awesome ;D\n", + Description: fmt.Sprintln("Describing is awesome ;D"), }, "super_secret_password": Variable{ Sensible: true, - Description: "Handle with care plz\n", + Description: fmt.Sprintln("Handle with care plz"), }, }, LocalVariables: Variables{ diff --git a/hcl2template/utils.go b/hcl2template/utils.go index 52429321b55..ad9a0cf6cee 100644 --- a/hcl2template/utils.go +++ b/hcl2template/utils.go @@ -37,7 +37,7 @@ func isDir(name string) (bool, error) { return s.IsDir(), nil } -func GetHCL2Files(filename string) (hclFiles, jsonFiles []string, diags hcl.Diagnostics) { +func GetHCL2Files(filename, hclSuffix, jsonSuffix string) (hclFiles, jsonFiles []string, diags hcl.Diagnostics) { isDir, err := isDir(filename) if err != nil { diags = append(diags, &hcl.Diagnostic{ @@ -48,12 +48,13 @@ func GetHCL2Files(filename string) (hclFiles, jsonFiles []string, diags hcl.Diag return nil, nil, diags } if !isDir { - if strings.HasSuffix(filename, hcl2JsonFileExt) { + if strings.HasSuffix(filename, jsonSuffix) { return nil, []string{filename}, diags } - if strings.HasSuffix(filename, hcl2FileExt) { + if strings.HasSuffix(filename, hclSuffix) { return []string{filename}, nil, diags } + return nil, nil, diags } fileInfos, err := ioutil.ReadDir(filename) @@ -71,20 +72,12 @@ func GetHCL2Files(filename string) (hclFiles, jsonFiles []string, diags hcl.Diag continue } filename := filepath.Join(filename, fileInfo.Name()) - if strings.HasSuffix(filename, hcl2FileExt) { + if strings.HasSuffix(filename, hclSuffix) { hclFiles = append(hclFiles, filename) - } else if strings.HasSuffix(filename, hcl2JsonFileExt) { + } else if strings.HasSuffix(filename, jsonSuffix) { jsonFiles = append(jsonFiles, filename) } } - if len(hclFiles)+len(jsonFiles) == 0 { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Could not find any config file in " + filename, - Detail: "A config file must be suffixed with `.pkr.hcl` or " + - "`.pkr.json`. A folder can be referenced.", - }) - } return hclFiles, jsonFiles, diags }