diff --git a/builtin/providers/test/provider.go b/builtin/providers/test/provider.go index 1c0fc574d3..59dd550855 100644 --- a/builtin/providers/test/provider.go +++ b/builtin/providers/test/provider.go @@ -24,6 +24,7 @@ func Provider() terraform.ResourceProvider { "test_resource_diff_suppress": testResourceDiffSuppress(), "test_resource_force_new": testResourceForceNew(), "test_resource_nested": testResourceNested(), + "test_resource_nested_set": testResourceNestedSet(), }, DataSourcesMap: map[string]*schema.Resource{ "test_data_source": testDataSource(), diff --git a/builtin/providers/test/resource_diff_suppress.go b/builtin/providers/test/resource_diff_suppress.go index 5c01a1d09d..cb5f7358f5 100644 --- a/builtin/providers/test/resource_diff_suppress.go +++ b/builtin/providers/test/resource_diff_suppress.go @@ -1,23 +1,36 @@ package test import ( + "fmt" + "math/rand" "strings" "github.com/hashicorp/terraform/helper/schema" ) 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{ Create: testResourceDiffSuppressCreate, Read: testResourceDiffSuppressRead, - Update: testResourceDiffSuppressUpdate, Delete: testResourceDiffSuppressDelete, + Update: testResourceDiffSuppressUpdate, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, Schema: map[string]*schema.Schema{ + "optional": { + Type: schema.TypeString, + Optional: true, + }, "val_to_upper": { Type: schema.TypeString, Required: true, @@ -29,18 +42,48 @@ func testResourceDiffSuppress() *schema.Resource { return strings.ToUpper(old) == strings.ToUpper(new) }, }, - "optional": { - Type: schema.TypeString, + "network": { + 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, + 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 { - 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 { diff --git a/builtin/providers/test/resource_diff_suppress_test.go b/builtin/providers/test/resource_diff_suppress_test.go index 59490e3584..89416f32a1 100644 --- a/builtin/providers/test/resource_diff_suppress_test.go +++ b/builtin/providers/test/resource_diff_suppress_test.go @@ -1,10 +1,14 @@ package test import ( + "errors" "strings" "testing" + "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" ) 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, + }, + }, + }) +} diff --git a/builtin/providers/test/resource_nested_set.go b/builtin/providers/test/resource_nested_set.go new file mode 100644 index 0000000000..c1e6520fb9 --- /dev/null +++ b/builtin/providers/test/resource_nested_set.go @@ -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 +} diff --git a/builtin/providers/test/resource_nested_set_test.go b/builtin/providers/test/resource_nested_set_test.go new file mode 100644 index 0000000000..fe28187011 --- /dev/null +++ b/builtin/providers/test/resource_nested_set_test.go @@ -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, + }, + }, + }) +} diff --git a/config/hcl2shim/flatmap.go b/config/hcl2shim/flatmap.go index 2f7954d76c..bcecb30df7 100644 --- a/config/hcl2shim/flatmap.go +++ b/config/hcl2shim/flatmap.go @@ -356,6 +356,11 @@ func hcl2ValueFromFlatmapSet(m map[string]string, prefix string, ty cty.Type) (c 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 { if !strings.HasPrefix(fullKey, prefix) { continue @@ -370,6 +375,12 @@ func hcl2ValueFromFlatmapSet(m map[string]string, prefix string, ty cty.Type) (c key = fullKey[:dot+len(prefix)] } + if seen[key] { + continue + } + + seen[key] = true + // The flatmap format doesn't allow us to distinguish between keys // that contain periods and nested objects, so by convention a // 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 { return cty.SetValEmpty(ety), nil } + return cty.SetVal(vals), nil } diff --git a/config/hcl2shim/flatmap_test.go b/config/hcl2shim/flatmap_test.go index 56c06c3dc5..07f93ac265 100644 --- a/config/hcl2shim/flatmap_test.go +++ b/config/hcl2shim/flatmap_test.go @@ -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 { diff --git a/configs/configschema/coerce_value.go b/configs/configschema/coerce_value.go index e6b163b9f7..bae5733df7 100644 --- a/configs/configschema/coerce_value.go +++ b/configs/configschema/coerce_value.go @@ -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) } 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 { 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) } 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 { attrs[typeName] = cty.SetValEmpty(blockS.ImpliedType()) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index a25aa67459..c668e9733b 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -3,7 +3,10 @@ package plugin import ( "encoding/json" "errors" + "regexp" + "sort" "strconv" + "strings" "github.com/zclconf/go-cty/cty" ctyconvert "github.com/zclconf/go-cty/cty/convert" @@ -387,7 +390,11 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso 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()) 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 newInstanceState.Attributes["id"] = newInstanceState.ID + newInstanceState.Attributes = normalizeFlatmapContainers(newInstanceState.Attributes) + newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -455,7 +464,12 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl 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{}) if len(req.PriorPrivate) > 0 { 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 } + // 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 - 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 { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil @@ -571,7 +610,11 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A 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{}) 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 { diff.Meta = private } @@ -617,6 +667,10 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A return resp, nil } + if newInstanceState != nil { + newInstanceState.Attributes = normalizeFlatmapContainers(newInstanceState.Attributes) + } + newStateVal := cty.NullVal(block.ImpliedType()) // 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} } +// 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 // 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. diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index 03f059a0b4..a06185fc4c 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -3,6 +3,8 @@ package plugin import ( "context" "fmt" + "reflect" + "strconv" "strings" "testing" "time" @@ -637,3 +639,34 @@ func TestGetSchemaTimeouts(t *testing.T) { 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) + } + }) + } +} diff --git a/helper/resource/state_shim.go b/helper/resource/state_shim.go index 9ad30d3ac9..9c698d16a4 100644 --- a/helper/resource/state_shim.go +++ b/helper/resource/state_shim.go @@ -71,7 +71,7 @@ func shimNewState(newState *states.State, schemas *terraform.Schemas) (*terrafor } 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 { diff --git a/helper/schema/resource.go b/helper/schema/resource.go index a26dfc9f88..d96bbcfde2 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -155,6 +155,27 @@ type Resource struct { 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. type CreateFunc func(*ResourceData, interface{}) error @@ -550,8 +571,7 @@ func (r *Resource) upgradeState(s *terraform.InstanceState, meta interface{}) (* return nil, err } - s = InstanceStateFromStateValue(stateVal, r.SchemaVersion) - return s, nil + return r.ShimInstanceStateFromValue(stateVal) } // InternalValidate should be called to validate the structure diff --git a/helper/schema/shims.go b/helper/schema/shims.go index 52ae7d7446..d99fb39651 100644 --- a/helper/schema/shims.go +++ b/helper/schema/shims.go @@ -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 // have already been done. 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() @@ -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) { 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) -} diff --git a/helper/schema/shims_test.go b/helper/schema/shims_test.go index 68186497bc..072c8ca7cb 100644 --- a/helper/schema/shims_test.go +++ b/helper/schema/shims_test.go @@ -599,6 +599,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "availability_zone": "foo", }, @@ -901,6 +902,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "delete": "false", }, @@ -953,43 +955,6 @@ func TestShimSchemaMap_Diff(t *testing.T) { 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", Schema: map[string]*Schema{ @@ -1109,6 +1074,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "ports.#": "3", "ports.0": "1", @@ -1137,6 +1103,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "ports.#": "2", "ports.0": "1", @@ -1353,6 +1320,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "ports.#": "0", }, @@ -1493,6 +1461,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "ports.#": "2", "ports.1": "1", @@ -1528,55 +1497,6 @@ func TestShimSchemaMap_Diff(t *testing.T) { 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", Schema: map[string]*Schema{ @@ -1592,6 +1512,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "availability_zone": "bar", "ports.#": "1", @@ -1634,6 +1555,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "ingress.#": "2", "ingress.80.ports.#": "1", @@ -1718,6 +1640,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "availability_zone": "foo", "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{ "availability_zone": &Schema{ Type: TypeString, @@ -1749,6 +1707,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "port": "80", }, @@ -1758,13 +1717,8 @@ func TestShimSchemaMap_Diff(t *testing.T) { "port": 80, }, - Diff: &terraform.InstanceDiff{ - Attributes: map[string]*terraform.ResourceAttrDiff{ - "availability_zone": &terraform.ResourceAttrDiff{ - NewComputed: true, - }, - }, - }, + // there is no computed diff when the instance exists already + Diff: nil, Err: false, }, @@ -1811,6 +1765,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "config_vars.%": "1", "config_vars.foo": "bar", @@ -1850,6 +1805,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "vars.%": "1", "vars.foo": "bar", @@ -1889,6 +1845,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "vars.%": "1", "vars.foo": "bar", @@ -1912,6 +1869,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "config_vars.#": "1", "config_vars.0.%": "1", @@ -1954,6 +1912,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "config_vars.#": "1", "config_vars.0.%": "2", @@ -2005,6 +1964,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "availability_zone": "bar", "address": "foo", @@ -2049,6 +2009,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "availability_zone": "bar", "ports.#": "1", @@ -2088,6 +2049,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "instances.#": "0", }, @@ -2275,6 +2237,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "vars.%": "0", }, @@ -2303,7 +2266,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, { - Name: " - Empty", + Name: "Empty", Schema: map[string]*Schema{}, State: &terraform.InstanceState{}, @@ -2324,6 +2287,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "some_threshold": "567.8", }, @@ -2376,6 +2340,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "block_device.#": "2", "block_device.616397234.delete_on_termination": "true", @@ -2410,6 +2375,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "port": "false", }, @@ -2453,6 +2419,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "route.#": "0", }, @@ -2476,6 +2443,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "active": "true", }, @@ -2503,6 +2471,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "instances.#": "1", "instances.3": "foo", @@ -2615,6 +2584,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "metadata_keys.#": "0", }, @@ -2675,6 +2645,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { Config: nil, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "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{ "service_account": &Schema{ Type: TypeList, @@ -2823,6 +2794,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "instances.#": "2", "instances.3": "333", @@ -2875,6 +2847,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "one": "false", "two": "true", @@ -2913,6 +2886,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { Schema: map[string]*Schema{}, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "id": "someid", }, @@ -2942,6 +2916,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "ports.#": "3", "ports.1": "1", @@ -2990,6 +2965,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "description": "foo", }, @@ -3023,7 +2999,9 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, }, - State: &terraform.InstanceState{}, + State: &terraform.InstanceState{ + ID: "id", + }, Config: map[string]interface{}{ "foo": "${var.foo}", @@ -3063,6 +3041,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "ports.#": "3", "ports.1": "1", @@ -3103,6 +3082,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "config.#": "2", "config.0": "a", @@ -3310,6 +3290,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "ports.#": "3", "ports.1": "1", @@ -3362,6 +3343,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { Schema: map[string]*Schema{}, State: &terraform.InstanceState{ + ID: "someid", Attributes: map[string]string{ "id": "someid", }, @@ -3397,6 +3379,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "etag": "foo", "version_id": "1", @@ -3442,6 +3425,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "foo": "bar", }, @@ -3471,6 +3455,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "attr": "bar", }, @@ -3508,6 +3493,7 @@ func TestShimSchemaMap_Diff(t *testing.T) { }, State: &terraform.InstanceState{ + ID: "id", Attributes: map[string]string{ "unrelated_set.#": "0", "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 { t.Fatalf("err: %s", err) } - if !cmp.Equal(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} + d, err := diffFromValues(stateVal, configVal, res, tc.CustomizeDiff) if err != nil { if !tc.Err { diff --git a/terraform/diff.go b/terraform/diff.go index 92b575086e..a28d7e4822 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -415,62 +415,278 @@ func (d *InstanceDiff) Unlock() { d.mu.Unlock() } // 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) 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. // We can use this to more easily apply the diff changes. 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 { 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 { - // to mark a destroy, we remove all attributes - 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 + return result, nil } - for attr, diff := range d.Attributes { - old, exists := attrs[attr] + // iterate over the schema rather than the attributes, so we can handle + // blocks separately from plain attributes + for name, attrSchema := range schema.Attributes { + var err error + var newAttrs map[string]string - if exists && - old != diff.Old && - // if new or old is unknown, then there's no mismatch - old != config.UnknownVariableValue && - diff.Old != config.UnknownVariableValue { - return base, fmt.Errorf("diff apply conflict for %s: diff expects %q, but prior value has %q", attr, diff.Old, old) + // handle non-block collections + switch ty := attrSchema.Type; { + case ty.IsListType() || ty.IsTupleType() || ty.IsMapType(): + newAttrs, err = d.applyCollectionDiff(name, attrs, attrSchema) + case ty.IsSetType(): + 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 { - 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 } + setDiff = true if diff.NewRemoved { - delete(attrs, attr) + // no longer in the set continue } - // sometimes helper/schema gives us values that aren't really a diff - if diff.Old == diff.New { + if diff.NewComputed { + result[k] = config.UnknownVariableValue 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()) - if err != nil { - return val, err + // use the existing values if there was no set diff at all + if !setDiff { + 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.