helper/schema: Hook CustomizeDiffFunc into diff logic

It's alive! CustomizeDiff logic now has been inserted into the diff
process. The test_resource_with_custom_diff resource provides some basic
testing and a reference implementation.

There should now be plenty of test coverage for this feature via the
tests added for ResourceDiff, and the basic test added to the
schemaMap.Diff test, and the test resource, but more can be added to
test any specific case that comes up otherwise.

builtin/providers/test/provider.go

@@ -17,8 +17,9 @@ func Provider() terraform.ResourceProvider {
 			},
 		},
 		ResourcesMap: map[string]*schema.Resource{
-			"test_resource":         testResource(),
-			"test_resource_gh12183": testResourceGH12183(),
+			"test_resource":                  testResource(),
+			"test_resource_gh12183":          testResourceGH12183(),
+			"test_resource_with_custom_diff": testResourceCustomDiff(),
 		},
 		DataSourcesMap: map[string]*schema.Resource{
 			"test_data_source":    testDataSource(),

builtin/providers/test/resource_with_custom_diff.go

@@ -0,0 +1,84 @@
+package test
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+func testResourceCustomDiff() *schema.Resource {
+	return &schema.Resource{
+		Create:        testResourceCustomDiffCreate,
+		Read:          testResourceCustomDiffRead,
+		CustomizeDiff: testResourceCustomDiffCustomizeDiff,
+		Update:        testResourceCustomDiffUpdate,
+		Delete:        testResourceCustomDiffDelete,
+		Schema: map[string]*schema.Schema{
+			"required": {
+				Type:     schema.TypeString,
+				Required: true,
+			},
+			"computed": {
+				Type:     schema.TypeInt,
+				Computed: true,
+			},
+			"index": {
+				Type:     schema.TypeInt,
+				Computed: true,
+			},
+			"veto": {
+				Type:     schema.TypeBool,
+				Optional: true,
+			},
+		},
+	}
+}
+
+func testResourceCustomDiffCreate(d *schema.ResourceData, meta interface{}) error {
+	d.SetId("testId")
+
+	// Required must make it through to Create
+	if _, ok := d.GetOk("required"); !ok {
+		return fmt.Errorf("missing attribute 'required', but it's required")
+	}
+
+	_, new := d.GetChange("computed")
+	expected := new.(int) - 1
+	actual := d.Get("index").(int)
+	if expected != actual {
+		return fmt.Errorf("expected computed to be 1 ahead of index, got computed: %d, index: %d", expected, actual)
+	}
+	d.Set("index", new)
+
+	return testResourceCustomDiffRead(d, meta)
+}
+
+func testResourceCustomDiffRead(d *schema.ResourceData, meta interface{}) error {
+	return nil
+}
+
+func testResourceCustomDiffCustomizeDiff(d *schema.ResourceDiff, meta interface{}) error {
+	if d.Get("veto").(bool) == true {
+		return fmt.Errorf("veto is true, diff vetoed")
+	}
+	// Note that this gets put into state after the update, regardless of whether
+	// or not anything is acted upon in the diff.
+	d.SetNew("computed", d.Get("computed").(int)+1)
+	return nil
+}
+
+func testResourceCustomDiffUpdate(d *schema.ResourceData, meta interface{}) error {
+	_, new := d.GetChange("computed")
+	expected := new.(int) - 1
+	actual := d.Get("index").(int)
+	if expected != actual {
+		return fmt.Errorf("expected computed to be 1 ahead of index, got computed: %d, index: %d", expected, actual)
+	}
+	d.Set("index", new)
+	return nil
+}
+
+func testResourceCustomDiffDelete(d *schema.ResourceData, meta interface{}) error {
+	d.SetId("")
+	return nil
+}

builtin/providers/test/resource_with_custom_diff_test.go

@@ -0,0 +1,47 @@
+package test
+
+import (
+	"fmt"
+	"regexp"
+	"testing"
+
+	"github.com/hashicorp/terraform/helper/resource"
+)
+
+// TestResourceWithCustomDiff test custom diff behaviour.
+func TestResourceWithCustomDiff(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: resourceWithCustomDiffConfig(false),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("test_resource_with_custom_diff.foo", "computed", "1"),
+					resource.TestCheckResourceAttr("test_resource_with_custom_diff.foo", "index", "1"),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+			{
+				Config: resourceWithCustomDiffConfig(false),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("test_resource_with_custom_diff.foo", "computed", "2"),
+					resource.TestCheckResourceAttr("test_resource_with_custom_diff.foo", "index", "2"),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+			{
+				Config:      resourceWithCustomDiffConfig(true),
+				ExpectError: regexp.MustCompile("veto is true, diff vetoed"),
+			},
+		},
+	})
+}
+
+func resourceWithCustomDiffConfig(veto bool) string {
+	return fmt.Sprintf(`
+resource "test_resource_with_custom_diff" "foo" {
+	required = "yep"
+	veto = %t
+}
+`, veto)
+}

