From da389d6cd4d586397eedda9375da0c0eca1a1dd4 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 14 Feb 2019 12:32:42 -0500 Subject: [PATCH] simple list diffs may also have missing elements Like was done for list blocks, simple lists of strings may be missing empty string elements, and any list may be implicitly truncated. --- builtin/providers/test/resource.go | 2 +- builtin/providers/test/resource_list.go | 7 +++ builtin/providers/test/resource_list_test.go | 58 +++++++++++++++++++ terraform/diff.go | 59 ++++++++++++++++++-- 4 files changed, 121 insertions(+), 5 deletions(-) diff --git a/builtin/providers/test/resource.go b/builtin/providers/test/resource.go index 490e539f9..79c5b4a45 100644 --- a/builtin/providers/test/resource.go +++ b/builtin/providers/test/resource.go @@ -171,7 +171,7 @@ func testResourceUpdate(d *schema.ResourceData, meta interface{}) error { if errMsg != "" { return errors.New(errMsg) } - return nil + return testResourceRead(d, meta) } func testResourceDelete(d *schema.ResourceData, meta interface{}) error { diff --git a/builtin/providers/test/resource_list.go b/builtin/providers/test/resource_list.go index 033e5f40c..1d8424293 100644 --- a/builtin/providers/test/resource_list.go +++ b/builtin/providers/test/resource_list.go @@ -30,6 +30,13 @@ func testResourceList() *schema.Resource { Optional: true, ForceNew: true, }, + "sublist": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, "sublist_block": { Type: schema.TypeList, Optional: true, diff --git a/builtin/providers/test/resource_list_test.go b/builtin/providers/test/resource_list_test.go index c1d8eca61..daa534f0f 100644 --- a/builtin/providers/test/resource_list_test.go +++ b/builtin/providers/test/resource_list_test.go @@ -218,3 +218,61 @@ resource "test_resource_list" "foo" { }, }) } + +func TestResourceList_emptyStrings(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_list" "foo" { + list_block { + sublist = ["a", ""] + } + + list_block { + sublist = [""] + } + + list_block { + sublist = ["", "c", ""] + } +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.0.sublist.0", "a"), + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.0.sublist.1", ""), + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.1.sublist.0", ""), + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.2.sublist.0", ""), + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.2.sublist.1", "c"), + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.2.sublist.2", ""), + ), + }, + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_list" "foo" { + list_block { + sublist = [""] + } + + list_block { + sublist = [] + } + + list_block { + sublist = ["", "c"] + } +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.0.sublist.#", "1"), + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.0.sublist.0", ""), + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.1.sublist.#", "0"), + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.2.sublist.1", "c"), + resource.TestCheckResourceAttr("test_resource_list.foo", "list_block.2.sublist.#", "2"), + ), + }, + }, + }) +} diff --git a/terraform/diff.go b/terraform/diff.go index e68f3d81e..3f26117a8 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -624,7 +624,7 @@ func (d *InstanceDiff) applyBlockDiff(path []string, attrs map[string]string, sc if err != nil { // this shouldn't happen since we added these // ourself, but make note of it just in case. - log.Printf("[ERROR] bas list index in %q: %s", k, err) + log.Printf("[ERROR] bad list index in %q: %s", k, err) continue } if i >= length { @@ -811,10 +811,61 @@ func (d *InstanceDiff) applyCollectionDiff(path []string, attrs map[string]strin } } - // Fill in the count value if it was missing for some reason: - if result[name+"."+idx] == "" { - result[name+"."+idx] = countFlatmapContainerValues(name+"."+idx, result) + // Just like in nested list blocks, for simple lists we may need to fill in + // missing empty strings. + countKey := name + "." + idx + count := result[countKey] + length, _ := strconv.Atoi(count) + + if count != "" && count != hcl2shim.UnknownVariableValue && + attrSchema.Type.Equals(cty.List(cty.String)) { + // insert empty strings into missing indexes + for i := 0; i < length; i++ { + key := fmt.Sprintf("%s.%d", name, i) + if _, ok := result[key]; !ok { + result[key] = "" + } + } } + + // now check for truncation in any type of list + if attrSchema.Type.IsListType() { + for key := range result { + if key == countKey { + continue + } + + if len(key) <= len(name)+1 { + // not sure what this is, but don't panic + continue + } + + index := key[len(name)+1:] + + // It is possible to have nested sets or maps, so look for another dot + dot := strings.Index(index, ".") + if dot > 0 { + index = index[:dot] + } + + // This shouldn't have any more dots, since the element type is only string. + num, err := strconv.Atoi(index) + if err != nil { + log.Printf("[ERROR] bad list index in %q: %s", currentKey, err) + continue + } + + if num >= length { + delete(result, key) + } + } + } + + // Fill in the count value if it was missing for some reason: + if result[countKey] == "" { + result[countKey] = countFlatmapContainerValues(countKey, result) + } + return result, nil }