diff --git a/builtin/providers/test/provider.go b/builtin/providers/test/provider.go index fc85ad4a4..a11e3f1cd 100644 --- a/builtin/providers/test/provider.go +++ b/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(), diff --git a/builtin/providers/test/resource_gh12183.go b/builtin/providers/test/resource_gh12183.go new file mode 100644 index 000000000..07f164de1 --- /dev/null +++ b/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 +} diff --git a/builtin/providers/test/resource_gh12183_test.go b/builtin/providers/test/resource_gh12183_test.go new file mode 100644 index 000000000..6b03b8027 --- /dev/null +++ b/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 + }, + }, + }, + }) +} diff --git a/flatmap/expand.go b/flatmap/expand.go index 331357ff3..422b1ffcc 100644 --- a/flatmap/expand.go +++ b/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) } diff --git a/flatmap/expand_test.go b/flatmap/expand_test.go index b5da3197b..53963a1ba 100644 --- a/flatmap/expand_test.go +++ b/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 { diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 0a3b7721f..f335c8aea 100644 --- a/helper/schema/schema_test.go +++ b/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{ diff --git a/terraform/state_test.go b/terraform/state_test.go index 2d57dafbb..694c9f751 100644 --- a/terraform/state_test.go +++ b/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