From 50a101afbd3cabbd2ed1adce02213ae16e894f5d Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 18 Mar 2019 17:15:23 -0700 Subject: [PATCH] lang: Consider "dynamic" blocks when resolving references The hcldec package has no awareness of the dynamic block extension, so the hcldec.Variables function misses any variables declared inside dynamic blocks. dynblock.VariablesHCLDec is a drop-in replacement for hcldec.Variables that _is_ aware of dynamic blocks, returning all of the same variables that hcldec would find naturally plus also any variables used inside the dynamic block "for_each" and "labels" arguments and inside the nested "content" block. --- lang/references.go | 10 ++- terraform/graph_builder_plan_test.go | 85 +++++++++++++++++++ .../graph-builder-plan-dynblock/dynblock.tf | 14 +++ 3 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 terraform/test-fixtures/graph-builder-plan-dynblock/dynblock.tf diff --git a/lang/references.go b/lang/references.go index 69dd0d82c..fab468a6e 100644 --- a/lang/references.go +++ b/lang/references.go @@ -1,8 +1,8 @@ package lang import ( + "github.com/hashicorp/hcl2/ext/dynblock" "github.com/hashicorp/hcl2/hcl" - "github.com/hashicorp/hcl2/hcldec" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/tfdiags" @@ -52,7 +52,13 @@ func ReferencesInBlock(body hcl.Body, schema *configschema.Block) ([]*addrs.Refe return nil, nil } spec := schema.DecoderSpec() - traversals := hcldec.Variables(body, spec) + + // We use dynblock.VariablesHCLDec instead of hcldec.Variables here because + // when we evaluate a block we'll apply the HCL dynamic block extension + // expansion to it first, and so we need this specialized version in order + // to properly understand what the dependencies will be once expanded. + // Otherwise, we'd miss references that only occur inside dynamic blocks. + traversals := dynblock.VariablesHCLDec(body, spec) return References(traversals) } diff --git a/terraform/graph_builder_plan_test.go b/terraform/graph_builder_plan_test.go index 25452a723..afdbedd2e 100644 --- a/terraform/graph_builder_plan_test.go +++ b/terraform/graph_builder_plan_test.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/providers" + "github.com/zclconf/go-cty/cty" ) func TestPlanGraphBuilder_impl(t *testing.T) { @@ -67,6 +68,90 @@ func TestPlanGraphBuilder(t *testing.T) { } } +func TestPlanGraphBuilder_dynamicBlock(t *testing.T) { + provider := &MockProvider{ + GetSchemaReturn: &ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_thing": { + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + "list": {Type: cty.List(cty.String), Computed: true}, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "nested": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": {Type: cty.String, Optional: true}, + }, + }, + }, + }, + }, + }, + }, + } + components := &basicComponentFactory{ + providers: map[string]providers.Factory{ + "test": providers.FactoryFixed(provider), + }, + } + + b := &PlanGraphBuilder{ + Config: testModule(t, "graph-builder-plan-dynblock"), + Components: components, + Schemas: &Schemas{ + Providers: map[string]*ProviderSchema{ + "test": provider.GetSchemaReturn, + }, + }, + DisableReduce: true, + } + + g, err := b.Build(addrs.RootModuleInstance) + if err != nil { + t.Fatalf("err: %s", err) + } + + if g.Path.String() != addrs.RootModuleInstance.String() { + t.Fatalf("wrong module path %q", g.Path) + } + + // This test is here to make sure we properly detect references inside + // the special "dynamic" block construct. The most important thing here + // is that at the end test_thing.c depends on both test_thing.a and + // test_thing.b. Other details might shift over time as other logic in + // the graph builders changes. + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(` +meta.count-boundary (EachMode fixup) + provider.test + test_thing.a + test_thing.b + test_thing.c +provider.test +provider.test (close) + provider.test + test_thing.a + test_thing.b + test_thing.c +root + meta.count-boundary (EachMode fixup) + provider.test (close) +test_thing.a + provider.test +test_thing.b + provider.test +test_thing.c + provider.test + test_thing.a + test_thing.b +`) + if actual != expected { + t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual) + } +} + func TestPlanGraphBuilder_targetModule(t *testing.T) { b := &PlanGraphBuilder{ Config: testModule(t, "graph-builder-plan-target-module-provider"), diff --git a/terraform/test-fixtures/graph-builder-plan-dynblock/dynblock.tf b/terraform/test-fixtures/graph-builder-plan-dynblock/dynblock.tf new file mode 100644 index 000000000..894696977 --- /dev/null +++ b/terraform/test-fixtures/graph-builder-plan-dynblock/dynblock.tf @@ -0,0 +1,14 @@ +resource "test_thing" "a" { +} + +resource "test_thing" "b" { +} + +resource "test_thing" "c" { + dynamic "nested" { + for_each = test_thing.a.list + content { + foo = test_thing.b.id + } + } +}