From f9f92da699d801c8beee28dbbb7c09e3dce2ae66 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 18 Mar 2019 16:28:30 -0700 Subject: [PATCH] ext/dynblock: Allow interrogation of _all_ references in blocks Our API previously had a function only for retrieving the variables used in the for_each and labels arguments used during an Expand call, and expected callers to then interrogate the resulting expanded block to find the other variables required to fully decode the content. That approach is insufficient for any application that needs to know the full set of required variables before any evaluation begins, such as when a dependency graph will be constructed to allow a topological traversal through blocks while evaluating. Now we have WalkVariables, which finds both the variables used to expand _and_ the variables within any blocks. This also renames WalkForEachVariables to WalkExpandVariables since that name is more accurate with the addition of the "label" argument into the expand-time dependency set. There is also a hcldec-based helper wrapper for each of those, allowing single-shot analysis of blocks for applications that use hcldec. This is a breaking change to the dynblock package API, because the old WalkForEachVariables and ForEachVariablesHCLDec functions are no longer present. --- ext/dynblock/variables.go | 54 ++++++++++++---- ext/dynblock/variables_hcldec.go | 26 +++++--- ext/dynblock/variables_test.go | 104 +++++++++++++++++++++---------- 3 files changed, 131 insertions(+), 53 deletions(-) diff --git a/ext/dynblock/variables.go b/ext/dynblock/variables.go index 9cb6716..90b4820 100644 --- a/ext/dynblock/variables.go +++ b/ext/dynblock/variables.go @@ -5,19 +5,31 @@ import ( "github.com/zclconf/go-cty/cty" ) -// WalkVariables begins the recursive process of walking the variables in the -// given body that are needed by any "for_each" or "labels" attributes in -// "dynamic" blocks. The result is a WalkVariablesNode, which can extract -// root-level variable traversals and produce a list of child nodes that -// also need to be processed by calling Visit. +// WalkVariables begins the recursive process of walking all expressions and +// nested blocks in the given body and its child bodies while taking into +// account any "dynamic" blocks. // // This function requires that the caller walk through the nested block // structure in the given body level-by-level so that an appropriate schema // can be provided at each level to inform further processing. This workflow // is thus easiest to use for calling applications that have some higher-level // schema representation available with which to drive this multi-step -// process. -func WalkForEachVariables(body hcl.Body) WalkVariablesNode { +// process. If your application uses the hcldec package, you may be able to +// use VariablesHCLDec instead for a more automatic approach. +func WalkVariables(body hcl.Body) WalkVariablesNode { + return WalkVariablesNode{ + body: body, + includeContent: true, + } +} + +// WalkExpandVariables is like Variables but it includes only the variables +// required for successful block expansion, ignoring any variables referenced +// inside block contents. The result is the minimal set of all variables +// required for a call to Expand, excluding variables that would only be +// needed to subsequently call Content or PartialContent on the expanded +// body. +func WalkExpandVariables(body hcl.Body) WalkVariablesNode { return WalkVariablesNode{ body: body, } @@ -26,6 +38,8 @@ func WalkForEachVariables(body hcl.Body) WalkVariablesNode { type WalkVariablesNode struct { body hcl.Body it *iteration + + includeContent bool } type WalkVariablesChild struct { @@ -50,6 +64,22 @@ func (n WalkVariablesNode) Visit(schema *hcl.BodySchema) (vars []hcl.Traversal, children = make([]WalkVariablesChild, 0, len(container.Blocks)) + if n.includeContent { + for _, attr := range container.Attributes { + for _, traversal := range attr.Expr.Variables() { + var ours, inherited bool + if n.it != nil { + ours = traversal.RootName() == n.it.IteratorName + _, inherited = n.it.Inherited[traversal.RootName()] + } + + if !(ours || inherited) { + vars = append(vars, traversal) + } + } + } + } + for _, block := range container.Blocks { switch block.Type { @@ -104,8 +134,9 @@ func (n WalkVariablesNode) Visit(schema *hcl.BodySchema) (vars []hcl.Traversal, children = append(children, WalkVariablesChild{ BlockTypeName: blockTypeName, Node: WalkVariablesNode{ - body: contentBlock.Body, - it: blockIt, + body: contentBlock.Body, + it: blockIt, + includeContent: n.includeContent, }, }) } @@ -114,8 +145,9 @@ func (n WalkVariablesNode) Visit(schema *hcl.BodySchema) (vars []hcl.Traversal, children = append(children, WalkVariablesChild{ BlockTypeName: block.Type, Node: WalkVariablesNode{ - body: block.Body, - it: n.it, + body: block.Body, + it: n.it, + includeContent: n.includeContent, }, }) diff --git a/ext/dynblock/variables_hcldec.go b/ext/dynblock/variables_hcldec.go index 480873a..a078d91 100644 --- a/ext/dynblock/variables_hcldec.go +++ b/ext/dynblock/variables_hcldec.go @@ -5,15 +5,25 @@ import ( "github.com/hashicorp/hcl2/hcldec" ) -// ForEachVariablesHCLDec is a wrapper around WalkForEachVariables that -// uses the given hcldec specification to automatically drive the recursive -// walk through nested blocks in the given body. +// VariablesHCLDec is a wrapper around WalkVariables that uses the given hcldec +// specification to automatically drive the recursive walk through nested +// blocks in the given body. // -// This provides more convenient access to all of the "for_each" and "labels" -// dependencies in a body for applications that are already using hcldec -// as a more convenient way to recursively decode body contents. -func ForEachVariablesHCLDec(body hcl.Body, spec hcldec.Spec) []hcl.Traversal { - rootNode := WalkForEachVariables(body) +// This is a drop-in replacement for hcldec.Variables which is able to treat +// blocks of type "dynamic" in the same special way that dynblock.Expand would, +// exposing both the variables referenced in the "for_each" and "labels" +// arguments and variables used in the nested "content" block. +func VariablesHCLDec(body hcl.Body, spec hcldec.Spec) []hcl.Traversal { + rootNode := WalkVariables(body) + return walkVariablesWithHCLDec(rootNode, spec) +} + +// ExpandVariablesHCLDec is like VariablesHCLDec but it includes only the +// minimal set of variables required to call Expand, ignoring variables that +// are referenced only inside normal block contents. See WalkExpandVariables +// for more information. +func ExpandVariablesHCLDec(body hcl.Body, spec hcldec.Spec) []hcl.Traversal { + rootNode := WalkExpandVariables(body) return walkVariablesWithHCLDec(rootNode, spec) } diff --git a/ext/dynblock/variables_test.go b/ext/dynblock/variables_test.go index 83748ef..5b4f492 100644 --- a/ext/dynblock/variables_test.go +++ b/ext/dynblock/variables_test.go @@ -13,13 +13,12 @@ import ( "github.com/hashicorp/hcl2/hcl/hclsyntax" ) -func TestForEachVariables(t *testing.T) { +func TestVariables(t *testing.T) { const src = ` # We have some references to things inside the "val" attribute inside each -# of our "b" blocks, but since our ForEachVariables walk only considers -# "for_each" and "labels" within a dynamic block we do _not_ expect these -# to be in the output. +# of our "b" blocks, which should be included in the result of WalkVariables +# but not WalkExpandVariables. a { dynamic "b" { @@ -56,11 +55,11 @@ dynamic "a" { content { b "foo" { - val = "${dyn_a.value} ${something_else_5}" - } + val = "${dyn_a.value} ${something_else_5}" + } - dynamic "b" { - for_each = some_list_4 + dynamic "b" { + for_each = some_list_4 labels = ["${dyn_a.value} ${b.value} ${a} ${something_else_6}"] content { val = "${dyn_a.value} ${b.value} ${something_else_7}" @@ -81,8 +80,9 @@ dynamic "a" { spec := &hcldec.BlockListSpec{ TypeName: "a", - Nested: &hcldec.BlockListSpec{ - TypeName: "b", + Nested: &hcldec.BlockMapSpec{ + TypeName: "b", + LabelNames: []string{"key"}, Nested: &hcldec.AttrSpec{ Name: "val", Type: cty.String, @@ -90,30 +90,66 @@ dynamic "a" { }, } - traversals := ForEachVariablesHCLDec(f.Body, spec) - got := make([]string, len(traversals)) - for i, traversal := range traversals { - got[i] = traversal.RootName() - } + t.Run("WalkVariables", func(t *testing.T) { + traversals := VariablesHCLDec(f.Body, spec) + got := make([]string, len(traversals)) + for i, traversal := range traversals { + got[i] = traversal.RootName() + } - // The block structure is traversed one level at a time, so the ordering - // here is reflecting first a pass of the root, then the first child - // under the root, then the first child under that, etc. - want := []string{ - "some_list_1", - "some_list_3", - "some_list_0", - "baz", - "something_else_0", - "some_list_2", - "b", // This is correct because it is referenced in a context where the iterator is overridden to be dyn_b - "something_else_3", - "some_list_4", - "a", // This is correct because it is referenced in a context where the iterator is overridden to be dyn_a - "something_else_6", - } + // The block structure is traversed one level at a time, so the ordering + // here is reflecting first a pass of the root, then the first child + // under the root, then the first child under that, etc. + want := []string{ + "some_list_1", + "some_list_3", + "some_list_0", + "baz", + "something_else_0", + "something_else_1", // Would not be included for WalkExpandVariables because it only appears in content + "some_list_2", + "b", // This is correct because it is referenced in a context where the iterator is overridden to be dyn_b + "something_else_3", + "something_else_2", // Would not be included for WalkExpandVariables because it only appears in content + "something_else_4", // Would not be included for WalkExpandVariables because it only appears in content + "some_list_4", + "a", // This is correct because it is referenced in a context where the iterator is overridden to be dyn_a + "something_else_6", + "something_else_5", // Would not be included for WalkExpandVariables because it only appears in content + "something_else_7", // Would not be included for WalkExpandVariables because it only appears in content + } - if !reflect.DeepEqual(got, want) { - t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(want)) - } + if !reflect.DeepEqual(got, want) { + t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(want)) + } + }) + + t.Run("WalkExpandVariables", func(t *testing.T) { + traversals := ExpandVariablesHCLDec(f.Body, spec) + got := make([]string, len(traversals)) + for i, traversal := range traversals { + got[i] = traversal.RootName() + } + + // The block structure is traversed one level at a time, so the ordering + // here is reflecting first a pass of the root, then the first child + // under the root, then the first child under that, etc. + want := []string{ + "some_list_1", + "some_list_3", + "some_list_0", + "baz", + "something_else_0", + "some_list_2", + "b", // This is correct because it is referenced in a context where the iterator is overridden to be dyn_b + "something_else_3", + "some_list_4", + "a", // This is correct because it is referenced in a context where the iterator is overridden to be dyn_a + "something_else_6", + } + + if !reflect.DeepEqual(got, want) { + t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(want)) + } + }) }