From 7dd0acc46b345a805420b6b9c91bfd1bf80d7c83 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 23 Jan 2019 19:17:10 -0500 Subject: [PATCH] don't count empty containers in diff.Apply If there were no matching keys, and there was no diff at all, don't set a zero count for the container. Normally Providers can't reliably detect empty vs unset here, but there are some cases that worked. --- builtin/providers/test/resource_test.go | 95 +++++++++++++++++++++++-- terraform/diff.go | 27 +++++-- 2 files changed, 109 insertions(+), 13 deletions(-) diff --git a/builtin/providers/test/resource_test.go b/builtin/providers/test/resource_test.go index 12bf568fa..05fd5c0c5 100644 --- a/builtin/providers/test/resource_test.go +++ b/builtin/providers/test/resource_test.go @@ -24,9 +24,95 @@ resource "test_resource" "foo" { } } `), - Check: func(s *terraform.State) error { - return nil - }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckNoResourceAttr( + "test_resource.foo", "list.#", + ), + ), + }, + }, + }) +} + +func TestResource_changedList(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + { + Config: strings.TrimSpace(` +resource "test_resource" "foo" { + required = "yep" + required_map = { + key = "value" + } +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckNoResourceAttr( + "test_resource.foo", "list.#", + ), + ), + }, + { + Config: strings.TrimSpace(` +resource "test_resource" "foo" { + required = "yep" + required_map = { + key = "value" + } + list = ["a"] +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource.foo", "list.#", "1", + ), + resource.TestCheckResourceAttr( + "test_resource.foo", "list.0", "a", + ), + ), + }, + { + Config: strings.TrimSpace(` +resource "test_resource" "foo" { + required = "yep" + required_map = { + key = "value" + } + list = ["a", "b"] +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource.foo", "list.#", "2", + ), + resource.TestCheckResourceAttr( + "test_resource.foo", "list.0", "a", + ), + resource.TestCheckResourceAttr( + "test_resource.foo", "list.1", "b", + ), + ), + }, + { + Config: strings.TrimSpace(` +resource "test_resource" "foo" { + required = "yep" + required_map = { + key = "value" + } + list = ["b"] +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource.foo", "list.#", "1", + ), + resource.TestCheckResourceAttr( + "test_resource.foo", "list.0", "b", + ), + ), }, }, }) @@ -165,9 +251,6 @@ resource "test_resource" "foo" { } } `), - Check: func(s *terraform.State) error { - return nil - }, }, resource.TestStep{ Config: strings.TrimSpace(` diff --git a/terraform/diff.go b/terraform/diff.go index bd886ebea..7751794ac 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -454,10 +454,10 @@ func (d *InstanceDiff) Apply(attrs map[string]string, schema *configschema.Block return result, nil } - return d.blockDiff(nil, attrs, schema) + return d.applyBlockDiff(nil, attrs, schema) } -func (d *InstanceDiff) blockDiff(path []string, attrs map[string]string, schema *configschema.Block) (map[string]string, error) { +func (d *InstanceDiff) applyBlockDiff(path []string, attrs map[string]string, schema *configschema.Block) (map[string]string, error) { result := map[string]string{} name := "" @@ -564,7 +564,7 @@ func (d *InstanceDiff) blockDiff(path []string, attrs map[string]string, schema } for k := range candidateKeys { - newAttrs, err := d.blockDiff(append(path, n, k), attrs, &block.Block) + newAttrs, err := d.applyBlockDiff(append(path, n, k), attrs, &block.Block) if err != nil { return result, err } @@ -735,24 +735,37 @@ func (d *InstanceDiff) applyCollectionDiff(path []string, attrs map[string]strin } // collect all the keys from the diff and the old state + noDiff := true keys := map[string]bool{} for k := range d.Attributes { + if !strings.HasPrefix(k, currentKey+".") { + continue + } + noDiff = false keys[k] = true } + + noAttrs := true for k := range attrs { + if !strings.HasPrefix(k, currentKey+".") { + continue + } + noAttrs = false keys[k] = true } + // If there's no diff and no attrs, then there's no value at all. + // This prevents an unexpected zero-count attribute in the attributes. + if noDiff && noAttrs { + return result, nil + } + idx := "#" if attrSchema.Type.IsMapType() { idx = "%" } for k := range keys { - if !strings.HasPrefix(k, currentKey+".") { - continue - } - // generate an schema placeholder for the values elSchema := &configschema.Attribute{ Type: attrSchema.Type.ElementType(),