Merge pull request #12210 from hashicorp/b-computed-elem

flatmap: mark computed list as a computed value in Expand

builtin/providers/test/provider.go

@@ -8,7 +8,8 @@ import (
 func Provider() terraform.ResourceProvider {
 	return &schema.Provider{
 		ResourcesMap: map[string]*schema.Resource{
-			"test_resource": testResource(),
+			"test_resource":         testResource(),
+			"test_resource_gh12183": testResourceGH12183(),
 		},
 		DataSourcesMap: map[string]*schema.Resource{
 			"test_data_source": testDataSource(),

builtin/providers/test/resource_gh12183.go

@@ -0,0 +1,64 @@
+package test
+
+import (
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+// This is a test resource to help reproduce GH-12183. This issue came up
+// as a complex mixing of core + helper/schema and while we added core tests
+// to cover some of the cases, this test helps top it off with an end-to-end
+// test.
+func testResourceGH12183() *schema.Resource {
+	return &schema.Resource{
+		Create: testResourceCreate_gh12183,
+		Read:   testResourceRead_gh12183,
+		Update: testResourceUpdate_gh12183,
+		Delete: testResourceDelete_gh12183,
+		Schema: map[string]*schema.Schema{
+			"key": &schema.Schema{
+				Type:     schema.TypeString,
+				Optional: true,
+			},
+
+			"config": &schema.Schema{
+				Type:     schema.TypeList,
+				Optional: true,
+				ForceNew: true,
+				MinItems: 1,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"name": {
+							Type:     schema.TypeString,
+							Required: true,
+						},
+
+						"rules": {
+							Type:     schema.TypeSet,
+							Computed: true,
+							Elem:     &schema.Schema{Type: schema.TypeString},
+							Set:      schema.HashString,
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+func testResourceCreate_gh12183(d *schema.ResourceData, meta interface{}) error {
+	d.SetId("testId")
+	return testResourceRead(d, meta)
+}
+
+func testResourceRead_gh12183(d *schema.ResourceData, meta interface{}) error {
+	return nil
+}
+
+func testResourceUpdate_gh12183(d *schema.ResourceData, meta interface{}) error {
+	return nil
+}
+
+func testResourceDelete_gh12183(d *schema.ResourceData, meta interface{}) error {
+	d.SetId("")
+	return nil
+}

builtin/providers/test/resource_gh12183_test.go

@@ -0,0 +1,37 @@
+package test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/terraform/helper/resource"
+	"github.com/hashicorp/terraform/terraform"
+)
+
+// Tests GH-12183. This would previously cause a crash. More granular
+// unit tests are scattered through helper/schema and terraform core for
+// this.
+func TestResourceGH12183_basic(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_gh12183" "a" {
+	config {
+		name = "hello"
+	}
+}
+
+resource "test_resource_gh12183" "b" {
+	key = "${lookup(test_resource_gh12183.a.config[0], "name")}"
+}
+				`),
+				Check: func(s *terraform.State) error {
+					return nil
+				},
+			},
+		},
+	})
+}

flatmap/expand.go

@@ -5,6 +5,8 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+
+	"github.com/hashicorp/hil"
 )
 
 // Expand takes a map and a key (prefix) and expands that value into
@@ -22,7 +24,14 @@ func Expand(m map[string]string, key string) interface{} {
 	}
 
 	// Check if the key is an array, and if so, expand the array
-	if _, ok := m[key+".#"]; ok {
+	if v, ok := m[key+".#"]; ok {
+		// If the count of the key is unknown, then just put the unknown
+		// value in the value itself. This will be detected by Terraform
+		// core later.
+		if v == hil.UnknownValue {
+			return v
+		}
+
 		return expandArray(m, key)
 	}
 

flatmap/expand_test.go

@@ -3,6 +3,8 @@ package flatmap
 import (
 	"reflect"
 	"testing"
+
+	"github.com/hashicorp/hil"
 )
 
 func TestExpand(t *testing.T) {
@@ -117,6 +119,21 @@ func TestExpand(t *testing.T) {
 			Key:    "set",
 			Output: []interface{}{"a", "b", "c"},
 		},
+
+		{
+			Map: map[string]string{
+				"struct.#":         "1",
+				"struct.0.name":    "hello",
+				"struct.0.rules.#": hil.UnknownValue,
+			},
+			Key: "struct",
+			Output: []interface{}{
+				map[string]interface{}{
+					"name":  "hello",
+					"rules": hil.UnknownValue,
+				},
+			},
+		},
 	}
 
 	for _, tc := range cases {

helper/schema/schema_test.go

@@ -861,6 +861,65 @@ func TestSchemaMap_Diff(t *testing.T) {
 			Err: false,
 		},
 
+		{
+			Name: "List with computed set",
+			Schema: map[string]*Schema{
+				"config": &Schema{
+					Type:     TypeList,
+					Optional: true,
+					ForceNew: true,
+					MinItems: 1,
+					Elem: &Resource{
+						Schema: map[string]*Schema{
+							"name": {
+								Type:     TypeString,
+								Required: true,
+							},
+
+							"rules": {
+								Type:     TypeSet,
+								Computed: true,
+								Elem:     &Schema{Type: TypeString},
+								Set:      HashString,
+							},
+						},
+					},
+				},
+			},
+
+			State: nil,
+
+			Config: map[string]interface{}{
+				"config": []interface{}{
+					map[string]interface{}{
+						"name": "hello",
+					},
+				},
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"config.#": &terraform.ResourceAttrDiff{
+						Old:         "0",
+						New:         "1",
+						RequiresNew: true,
+					},
+
+					"config.0.name": &terraform.ResourceAttrDiff{
+						Old: "",
+						New: "hello",
+					},
+
+					"config.0.rules.#": &terraform.ResourceAttrDiff{
+						Old:         "",
+						NewComputed: true,
+					},
+				},
+			},
+
+			Err: false,
+		},
+
 		{
 			Name: "Set",
 			Schema: map[string]*Schema{

terraform/state_test.go

@@ -1456,6 +1456,45 @@ func TestInstanceState_MergeDiffRemoveCounts(t *testing.T) {
 	}
 }
 
+// GH-12183. This tests that a list with a computed set generates the
+// right partial state. This never failed but is put here for completion
+// of the test case for GH-12183.
+func TestInstanceState_MergeDiff_computedSet(t *testing.T) {
+	is := InstanceState{}
+
+	diff := &InstanceDiff{
+		Attributes: map[string]*ResourceAttrDiff{
+			"config.#": &ResourceAttrDiff{
+				Old:         "0",
+				New:         "1",
+				RequiresNew: true,
+			},
+
+			"config.0.name": &ResourceAttrDiff{
+				Old: "",
+				New: "hello",
+			},
+
+			"config.0.rules.#": &ResourceAttrDiff{
+				Old:         "",
+				NewComputed: true,
+			},
+		},
+	}
+
+	is2 := is.MergeDiff(diff)
+
+	expected := map[string]string{
+		"config.#":         "1",
+		"config.0.name":    "hello",
+		"config.0.rules.#": config.UnknownVariableValue,
+	}
+
+	if !reflect.DeepEqual(expected, is2.Attributes) {
+		t.Fatalf("bad: %#v", is2.Attributes)
+	}
+}
+
 func TestInstanceState_MergeDiff_nil(t *testing.T) {
 	var is *InstanceState