Merge pull request #19384 from hashicorp/jbardin/nested-sets

New Attribute and Diff handling in shims
This commit is contained in:
James Bardin 2018-11-16 11:55:41 -05:00 committed by GitHub
commit 3716db3865
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 1122 additions and 149 deletions

View File

@ -24,6 +24,7 @@ func Provider() terraform.ResourceProvider {
"test_resource_diff_suppress": testResourceDiffSuppress(), "test_resource_diff_suppress": testResourceDiffSuppress(),
"test_resource_force_new": testResourceForceNew(), "test_resource_force_new": testResourceForceNew(),
"test_resource_nested": testResourceNested(), "test_resource_nested": testResourceNested(),
"test_resource_nested_set": testResourceNestedSet(),
}, },
DataSourcesMap: map[string]*schema.Resource{ DataSourcesMap: map[string]*schema.Resource{
"test_data_source": testDataSource(), "test_data_source": testDataSource(),

View File

@ -1,23 +1,36 @@
package test package test
import ( import (
"fmt"
"math/rand"
"strings" "strings"
"github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/schema"
) )
func testResourceDiffSuppress() *schema.Resource { func testResourceDiffSuppress() *schema.Resource {
diffSuppress := func(k, old, new string, d *schema.ResourceData) bool {
if old == "" || strings.Contains(new, "replace") {
return false
}
return true
}
return &schema.Resource{ return &schema.Resource{
Create: testResourceDiffSuppressCreate, Create: testResourceDiffSuppressCreate,
Read: testResourceDiffSuppressRead, Read: testResourceDiffSuppressRead,
Update: testResourceDiffSuppressUpdate,
Delete: testResourceDiffSuppressDelete, Delete: testResourceDiffSuppressDelete,
Update: testResourceDiffSuppressUpdate,
Importer: &schema.ResourceImporter{ Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough, State: schema.ImportStatePassthrough,
}, },
Schema: map[string]*schema.Schema{ Schema: map[string]*schema.Schema{
"optional": {
Type: schema.TypeString,
Optional: true,
},
"val_to_upper": { "val_to_upper": {
Type: schema.TypeString, Type: schema.TypeString,
Required: true, Required: true,
@ -29,18 +42,48 @@ func testResourceDiffSuppress() *schema.Resource {
return strings.ToUpper(old) == strings.ToUpper(new) return strings.ToUpper(old) == strings.ToUpper(new)
}, },
}, },
"optional": { "network": {
Type: schema.TypeString, Type: schema.TypeString,
Optional: true,
Default: "default",
ForceNew: true,
DiffSuppressFunc: diffSuppress,
},
"subnetwork": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
DiffSuppressFunc: diffSuppress,
},
"node_pool": {
Type: schema.TypeList,
Optional: true, Optional: true,
Computed: true,
ForceNew: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"name": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},
},
},
}, },
}, },
} }
} }
func testResourceDiffSuppressCreate(d *schema.ResourceData, meta interface{}) error { func testResourceDiffSuppressCreate(d *schema.ResourceData, meta interface{}) error {
d.SetId("testId") d.Set("network", "modified")
d.Set("subnetwork", "modified")
return testResourceRead(d, meta) id := fmt.Sprintf("%x", rand.Int63())
d.SetId(id)
return nil
} }
func testResourceDiffSuppressRead(d *schema.ResourceData, meta interface{}) error { func testResourceDiffSuppressRead(d *schema.ResourceData, meta interface{}) error {

View File

@ -1,10 +1,14 @@
package test package test
import ( import (
"errors"
"strings" "strings"
"testing" "testing"
"github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
) )
func TestResourceDiffSuppress_create(t *testing.T) { func TestResourceDiffSuppress_create(t *testing.T) {
@ -45,3 +49,78 @@ resource "test_resource_diff_suppress" "foo" {
}, },
}) })
} }
func TestResourceDiffSuppress_updateIgnoreChanges(t *testing.T) {
// None of these steps should replace the instance
id := ""
checkFunc := func(s *terraform.State) error {
root := s.ModuleByPath(addrs.RootModuleInstance)
res := root.Resources["test_resource_diff_suppress.foo"]
if id != "" && res.Primary.ID != id {
return errors.New("expected no resource replacement")
}
id = res.Primary.ID
return nil
}
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_diff_suppress" "foo" {
val_to_upper = "foo"
network = "foo"
subnetwork = "foo"
node_pool {
name = "default-pool"
}
lifecycle {
ignore_changes = ["node_pool"]
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_diff_suppress" "foo" {
val_to_upper = "foo"
network = "ignored"
subnetwork = "ignored"
node_pool {
name = "default-pool"
}
lifecycle {
ignore_changes = ["node_pool"]
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_diff_suppress" "foo" {
val_to_upper = "foo"
network = "ignored"
subnetwork = "ignored"
node_pool {
name = "ignored"
}
lifecycle {
ignore_changes = ["node_pool"]
}
}
`),
Check: checkFunc,
},
},
})
}

View File

@ -0,0 +1,127 @@
package test
import (
"fmt"
"math/rand"
"github.com/hashicorp/terraform/helper/schema"
)
func testResourceNestedSet() *schema.Resource {
return &schema.Resource{
Create: testResourceNestedSetCreate,
Read: testResourceNestedSetRead,
Delete: testResourceNestedSetDelete,
Update: testResourceNestedSetUpdate,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},
Schema: map[string]*schema.Schema{
"optional": {
Type: schema.TypeBool,
Optional: true,
},
"force_new": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
},
"single": {
Type: schema.TypeSet,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"value": {
Type: schema.TypeString,
ForceNew: true,
Required: true,
},
"optional": {
Type: schema.TypeString,
ForceNew: true,
Optional: true,
Computed: true,
},
},
},
},
"multi": {
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"set": {
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"required": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"optional_int": {
Type: schema.TypeInt,
Optional: true,
},
"bool": {
Type: schema.TypeBool,
Optional: true,
},
},
},
},
"optional": {
Type: schema.TypeString,
// commenting this causes it to get missed during apply
//ForceNew: true,
Optional: true,
},
"bool": {
Type: schema.TypeBool,
Optional: true,
},
},
},
},
},
}
}
func testResourceNestedSetCreate(d *schema.ResourceData, meta interface{}) error {
id := fmt.Sprintf("%x", rand.Int63())
d.SetId(id)
// replicate some awkward handling of a computed value in a set
set := d.Get("single").(*schema.Set)
l := set.List()
if len(l) == 1 {
if s, ok := l[0].(map[string]interface{}); ok {
if v, _ := s["optional"].(string); v == "" {
s["optional"] = id
}
}
}
d.Set("single", set)
return testResourceNestedRead(d, meta)
}
func testResourceNestedSetRead(d *schema.ResourceData, meta interface{}) error {
return nil
}
func testResourceNestedSetDelete(d *schema.ResourceData, meta interface{}) error {
d.SetId("")
return nil
}
func testResourceNestedSetUpdate(d *schema.ResourceData, meta interface{}) error {
return nil
}

View File

@ -0,0 +1,307 @@
package test
import (
"errors"
"fmt"
"strings"
"testing"
"github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)
func TestResourceNestedSet_basic(t *testing.T) {
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
single {
value = "bar"
}
}
`),
},
},
})
}
// The set should not be generated because of it's computed value
func TestResourceNestedSet_noSet(t *testing.T) {
checkFunc := func(s *terraform.State) error {
root := s.ModuleByPath(addrs.RootModuleInstance)
res := root.Resources["test_resource_nested_set.foo"]
for k, v := range res.Primary.Attributes {
if strings.HasPrefix(k, "single") && k != "single.#" {
return fmt.Errorf("unexpected set value: %s:%s", k, v)
}
}
return nil
}
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
}
`),
Check: checkFunc,
},
},
})
}
func TestResourceNestedSet_addRemove(t *testing.T) {
var id string
checkFunc := func(s *terraform.State) error {
root := s.ModuleByPath(addrs.RootModuleInstance)
res := root.Resources["test_resource_nested_set.foo"]
if res.Primary.ID == id {
return errors.New("expected new resource")
}
id = res.Primary.ID
return nil
}
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
single {
value = "bar"
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
single {
value = "bar"
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
single {
value = "bar"
optional = "baz"
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
}
`),
Check: checkFunc,
},
},
})
}
func TestResourceNestedSet_multiAddRemove(t *testing.T) {
checkFunc := func(s *terraform.State) error {
return nil
}
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
multi {
optional = "bar"
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
multi {
set {
required = "val"
}
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
multi {
set {
required = "new"
}
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
multi {
set {
required = "new"
optional_int = 3
}
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
single {
value = "bar"
optional = "baz"
}
multi {
set {
required = "new"
optional_int = 3
}
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
optional = true
single {
value = "bar"
optional = "baz"
}
multi {
set {
required = "new"
optional_int = 3
}
}
}
`),
Check: checkFunc,
},
},
})
}
func TestResourceNestedSet_forceNewEmptyString(t *testing.T) {
var id string
step := 0
checkFunc := func(s *terraform.State) error {
root := s.ModuleByPath(addrs.RootModuleInstance)
res := root.Resources["test_resource_nested_set.foo"]
defer func() {
step++
id = res.Primary.ID
}()
if step == 2 && res.Primary.ID == id {
// setting an empty string currently does not trigger ForceNew, but
// it should in the future.
return nil
}
if res.Primary.ID == id {
return errors.New("expected new resource")
}
return nil
}
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
multi {
set {
required = "val"
}
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
multi {
set {
required = ""
}
}
}
`),
Check: checkFunc,
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
force_new = ""
}
`),
Check: checkFunc,
},
},
})
}

View File

@ -356,6 +356,11 @@ func hcl2ValueFromFlatmapSet(m map[string]string, prefix string, ty cty.Type) (c
return cty.UnknownVal(ty), nil return cty.UnknownVal(ty), nil
} }
// Keep track of keys we've seen, se we don't add the same set value
// multiple times. The cty.Set will normally de-duplicate values, but we may
// have unknown values that would not show as equivalent.
seen := map[string]bool{}
for fullKey := range m { for fullKey := range m {
if !strings.HasPrefix(fullKey, prefix) { if !strings.HasPrefix(fullKey, prefix) {
continue continue
@ -370,6 +375,12 @@ func hcl2ValueFromFlatmapSet(m map[string]string, prefix string, ty cty.Type) (c
key = fullKey[:dot+len(prefix)] key = fullKey[:dot+len(prefix)]
} }
if seen[key] {
continue
}
seen[key] = true
// The flatmap format doesn't allow us to distinguish between keys // The flatmap format doesn't allow us to distinguish between keys
// that contain periods and nested objects, so by convention a // that contain periods and nested objects, so by convention a
// map is only ever of primitive type in flatmap, and we just assume // map is only ever of primitive type in flatmap, and we just assume
@ -386,5 +397,6 @@ func hcl2ValueFromFlatmapSet(m map[string]string, prefix string, ty cty.Type) (c
if len(vals) == 0 { if len(vals) == 0 {
return cty.SetValEmpty(ety), nil return cty.SetValEmpty(ety), nil
} }
return cty.SetVal(vals), nil return cty.SetVal(vals), nil
} }

View File

@ -635,6 +635,50 @@ func TestHCL2ValueFromFlatmap(t *testing.T) {
}), }),
}), }),
}, },
{
Flatmap: map[string]string{
"single.#": "1",
"single.~1.value": "a",
"single.~1.optional": UnknownVariableValue,
"two.#": "2",
"two.~2381914684.value": "a",
"two.~2381914684.optional": UnknownVariableValue,
"two.~2798940671.value": "b",
"two.~2798940671.optional": UnknownVariableValue,
},
Type: cty.Object(map[string]cty.Type{
"single": cty.Set(
cty.Object(map[string]cty.Type{
"value": cty.String,
"optional": cty.String,
}),
),
"two": cty.Set(
cty.Object(map[string]cty.Type{
"optional": cty.String,
"value": cty.String,
}),
),
}),
Want: cty.ObjectVal(map[string]cty.Value{
"single": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"value": cty.StringVal("a"),
"optional": cty.UnknownVal(cty.String),
}),
}),
"two": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"value": cty.StringVal("a"),
"optional": cty.UnknownVal(cty.String),
}),
cty.ObjectVal(map[string]cty.Value{
"value": cty.StringVal("b"),
"optional": cty.UnknownVal(cty.String),
}),
}),
}),
},
} }
for _, test := range tests { for _, test := range tests {

View File

@ -113,7 +113,7 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("insufficient items for attribute %q; must have at least %d", typeName, blockS.MinItems) return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("insufficient items for attribute %q; must have at least %d", typeName, blockS.MinItems)
} }
if l > blockS.MaxItems && blockS.MaxItems > 0 { if l > blockS.MaxItems && blockS.MaxItems > 0 {
return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("too many items for attribute %q; must have at least %d", typeName, blockS.MinItems) return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("too many items for attribute %q; cannot have more than %d", typeName, blockS.MaxItems)
} }
if l == 0 { if l == 0 {
attrs[typeName] = cty.ListValEmpty(blockS.ImpliedType()) attrs[typeName] = cty.ListValEmpty(blockS.ImpliedType())
@ -161,7 +161,7 @@ func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("insufficient items for attribute %q; must have at least %d", typeName, blockS.MinItems) return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("insufficient items for attribute %q; must have at least %d", typeName, blockS.MinItems)
} }
if l > blockS.MaxItems && blockS.MaxItems > 0 { if l > blockS.MaxItems && blockS.MaxItems > 0 {
return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("too many items for attribute %q; must have at least %d", typeName, blockS.MinItems) return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("too many items for attribute %q; cannot have more than %d", typeName, blockS.MaxItems)
} }
if l == 0 { if l == 0 {
attrs[typeName] = cty.SetValEmpty(blockS.ImpliedType()) attrs[typeName] = cty.SetValEmpty(blockS.ImpliedType())

View File

@ -3,7 +3,10 @@ package plugin
import ( import (
"encoding/json" "encoding/json"
"errors" "errors"
"regexp"
"sort"
"strconv" "strconv"
"strings"
"github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty"
ctyconvert "github.com/zclconf/go-cty/cty/convert" ctyconvert "github.com/zclconf/go-cty/cty/convert"
@ -387,7 +390,11 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso
return resp, nil return resp, nil
} }
instanceState := schema.InstanceStateFromStateValue(stateVal, res.SchemaVersion) instanceState, err := res.ShimInstanceStateFromValue(stateVal)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
newInstanceState, err := res.RefreshWithoutUpgrade(instanceState, s.provider.Meta()) newInstanceState, err := res.RefreshWithoutUpgrade(instanceState, s.provider.Meta())
if err != nil { if err != nil {
@ -412,6 +419,8 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso
// helper/schema should always copy the ID over, but do it again just to be safe // helper/schema should always copy the ID over, but do it again just to be safe
newInstanceState.Attributes["id"] = newInstanceState.ID newInstanceState.Attributes["id"] = newInstanceState.ID
newInstanceState.Attributes = normalizeFlatmapContainers(newInstanceState.Attributes)
newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType()) newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType())
if err != nil { if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
@ -455,7 +464,12 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
Type: req.TypeName, Type: req.TypeName,
} }
priorState := schema.InstanceStateFromStateValue(priorStateVal, res.SchemaVersion) priorState, err := res.ShimInstanceStateFromValue(priorStateVal)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
priorPrivate := make(map[string]interface{}) priorPrivate := make(map[string]interface{})
if len(req.PriorPrivate) > 0 { if len(req.PriorPrivate) > 0 {
if err := json.Unmarshal(req.PriorPrivate, &priorPrivate); err != nil { if err := json.Unmarshal(req.PriorPrivate, &priorPrivate); err != nil {
@ -489,8 +503,33 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
return resp, nil return resp, nil
} }
// strip out non-diffs
for k, v := range diff.Attributes {
if v.New == v.Old && !v.NewComputed && !v.NewRemoved {
delete(diff.Attributes, k)
}
}
if priorState == nil {
priorState = &terraform.InstanceState{}
}
// now we need to apply the diff to the prior state, so get the planned state // now we need to apply the diff to the prior state, so get the planned state
plannedStateVal, err := schema.ApplyDiff(priorStateVal, diff, block) plannedAttrs, err := diff.Apply(priorState.Attributes, block)
plannedAttrs = normalizeFlatmapContainers(plannedAttrs)
plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, block.ImpliedType())
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
plannedStateVal, err = block.CoerceValue(plannedStateVal)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
if err != nil { if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil return resp, nil
@ -571,7 +610,11 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
Type: req.TypeName, Type: req.TypeName,
} }
priorState := schema.InstanceStateFromStateValue(priorStateVal, res.SchemaVersion) priorState, err := res.ShimInstanceStateFromValue(priorStateVal)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
private := make(map[string]interface{}) private := make(map[string]interface{})
if len(req.PlannedPrivate) > 0 { if len(req.PlannedPrivate) > 0 {
@ -607,6 +650,13 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
} }
} }
// strip out non-diffs
for k, v := range diff.Attributes {
if v.New == v.Old && !v.NewComputed && !v.NewRemoved {
delete(diff.Attributes, k)
}
}
if private != nil { if private != nil {
diff.Meta = private diff.Meta = private
} }
@ -617,6 +667,10 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
return resp, nil return resp, nil
} }
if newInstanceState != nil {
newInstanceState.Attributes = normalizeFlatmapContainers(newInstanceState.Attributes)
}
newStateVal := cty.NullVal(block.ImpliedType()) newStateVal := cty.NullVal(block.ImpliedType())
// We keep the null val if we destroyed the resource, otherwise build the // We keep the null val if we destroyed the resource, otherwise build the
@ -790,6 +844,62 @@ func pathToAttributePath(path cty.Path) *proto.AttributePath {
return &proto.AttributePath{Steps: steps} return &proto.AttributePath{Steps: steps}
} }
// normalizeFlatmapContainers removes empty containers, and fixes counts in a
// set of flatmapped attributes.
func normalizeFlatmapContainers(attrs map[string]string) map[string]string {
keyRx := regexp.MustCompile(`.*\.[%#]$`)
// find container keys
var keys []string
for k := range attrs {
if keyRx.MatchString(k) {
keys = append(keys, k)
}
}
// sort the keys in reverse, so that we check the longest subkeys first
sort.Slice(keys, func(i, j int) bool {
a, b := keys[i], keys[j]
if strings.HasPrefix(a, b) {
return true
}
if strings.HasPrefix(b, a) {
return false
}
return a > b
})
for _, k := range keys {
prefix := k[:len(k)-1]
indexes := map[string]int{}
for cand := range attrs {
if cand == k {
continue
}
if strings.HasPrefix(cand, prefix) {
idx := cand[len(prefix):]
dot := strings.Index(idx, ".")
if dot > 0 {
idx = idx[:dot]
}
indexes[idx]++
}
}
if len(indexes) > 0 {
attrs[k] = strconv.Itoa(len(indexes))
} else {
delete(attrs, k)
}
}
return attrs
}
// helper/schema throws away timeout values from the config and stores them in // helper/schema throws away timeout values from the config and stores them in
// the Private/Meta fields. we need to copy those values into the planned state // the Private/Meta fields. we need to copy those values into the planned state
// so that core doesn't see a perpetual diff with the timeout block. // so that core doesn't see a perpetual diff with the timeout block.

View File

@ -3,6 +3,8 @@ package plugin
import ( import (
"context" "context"
"fmt" "fmt"
"reflect"
"strconv"
"strings" "strings"
"testing" "testing"
"time" "time"
@ -637,3 +639,34 @@ func TestGetSchemaTimeouts(t *testing.T) {
t.Fatal("missing default timeout in schema") t.Fatal("missing default timeout in schema")
} }
} }
func TestNormalizeFlatmapContainers(t *testing.T) {
for i, tc := range []struct {
attrs map[string]string
expect map[string]string
}{
{
attrs: map[string]string{"id": "1", "multi.2.set.#": "1", "multi.1.set.#": "0", "single.#": "0"},
expect: map[string]string{"id": "1"},
},
{
attrs: map[string]string{"id": "1", "multi.2.set.#": "2", "multi.2.set.1.foo": "bar", "multi.1.set.#": "0", "single.#": "0"},
expect: map[string]string{"id": "1", "multi.2.set.#": "1", "multi.2.set.1.foo": "bar"},
},
{
attrs: map[string]string{"id": "78629a0f5f3f164f", "multi.#": "1"},
expect: map[string]string{"id": "78629a0f5f3f164f"},
},
{
attrs: map[string]string{"multi.529860700.set.#": "1", "multi.#": "1", "id": "78629a0f5f3f164f"},
expect: map[string]string{"id": "78629a0f5f3f164f"},
},
} {
t.Run(strconv.Itoa(i), func(t *testing.T) {
got := normalizeFlatmapContainers(tc.attrs)
if !reflect.DeepEqual(tc.expect, got) {
t.Fatalf("expected:\n%#v\ngot:\n%#v\n", tc.expect, got)
}
})
}
}

View File

@ -71,7 +71,7 @@ func shimNewState(newState *states.State, schemas *terraform.Schemas) (*terrafor
} }
if resSchema == nil { if resSchema == nil {
return nil, fmt.Errorf("mising resource schema for %q in %q", resType, providerType) return nil, fmt.Errorf("missing resource schema for %q in %q", resType, providerType)
} }
for key, i := range res.Instances { for key, i := range res.Instances {

View File

@ -155,6 +155,27 @@ type Resource struct {
Timeouts *ResourceTimeout Timeouts *ResourceTimeout
} }
// ShimInstanceStateFromValue converts a cty.Value to a
// terraform.InstanceState.
func (r *Resource) ShimInstanceStateFromValue(state cty.Value) (*terraform.InstanceState, error) {
// Get the raw shimmed value. While this is correct, the set hashes don't
// match those from the Schema.
s := terraform.NewInstanceStateShimmedFromValue(state, r.SchemaVersion)
// We now rebuild the state through the ResourceData, so that the set indexes
// match what helper/schema expects.
data, err := schemaMap(r.Schema).Data(s, nil)
if err != nil {
return nil, err
}
s = data.State()
if s == nil {
s = &terraform.InstanceState{}
}
return s, nil
}
// See Resource documentation. // See Resource documentation.
type CreateFunc func(*ResourceData, interface{}) error type CreateFunc func(*ResourceData, interface{}) error
@ -550,8 +571,7 @@ func (r *Resource) upgradeState(s *terraform.InstanceState, meta interface{}) (*
return nil, err return nil, err
} }
s = InstanceStateFromStateValue(stateVal, r.SchemaVersion) return r.ShimInstanceStateFromValue(stateVal)
return s, nil
} }
// InternalValidate should be called to validate the structure // InternalValidate should be called to validate the structure