helper/schema/backend.go

@@ -65,7 +65,7 @@ func (b *Backend) Configure(c *terraform.ResourceConfig) error {
 
 	// Get a ResourceData for this configuration. To do this, we actually
 	// generate an intermediary "diff" although that is never exposed.
-	diff, err := sm.Diff(nil, c)
+	diff, err := sm.Diff(nil, c, nil, nil)
 	if err != nil {
 		return err
 	}

helper/schema/provider.go

@@ -251,7 +251,7 @@ func (p *Provider) Configure(c *terraform.ResourceConfig) error {
 
 	// Get a ResourceData for this configuration. To do this, we actually
 	// generate an intermediary "diff" although that is never exposed.
-	diff, err := sm.Diff(nil, c)
+	diff, err := sm.Diff(nil, c, nil, p.meta)
 	if err != nil {
 		return err
 	}
@@ -293,7 +293,7 @@ func (p *Provider) Diff(
 		return nil, fmt.Errorf("unknown resource type: %s", info.Type)
 	}
 
-	return r.Diff(s, c)
+	return r.Diff(s, c, p.meta)
 }
 
 // Refresh implementation of terraform.ResourceProvider interface.
@@ -410,7 +410,7 @@ func (p *Provider) ReadDataDiff(
 		return nil, fmt.Errorf("unknown data source: %s", info.Type)
 	}
 
-	return r.Diff(nil, c)
+	return r.Diff(nil, c, p.meta)
 }
 
 // RefreshData implementation of terraform.ResourceProvider interface.

helper/schema/provisioner.go

@@ -146,7 +146,7 @@ func (p *Provisioner) Apply(
 		}
 
 		sm := schemaMap(p.ConnSchema)
-		diff, err := sm.Diff(nil, terraform.NewResourceConfig(c))
+		diff, err := sm.Diff(nil, terraform.NewResourceConfig(c), nil, nil)
 		if err != nil {
 			return err
 		}
