From 9181a435aaf15616bd2e5bc7907d67492d02eba9 Mon Sep 17 00:00:00 2001 From: Sylvia Moss Date: Wed, 5 Aug 2020 17:41:20 +0200 Subject: [PATCH] Make max_retries a string to allow variable interpolation (#9673) --- hcl2template/parser.go | 3 +- .../provisioner_variable_decoding.pkr.hcl | 21 +++++ hcl2template/types.build.go | 4 +- hcl2template/types.build.provisioners.go | 4 +- hcl2template/types.variables_test.go | 92 ++++++++++++++++++- packer/core.go | 16 +++- packer/core_test.go | 21 ++++- packer/test-fixtures/build-prov-retry.json | 14 ++- template/parse.go | 20 +++- template/parse_test.go | 2 +- template/template.go | 2 +- template/template.hcl2spec.go | 4 +- 12 files changed, 179 insertions(+), 24 deletions(-) create mode 100644 hcl2template/testdata/variables/provisioner_variable_decoding.pkr.hcl diff --git a/hcl2template/parser.go b/hcl2template/parser.go index c14dd3a6b..776d6abec 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -61,7 +61,6 @@ const ( // build; sources(builders)/provisioners/posts-processors will not be started // and their contents wont be verified; Most syntax errors will cause an error. func (p *Parser) Parse(filename string, varFiles []string, argVars map[string]string) (*PackerConfig, hcl.Diagnostics) { - var files []*hcl.File var diags hcl.Diagnostics @@ -225,7 +224,7 @@ func (p *Parser) decodeConfig(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics { cfg.Sources[ref] = source case buildLabel: - build, moreDiags := p.decodeBuildConfig(block) + build, moreDiags := p.decodeBuildConfig(block, cfg) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue diff --git a/hcl2template/testdata/variables/provisioner_variable_decoding.pkr.hcl b/hcl2template/testdata/variables/provisioner_variable_decoding.pkr.hcl new file mode 100644 index 000000000..f6a3b07d9 --- /dev/null +++ b/hcl2template/testdata/variables/provisioner_variable_decoding.pkr.hcl @@ -0,0 +1,21 @@ +variables { + max_retries = "1" + max_retries_int = 1 +} + +source "null" "null-builder" { + communicator = "none" +} + +build { + sources = [ + "source.null.null-builder", + ] + + provisioner "shell" { + max_retries = var.max_retries_int + } + provisioner "shell" { + max_retries = var.max_retries + } +} \ No newline at end of file diff --git a/hcl2template/types.build.go b/hcl2template/types.build.go index fa83dc3df..77c78dfac 100644 --- a/hcl2template/types.build.go +++ b/hcl2template/types.build.go @@ -70,7 +70,7 @@ type Builds []*BuildBlock // decodeBuildConfig is called when a 'build' block has been detected. It will // load the references to the contents of the build block. -func (p *Parser) decodeBuildConfig(block *hcl.Block) (*BuildBlock, hcl.Diagnostics) { +func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildBlock, hcl.Diagnostics) { build := &BuildBlock{} var b struct { @@ -123,7 +123,7 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block) (*BuildBlock, hcl.Diagnosti } build.Sources = append(build.Sources, ref) case buildProvisionerLabel: - p, moreDiags := p.decodeProvisioner(block) + p, moreDiags := p.decodeProvisioner(block, cfg) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue diff --git a/hcl2template/types.build.provisioners.go b/hcl2template/types.build.provisioners.go index 519a72e72..0d9d07607 100644 --- a/hcl2template/types.build.provisioners.go +++ b/hcl2template/types.build.provisioners.go @@ -70,7 +70,7 @@ func (p *ProvisionerBlock) String() string { return fmt.Sprintf(buildProvisionerLabel+"-block %q %q", p.PType, p.PName) } -func (p *Parser) decodeProvisioner(block *hcl.Block) (*ProvisionerBlock, hcl.Diagnostics) { +func (p *Parser) decodeProvisioner(block *hcl.Block, cfg *PackerConfig) (*ProvisionerBlock, hcl.Diagnostics) { var b struct { Name string `hcl:"name,optional"` PauseBefore string `hcl:"pause_before,optional"` @@ -80,7 +80,7 @@ func (p *Parser) decodeProvisioner(block *hcl.Block) (*ProvisionerBlock, hcl.Dia Except []string `hcl:"except,optional"` Rest hcl.Body `hcl:",remain"` } - diags := gohcl.DecodeBody(block.Body, nil, &b) + diags := gohcl.DecodeBody(block.Body, cfg.EvalContext(nil), &b) if diags.HasErrors() { return nil, diags } diff --git a/hcl2template/types.variables_test.go b/hcl2template/types.variables_test.go index 15e014a66..b36b6e44f 100644 --- a/hcl2template/types.variables_test.go +++ b/hcl2template/types.variables_test.go @@ -6,12 +6,12 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" - "github.com/hashicorp/hcl/v2" "github.com/hashicorp/packer/builder/null" + . "github.com/hashicorp/packer/hcl2template/internal" "github.com/hashicorp/packer/packer" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" ) func TestParse_variables(t *testing.T) { @@ -271,6 +271,92 @@ func TestParse_variables(t *testing.T) { []packer.Build{}, false, }, + + {"provisioner variable decoding", + defaultParser, + parseTestArgs{"testdata/variables/provisioner_variable_decoding.pkr.hcl", nil, nil}, + &PackerConfig{ + Basedir: filepath.Join("testdata", "variables"), + InputVariables: Variables{ + "max_retries": &Variable{ + Name: "max_retries", + DefaultValue: cty.StringVal("1"), + Type: cty.String, + }, + "max_retries_int": &Variable{ + Name: "max_retries_int", + DefaultValue: cty.NumberIntVal(1), + Type: cty.Number, + }, + }, + Sources: map[SourceRef]SourceBlock{ + SourceRef{Type: "null", Name: "null-builder"}: SourceBlock{ + Name: "null-builder", + Type: "null", + }, + }, + Builds: Builds{ + &BuildBlock{ + Sources: []SourceRef{ + {Type: "null", Name: "null-builder"}, + }, + ProvisionerBlocks: []*ProvisionerBlock{ + { + PType: "shell", + MaxRetries: 1, + }, + { + PType: "shell", + MaxRetries: 1, + }, + }, + }, + }, + }, + false, false, + []packer.Build{&packer.CoreBuild{ + Type: "null.null-builder", + Prepared: true, + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{ + { + PType: "shell", + Provisioner: &packer.RetriedProvisioner{ + MaxRetries: 1, + Provisioner: &HCL2Provisioner{ + Provisioner: &MockProvisioner{ + Config: MockConfig{ + NestedMockConfig: NestedMockConfig{ + Tags: []MockTag{}, + }, + NestedSlice: []NestedMockConfig{}, + }, + }, + }, + }, + }, + { + PType: "shell", + Provisioner: &packer.RetriedProvisioner{ + MaxRetries: 1, + Provisioner: &HCL2Provisioner{ + Provisioner: &MockProvisioner{ + Config: MockConfig{ + NestedMockConfig: NestedMockConfig{ + Tags: []MockTag{}, + }, + NestedSlice: []NestedMockConfig{}, + }, + }, + }, + }, + }, + }, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + }, + }, + false, + }, } testParse(t, tests) } diff --git a/packer/core.go b/packer/core.go index 23abdc983..aa43d8fbc 100644 --- a/packer/core.go +++ b/packer/core.go @@ -6,6 +6,7 @@ import ( "log" "regexp" "sort" + "strconv" "strings" ttmp "text/template" @@ -191,9 +192,20 @@ func (c *Core) generateCoreBuildProvisioner(rawP *template.Provisioner, rawName Provisioner: provisioner, } } - if rawP.MaxRetries != 0 { + maxRetries := 0 + if rawP.MaxRetries != "" { + renderedMaxRetries, err := interpolate.Render(rawP.MaxRetries, c.Context()) + if err != nil { + return cbp, fmt.Errorf("failed to interpolate `max_retries`: %s", err.Error()) + } + maxRetries, err = strconv.Atoi(renderedMaxRetries) + if err != nil { + return cbp, fmt.Errorf("`max_retries` must be a valid integer: %s", err.Error()) + } + } + if maxRetries != 0 { provisioner = &RetriedProvisioner{ - MaxRetries: rawP.MaxRetries, + MaxRetries: maxRetries, Provisioner: provisioner, } } diff --git a/packer/core_test.go b/packer/core_test.go index 1a4f865df..864080072 100644 --- a/packer/core_test.go +++ b/packer/core_test.go @@ -799,7 +799,13 @@ func TestCoreBuild_provRetry(t *testing.T) { config := TestCoreConfig(t) testCoreTemplate(t, config, fixtureDir("build-prov-retry.json")) b := TestBuilder(t, config, "test") - p := TestProvisioner(t, config, "test") + pString := new(MockProvisioner) + pInt := new(MockProvisioner) + config.Components.ProvisionerStore = MapOfProvisioner{ + "test-string": func() (Provisioner, error) { return pString, nil }, + // backwards compatibility + "test-integer": func() (Provisioner, error) { return pInt, nil }, + } core := TestCore(t, config) b.ArtifactId = "hello" @@ -814,7 +820,10 @@ func TestCoreBuild_provRetry(t *testing.T) { } ui := testUi() - p.ProvFunc = func(ctx context.Context) error { + pInt.ProvFunc = func(ctx context.Context) error { + return errors.New("failed") + } + pString.ProvFunc = func(ctx context.Context) error { return errors.New("failed") } @@ -829,7 +838,11 @@ func TestCoreBuild_provRetry(t *testing.T) { if artifact[0].Id() != b.ArtifactId { t.Fatalf("bad: %s", artifact[0].Id()) } - if !p.ProvRetried { - t.Fatal("provisioner should retry") + if !pString.ProvRetried { + t.Fatal("provisioner should retry for max_retries string value") + } + // backwards compatibility + if !pInt.ProvRetried { + t.Fatal("provisioner should retry for max_retries integer value") } } diff --git a/packer/test-fixtures/build-prov-retry.json b/packer/test-fixtures/build-prov-retry.json index e2128855f..ebfd54f99 100644 --- a/packer/test-fixtures/build-prov-retry.json +++ b/packer/test-fixtures/build-prov-retry.json @@ -3,8 +3,14 @@ "type": "test" }], - "provisioners": [{ - "type": "test", - "max_retries": 1 - }] + "provisioners": [ + { + "type": "test-integer", + "max_retries": 1 + }, + { + "type": "test-string", + "max_retries": "1" + } + ] } diff --git a/template/parse.go b/template/parse.go index 830a0ac8c..6bd4b8fa9 100644 --- a/template/parse.go +++ b/template/parse.go @@ -59,7 +59,7 @@ func (r *rawTemplate) MarshalJSON() ([]byte, error) { func (r *rawTemplate) decodeProvisioner(raw interface{}) (Provisioner, error) { var p Provisioner - if err := r.decoder(&p, nil).Decode(raw); err != nil { + if err := r.weakDecoder(&p, nil).Decode(raw); err != nil { return p, fmt.Errorf("Error decoding provisioner: %s", err) } @@ -287,6 +287,24 @@ func (r *rawTemplate) decoder( return d } +func (r *rawTemplate) weakDecoder( + result interface{}, + md *mapstructure.Metadata) *mapstructure.Decoder { + d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + WeaklyTypedInput: true, + DecodeHook: mapstructure.StringToTimeDurationHookFunc(), + Metadata: md, + Result: result, + }) + if err != nil { + // This really shouldn't happen since we have firm control over + // all the arguments and they're all unit tested. So we use a + // panic here to note this would definitely be a bug. + panic(err) + } + return d +} + func (r *rawTemplate) parsePostProcessor( i int, raw interface{}) ([]map[string]interface{}, error) { switch v := raw.(type) { diff --git a/template/parse_test.go b/template/parse_test.go index 0b25e973b..2dc7acde9 100644 --- a/template/parse_test.go +++ b/template/parse_test.go @@ -112,7 +112,7 @@ func TestParse(t *testing.T) { Provisioners: []*Provisioner{ { Type: "something", - MaxRetries: 5, + MaxRetries: "5", }, }, }, diff --git a/template/template.go b/template/template.go index 6d810a656..decd2d23c 100644 --- a/template/template.go +++ b/template/template.go @@ -141,7 +141,7 @@ type Provisioner struct { Config map[string]interface{} `json:"config,omitempty"` Override map[string]interface{} `json:"override,omitempty"` PauseBefore time.Duration `mapstructure:"pause_before" json:"pause_before,omitempty"` - MaxRetries int `mapstructure:"max_retries" json:"max_retries,omitempty"` + MaxRetries string `mapstructure:"max_retries" json:"max_retries,omitempty"` Timeout time.Duration `mapstructure:"timeout" json:"timeout,omitempty"` } diff --git a/template/template.hcl2spec.go b/template/template.hcl2spec.go index 685fe3b91..6be7b93e7 100644 --- a/template/template.hcl2spec.go +++ b/template/template.hcl2spec.go @@ -15,7 +15,7 @@ type FlatProvisioner struct { Config map[string]interface{} `json:"config,omitempty" cty:"config" hcl:"config"` Override map[string]interface{} `json:"override,omitempty" cty:"override" hcl:"override"` PauseBefore *string `mapstructure:"pause_before" json:"pause_before,omitempty" cty:"pause_before" hcl:"pause_before"` - MaxRetries *int `mapstructure:"max_retries" json:"max_retries,omitempty" cty:"max_retries" hcl:"max_retries"` + MaxRetries *string `mapstructure:"max_retries" json:"max_retries,omitempty" cty:"max_retries" hcl:"max_retries"` Timeout *string `mapstructure:"timeout" json:"timeout,omitempty" cty:"timeout" hcl:"timeout"` } @@ -37,7 +37,7 @@ func (*FlatProvisioner) HCL2Spec() map[string]hcldec.Spec { "config": &hcldec.AttrSpec{Name: "config", Type: cty.Map(cty.String), Required: false}, "override": &hcldec.AttrSpec{Name: "override", Type: cty.Map(cty.String), Required: false}, "pause_before": &hcldec.AttrSpec{Name: "pause_before", Type: cty.String, Required: false}, - "max_retries": &hcldec.AttrSpec{Name: "max_retries", Type: cty.Number, Required: false}, + "max_retries": &hcldec.AttrSpec{Name: "max_retries", Type: cty.String, Required: false}, "timeout": &hcldec.AttrSpec{Name: "timeout", Type: cty.String, Required: false}, } return s