diff --git a/helper/schema/resource_diff.go b/helper/schema/resource_diff.go index 6727f04b06..96e1d1cbd3 100644 --- a/helper/schema/resource_diff.go +++ b/helper/schema/resource_diff.go @@ -124,9 +124,6 @@ type ResourceDiff struct { // diff, and the new diff. multiReader *MultiLevelFieldReader - // A writer that writes overridden old fields. - oldWriter *MapFieldWriter - // A writer that writes overridden new fields. newWriter *newValueWriter @@ -146,7 +143,6 @@ func newResourceDiff(schema map[string]*Schema, config *terraform.ResourceConfig schema: schema, } - d.oldWriter = &MapFieldWriter{Schema: d.schema} d.newWriter = &newValueWriter{ MapFieldWriter: &MapFieldWriter{Schema: d.schema}, } @@ -175,11 +171,7 @@ func newResourceDiff(schema map[string]*Schema, config *terraform.ResourceConfig }, } } - readers["newDiffOld"] = &MapFieldReader{ - Schema: d.schema, - Map: BasicMapReader(d.oldWriter.Map()), - } - readers["newDiffNew"] = &newValueReader{ + readers["newDiff"] = &newValueReader{ MapFieldReader: &MapFieldReader{ Schema: d.schema, Map: BasicMapReader(d.newWriter.Map()), @@ -191,8 +183,7 @@ func newResourceDiff(schema map[string]*Schema, config *terraform.ResourceConfig "state", "config", "diff", - "newDiffOld", - "newDiffNew", + "newDiff", }, Readers: readers, @@ -261,7 +252,10 @@ func (d *ResourceDiff) diffChange(key string) (interface{}, interface{}, bool, b // // This function is only allowed on computed attributes. func (d *ResourceDiff) SetNew(key string, value interface{}) error { - return d.SetDiff(key, d.get(strings.Split(key, "."), "state").Value, value, false) + if !d.schema[key].Computed { + return fmt.Errorf("SetNew only operates on computed keys - %s is not one", key) + } + return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, value, false) } // SetNewComputed functions like SetNew, except that it blanks out a new value @@ -269,33 +263,18 @@ func (d *ResourceDiff) SetNew(key string, value interface{}) error { // // This function is only allowed on computed attributes. func (d *ResourceDiff) SetNewComputed(key string) error { - return d.SetDiff(key, d.get(strings.Split(key, "."), "state").Value, nil, true) -} - -// SetDiff allows the setting of both old and new values for the diff -// referenced by a given key. This can be used to completely override -// Terraform's own diff behaviour, and can be used in conjunction with Clear or -// ClearAll to construct a compleletely new diff based off of provider logic -// alone. -// -// This function is only allowed on computed attributes. -func (d *ResourceDiff) SetDiff(key string, old, new interface{}, computed bool) error { if !d.schema[key].Computed { - return fmt.Errorf("SetNew, SetNewComputed, and SetDiff are allowed on computed attributes only - %s is not one", key) + return fmt.Errorf("SetNewComputed only operates on computed keys - %s is not one", key) } - - return d.setDiff(key, old, new, computed) + return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, nil, true) } +// setDiff performs common diff setting behaviour. func (d *ResourceDiff) setDiff(key string, old, new interface{}, computed bool) error { if err := d.clear(key); err != nil { return err } - if err := d.oldWriter.WriteField(strings.Split(key, "."), old); err != nil { - return fmt.Errorf("Cannot set old diff value for key %s: %s", key, err) - } - if err := d.newWriter.WriteField(strings.Split(key, "."), new, computed); err != nil { return fmt.Errorf("Cannot set new diff value for key %s: %s", key, err) } @@ -341,7 +320,7 @@ func (d *ResourceDiff) GetChange(key string) (interface{}, interface{}) { // new diff levels to provide data consistent with the current state of the // customized diff. func (d *ResourceDiff) GetOk(key string) (interface{}, bool) { - r := d.get(strings.Split(key, "."), "newDiffNew") + r := d.get(strings.Split(key, "."), "newDiff") exists := r.Exists && !r.Computed if exists { // If it exists, we also want to verify it is not the zero-value. @@ -394,19 +373,16 @@ func (d *ResourceDiff) Id() string { // results from the exact levels for the new diff, then from state and diff as // per normal. func (d *ResourceDiff) getChange(key string) (getResult, getResult) { - old := d.getExact(strings.Split(key, "."), "newDiffOld") - new := d.getExact(strings.Split(key, "."), "newDiffNew") - - if old.Exists || new.Exists { - // If one of these values exists, then SetDiff has operated on this value, - // and as such the newDiff level should be used. - return old, new + old := d.get(strings.Split(key, "."), "state") + var new getResult + for p := range d.updatedKeys { + if childAddrOf(key, p) { + new = d.getExact(strings.Split(key, "."), "newDiff") + goto done + } } - - // If we haven't set this in the new diff, then we want to get the default - // levels as if we were using ResourceData normally. - old = d.get(strings.Split(key, "."), "state") - new = d.get(strings.Split(key, "."), "diff") + new = d.get(strings.Split(key, "."), "newDiff") +done: return old, new } @@ -453,3 +429,11 @@ func (d *ResourceDiff) finalizeResult(addr []string, result FieldReadResult) get Schema: schema, } } + +// childAddrOf does a comparison of two addresses to see if one is the child of +// the other. +func childAddrOf(child, parent string) bool { + cs := strings.Split(child, ".") + ps := strings.Split(parent, ".") + return reflect.DeepEqual(ps, cs[:len(ps)]) +} diff --git a/helper/schema/resource_diff_test.go b/helper/schema/resource_diff_test.go index 35b197c3e8..a8bde41a03 100644 --- a/helper/schema/resource_diff_test.go +++ b/helper/schema/resource_diff_test.go @@ -1,7 +1,6 @@ package schema import ( - "fmt" "reflect" "testing" @@ -62,12 +61,11 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) }, }, Key: "foo", - OldValue: fmt.Sprintf("%sbar", oldPrefix), NewValue: "qux", Expected: &terraform.InstanceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ "foo": &terraform.ResourceAttrDiff{ - Old: fmt.Sprintf("%sbar", oldPrefix), + Old: "bar", New: func() string { if computed { return "" @@ -113,7 +111,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) }, }, Key: "foo", - OldValue: []interface{}{fmt.Sprintf("%sbar", oldPrefix)}, NewValue: []interface{}{"qux"}, Expected: &terraform.InstanceDiff{ Attributes: func() map[string]*terraform.ResourceAttrDiff { @@ -129,8 +126,8 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) Old: "", New: "qux", } - result[fmt.Sprintf("foo.%d", HashString(fmt.Sprintf("%sbar", oldPrefix)))] = &terraform.ResourceAttrDiff{ - Old: fmt.Sprintf("%sbar", oldPrefix), + result["foo.1996459178"] = &terraform.ResourceAttrDiff{ + Old: "bar", New: "", NewRemoved: true, } @@ -167,7 +164,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) }, }, Key: "foo", - OldValue: []interface{}{fmt.Sprintf("%sbar", oldPrefix)}, NewValue: []interface{}{"qux"}, Expected: &terraform.InstanceDiff{ Attributes: func() map[string]*terraform.ResourceAttrDiff { @@ -180,7 +176,7 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) } } else { result["foo.0"] = &terraform.ResourceAttrDiff{ - Old: fmt.Sprintf("%sbar", oldPrefix), + Old: "bar", New: "qux", } } @@ -215,7 +211,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) }, }, Key: "foo", - OldValue: map[string]interface{}{"bar": fmt.Sprintf("%sbaz", oldPrefix)}, NewValue: map[string]interface{}{"bar": "quux"}, Expected: &terraform.InstanceDiff{ Attributes: func() map[string]*terraform.ResourceAttrDiff { @@ -233,7 +228,7 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) } } else { result["foo.bar"] = &terraform.ResourceAttrDiff{ - Old: fmt.Sprintf("%sbaz", oldPrefix), + Old: "baz", New: "quux", } } @@ -272,7 +267,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) }, }, Key: "one", - OldValue: fmt.Sprintf("%stwo", oldPrefix), NewValue: "four", Expected: &terraform.InstanceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ @@ -281,7 +275,7 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) New: "baz", }, "one": &terraform.ResourceAttrDiff{ - Old: fmt.Sprintf("%stwo", oldPrefix), + Old: "two", New: func() string { if computed { return "" @@ -323,7 +317,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) }, }, Key: "one", - OldValue: fmt.Sprintf("%stwo", oldPrefix), NewValue: "three", Expected: &terraform.InstanceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ @@ -332,7 +325,7 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) New: "baz", }, "one": &terraform.ResourceAttrDiff{ - Old: fmt.Sprintf("%stwo", oldPrefix), + Old: "two", New: func() string { if computed { return "" @@ -410,30 +403,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) }, }, Key: "top", - OldValue: NewSet(testSetFunc, []interface{}{ - map[string]interface{}{ - "foo": 1, - "bar": 4 + oldOffset, - }, - map[string]interface{}{ - "foo": 13 + oldOffset, - "bar": 12, - }, - }), - NewValue: NewSet(testSetFunc, []interface{}{ - map[string]interface{}{ - "foo": 1, - "bar": 4, - }, - map[string]interface{}{ - "foo": 13, - "bar": 12, - }, - map[string]interface{}{ - "foo": 21, - "bar": 22, - }, - }), Expected: &terraform.InstanceDiff{ Attributes: func() map[string]*terraform.ResourceAttrDiff { result := make(map[string]*terraform.ResourceAttrDiff) @@ -493,12 +462,11 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) Config: testConfig(t, map[string]interface{}{}), Diff: &terraform.InstanceDiff{Attributes: map[string]*terraform.ResourceAttrDiff{}}, Key: "foo", - OldValue: fmt.Sprintf("%sbar", oldPrefix), NewValue: "baz", Expected: &terraform.InstanceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ "foo": &terraform.ResourceAttrDiff{ - Old: fmt.Sprintf("%sbar", oldPrefix), + Old: "bar", New: func() string { if computed { return "" @@ -535,7 +503,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) }, }, Key: "foo", - OldValue: fmt.Sprintf("%sbar", oldPrefix), NewValue: "qux", ExpectedError: true, }, @@ -596,33 +563,6 @@ func TestSetNewComputed(t *testing.T) { } } -func TestSetDiff(t *testing.T) { - testCases := testDiffCases(t, "testSetDiff", 1, false) - for _, tc := range testCases { - t.Run(tc.Name, func(t *testing.T) { - m := schemaMap(tc.Schema) - d := newResourceDiff(tc.Schema, tc.Config, tc.State, tc.Diff) - err := d.SetDiff(tc.Key, tc.OldValue, tc.NewValue, false) - switch { - case err != nil && !tc.ExpectedError: - t.Fatalf("bad: %s", err) - case err == nil && tc.ExpectedError: - t.Fatalf("Expected error, got none") - case err != nil && tc.ExpectedError: - return - } - for _, k := range d.UpdatedKeys() { - if err := m.diff(k, m[k], tc.Diff, d, false); err != nil { - t.Fatalf("bad: %s", err) - } - } - if !reflect.DeepEqual(tc.Expected, tc.Diff) { - t.Fatalf("Expected %s, got %s", spew.Sdump(tc.Expected), spew.Sdump(tc.Diff)) - } - }) - } -} - func TestForceNew(t *testing.T) { cases := []resourceDiffTestCase{ resourceDiffTestCase{