@@ -160,7 +160,7 @@ func (p *Provisioner) Apply(
 		// Build the configuration data. Doing this requires making a "diff"
 		// even though that's never used. We use that just to get the correct types.
 		configMap := schemaMap(p.Schema)
-		diff, err := configMap.Diff(nil, c)
+		diff, err := configMap.Diff(nil, c, nil, nil)
 		if err != nil {
 			return err
 		}

helper/schema/resource.go

@@ -219,7 +219,8 @@ func (r *Resource) Apply(
 // ResourceProvider interface.
 func (r *Resource) Diff(
 	s *terraform.InstanceState,
-	c *terraform.ResourceConfig) (*terraform.InstanceDiff, error) {
+	c *terraform.ResourceConfig,
+	meta interface{}) (*terraform.InstanceDiff, error) {
 
 	t := &ResourceTimeout{}
 	err := t.ConfigDecode(r, c)
@@ -228,7 +229,7 @@ func (r *Resource) Diff(
 		return nil, fmt.Errorf("[ERR] Error decoding timeout: %s", err)
 	}
 
-	instanceDiff, err := schemaMap(r.Schema).Diff(s, c)
+	instanceDiff, err := schemaMap(r.Schema).Diff(s, c, r.CustomizeDiff, meta)
 	if err != nil {
 		return instanceDiff, err
 	}

helper/schema/resource_diff.go

@@ -259,7 +259,7 @@ func (d *ResourceDiff) diffChange(key string) (interface{}, interface{}, bool, b
 // doing so will taint any existing diff for this key and will remove it from
 // the catalog.
 func (d *ResourceDiff) SetNew(key string, value interface{}) error {
-	return d.SetDiff(key, d.getExact(strings.Split(key, "."), "state").Value, value, false)
+	return d.SetDiff(key, d.get(strings.Split(key, "."), "state").Value, value, false)
 }
 
 // SetNewComputed functions like SetNew, except that it sets the new value to
@@ -267,7 +267,7 @@ func (d *ResourceDiff) SetNew(key string, value interface{}) error {
 //
 // This function is only allowed on computed keys.
 func (d *ResourceDiff) SetNewComputed(key string) error {
-	return d.SetDiff(key, d.getExact(strings.Split(key, "."), "state").Value, d.schema[key].ZeroValue(), true)
+	return d.SetDiff(key, d.get(strings.Split(key, "."), "state").Value, d.schema[key].ZeroValue(), true)
 }
 
 // SetDiff allows the setting of both old and new values for the diff

helper/schema/resource_test.go

@@ -226,7 +226,7 @@ func TestResourceDiff_Timeout_diff(t *testing.T) {
 	var s *terraform.InstanceState = nil
 	conf := terraform.NewResourceConfig(raw)
 
-	actual, err := r.Diff(s, conf)
+	actual, err := r.Diff(s, conf, nil)
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}

helper/schema/schema.go

@@ -385,7 +385,9 @@ func (m *schemaMap) DeepCopy() schemaMap {
 // state, and configuration.
 func (m schemaMap) Diff(
 	s *terraform.InstanceState,
-	c *terraform.ResourceConfig) (*terraform.InstanceDiff, error) {
+	c *terraform.ResourceConfig,
+	customizeFunc CustomizeDiffFunc,
+	meta interface{}) (*terraform.InstanceDiff, error) {
 	result := new(terraform.InstanceDiff)
 	result.Attributes = make(map[string]*terraform.ResourceAttrDiff)
 
@@ -407,6 +409,22 @@ func (m schemaMap) Diff(
 		}
 	}
 
+	// If this is a non-destroy diff, call any custom diff logic that has been
+	// defined.
+	if !result.Destroy && !result.DestroyTainted && customizeFunc != nil {
+		mc := m.DeepCopy()
+		rd := newResourceDiff(mc, c, s, result)
+		if err := customizeFunc(rd, meta); err != nil {
+			return nil, err
+		}
+		for _, k := range rd.UpdatedKeys() {
+			err := m.diff(k, mc[k], result, rd, false)
+			if err != nil {
+				return nil, err
+			}
+		}
+	}
+
 	// If the diff requires a new resource, then we recompute the diff
 	// so we have the complete new resource diff, and preserve the
 	// RequiresNew fields where necessary so the user knows exactly what
@@ -432,6 +450,21 @@ func (m schemaMap) Diff(
 			}
 		}
 
+		// Re-run customization
+		if customizeFunc != nil {
+			mc := m.DeepCopy()
+			rd := newResourceDiff(mc, c, d.state, result2)
+			if err := customizeFunc(rd, meta); err != nil {
+				return nil, err
+			}
+			for _, k := range rd.UpdatedKeys() {
+				err := m.diff(k, mc[k], result2, rd, false)
+				if err != nil {
+					return nil, err
+				}
+			}
+		}
+
 		// Force all the fields to not force a new since we know what we
 		// want to force new.
 		for k, attr := range result2.Attributes {

helper/schema/schema_test.go

@@ -138,6 +138,7 @@ func TestSchemaMap_Diff(t *testing.T) {
 		State           *terraform.InstanceState
 		Config          map[string]interface{}
 		ConfigVariables map[string]ast.Variable
+		CustomizeDiff   CustomizeDiffFunc
 		Diff            *terraform.InstanceDiff
 		Err             bool
 	}{
@@ -2823,6 +2824,43 @@ func TestSchemaMap_Diff(t *testing.T) {
 
 			Err: false,
 		},
+
+		{
+			Name: "overridden diff with a CustomizeDiff function",
+			Schema: map[string]*Schema{
+				"availability_zone": &Schema{
+					Type:     TypeString,
+					Optional: true,
+					Computed: true,
+					ForceNew: 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
+				}
+				return nil
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"availability_zone": &terraform.ResourceAttrDiff{
+						Old:         "",
+						New:         "bar",
+						RequiresNew: true,
+					},
+				},
+			},
+
+			Err: false,
+		},
 	}
 
 	for i, tc := range cases {
@@ -2838,8 +2876,7 @@ func TestSchemaMap_Diff(t *testing.T) {
 				}
 			}
 
-			d, err := schemaMap(tc.Schema).Diff(
-				tc.State, terraform.NewResourceConfig(c))
+			d, err := schemaMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), tc.CustomizeDiff, nil)
 			if err != nil != tc.Err {
 				t.Fatalf("err: %s", err)
 			}
@@ -3689,8 +3726,7 @@ func TestSchemaMap_DiffSuppress(t *testing.T) {
 				}
 			}
 
-			d, err := schemaMap(tc.Schema).Diff(
-				tc.State, terraform.NewResourceConfig(c))
+			d, err := schemaMap(tc.Schema).Diff(tc.State, terraform.NewResourceConfig(c), nil, nil)
 			if err != nil != tc.Err {
 				t.Fatalf("#%q err: %s", tn, err)
 			}

helper/schema/testing.go

@@ -18,7 +18,7 @@ func TestResourceDataRaw(
 	}
 
 	sm := schemaMap(schema)
-	diff, err := sm.Diff(nil, terraform.NewResourceConfig(c))
+	diff, err := sm.Diff(nil, terraform.NewResourceConfig(c), nil, nil)
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}