flatmap: mark computed list as a computed value in Expand

Fixes #12183

The fix is in flatmap for this but the entire issue is a bit more
complex. Given a schema with a computed set, if you reference it like
this:

    lookup(attr[0], "field")

And "attr" contains a computed set within it, it would panic even though
"field" is available. There were a couple avenues I could've taken to
fix this:

1.) Any complex value containing any unknown value at any point is
entirely unknown.

2.) Only the specific part of the complex value is unknown.

I took route 2 so that the above works without any computed (since
"name" is not computed but something else is). This may actually have an
effect on other parts of Terraform configs, however those similar
configs would've simply crashed previously so it shouldn't break any
pre-existing configs.
This commit is contained in:
Mitchell Hashimoto 2017-02-23 10:03:59 -08:00
parent 93e38c8fa8
commit c6d0333dc0
No known key found for this signature in database
GPG Key ID: 744E147AA52F5B0A
7 changed files with 228 additions and 2 deletions

View File

@ -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(),

View File

@ -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
}

View File

@ -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
},
},
},
})
}

View File

@ -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)
}

View File

@ -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 {

View File

@ -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{

View File

@ -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