core + provider/terraform: Fix outputs in remote state

builtin/providers/terraform/data_source_state.go

@@ -13,19 +13,19 @@ func dataSourceRemoteState() *schema.Resource {
 		Read: dataSourceRemoteStateRead,
 
 		Schema: map[string]*schema.Schema{
-			"backend": &schema.Schema{
+			"backend": {
 				Type:     schema.TypeString,
 				Required: true,
 			},
 
-			"config": &schema.Schema{
+			"config": {
 				Type:     schema.TypeMap,
 				Optional: true,
 			},
 
-			"output": &schema.Schema{
-				Type:     schema.TypeMap,
-				Computed: true,
+			"__has_dynamic_attributes": {
+				Type:     schema.TypeString,
+				Optional: true,
 			},
 		},
 	}
@@ -52,16 +52,17 @@ func dataSourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}
 
-	var outputs map[string]interface{}
-	if !state.State().Empty() {
-		outputValueMap := make(map[string]string)
-		for key, output := range state.State().RootModule().Outputs {
-			//This is ok for 0.6.17 as outputs will have been strings
-			outputValueMap[key] = output.Value.(string)
-		}
+	d.SetId(time.Now().UTC().String())
+
+	outputMap := make(map[string]interface{})
+	for key, val := range state.State().RootModule().Outputs {
+		outputMap[key] = val.Value
 	}
 
-	d.SetId(time.Now().UTC().String())
-	d.Set("output", outputs)
+	mappedOutputs := remoteStateFlatten(outputMap)
+
+	for key, val := range mappedOutputs {
+		d.UnsafeSetFieldRaw(key, val)
+	}
 	return nil
 }

builtin/providers/terraform/data_source_state_test.go

@@ -8,12 +8,13 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func TestAccState_basic(t *testing.T) {
+func TestState_basic(t *testing.T) {
 	resource.Test(t, resource.TestCase{
-		PreCheck:  func() { testAccPreCheck(t) },
-		Providers: testAccProviders,
+		OverrideEnvVar: true,
+		PreCheck:       func() { testAccPreCheck(t) },
+		Providers:      testAccProviders,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccState_basic,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckStateValue(
@@ -24,6 +25,26 @@ func TestAccState_basic(t *testing.T) {
 	})
 }
 
+func TestState_complexOutputs(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		OverrideEnvVar: true,
+		PreCheck:       func() { testAccPreCheck(t) },
+		Providers:      testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccState_complexOutputs,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStateValue("terraform_remote_state.foo", "backend", "_local"),
+					testAccCheckStateValue("terraform_remote_state.foo", "config.path", "./test-fixtures/complex_outputs.tfstate"),
+					testAccCheckStateValue("terraform_remote_state.foo", "computed_set.#", "2"),
+					testAccCheckStateValue("terraform_remote_state.foo", `map.%`, "2"),
+					testAccCheckStateValue("terraform_remote_state.foo", `map.key`, "test"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckStateValue(id, name, value string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[id]
@@ -34,7 +55,7 @@ func testAccCheckStateValue(id, name, value string) resource.TestCheckFunc {
 			return fmt.Errorf("No ID is set")
 		}
 
-		v := rs.Primary.Attributes["output."+name]
+		v := rs.Primary.Attributes[name]
 		if v != value {
 			return fmt.Errorf(
 				"Value for %s is %s, not %s", name, v, value)
@@ -52,3 +73,12 @@ resource "terraform_remote_state" "foo" {
 		path = "./test-fixtures/basic.tfstate"
 	}
 }`
+
+const testAccState_complexOutputs = `
+resource "terraform_remote_state" "foo" {
+	backend = "_local"
+
+	config {
+		path = "./test-fixtures/complex_outputs.tfstate"
+	}
+}`

builtin/providers/terraform/flatten.go

@@ -0,0 +1,76 @@
+package terraform
+
+import (
+	"fmt"
+	"reflect"
+)
+
+// remoteStateFlatten takes a structure and turns into a flat map[string]string.
+//
+// Within the "thing" parameter, only primitive values are allowed. Structs are
+// not supported. Therefore, it can only be slices, maps, primitives, and
+// any combination of those together.
+//
+// The difference between this version and the version in package flatmap is that
+// we add the count key for maps in this version, and return a normal
+// map[string]string instead of a flatmap.Map
+func remoteStateFlatten(thing map[string]interface{}) map[string]string {
+	result := make(map[string]string)
+
+	for k, raw := range thing {
+		flatten(result, k, reflect.ValueOf(raw))
+	}
+
+	return result
+}
+
+func flatten(result map[string]string, prefix string, v reflect.Value) {
+	if v.Kind() == reflect.Interface {
+		v = v.Elem()
+	}
+
+	switch v.Kind() {
+	case reflect.Bool:
+		if v.Bool() {
+			result[prefix] = "true"
+		} else {
+			result[prefix] = "false"
+		}
+	case reflect.Int:
+		result[prefix] = fmt.Sprintf("%d", v.Int())
+	case reflect.Map:
+		flattenMap(result, prefix, v)
+	case reflect.Slice:
+		flattenSlice(result, prefix, v)
+	case reflect.String:
+		result[prefix] = v.String()
+	default:
+		panic(fmt.Sprintf("Unknown: %s", v))
+	}
+}
+
+func flattenMap(result map[string]string, prefix string, v reflect.Value) {
+	mapKeys := v.MapKeys()
+
+	result[fmt.Sprintf("%s.%%", prefix)] = fmt.Sprintf("%d", len(mapKeys))
+	for _, k := range mapKeys {
+		if k.Kind() == reflect.Interface {
+			k = k.Elem()
+		}
+
+		if k.Kind() != reflect.String {
+			panic(fmt.Sprintf("%s: map key is not string: %s", prefix, k))
+		}
+
+		flatten(result, fmt.Sprintf("%s.%s", prefix, k.String()), v.MapIndex(k))
+	}
+}
+
+func flattenSlice(result map[string]string, prefix string, v reflect.Value) {
+	prefix = prefix + "."
+
+	result[prefix+"#"] = fmt.Sprintf("%d", v.Len())
+	for i := 0; i < v.Len(); i++ {
+		flatten(result, fmt.Sprintf("%s%d", prefix, i), v.Index(i))
+	}
+}

builtin/providers/terraform/test-fixtures/basic.tfstate

@@ -1,4 +1,5 @@
 {
+    "version": 1,
     "modules": [{
         "path": ["root"],
         "outputs": { "foo": "bar" }

builtin/providers/terraform/test-fixtures/complex_outputs.tfstate

@@ -0,0 +1,88 @@
+{
+    "version": 3,
+    "terraform_version": "0.7.0",
+    "serial": 3,
+    "modules": [
+        {
+            "path": [
+                "root"
+            ],
+            "outputs": {
+                "computed_map": {
+                    "sensitive": false,
+                    "type": "map",
+                    "value": {
+                        "key1": "value1"
+                    }
+                },
+                "computed_set": {
+                    "sensitive": false,
+                    "type": "list",
+                    "value": [
+                        "setval1",
+                        "setval2"
+                    ]
+                },
+                "map": {
+                    "sensitive": false,
+                    "type": "map",
+                    "value": {
+                        "key": "test",
+                        "test": "test"
+                    }
+                },
+                "set": {
+                    "sensitive": false,
+                    "type": "list",
+                    "value": [
+                        "test1",
+                        "test2"
+                    ]
+                }
+            },
+            "resources": {
+                "test_resource.main": {
+                    "type": "test_resource",
+                    "primary": {
+                        "id": "testId",
+                        "attributes": {
+                            "computed_list.#": "2",
+                            "computed_list.0": "listval1",
+                            "computed_list.1": "listval2",
+                            "computed_map.%": "1",
+                            "computed_map.key1": "value1",
+                            "computed_read_only": "value_from_api",
+                            "computed_read_only_force_new": "value_from_api",
+                            "computed_set.#": "2",
+                            "computed_set.2337322984": "setval1",
+                            "computed_set.307881554": "setval2",
+                            "id": "testId",
+                            "list_of_map.#": "2",
+                            "list_of_map.0.%": "2",
+                            "list_of_map.0.key1": "value1",
+                            "list_of_map.0.key2": "value2",
+                            "list_of_map.1.%": "2",
+                            "list_of_map.1.key3": "value3",
+                            "list_of_map.1.key4": "value4",
+                            "map.%": "2",
+                            "map.key": "test",
+                            "map.test": "test",
+                            "map_that_look_like_set.%": "2",
+                            "map_that_look_like_set.12352223": "hello",
+                            "map_that_look_like_set.36234341": "world",
+                            "optional_computed_map.%": "0",
+                            "required": "Hello World",
+                            "required_map.%": "3",
+                            "required_map.key1": "value1",
+                            "required_map.key2": "value2",
+                            "required_map.key3": "value3",
+                            "set.#": "2",
+                            "set.2326977762": "test1",
+                            "set.331058520": "test2"
+                        }
+                    }
+                }
+            }
+        }
+    ]
+}

helper/schema/field_writer_map.go

@@ -29,6 +29,16 @@ func (w *MapFieldWriter) Map() map[string]string {
 	return w.result
 }
 
+func (w *MapFieldWriter) unsafeWriteField(addr string, value string) {
+	w.lock.Lock()
+	defer w.lock.Unlock()
+	if w.result == nil {
+		w.result = make(map[string]string)
+	}
+
+	w.result[addr] = value
+}
+
 func (w *MapFieldWriter) WriteField(addr []string, value interface{}) error {
 	w.lock.Lock()
 	defer w.lock.Unlock()

helper/schema/resource_data.go

@@ -1,6 +1,7 @@
 package schema
 
 import (
+	"log"
 	"reflect"
 	"strings"
 	"sync"
@@ -44,7 +45,14 @@ type getResult struct {
 	Schema         *Schema
 }
 
-var getResultEmpty getResult
+// UnsafeSetFieldRaw allows setting arbitrary values in state to arbitrary
+// values, bypassing schema. This MUST NOT be used in normal circumstances -
+// it exists only to support the remote_state data source.
+func (d *ResourceData) UnsafeSetFieldRaw(key string, value string) {
+	d.once.Do(d.init)
+
+	d.setWriter.unsafeWriteField(key, value)
+}
 
 // Get returns the data for the given key, or nil if the key doesn't exist
 // in the schema.
@@ -242,6 +250,17 @@ func (d *ResourceData) State() *terraform.InstanceState {
 		return nil
 	}
 
+	// Look for a magic key in the schema that determines we skip the
+	// integrity check of fields existing in the schema, allowing dynamic
+	// keys to be created.
+	hasDynamicAttributes := false
+	for k, _ := range d.schema {
+		if k == "__has_dynamic_attributes" {
+			hasDynamicAttributes = true
+			log.Printf("[INFO] Resource %s has dynamic attributes", result.ID)
+		}
+	}
+
 	// In order to build the final state attributes, we read the full
 	// attribute set as a map[string]interface{}, write it to a MapFieldWriter,
 	// and then use that map.
@@ -263,12 +282,27 @@ func (d *ResourceData) State() *terraform.InstanceState {
 			}
 		}
 	}
+
 	mapW := &MapFieldWriter{Schema: d.schema}
 	if err := mapW.WriteField(nil, rawMap); err != nil {
 		return nil
 	}
 
 	result.Attributes = mapW.Map()
+
+	if hasDynamicAttributes {
+		// If we have dynamic attributes, just copy the attributes map
+		// one for one into the result attributes.
+		for k, v := range d.setWriter.Map() {
+			// Don't clobber schema values. This limits usage of dynamic
+			// attributes to names which _do not_ conflict with schema
+			// keys!
+			if _, ok := result.Attributes[k]; !ok {
+				result.Attributes[k] = v
+			}
+		}
+	}
+
 	if d.newState != nil {
 		result.Ephemeral = d.newState.Ephemeral
 	}