View File

@ -23,7 +23,10 @@ func DiffFromValues(prior, planned cty.Value, res *Resource) (*terraform.Instanc
// only needs to be created for the apply operation, and any customizations // only needs to be created for the apply operation, and any customizations
// have already been done. // have already been done.
func diffFromValues(prior, planned cty.Value, res *Resource, cust CustomizeDiffFunc) (*terraform.InstanceDiff, error) { func diffFromValues(prior, planned cty.Value, res *Resource, cust CustomizeDiffFunc) (*terraform.InstanceDiff, error) {
instanceState := InstanceStateFromStateValue(prior, res.SchemaVersion) instanceState, err := res.ShimInstanceStateFromValue(prior)
if err != nil {
return nil, err
}
configSchema := res.CoreConfigSchema() configSchema := res.CoreConfigSchema()
@ -85,11 +88,3 @@ func JSONMapToStateValue(m map[string]interface{}, block *configschema.Block) (c
func StateValueFromInstanceState(is *terraform.InstanceState, ty cty.Type) (cty.Value, error) { func StateValueFromInstanceState(is *terraform.InstanceState, ty cty.Type) (cty.Value, error) {
return is.AttrsAsObjectValue(ty) return is.AttrsAsObjectValue(ty)
} }
// InstanceStateFromStateValue converts a cty.Value to a
// terraform.InstanceState. This function requires the schema version used by
// the provider, because the legacy providers used the private Meta data in the
// InstanceState to store the schema version.
func InstanceStateFromStateValue(state cty.Value, schemaVersion int) *terraform.InstanceState {
return terraform.NewInstanceStateShimmedFromValue(state, schemaVersion)
}

View File

@ -599,6 +599,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"availability_zone": "foo", "availability_zone": "foo",
}, },
@ -901,6 +902,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"delete": "false", "delete": "false",
}, },
@ -953,43 +955,6 @@ func TestShimSchemaMap_Diff(t *testing.T) {
Err: false, Err: false,
}, },
/*
// disabled for shims
// there is no longer any "list promotion"
{
Name: "List decode with promotion",
Schema: map[string]*Schema{
"ports": &Schema{
Type: TypeList,
Required: true,
Elem: &Schema{Type: TypeInt},
PromoteSingle: true,
},
},
State: nil,
Config: map[string]interface{}{
"ports": "5",
},
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"ports.#": &terraform.ResourceAttrDiff{
Old: "0",
New: "1",
},
"ports.0": &terraform.ResourceAttrDiff{
Old: "",
New: "5",
},
},
},
Err: false,
},
*/
{ {
Name: "List decode with promotion with list", Name: "List decode with promotion with list",
Schema: map[string]*Schema{ Schema: map[string]*Schema{
@ -1109,6 +1074,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"ports.#": "3", "ports.#": "3",
"ports.0": "1", "ports.0": "1",
@ -1137,6 +1103,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"ports.#": "2", "ports.#": "2",
"ports.0": "1", "ports.0": "1",
@ -1353,6 +1320,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"ports.#": "0", "ports.#": "0",
}, },
@ -1493,6 +1461,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"ports.#": "2", "ports.#": "2",
"ports.1": "1", "ports.1": "1",
@ -1528,55 +1497,6 @@ func TestShimSchemaMap_Diff(t *testing.T) {
Err: false, Err: false,
}, },
/*
// disabled for shims
// you can't remove a required attribute
{
Name: "Set-7",
Schema: map[string]*Schema{
"ports": &Schema{
Type: TypeSet,
Required: true,
Elem: &Schema{Type: TypeInt},
Set: func(a interface{}) int {
return a.(int)
},
},
},
State: &terraform.InstanceState{
Attributes: map[string]string{
"ports.#": "2",
"ports.1": "1",
"ports.2": "2",
},
},
Config: map[string]interface{}{},
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"ports.#": &terraform.ResourceAttrDiff{
Old: "2",
New: "0",
},
"ports.1": &terraform.ResourceAttrDiff{
Old: "1",
New: "0",
NewRemoved: true,
},
"ports.2": &terraform.ResourceAttrDiff{
Old: "2",
New: "0",
NewRemoved: true,
},
},
},
Err: false,
},
*/
{ {
Name: "Set-8", Name: "Set-8",
Schema: map[string]*Schema{ Schema: map[string]*Schema{
@ -1592,6 +1512,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"availability_zone": "bar", "availability_zone": "bar",
"ports.#": "1", "ports.#": "1",
@ -1634,6 +1555,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"ingress.#": "2", "ingress.#": "2",
"ingress.80.ports.#": "1", "ingress.80.ports.#": "1",
@ -1718,6 +1640,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"availability_zone": "foo", "availability_zone": "foo",
"port": "80", "port": "80",
@ -1734,7 +1657,42 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
{ {
Name: "", Name: "computed",
Schema: map[string]*Schema{
"availability_zone": &Schema{
Type: TypeString,
Computed: true,
ComputedWhen: []string{"port"},
},
"port": &Schema{
Type: TypeInt,
Optional: true,
},
},
State: nil,
Config: map[string]interface{}{
"port": 80,
},
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"availability_zone": &terraform.ResourceAttrDiff{
NewComputed: true,
},
"port": &terraform.ResourceAttrDiff{
New: "80",
},
},
},
Err: false,
},
{
Name: "computed, exists",
Schema: map[string]*Schema{ Schema: map[string]*Schema{
"availability_zone": &Schema{ "availability_zone": &Schema{
Type: TypeString, Type: TypeString,
@ -1749,6 +1707,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"port": "80", "port": "80",
}, },
@ -1758,13 +1717,8 @@ func TestShimSchemaMap_Diff(t *testing.T) {
"port": 80, "port": 80,
}, },
Diff: &terraform.InstanceDiff{ // there is no computed diff when the instance exists already
Attributes: map[string]*terraform.ResourceAttrDiff{ Diff: nil,
"availability_zone": &terraform.ResourceAttrDiff{
NewComputed: true,
},
},
},
Err: false, Err: false,
}, },
@ -1811,6 +1765,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"config_vars.%": "1", "config_vars.%": "1",
"config_vars.foo": "bar", "config_vars.foo": "bar",
@ -1850,6 +1805,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"vars.%": "1", "vars.%": "1",
"vars.foo": "bar", "vars.foo": "bar",
@ -1889,6 +1845,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"vars.%": "1", "vars.%": "1",
"vars.foo": "bar", "vars.foo": "bar",
@ -1912,6 +1869,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"config_vars.#": "1", "config_vars.#": "1",
"config_vars.0.%": "1", "config_vars.0.%": "1",
@ -1954,6 +1912,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"config_vars.#": "1", "config_vars.#": "1",
"config_vars.0.%": "2", "config_vars.0.%": "2",
@ -2005,6 +1964,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"availability_zone": "bar", "availability_zone": "bar",
"address": "foo", "address": "foo",
@ -2049,6 +2009,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"availability_zone": "bar", "availability_zone": "bar",
"ports.#": "1", "ports.#": "1",
@ -2088,6 +2049,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"instances.#": "0", "instances.#": "0",
}, },
@ -2275,6 +2237,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"vars.%": "0", "vars.%": "0",
}, },
@ -2303,7 +2266,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
{ {
Name: " - Empty", Name: "Empty",
Schema: map[string]*Schema{}, Schema: map[string]*Schema{},
State: &terraform.InstanceState{}, State: &terraform.InstanceState{},
@ -2324,6 +2287,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"some_threshold": "567.8", "some_threshold": "567.8",
}, },
@ -2376,6 +2340,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"block_device.#": "2", "block_device.#": "2",
"block_device.616397234.delete_on_termination": "true", "block_device.616397234.delete_on_termination": "true",
@ -2410,6 +2375,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"port": "false", "port": "false",
}, },
@ -2453,6 +2419,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"route.#": "0", "route.#": "0",
}, },
@ -2476,6 +2443,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"active": "true", "active": "true",
}, },
@ -2503,6 +2471,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"instances.#": "1", "instances.#": "1",
"instances.3": "foo", "instances.3": "foo",
@ -2615,6 +2584,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"metadata_keys.#": "0", "metadata_keys.#": "0",
}, },
@ -2675,6 +2645,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
Config: nil, Config: nil,
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"tags.%": "0", "tags.%": "0",
}, },
@ -2743,7 +2714,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
{ {
Name: ": StateFunc in nested set (#1759)", Name: "StateFunc in nested set (#1759)",
Schema: map[string]*Schema{ Schema: map[string]*Schema{
"service_account": &Schema{ "service_account": &Schema{
Type: TypeList, Type: TypeList,
@ -2823,6 +2794,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"instances.#": "2", "instances.#": "2",
"instances.3": "333", "instances.3": "333",
@ -2875,6 +2847,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"one": "false", "one": "false",
"two": "true", "two": "true",
@ -2913,6 +2886,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
Schema: map[string]*Schema{}, Schema: map[string]*Schema{},
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"id": "someid", "id": "someid",
}, },
@ -2942,6 +2916,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"ports.#": "3", "ports.#": "3",
"ports.1": "1", "ports.1": "1",
@ -2990,6 +2965,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"description": "foo", "description": "foo",
}, },
@ -3023,7 +2999,9 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
}, },
State: &terraform.InstanceState{}, State: &terraform.InstanceState{
ID: "id",
},
Config: map[string]interface{}{ Config: map[string]interface{}{
"foo": "${var.foo}", "foo": "${var.foo}",
@ -3063,6 +3041,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"ports.#": "3", "ports.#": "3",
"ports.1": "1", "ports.1": "1",
@ -3103,6 +3082,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"config.#": "2", "config.#": "2",
"config.0": "a", "config.0": "a",
@ -3310,6 +3290,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"ports.#": "3", "ports.#": "3",
"ports.1": "1", "ports.1": "1",
@ -3362,6 +3343,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
Schema: map[string]*Schema{}, Schema: map[string]*Schema{},
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "someid",
Attributes: map[string]string{ Attributes: map[string]string{
"id": "someid", "id": "someid",
}, },
@ -3397,6 +3379,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"etag": "foo", "etag": "foo",
"version_id": "1", "version_id": "1",
@ -3442,6 +3425,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"foo": "bar", "foo": "bar",
}, },
@ -3471,6 +3455,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"attr": "bar", "attr": "bar",
}, },
@ -3508,6 +3493,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
}, },
State: &terraform.InstanceState{ State: &terraform.InstanceState{
ID: "id",
Attributes: map[string]string{ Attributes: map[string]string{
"unrelated_set.#": "0", "unrelated_set.#": "0",
"stream_enabled": "true", "stream_enabled": "true",
@ -3549,11 +3535,10 @@ func TestShimSchemaMap_Diff(t *testing.T) {
} }
{ {
d, err := InternalMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), tc.CustomizeDiff, nil, false) d, err := schemaMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), tc.CustomizeDiff, nil, false)
if err != nil != tc.Err { if err != nil != tc.Err {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
if !cmp.Equal(d, tc.Diff, equateEmpty) { if !cmp.Equal(d, tc.Diff, equateEmpty) {
t.Fatal(cmp.Diff(d, tc.Diff, equateEmpty)) t.Fatal(cmp.Diff(d, tc.Diff, equateEmpty))
} }
@ -3597,6 +3582,7 @@ func TestShimSchemaMap_Diff(t *testing.T) {
} }
res := &Resource{Schema: tc.Schema} res := &Resource{Schema: tc.Schema}
d, err := diffFromValues(stateVal, configVal, res, tc.CustomizeDiff) d, err := diffFromValues(stateVal, configVal, res, tc.CustomizeDiff)
if err != nil { if err != nil {
if !tc.Err { if !tc.Err {

View File

@ -415,62 +415,278 @@ func (d *InstanceDiff) Unlock() { d.mu.Unlock() }
// This method is intended for shimming old subsystems that still use this // This method is intended for shimming old subsystems that still use this
// legacy diff type to work with the new-style types. // legacy diff type to work with the new-style types.
func (d *InstanceDiff) ApplyToValue(base cty.Value, schema *configschema.Block) (cty.Value, error) { func (d *InstanceDiff) ApplyToValue(base cty.Value, schema *configschema.Block) (cty.Value, error) {
// We always build a new value here, even if the given diff is "empty",
// because we might be planning to create a new instance that happens
// to have no attributes set, and so we want to produce an empty object
// rather than just echoing back the null old value.
// Create an InstanceState attributes from our existing state. // Create an InstanceState attributes from our existing state.
// We can use this to more easily apply the diff changes. // We can use this to more easily apply the diff changes.
attrs := hcl2shim.FlatmapValueFromHCL2(base) attrs := hcl2shim.FlatmapValueFromHCL2(base)
applied, err := d.Apply(attrs, schema)
if err != nil {
return base, err
}
val, err := hcl2shim.HCL2ValueFromFlatmap(applied, schema.ImpliedType())
if err != nil {
return base, err
}
return schema.CoerceValue(val)
}
// Apply applies the diff to the provided flatmapped attributes,
// returning the new instance attributes.
//
// This method is intended for shimming old subsystems that still use this
// legacy diff type to work with the new-style types.
func (d *InstanceDiff) Apply(attrs map[string]string, schema *configschema.Block) (map[string]string, error) {
// We always build a new value here, even if the given diff is "empty",
// because we might be planning to create a new instance that happens
// to have no attributes set, and so we want to produce an empty object
// rather than just echoing back the null old value.
if attrs == nil { if attrs == nil {
attrs = map[string]string{} attrs = map[string]string{}
} }
return d.applyDiff(attrs, schema)
}
func (d *InstanceDiff) applyDiff(attrs map[string]string, schema *configschema.Block) (map[string]string, error) {
// Rather applying the diff to mutate the attrs, we'll copy new values into
// here to avoid the possibility of leaving stale values.
result := map[string]string{}
if d.Destroy || d.DestroyDeposed || d.DestroyTainted { if d.Destroy || d.DestroyDeposed || d.DestroyTainted {
// to mark a destroy, we remove all attributes return result, nil
attrs = map[string]string{}
} else if attrs["id"] == "" || d.RequiresNew() {
// Since "id" is always computed, make sure it always has a value. Set
// it as unknown to generate the correct cty.Value
attrs["id"] = config.UnknownVariableValue
} }
for attr, diff := range d.Attributes { // iterate over the schema rather than the attributes, so we can handle
old, exists := attrs[attr] // blocks separately from plain attributes
for name, attrSchema := range schema.Attributes {
var err error
var newAttrs map[string]string
if exists && // handle non-block collections
old != diff.Old && switch ty := attrSchema.Type; {
// if new or old is unknown, then there's no mismatch case ty.IsListType() || ty.IsTupleType() || ty.IsMapType():
old != config.UnknownVariableValue && newAttrs, err = d.applyCollectionDiff(name, attrs, attrSchema)
diff.Old != config.UnknownVariableValue { case ty.IsSetType():
return base, fmt.Errorf("diff apply conflict for %s: diff expects %q, but prior value has %q", attr, diff.Old, old) newAttrs, err = d.applySetDiff(name, attrs, schema)
default:
newAttrs, err = d.applyAttrDiff(name, attrs, attrSchema)
}
if err != nil {
return result, err
}
for k, v := range newAttrs {
result[k] = v
}
}
for name, block := range schema.BlockTypes {
newAttrs, err := d.applySetDiff(name, attrs, &block.Block)
if err != nil {
return result, err
}
for k, v := range newAttrs {
result[k] = v
}
}
return result, nil
}
func (d *InstanceDiff) applyAttrDiff(attrName string, oldAttrs map[string]string, attrSchema *configschema.Attribute) (map[string]string, error) {
result := map[string]string{}
diff := d.Attributes[attrName]
old, exists := oldAttrs[attrName]
if diff != nil && diff.NewComputed {
result[attrName] = config.UnknownVariableValue
return result, nil
}
if attrName == "id" {
if old == "" {
result["id"] = config.UnknownVariableValue
} else {
result["id"] = old
}
return result, nil
}
// attribute diffs are sometimes missed, so assume no diff means keep the
// old value
if diff == nil {
if exists {
result[attrName] = old
} else {
// We need required values, so set those with an empty value. It
// must be set in the config, since if it were missing it would have
// failed validation.
if attrSchema.Required {
result[attrName] = ""
}
}
return result, nil
}
// check for missmatched diff values
if exists &&
old != diff.Old &&
old != config.UnknownVariableValue &&
diff.Old != config.UnknownVariableValue {
return result, fmt.Errorf("diff apply conflict for %s: diff expects %q, but prior value has %q", attrName, diff.Old, old)
}
if attrSchema.Computed && diff.NewComputed {
result[attrName] = config.UnknownVariableValue
return result, nil
}
if diff.NewRemoved {
// don't set anything in the new value
return result, nil
}
result[attrName] = diff.New
return result, nil
}
func (d *InstanceDiff) applyCollectionDiff(attrName string, oldAttrs map[string]string, attrSchema *configschema.Attribute) (map[string]string, error) {
result := map[string]string{}
// check the index first for special handling
for k, diff := range d.Attributes {
// check the index value, which can be set, and 0
if k == attrName+".#" || k == attrName+".%" {
if diff.NewRemoved {
return result, nil
}
if diff.NewComputed {
result[k] = config.UnknownVariableValue
return result, nil
}
// do what the diff tells us to here, so that it's consistent with applies
if diff.New == "0" {
result[k] = "0"
return result, nil
}
}
}
// collect all the keys from the diff and the old state
keys := map[string]bool{}
for k := range d.Attributes {
keys[k] = true
}
for k := range oldAttrs {
keys[k] = true
}
idx := attrName + ".#"
if attrSchema.Type.IsMapType() {
idx = attrName + ".%"
}
// record if we got the index from the diff
setIndex := false
for k := range keys {
if !strings.HasPrefix(k, attrName+".") {
continue
}
// we need to verify if we saw the index later
if k == idx {
setIndex = true
}
res, err := d.applyAttrDiff(k, oldAttrs, attrSchema)
if err != nil {
return result, err
}
for k, v := range res {
result[k] = v
}
}
// Verify we have the index count.
// If it wasn't added from a diff, check it from the previous value.
// Make sure we keep the count if it existed before, so we can tell if it
// existed, or was null.
if !setIndex {
old := oldAttrs[idx]
if old != "" {
result[idx] = old
}
}
return result, nil
}
func (d *InstanceDiff) applySetDiff(attrName string, oldAttrs map[string]string, block *configschema.Block) (map[string]string, error) {
result := map[string]string{}
idx := attrName + ".#"
// first find the index diff
for k, diff := range d.Attributes {
if k != idx {
continue
} }
if diff.NewComputed { if diff.NewComputed {
attrs[attr] = config.UnknownVariableValue result[k] = config.UnknownVariableValue
return result, nil
}
}
// Flag if there was a diff used in the set at all.
// If not, take the pre-existing set values
setDiff := false
// here we're trusting the diff to supply all the known items
for k, diff := range d.Attributes {
if !strings.HasPrefix(k, attrName+".") {
continue continue
} }
setDiff = true
if diff.NewRemoved { if diff.NewRemoved {
delete(attrs, attr) // no longer in the set
continue continue
} }
// sometimes helper/schema gives us values that aren't really a diff if diff.NewComputed {
if diff.Old == diff.New { result[k] = config.UnknownVariableValue
continue continue
} }
attrs[attr] = diff.New // helper/schema doesn't list old removed values, but since the set
// exists NewRemoved may not be true.
if diff.New == "" && diff.Old == "" {
continue
}
result[k] = diff.New
} }
val, err := hcl2shim.HCL2ValueFromFlatmap(attrs, schema.ImpliedType()) // use the existing values if there was no set diff at all
if err != nil { if !setDiff {
return val, err for k, v := range oldAttrs {
if strings.HasPrefix(k, attrName+".") {
result[k] = v
}
}
} }
return schema.CoerceValue(val) return result, nil
} }
// ResourceAttrDiff is the diff of a single attribute of a resource. // ResourceAttrDiff is the diff of a single attribute of a resource.