From ae0be75ae09efb191ecd22f27389b16521c771fb Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Thu, 24 Jan 2019 17:41:30 -0800
Subject: [PATCH] helper/schema: TypeMap of Resource is actually of TypeString

Historically helper/schema did not support non-primitive map attributes
because they cannot be represented unambiguously in flatmap. When we
initially implemented CoreConfigSchema here we mapped that situation to
a nested block of mode NestingMap, even though that'd never worked until
now, assuming that it'd be harmless because providers wouldn't be using
it.

It turns out that some providers are, in fact, incorrectly populating
a TypeMap schema with Elem: &schema.Resource, apparently under the false
assumption that it would constrain the keys allowed in the map. In
practice, helper/schema has just been ignoring this and treating such
attributes as map of string. (#20076)

In order to preserve the behavior of these existing incorrectly-specified
attribute definitions, here we mimic the helper/schema behavior by
presenting as an attribute of type map(string).

These attributes have also been shown in some documentation as nested
blocks (with no equals sign), so that'll need to be fixed in user
configurations as they upgrade to Terraform 0.12. However, the existing
upgrade tool rules will take care of that as a natural consequence of the
name being indicated as an attribute in the schema, rather than as a block
type.

This fixes #20076.
---
 helper/schema/core_schema.go      | 15 +++++++++++++++
 helper/schema/core_schema_test.go | 17 ++++++++++++-----
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/helper/schema/core_schema.go b/helper/schema/core_schema.go
index 20146ec12..727b164b4 100644
--- a/helper/schema/core_schema.go
+++ b/helper/schema/core_schema.go
@@ -39,6 +39,21 @@ func (m schemaMap) CoreConfigSchema() *configschema.Block {
 			ret.Attributes[name] = schema.coreConfigSchemaAttribute()
 			continue
 		}
+		if schema.Type == TypeMap {
+			// For TypeMap in particular, it isn't valid for Elem to be a
+			// *Resource (since that would be ambiguous in flatmap) and
+			// so Elem is treated as a TypeString schema if so. This matches
+			// how the field readers treat this situation, for compatibility
+			// with configurations targeting Terraform 0.11 and earlier.
+			if _, isResource := schema.Elem.(*Resource); isResource {
+				sch := *schema // shallow copy
+				sch.Elem = &Schema{
+					Type: TypeString,
+				}
+				ret.Attributes[name] = sch.coreConfigSchemaAttribute()
+				continue
+			}
+		}
 		switch schema.Elem.(type) {
 		case *Schema, ValueType:
 			ret.Attributes[name] = schema.coreConfigSchemaAttribute()
diff --git a/helper/schema/core_schema_test.go b/helper/schema/core_schema_test.go
index d07c897e2..b20f164e8 100644
--- a/helper/schema/core_schema_test.go
+++ b/helper/schema/core_schema_test.go
@@ -204,7 +204,18 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) {
 				},
 			},
 			testResource(&configschema.Block{
-				Attributes: map[string]*configschema.Attribute{},
+				Attributes: map[string]*configschema.Attribute{
+					// This one becomes a string attribute because helper/schema
+					// doesn't actually support maps of resource. The given
+					// "Elem" is just ignored entirely here, which is important
+					// because that is also true of the helper/schema logic and
+					// existing providers rely on this being ignored for
+					// correct operation.
+					"map": {
+						Type:     cty.Map(cty.String),
+						Optional: true,
+					},
+				},
 				BlockTypes: map[string]*configschema.NestedBlock{
 					"list": {
 						Nesting:  configschema.NestingList,
@@ -217,10 +228,6 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) {
 						Block:    configschema.Block{},
 						MinItems: 1, // because schema is Required
 					},
-					"map": {
-						Nesting: configschema.NestingMap,
-						Block:   configschema.Block{},
-					},
 				},
 			}),
 		},