more precise handling of ComputedKeys in config

With the new ConfigModeAttr, we can now have complex structures come in
as attributes rather than blocks. Previously attributes were either
known, or unknown, and there was no reason to descend into them. We now
need to record the complete path to unknown values within complex
attributes to create a proper diff after shimming the 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{