From fe11724678289a94c69af9101b45e4640be615b1 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 25 Jun 2019 13:37:55 -0400 Subject: [PATCH 1/2] test for panic when readin empty map --- builtin/providers/test/resource_list.go | 5 ++++ builtin/providers/test/resource_list_test.go | 31 ++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/builtin/providers/test/resource_list.go b/builtin/providers/test/resource_list.go index a8b871369..895298ebb 100644 --- a/builtin/providers/test/resource_list.go +++ b/builtin/providers/test/resource_list.go @@ -137,6 +137,11 @@ func testResourceList() *schema.Resource { }, }, }, + "map_list": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeMap}, + }, }, } } diff --git a/builtin/providers/test/resource_list_test.go b/builtin/providers/test/resource_list_test.go index 00ec5491c..876a81fd5 100644 --- a/builtin/providers/test/resource_list_test.go +++ b/builtin/providers/test/resource_list_test.go @@ -103,6 +103,37 @@ resource "test_resource_list" "foo" { }) } +func TestResourceList_mapList(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +variable "map" { + type = map(string) + default = {} +} + +resource "test_resource_list" "foo" { + map_list = [ + { + a = "1" + }, + var.map + ] +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource_list.foo", "map_list.1", "", + ), + ), + }, + }, + }) +} + func TestResourceList_sublist(t *testing.T) { resource.UnitTest(t, resource.TestCase{ Providers: testAccProviders, From cd3ac50ddb8eb655a4596ff3ff2a5356a0296342 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 25 Jun 2019 14:12:01 -0400 Subject: [PATCH 2/2] prevent sdk panics in 2 specific cases Fix 2 specific panics in the sdk when reading nil or computed maps from various configurations. The legacy sdk code is too dependent on undefined behavior to attempt to find and fix the root cause at this point. Since the code is essentially frozen for future development, these changes are specifically targeted to only prevent panics from within providers. Because any code effected by these changes would have panicked, there cannot be anything depending on the behavior, and these should be safe to fix. --- helper/schema/field_reader_config.go | 3 +++ helper/schema/field_reader_diff.go | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/helper/schema/field_reader_config.go b/helper/schema/field_reader_config.go index 808375ceb..6ad3f13cb 100644 --- a/helper/schema/field_reader_config.go +++ b/helper/schema/field_reader_config.go @@ -219,6 +219,9 @@ func (r *ConfigFieldReader) readMap(k string, schema *Schema) (FieldReadResult, v, _ := r.Config.Get(key) result[ik] = v } + case nil: + // the map may have been empty on the configuration, so we leave the + // empty result default: panic(fmt.Sprintf("unknown type: %#v", mraw)) } diff --git a/helper/schema/field_reader_diff.go b/helper/schema/field_reader_diff.go index ae35b4a87..3e70acf0b 100644 --- a/helper/schema/field_reader_diff.go +++ b/helper/schema/field_reader_diff.go @@ -95,7 +95,9 @@ func (r *DiffFieldReader) readMap( return FieldReadResult{}, err } if source.Exists { - result = source.Value.(map[string]interface{}) + // readMap may return a nil value, or an unknown value placeholder in + // some cases, causing the type assertion to panic if we don't assign the ok value + result, _ = source.Value.(map[string]interface{}) resultSet = true }