From ee769188d745281d9d75e99273ba7162ca55670d Mon Sep 17 00:00:00 2001 From: Chris Marchesi Date: Sun, 28 May 2017 09:48:15 -0700 Subject: [PATCH] helper/schema: More CustomizeDiff test cases Added a few more test cases for CustomizeDiff, caught another error in the process. I think this is ready for review now, and possibly some real-world testing of the waters by way of porting some resources that would benefit from the feature. --- helper/schema/schema.go | 2 +- helper/schema/schema_test.go | 251 ++++++++++++++++++++++++++++++++++- 2 files changed, 251 insertions(+), 2 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 998a104ca..ff315e7df 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -451,7 +451,7 @@ func (m schemaMap) Diff( } // Re-run customization - if customizeFunc != nil { + if !result2.Destroy && !result2.DestroyTainted && customizeFunc != nil { mc := m.DeepCopy() rd := newResourceDiff(mc, c, d.state, result2) if err := customizeFunc(rd, meta); err != nil { diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 089a370e5..3698fe0f4 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -2,6 +2,7 @@ package schema import ( "bytes" + "errors" "fmt" "os" "reflect" @@ -2826,7 +2827,46 @@ func TestSchemaMap_Diff(t *testing.T) { }, { - Name: "overridden diff with a CustomizeDiff function", + Name: "overridden diff with a CustomizeDiff function, ForceNew not in schema", + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + }, + }, + + State: nil, + + Config: map[string]interface{}{ + "availability_zone": "foo", + }, + + CustomizeDiff: func(d *ResourceDiff, meta interface{}) error { + if err := d.SetNew("availability_zone", "bar"); err != nil { + return err + } + if err := d.ForceNew("availability_zone"); err != nil { + return err + } + return nil + }, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "availability_zone": &terraform.ResourceAttrDiff{ + Old: "", + New: "bar", + RequiresNew: true, + }, + }, + }, + + Err: false, + }, + + { + Name: "overridden diff with a CustomizeDiff function, ForceNew in schema", Schema: map[string]*Schema{ "availability_zone": &Schema{ Type: TypeString, @@ -2861,6 +2901,215 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + + { + Name: "required field with computed diff added with CustomizeDiff function", + Schema: map[string]*Schema{ + "ami_id": &Schema{ + Type: TypeString, + Required: true, + }, + "instance_id": &Schema{ + Type: TypeString, + Computed: true, + }, + }, + + State: nil, + + Config: map[string]interface{}{ + "ami_id": "foo", + }, + + CustomizeDiff: func(d *ResourceDiff, meta interface{}) error { + if err := d.SetNew("instance_id", "bar"); err != nil { + return err + } + return nil + }, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "ami_id": &terraform.ResourceAttrDiff{ + Old: "", + New: "foo", + }, + "instance_id": &terraform.ResourceAttrDiff{ + Old: "", + New: "bar", + }, + }, + }, + + Err: false, + }, + + { + Name: "Set ForceNew only marks the changing element as ForceNew - CustomizeDiffFunc edition", + Schema: map[string]*Schema{ + "ports": &Schema{ + Type: TypeSet, + Optional: true, + Computed: true, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, + }, + + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "ports.#": "3", + "ports.1": "1", + "ports.2": "2", + "ports.4": "4", + }, + }, + + Config: map[string]interface{}{ + "ports": []interface{}{5, 2, 6}, + }, + + CustomizeDiff: func(d *ResourceDiff, meta interface{}) error { + if err := d.SetNew("ports", []interface{}{5, 2, 1}); err != nil { + return err + } + if err := d.ForceNew("ports"); err != nil { + return err + } + return nil + }, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "ports.#": &terraform.ResourceAttrDiff{ + Old: "3", + New: "3", + }, + "ports.1": &terraform.ResourceAttrDiff{ + Old: "1", + New: "1", + }, + "ports.2": &terraform.ResourceAttrDiff{ + Old: "2", + New: "2", + }, + "ports.5": &terraform.ResourceAttrDiff{ + Old: "", + New: "5", + RequiresNew: true, + }, + "ports.4": &terraform.ResourceAttrDiff{ + Old: "4", + New: "0", + NewRemoved: true, + RequiresNew: true, + }, + }, + }, + }, + + { + Name: "tainted resource does not run CustomizeDiffFunc", + Schema: map[string]*Schema{}, + + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "id": "someid", + }, + Tainted: true, + }, + + Config: map[string]interface{}{}, + + CustomizeDiff: func(d *ResourceDiff, meta interface{}) error { + return errors.New("diff customization should not have run") + }, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{}, + DestroyTainted: true, + }, + + Err: false, + }, + + { + Name: "NewComputed based on a conditional with CustomizeDiffFunc", + Schema: map[string]*Schema{ + "etag": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + }, + "version_id": &Schema{ + Type: TypeString, + Computed: true, + }, + }, + + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "etag": "foo", + "version_id": "1", + }, + }, + + Config: map[string]interface{}{ + "etag": "bar", + }, + + CustomizeDiff: func(d *ResourceDiff, meta interface{}) error { + if d.HasChange("etag") { + d.SetNewComputed("version_id") + } + return nil + }, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "etag": &terraform.ResourceAttrDiff{ + Old: "foo", + New: "bar", + }, + "version_id": &terraform.ResourceAttrDiff{ + Old: "1", + New: "", + NewComputed: true, + }, + }, + }, + + Err: false, + }, + + { + Name: "vetoing a diff", + Schema: map[string]*Schema{ + "foo": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + }, + }, + + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "foo": "bar", + }, + }, + + Config: map[string]interface{}{ + "foo": "baz", + }, + + CustomizeDiff: func(d *ResourceDiff, meta interface{}) error { + return fmt.Errorf("diff vetoed") + }, + + Err: true, + }, } for i, tc := range cases {