From 554cedab8a6ce7ba4ab1927ce11421e24aeeea39 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Thu, 15 Aug 2019 09:29:26 -0400
Subject: [PATCH] don't validate Min/Max block vals in CoerceValue

A provider may not have the data to fill in required block values in all
cases during the resource Read operation. This is more common in import,
because there is no initial configuration or state, and it's possible
some values are only provided in the configuration.

The original intent of MinItems and MaxItems in the schema was to
enforce configuration constraints, not to enforce what the resource
could save in the state.  Since the configuration is already statically
validated, and the Schema is validated against the configuration in a
separate step, we can drop these extra validation constraints in
CoerceValue and relax it to only ensure the types conform to what is
expected.
---
 configs/configschema/coerce_value.go      | 24 ++-----------------
 configs/configschema/coerce_value_test.go | 28 -----------------------
 2 files changed, 2 insertions(+), 50 deletions(-)

diff --git a/configs/configschema/coerce_value.go b/configs/configschema/coerce_value.go
index 7996c383a..23a48e1be 100644
--- a/configs/configschema/coerce_value.go
+++ b/configs/configschema/coerce_value.go
@@ -114,14 +114,6 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
 				}
 				l := coll.LengthInt()
 
-				// Assume that if there are unknowns this could have come from
-				// a dynamic block, and we can't validate MinItems yet.
-				if l < blockS.MinItems && coll.IsWhollyKnown() {
-					return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("insufficient items for attribute %q; must have at least %d", typeName, blockS.MinItems)
-				}
-				if l > blockS.MaxItems && blockS.MaxItems > 0 {
-					return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("too many items for attribute %q; cannot have more than %d", typeName, blockS.MaxItems)
-				}
 				if l == 0 {
 					attrs[typeName] = cty.ListValEmpty(blockS.ImpliedType())
 					continue
@@ -140,10 +132,8 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
 					}
 				}
 				attrs[typeName] = cty.ListVal(elems)
-			case blockS.MinItems == 0:
-				attrs[typeName] = cty.ListValEmpty(blockS.ImpliedType())
 			default:
-				return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("attribute %q is required", typeName)
+				attrs[typeName] = cty.ListValEmpty(blockS.ImpliedType())
 			}
 
 		case NestingSet:
@@ -165,14 +155,6 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
 				}
 				l := coll.LengthInt()
 
-				// Assume that if there are unknowns this could have come from
-				// a dynamic block, and we can't validate MinItems yet.
-				if l < blockS.MinItems && coll.IsWhollyKnown() {
-					return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("insufficient items for attribute %q; must have at least %d", typeName, blockS.MinItems)
-				}
-				if l > blockS.MaxItems && blockS.MaxItems > 0 {
-					return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("too many items for attribute %q; cannot have more than %d", typeName, blockS.MaxItems)
-				}
 				if l == 0 {
 					attrs[typeName] = cty.SetValEmpty(blockS.ImpliedType())
 					continue
@@ -191,10 +173,8 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
 					}
 				}
 				attrs[typeName] = cty.SetVal(elems)
-			case blockS.MinItems == 0:
-				attrs[typeName] = cty.SetValEmpty(blockS.ImpliedType())
 			default:
-				return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("attribute %q is required", typeName)
+				attrs[typeName] = cty.SetValEmpty(blockS.ImpliedType())
 			}
 
 		case NestingMap:
diff --git a/configs/configschema/coerce_value_test.go b/configs/configschema/coerce_value_test.go
index 7fd07856d..084107878 100644
--- a/configs/configschema/coerce_value_test.go
+++ b/configs/configschema/coerce_value_test.go
@@ -291,34 +291,6 @@ func TestCoerceValue(t *testing.T) {
 			cty.DynamicVal,
 			`attribute "foo" is required`,
 		},
-		"missing required list block": {
-			&Block{
-				BlockTypes: map[string]*NestedBlock{
-					"foo": {
-						Block:    Block{},
-						Nesting:  NestingList,
-						MinItems: 1,
-					},
-				},
-			},
-			cty.EmptyObjectVal,
-			cty.DynamicVal,
-			`attribute "foo" is required`,
-		},
-		"missing required set block": {
-			&Block{
-				BlockTypes: map[string]*NestedBlock{
-					"foo": {
-						Block:    Block{},
-						Nesting:  NestingList,
-						MinItems: 1,
-					},
-				},
-			},
-			cty.EmptyObjectVal,
-			cty.DynamicVal,
-			`attribute "foo" is required`,
-		},
 		"unknown nested list": {
 			&Block{
 				Attributes: map[string]*Attribute{