From bb118c37a276059e3a22158f5479efc635a083f6 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 15 Apr 2019 15:25:00 -0700 Subject: [PATCH] configs: Handle "dynamic" blocks as special during override merging Previously we were treating "dynamic" blocks in configuration the same as any other block type when merging config bodies, so that dynamic blocks in the override would override any dynamic blocks present in the base, without considering the dynamic block type. It's more useful and intuitive for us to treat dynamic blocks as if they are instances of their given block type for the purpose of overriding. That means a foo block can be overridden by a dynamic "foo" block and vice-versa, and dynamic blocks of different types do not interact at all during overriding. This requires us to recognize dynamic blocks and treat them specially during decoding of a merged body. We leave them unexpanded here because this package is not responsible for dynamic block expansion (that happens in the sibling "lang" package) but we do decode them enough to recognize their labels so we can treat them as if they were blocks of the labelled type. --- configs/module_merge_body.go | 26 ++++++-- configs/module_merge_test.go | 63 +++++++++++++++++++ .../override-dynamic-block-base/a_override.tf | 6 ++ .../override-dynamic-block-base/base.tf | 9 +++ .../a_override.tf | 9 +++ .../override-dynamic-block-override/base.tf | 6 ++ configs/util.go | 18 ++++++ 7 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 configs/test-fixtures/valid-modules/override-dynamic-block-base/a_override.tf create mode 100644 configs/test-fixtures/valid-modules/override-dynamic-block-base/base.tf create mode 100644 configs/test-fixtures/valid-modules/override-dynamic-block-override/a_override.tf create mode 100644 configs/test-fixtures/valid-modules/override-dynamic-block-override/base.tf diff --git a/configs/module_merge_body.go b/configs/module_merge_body.go index bb40e2683..0ed561eee 100644 --- a/configs/module_merge_body.go +++ b/configs/module_merge_body.go @@ -40,11 +40,12 @@ var _ hcl.Body = mergeBody{} func (b mergeBody) Content(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Diagnostics) { var diags hcl.Diagnostics - oSchema := schemaForOverrides(schema) + baseSchema := schemaWithDynamic(schema) + overrideSchema := schemaWithDynamic(schemaForOverrides(schema)) - baseContent, cDiags := b.Base.Content(schema) + baseContent, _, cDiags := b.Base.PartialContent(baseSchema) diags = append(diags, cDiags...) - overrideContent, cDiags := b.Override.Content(oSchema) + overrideContent, _, cDiags := b.Override.PartialContent(overrideSchema) diags = append(diags, cDiags...) content := b.prepareContent(baseContent, overrideContent) @@ -54,11 +55,12 @@ func (b mergeBody) Content(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Diagno func (b mergeBody) PartialContent(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Body, hcl.Diagnostics) { var diags hcl.Diagnostics - oSchema := schemaForOverrides(schema) + baseSchema := schemaWithDynamic(schema) + overrideSchema := schemaWithDynamic(schemaForOverrides(schema)) - baseContent, baseRemain, cDiags := b.Base.PartialContent(schema) + baseContent, baseRemain, cDiags := b.Base.PartialContent(baseSchema) diags = append(diags, cDiags...) - overrideContent, overrideRemain, cDiags := b.Override.PartialContent(oSchema) + overrideContent, overrideRemain, cDiags := b.Override.PartialContent(overrideSchema) diags = append(diags, cDiags...) content := b.prepareContent(baseContent, overrideContent) @@ -90,9 +92,21 @@ func (b mergeBody) prepareContent(base *hcl.BodyContent, override *hcl.BodyConte overriddenBlockTypes := make(map[string]bool) for _, block := range override.Blocks { + if block.Type == "dynamic" { + overriddenBlockTypes[block.Labels[0]] = true + continue + } overriddenBlockTypes[block.Type] = true } for _, block := range base.Blocks { + // We skip over dynamic blocks whose type label is an overridden type + // but note that below we do still leave them as dynamic blocks in + // the result because expanding the dynamic blocks that are left is + // done much later during the core graph walks, where we can safely + // evaluate the expressions. + if block.Type == "dynamic" && overriddenBlockTypes[block.Labels[0]] { + continue + } if overriddenBlockTypes[block.Type] { continue } diff --git a/configs/module_merge_test.go b/configs/module_merge_test.go index a89ba1551..a540b0a85 100644 --- a/configs/module_merge_test.go +++ b/configs/module_merge_test.go @@ -136,3 +136,66 @@ func TestModuleOverrideModule(t *testing.T) { assertResultDeepEqual(t, gotArgs, wantArgs) } + +func TestModuleOverrideDynamic(t *testing.T) { + schema := &hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{ + {Type: "foo"}, + {Type: "dynamic", LabelNames: []string{"type"}}, + }, + } + + t.Run("base is dynamic", func(t *testing.T) { + mod, diags := testModuleFromDir("test-fixtures/valid-modules/override-dynamic-block-base") + assertNoDiagnostics(t, diags) + if mod == nil { + t.Fatalf("module is nil") + } + + if _, exists := mod.ManagedResources["test.foo"]; !exists { + t.Fatalf("no module 'example'") + } + if len(mod.ManagedResources) != 1 { + t.Fatalf("wrong number of managed resources in result %d; want 1", len(mod.ManagedResources)) + } + + body := mod.ManagedResources["test.foo"].Config + content, diags := body.Content(schema) + assertNoDiagnostics(t, diags) + + if len(content.Blocks) != 1 { + t.Fatalf("wrong number of blocks in result %d; want 1", len(content.Blocks)) + } + if got, want := content.Blocks[0].Type, "foo"; got != want { + t.Fatalf("wrong block type %q; want %q", got, want) + } + }) + t.Run("override is dynamic", func(t *testing.T) { + mod, diags := testModuleFromDir("test-fixtures/valid-modules/override-dynamic-block-override") + assertNoDiagnostics(t, diags) + if mod == nil { + t.Fatalf("module is nil") + } + + if _, exists := mod.ManagedResources["test.foo"]; !exists { + t.Fatalf("no module 'example'") + } + if len(mod.ManagedResources) != 1 { + t.Fatalf("wrong number of managed resources in result %d; want 1", len(mod.ManagedResources)) + } + + body := mod.ManagedResources["test.foo"].Config + content, diags := body.Content(schema) + assertNoDiagnostics(t, diags) + + if len(content.Blocks) != 1 { + t.Fatalf("wrong number of blocks in result %d; want 1", len(content.Blocks)) + } + if got, want := content.Blocks[0].Type, "dynamic"; got != want { + t.Fatalf("wrong block type %q; want %q", got, want) + } + if got, want := content.Blocks[0].Labels[0], "foo"; got != want { + t.Fatalf("wrong dynamic block label %q; want %q", got, want) + } + }) +} diff --git a/configs/test-fixtures/valid-modules/override-dynamic-block-base/a_override.tf b/configs/test-fixtures/valid-modules/override-dynamic-block-base/a_override.tf new file mode 100644 index 000000000..e168fac47 --- /dev/null +++ b/configs/test-fixtures/valid-modules/override-dynamic-block-base/a_override.tf @@ -0,0 +1,6 @@ + +resource "test" "foo" { + foo { + from = "override" + } +} diff --git a/configs/test-fixtures/valid-modules/override-dynamic-block-base/base.tf b/configs/test-fixtures/valid-modules/override-dynamic-block-base/base.tf new file mode 100644 index 000000000..0f3ad4ca7 --- /dev/null +++ b/configs/test-fixtures/valid-modules/override-dynamic-block-base/base.tf @@ -0,0 +1,9 @@ + +resource "test" "foo" { + dynamic "foo" { + for_each = [] + content { + from = "base" + } + } +} diff --git a/configs/test-fixtures/valid-modules/override-dynamic-block-override/a_override.tf b/configs/test-fixtures/valid-modules/override-dynamic-block-override/a_override.tf new file mode 100644 index 000000000..d37ce790e --- /dev/null +++ b/configs/test-fixtures/valid-modules/override-dynamic-block-override/a_override.tf @@ -0,0 +1,9 @@ + +resource "test" "foo" { + dynamic "foo" { + for_each = [] + content { + from = "override" + } + } +} diff --git a/configs/test-fixtures/valid-modules/override-dynamic-block-override/base.tf b/configs/test-fixtures/valid-modules/override-dynamic-block-override/base.tf new file mode 100644 index 000000000..3ed55a6b5 --- /dev/null +++ b/configs/test-fixtures/valid-modules/override-dynamic-block-override/base.tf @@ -0,0 +1,6 @@ + +resource "test" "foo" { + foo { + from = "base" + } +} diff --git a/configs/util.go b/configs/util.go index 002bb8cb8..5fbde4310 100644 --- a/configs/util.go +++ b/configs/util.go @@ -43,3 +43,21 @@ func schemaForOverrides(schema *hcl.BodySchema) *hcl.BodySchema { return ret } + +// schemaWithDynamic takes a *hcl.BodySchema and produces a new one that +// is equivalent except that it accepts an additional block type "dynamic" with +// a single label, used to recognize usage of the HCL dynamic block extension. +func schemaWithDynamic(schema *hcl.BodySchema) *hcl.BodySchema { + ret := &hcl.BodySchema{ + Attributes: schema.Attributes, + Blocks: make([]hcl.BlockHeaderSchema, len(schema.Blocks), len(schema.Blocks)+1), + } + + copy(ret.Blocks, schema.Blocks) + ret.Blocks = append(ret.Blocks, hcl.BlockHeaderSchema{ + Type: "dynamic", + LabelNames: []string{"type"}, + }) + + return ret +}