From 7ee433955536db3fdff2dba9548cd2e88ae8a43c Mon Sep 17 00:00:00 2001 From: Paddy Carver Date: Wed, 5 Sep 2018 17:29:03 -0700 Subject: [PATCH 1/3] Allow for nested fields with checkKey on ResourceDiffs. When checking if a ResourceDiff key is valid, allow for keys that exist on sub-blocks. This allows things like diff.Clear to be called on sub-block fields, instead of just on top-level fields. --- helper/schema/resource_diff.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/helper/schema/resource_diff.go b/helper/schema/resource_diff.go index 92b891fc5b32..b32a2431a775 100644 --- a/helper/schema/resource_diff.go +++ b/helper/schema/resource_diff.go @@ -536,11 +536,15 @@ func childAddrOf(child, parent string) bool { // checkKey checks the key to make sure it exists and is computed. func (d *ResourceDiff) checkKey(key, caller string) error { - s, ok := d.schema[key] - if !ok { + keyParts := strings.Split(key, ".") + var schema *Schema + schemaL := addrToSchema(keyParts, d.schema) + if len(schemaL) > 0 { + schema = schemaL[len(schemaL)-1] + } else { return fmt.Errorf("%s: invalid key: %s", caller, key) } - if !s.Computed { + if !schema.Computed { return fmt.Errorf("%s only operates on computed keys - %s is not one", caller, key) } return nil From 393c32624a3ad082da0cd111d38deb8f70333f7b Mon Sep 17 00:00:00 2001 From: Paddy Carver Date: Thu, 20 Sep 2018 10:52:36 -0700 Subject: [PATCH 2/3] Add tests to ensure clearing sub-blocks works. --- helper/schema/resource_diff_test.go | 105 ++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/helper/schema/resource_diff_test.go b/helper/schema/resource_diff_test.go index fdb8937e52f8..52c02a6abe7e 100644 --- a/helper/schema/resource_diff_test.go +++ b/helper/schema/resource_diff_test.go @@ -1028,6 +1028,111 @@ func TestClear(t *testing.T) { }, }, }, + resourceDiffTestCase{ + Name: "basic sub-block diff", + Schema: map[string]*Schema{ + "foo": &Schema{ + Type: TypeList, + Optional: true, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "bar": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + }, + "baz": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "foo.0.bar": "bar1", + "foo.0.baz": "baz1", + }, + }, + Config: testConfig(t, map[string]interface{}{ + "foo": []map[string]interface{}{ + map[string]interface{}{ + "bar": "bar2", + "baz": "baz1", + }, + }, + }), + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "foo.0.bar": &terraform.ResourceAttrDiff{ + Old: "bar1", + New: "bar2", + }, + }, + }, + Key: "foo.0.bar", + Expected: &terraform.InstanceDiff{Attributes: map[string]*terraform.ResourceAttrDiff{}}, + }, + resourceDiffTestCase{ + Name: "sub-block diff only partial clear", + Schema: map[string]*Schema{ + "foo": &Schema{ + Type: TypeList, + Optional: true, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "bar": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + }, + "baz": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "foo.0.bar": "bar1", + "foo.0.baz": "baz1", + }, + }, + Config: testConfig(t, map[string]interface{}{ + "foo": []map[string]interface{}{ + map[string]interface{}{ + "bar": "bar2", + "baz": "baz2", + }, + }, + }), + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "foo.0.bar": &terraform.ResourceAttrDiff{ + Old: "bar1", + New: "bar2", + }, + "foo.0.baz": &terraform.ResourceAttrDiff{ + Old: "baz1", + New: "baz2", + }, + }, + }, + Key: "foo.0.bar", + Expected: &terraform.InstanceDiff{Attributes: map[string]*terraform.ResourceAttrDiff{ + "foo.0.baz": &terraform.ResourceAttrDiff{ + Old: "baz1", + New: "baz2", + }, + }}, + }, } for _, tc := range cases { t.Run(tc.Name, func(t *testing.T) { From 2eadc8f625d8819a3de13ab3321fa93523b34fc5 Mon Sep 17 00:00:00 2001 From: Paddy Carver Date: Wed, 26 Sep 2018 12:38:38 -0700 Subject: [PATCH 3/3] Don't allow sub-blocks for SetNew. You can't set individual items in lists, you can only set the list as a whole, so we shouldn't allow sub-blocks to SetNew or SetNewComputed. --- helper/schema/resource_diff.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/helper/schema/resource_diff.go b/helper/schema/resource_diff.go index b32a2431a775..7db3decc5f2d 100644 --- a/helper/schema/resource_diff.go +++ b/helper/schema/resource_diff.go @@ -231,7 +231,7 @@ func (d *ResourceDiff) UpdatedKeys() []string { // Note that this does not wipe an override. This function is only allowed on // computed keys. func (d *ResourceDiff) Clear(key string) error { - if err := d.checkKey(key, "Clear"); err != nil { + if err := d.checkKey(key, "Clear", true); err != nil { return err } @@ -287,7 +287,7 @@ 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 { - if err := d.checkKey(key, "SetNew"); err != nil { + if err := d.checkKey(key, "SetNew", false); err != nil { return err } @@ -299,7 +299,7 @@ func (d *ResourceDiff) SetNew(key string, value interface{}) error { // // This function is only allowed on computed attributes. func (d *ResourceDiff) SetNewComputed(key string) error { - if err := d.checkKey(key, "SetNewComputed"); err != nil { + if err := d.checkKey(key, "SetNewComputed", false); err != nil { return err } @@ -535,13 +535,21 @@ func childAddrOf(child, parent string) bool { } // checkKey checks the key to make sure it exists and is computed. -func (d *ResourceDiff) checkKey(key, caller string) error { - keyParts := strings.Split(key, ".") +func (d *ResourceDiff) checkKey(key, caller string, nested bool) error { var schema *Schema - schemaL := addrToSchema(keyParts, d.schema) - if len(schemaL) > 0 { - schema = schemaL[len(schemaL)-1] + if nested { + keyParts := strings.Split(key, ".") + schemaL := addrToSchema(keyParts, d.schema) + if len(schemaL) > 0 { + schema = schemaL[len(schemaL)-1] + } } else { + s, ok := d.schema[key] + if ok { + schema = s + } + } + if schema == nil { return fmt.Errorf("%s: invalid key: %s", caller, key) } if !schema.Computed {