From 3ddb17ad86c65c10cd246bec9da763e8b1c7c254 Mon Sep 17 00:00:00 2001 From: Sylvia Moss Date: Thu, 4 Feb 2021 11:25:44 +0100 Subject: [PATCH] Don't allow data sources to be used inside data sources (#10559) --- go.mod | 2 +- go.sum | 2 + hcl2template/parser.go | 2 +- hcl2template/plugin.go | 6 +-- .../testdata/datasources/not-allowed.pkr.hcl | 7 ++++ hcl2template/types.build.provisioners.go | 2 +- hcl2template/types.datasource.go | 2 +- hcl2template/types.datasource_test.go | 27 ++++++++++++ hcl2template/types.packer_config.go | 41 +++++++++++++++---- vendor/modules.txt | 2 +- 10 files changed, 77 insertions(+), 16 deletions(-) create mode 100644 hcl2template/testdata/datasources/not-allowed.pkr.hcl diff --git a/go.mod b/go.mod index 1cf4013ce..2ac755284 100644 --- a/go.mod +++ b/go.mod @@ -50,7 +50,7 @@ require ( github.com/hashicorp/go-uuid v1.0.2 github.com/hashicorp/go-version v1.2.0 github.com/hashicorp/hcl/v2 v2.8.0 - github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210127164048-448d64e93ee6 + github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210203104625-430d35d16bac github.com/hashicorp/vault/api v1.0.4 github.com/hetznercloud/hcloud-go v1.15.1 github.com/hyperonecom/h1-client-go v0.0.0-20191203060043-b46280e4c4a4 diff --git a/go.sum b/go.sum index 5918fcadc..9df99a58d 100644 --- a/go.sum +++ b/go.sum @@ -407,6 +407,8 @@ github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210127164048-448d64e93ee6 h1: github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210127164048-448d64e93ee6/go.mod h1:GNb0WNs7zibb8vzUZce1As64z2AW0FEMwhe2J7/NW5I= github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210128163027-e8ca18774ef6 h1:cIEMJEQNtAyx1t0aOodcQT7B6qjbQFJ8p6Az1nfK3fw= github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210128163027-e8ca18774ef6/go.mod h1:GNb0WNs7zibb8vzUZce1As64z2AW0FEMwhe2J7/NW5I= +github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210203104625-430d35d16bac h1:C/4PlC3GMp3xqf0/Q9SacTOywDAHNaxKA0QmuDWYBNs= +github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210203104625-430d35d16bac/go.mod h1:GNb0WNs7zibb8vzUZce1As64z2AW0FEMwhe2J7/NW5I= github.com/hashicorp/serf v0.8.2 h1:YZ7UKsJv+hKjqGVUUbtE3HNj79Eln2oQ75tniF6iPt0= github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= github.com/hashicorp/serf v0.9.2 h1:yJoyfZXo4Pk2p/M/viW+YLibBFiIbKoP79gu7kDAFP0= diff --git a/hcl2template/parser.go b/hcl2template/parser.go index b7d807962..15c6cc37c 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -373,7 +373,7 @@ func (p *Parser) parseConfig(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics { func (p *Parser) decodeDatasources(file *hcl.File, cfg *PackerConfig) hcl.Diagnostics { var diags hcl.Diagnostics - body := dynblock.Expand(file.Body, cfg.EvalContext(nil)) + body := dynblock.Expand(file.Body, cfg.EvalContext(DatasourceContext, nil)) content, moreDiags := body.Content(configSchema) diags = append(diags, moreDiags...) diff --git a/hcl2template/plugin.go b/hcl2template/plugin.go index db40fe38b..17f850cb6 100644 --- a/hcl2template/plugin.go +++ b/hcl2template/plugin.go @@ -143,7 +143,7 @@ func (cfg *PackerConfig) initializeBlocks() hcl.Diagnostics { body = hcl.MergeBodies([]hcl.Body{body, srcUsage.Body}) } // expand any dynamic block. - body = dynblock.Expand(body, cfg.EvalContext(nil)) + body = dynblock.Expand(body, cfg.EvalContext(BuildContext, nil)) srcUsage.Body = body } @@ -158,7 +158,7 @@ func (cfg *PackerConfig) initializeBlocks() hcl.Diagnostics { }) } // Allow rest of the body to have dynamic blocks - provBlock.HCL2Ref.Rest = dynblock.Expand(provBlock.HCL2Ref.Rest, cfg.EvalContext(nil)) + provBlock.HCL2Ref.Rest = dynblock.Expand(provBlock.HCL2Ref.Rest, cfg.EvalContext(BuildContext, nil)) } for _, ppList := range build.PostProcessorsLists { @@ -172,7 +172,7 @@ func (cfg *PackerConfig) initializeBlocks() hcl.Diagnostics { }) } // Allow the rest of the body to have dynamic blocks - ppBlock.HCL2Ref.Rest = dynblock.Expand(ppBlock.HCL2Ref.Rest, cfg.EvalContext(nil)) + ppBlock.HCL2Ref.Rest = dynblock.Expand(ppBlock.HCL2Ref.Rest, cfg.EvalContext(BuildContext, nil)) } } diff --git a/hcl2template/testdata/datasources/not-allowed.pkr.hcl b/hcl2template/testdata/datasources/not-allowed.pkr.hcl new file mode 100644 index 000000000..5e7d0a147 --- /dev/null +++ b/hcl2template/testdata/datasources/not-allowed.pkr.hcl @@ -0,0 +1,7 @@ +data "amazon-ami" "test_0" { + string = "string" +} + +data "amazon-ami" "test_1" { + string = data.amazon-ami.test_0.string +} \ No newline at end of file diff --git a/hcl2template/types.build.provisioners.go b/hcl2template/types.build.provisioners.go index dce5d39ad..17adb7c83 100644 --- a/hcl2template/types.build.provisioners.go +++ b/hcl2template/types.build.provisioners.go @@ -84,7 +84,7 @@ func (p *Parser) decodeProvisioner(block *hcl.Block, cfg *PackerConfig) (*Provis Override cty.Value `hcl:"override,optional"` Rest hcl.Body `hcl:",remain"` } - diags := gohcl.DecodeBody(block.Body, cfg.EvalContext(nil), &b) + diags := gohcl.DecodeBody(block.Body, cfg.EvalContext(BuildContext, nil), &b) if diags.HasErrors() { return nil, diags } diff --git a/hcl2template/types.datasource.go b/hcl2template/types.datasource.go index 0484927b5..c25ab5b1a 100644 --- a/hcl2template/types.datasource.go +++ b/hcl2template/types.datasource.go @@ -102,7 +102,7 @@ func (cfg *PackerConfig) startDatasource(dataSourceStore packer.DatasourceStore, }) } body := block.Body - decoded, moreDiags := decodeHCL2Spec(body, cfg.EvalContext(nil), datasource) + decoded, moreDiags := decodeHCL2Spec(body, cfg.EvalContext(DatasourceContext, nil), datasource) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { return nil, diags diff --git a/hcl2template/types.datasource_test.go b/hcl2template/types.datasource_test.go index 441c8c881..c50d5733f 100644 --- a/hcl2template/types.datasource_test.go +++ b/hcl2template/types.datasource_test.go @@ -53,6 +53,33 @@ func TestParse_datasource(t *testing.T) { nil, false, }, + {"not allowed usage of data source within another data source", + defaultParser, + parseTestArgs{"testdata/datasources/not-allowed.pkr.hcl", nil, nil}, + &PackerConfig{ + CorePackerVersionString: lockedVersion, + Basedir: filepath.Join("testdata", "datasources"), + Datasources: Datasources{ + { + Type: "amazon-ami", + Name: "test_0", + }: { + Type: "amazon-ami", + Name: "test_0", + }, + { + Type: "amazon-ami", + Name: "test_1", + }: { + Type: "amazon-ami", + Name: "test_1", + }, + }, + }, + true, true, + nil, + false, + }, {"inexistent source", defaultParser, parseTestArgs{"testdata/datasources/inexistent.pkr.hcl", nil, nil}, diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index fc8d6fa6b..159d3e5a9 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -73,13 +73,22 @@ const ( dataAccessor = "data" ) +type BlockContext int + +const ( + InputVariableContext BlockContext = iota + LocalContext + BuildContext + DatasourceContext + NilContext +) + // EvalContext returns the *hcl.EvalContext that will be passed to an hcl // decoder in order to tell what is the actual value of a var or a local and // the list of defined functions. -func (cfg *PackerConfig) EvalContext(variables map[string]cty.Value) *hcl.EvalContext { +func (cfg *PackerConfig) EvalContext(ctx BlockContext, variables map[string]cty.Value) *hcl.EvalContext { inputVariables, _ := cfg.InputVariables.Values() localVariables, _ := cfg.LocalVariables.Values() - datasourceVariables, _ := cfg.Datasources.Values() ectx := &hcl.EvalContext{ Functions: Functions(cfg.Basedir), Variables: map[string]cty.Value{ @@ -97,9 +106,25 @@ func (cfg *PackerConfig) EvalContext(variables map[string]cty.Value) *hcl.EvalCo "cwd": cty.StringVal(strings.ReplaceAll(cfg.Cwd, `\`, `/`)), "root": cty.StringVal(strings.ReplaceAll(cfg.Basedir, `\`, `/`)), }), - dataAccessor: cty.ObjectVal(datasourceVariables), }, } + + // Currently the places where you can make references to other blocks + // from one is very 'procedural', and in this specific case, we could make + // the data sources available to other datasources, but this would be + // order dependant, meaning that if you define two datasources in two + // different blocks, the second one can use the first one, but not the + // other way around; which would be totally confusing; so - for now - + // datasources can't use other datasources. + // In the future we'd like to load and execute HCL blocks using a graph + // dependency tree, so that any block can use any block whatever the + // order. + switch ctx { + case LocalContext, BuildContext: + datasourceVariables, _ := cfg.Datasources.Values() + ectx.Variables[dataAccessor] = cty.ObjectVal(datasourceVariables) + } + for k, v := range variables { ectx.Variables[k] = v } @@ -229,7 +254,7 @@ func (c *PackerConfig) evaluateLocalVariables(locals []*LocalBlock) hcl.Diagnost func (c *PackerConfig) evaluateLocalVariable(local *LocalBlock) hcl.Diagnostics { var diags hcl.Diagnostics - value, moreDiags := local.Expr.Value(c.EvalContext(nil)) + value, moreDiags := local.Expr.Value(c.EvalContext(LocalContext, nil)) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { return diags @@ -441,7 +466,7 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu } } - builder, moreDiags, generatedVars := cfg.startBuilder(srcUsage, cfg.EvalContext(nil), opts) + builder, moreDiags, generatedVars := cfg.startBuilder(srcUsage, cfg.EvalContext(BuildContext, nil), opts) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue @@ -463,12 +488,12 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu buildAccessor: cty.ObjectVal(unknownBuildValues), } - provisioners, moreDiags := cfg.getCoreBuildProvisioners(srcUsage, build.ProvisionerBlocks, cfg.EvalContext(variables)) + provisioners, moreDiags := cfg.getCoreBuildProvisioners(srcUsage, build.ProvisionerBlocks, cfg.EvalContext(BuildContext, variables)) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue } - pps, moreDiags := cfg.getCoreBuildPostProcessors(srcUsage, build.PostProcessorsLists, cfg.EvalContext(variables)) + pps, moreDiags := cfg.getCoreBuildPostProcessors(srcUsage, build.PostProcessorsLists, cfg.EvalContext(BuildContext, variables)) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue @@ -610,7 +635,7 @@ func (p *PackerConfig) handleEval(line string) (out string, exit bool, diags hcl return "", false, diags } - val, valueDiags := expr.Value(p.EvalContext(nil)) + val, valueDiags := expr.Value(p.EvalContext(NilContext, nil)) diags = append(diags, valueDiags...) if valueDiags.HasErrors() { return "", false, diags diff --git a/vendor/modules.txt b/vendor/modules.txt index 39941c3d4..832954d8f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -360,7 +360,7 @@ github.com/hashicorp/hcl/v2/hclparse github.com/hashicorp/hcl/v2/hclsyntax github.com/hashicorp/hcl/v2/hclwrite github.com/hashicorp/hcl/v2/json -# github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210127164048-448d64e93ee6 +# github.com/hashicorp/packer-plugin-sdk v0.0.11-0.20210203104625-430d35d16bac github.com/hashicorp/packer-plugin-sdk/acctest github.com/hashicorp/packer-plugin-sdk/acctest/provisioneracc github.com/hashicorp/packer-plugin-sdk/acctest/testutils