Merge pull request #21209 from hashicorp/jbardin/computeds-in-config-shim

more precise handling of ComputedKeys in config

terraform/resource.go

@@ -2,7 +2,6 @@ package terraform
 
 import (
 	"fmt"
-	"log"
 	"reflect"
 	"sort"
 	"strconv"
@@ -236,7 +235,7 @@ func NewResourceConfigShimmed(val cty.Value, schema *configschema.Block) *Resour
 		// schema here so that we can preserve the expected invariant
 		// that an attribute is always either wholly known or wholly unknown, while
 		// a child block can be partially unknown.
-		ret.ComputedKeys = newResourceConfigShimmedComputedKeys(val, schema, "")
+		ret.ComputedKeys = newResourceConfigShimmedComputedKeys(val, "")
 	} else {
 		ret.Config = make(map[string]interface{})
 	}
@@ -245,72 +244,45 @@ func NewResourceConfigShimmed(val cty.Value, schema *configschema.Block) *Resour
 	return ret
 }
 
-// newResourceConfigShimmedComputedKeys finds all of the unknown values in the
+// Record the any config values in ComputedKeys. This field had been unused in
-// given object, which must conform to the given schema, returning them in
+// helper/schema, but in the new protocol we're using this so that the SDK can
-// the format that's expected for ResourceConfig.ComputedKeys.
+// now handle having an unknown collection. The legacy diff code doesn't
-func newResourceConfigShimmedComputedKeys(obj cty.Value, schema *configschema.Block, prefix string) []string {
+// properly handle the unknown, because it can't be expressed in the same way
+// between the config and diff.
+func newResourceConfigShimmedComputedKeys(val cty.Value, path string) []string {
 	var ret []string
-	ty := obj.Type()
+	ty := val.Type()
 
-	if schema == nil {
+	if val.IsNull() {
-		log.Printf("[WARN] NewResourceConfigShimmed: can't identify computed keys because no schema is available")
+		return ret
-		return nil
 	}
 
-	for attrName := range schema.Attributes {
+	if !val.IsKnown() {
-		if !ty.HasAttribute(attrName) {
+		// we shouldn't have an entirely unknown resource, but prevent empty
-			// Should never happen, but we'll tolerate it anyway
+		// strings just in case
-			continue
+		if len(path) > 0 {
-		}
+			ret = append(ret, path)
-
-		attrVal := obj.GetAttr(attrName)
-		if !attrVal.IsWhollyKnown() {
-			ret = append(ret, prefix+attrName)
 		}
+		return ret
 	}
 
-	for typeName, blockS := range schema.BlockTypes {
+	if path != "" {
-		if !ty.HasAttribute(typeName) {
+		path += "."
-			// Should never happen, but we'll tolerate it anyway
+	}
-			continue
+	switch {
-		}
+	case ty.IsListType(), ty.IsTupleType(), ty.IsSetType():
-
+		i := 0
-		blockVal := obj.GetAttr(typeName)
+		for it := val.ElementIterator(); it.Next(); i++ {
-		if blockVal.IsNull() || !blockVal.IsKnown() {
+			_, subVal := it.Element()
-			continue
+			keys := newResourceConfigShimmedComputedKeys(subVal, fmt.Sprintf("%s%d", path, i))
-		}
+			ret = append(ret, keys...)
-
+		}
-		switch blockS.Nesting {
+
-		case configschema.NestingSingle, configschema.NestingGroup:
+	case ty.IsMapType(), ty.IsObjectType():
-			keys := newResourceConfigShimmedComputedKeys(blockVal, &blockS.Block, fmt.Sprintf("%s%s.", prefix, typeName))
+		for it := val.ElementIterator(); it.Next(); {
+			subK, subVal := it.Element()
+			keys := newResourceConfigShimmedComputedKeys(subVal, fmt.Sprintf("%s%s", path, subK.AsString()))
 			ret = append(ret, keys...)
-		case configschema.NestingList, configschema.NestingSet:
-			// Producing computed keys items for sets is not really useful
-			// since they are not usefully addressable anyway, but we'll treat
-			// them like lists just so that ret.ComputedKeys accounts for them
-			// all. Our legacy system didn't support sets here anyway, so
-			// treating them as lists is the most accurate translation. Although
-			// set traversal isn't in any particular order, it is _stable_ as
-			// long as the list isn't mutated, and so we know we'll see the
-			// same order here as hcl2shim.ConfigValueFromHCL2 would've seen
-			// inside NewResourceConfigShimmed above.
-			i := 0
-			for it := blockVal.ElementIterator(); it.Next(); i++ {
-				_, subVal := it.Element()
-				subPrefix := fmt.Sprintf("%s%s.%d.", prefix, typeName, i)
-				keys := newResourceConfigShimmedComputedKeys(subVal, &blockS.Block, subPrefix)
-				ret = append(ret, keys...)
-			}
-		case configschema.NestingMap:
-			for it := blockVal.ElementIterator(); it.Next(); {
-				subK, subVal := it.Element()
-				subPrefix := fmt.Sprintf("%s%s.%s.", prefix, typeName, subK.AsString())
-				keys := newResourceConfigShimmedComputedKeys(subVal, &blockS.Block, subPrefix)
-				ret = append(ret, keys...)
-			}
-		default:
-			// Should never happen, since the above is exhaustive.
-			panic(fmt.Errorf("unsupported block nesting type %s", blockS.Nesting))
 		}
 	}
 

terraform/resource_test.go

@@ -919,6 +919,7 @@ func TestNewResourceConfigShimmed(t *testing.T) {
 				},
 			},
 			Expected: &ResourceConfig{
+				ComputedKeys: []string{"bar", "baz"},
 				Raw: map[string]interface{}{
 					"bar": config.UnknownVariableValue,
 					"baz": config.UnknownVariableValue,
@@ -981,6 +982,115 @@ func TestNewResourceConfigShimmed(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name: "unknown in set",
+			Val: cty.ObjectVal(map[string]cty.Value{
+				"bar": cty.SetVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"val": cty.UnknownVal(cty.String),
+					}),
+				}),
+			}),
+			Schema: &configschema.Block{
+				BlockTypes: map[string]*configschema.NestedBlock{
+					"bar": {
+						Block: configschema.Block{
+							Attributes: map[string]*configschema.Attribute{
+								"val": {
+									Type:     cty.String,
+									Optional: true,
+								},
+							},
+						},
+						Nesting: configschema.NestingSet,
+					},
+				},
+			},
+			Expected: &ResourceConfig{
+				ComputedKeys: []string{"bar.0.val"},
+				Raw: map[string]interface{}{
+					"bar": []interface{}{map[string]interface{}{
+						"val": "74D93920-ED26-11E3-AC10-0800200C9A66",
+					}},
+				},
+				Config: map[string]interface{}{
+					"bar": []interface{}{map[string]interface{}{
+						"val": "74D93920-ED26-11E3-AC10-0800200C9A66",
+					}},
+				},
+			},
+		},
+		{
+			Name: "unknown in attribute sets",
+			Val: cty.ObjectVal(map[string]cty.Value{
+				"bar": cty.SetVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"val": cty.UnknownVal(cty.String),
+					}),
+				}),
+				"baz": cty.SetVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"obj": cty.UnknownVal(cty.Object(map[string]cty.Type{
+							"attr": cty.List(cty.String),
+						})),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"obj": cty.ObjectVal(map[string]cty.Value{
+							"attr": cty.UnknownVal(cty.List(cty.String)),
+						}),
+					}),
+				}),
+			}),
+			Schema: &configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"bar": &configschema.Attribute{
+						Type: cty.Set(cty.Object(map[string]cty.Type{
+							"val": cty.String,
+						})),
+					},
+					"baz": &configschema.Attribute{
+						Type: cty.Set(cty.Object(map[string]cty.Type{
+							"obj": cty.Object(map[string]cty.Type{
+								"attr": cty.List(cty.String),
+							}),
+						})),
+					},
+				},
+			},
+			Expected: &ResourceConfig{
+				ComputedKeys: []string{"bar.0.val", "baz.0.obj.attr", "baz.1.obj"},
+				Raw: map[string]interface{}{
+					"bar": []interface{}{map[string]interface{}{
+						"val": "74D93920-ED26-11E3-AC10-0800200C9A66",
+					}},
+					"baz": []interface{}{
+						map[string]interface{}{
+							"obj": map[string]interface{}{
+								"attr": "74D93920-ED26-11E3-AC10-0800200C9A66",
+							},
+						},
+						map[string]interface{}{
+							"obj": "74D93920-ED26-11E3-AC10-0800200C9A66",
+						},
+					},
+				},
+				Config: map[string]interface{}{
+					"bar": []interface{}{map[string]interface{}{
+						"val": "74D93920-ED26-11E3-AC10-0800200C9A66",
+					}},
+					"baz": []interface{}{
+						map[string]interface{}{
+							"obj": map[string]interface{}{
+								"attr": "74D93920-ED26-11E3-AC10-0800200C9A66",
+							},
+						},
+						map[string]interface{}{
+							"obj": "74D93920-ED26-11E3-AC10-0800200C9A66",
+						},
+					},
+				},
+			},
+		},
 		{
 			Name: "null blocks",
 			Val: cty.ObjectVal(map[string]cty.Value{