From 3ac0cdf0c06de0dd8bc8e7de1843a7de1c289349 Mon Sep 17 00:00:00 2001
From: Chris Marchesi <chrism@vancluevertech.com>
Date: Thu, 1 Jun 2017 09:19:03 -0700
Subject: [PATCH] helper/schema: Better ResourceDiff schema key validation

This fixes nil pointer issues that could come up if an invalid key was
referenced (ie: not one in the schema). Also ships a helper validation
function to streamline things.
---
 helper/schema/resource_diff.go      | 28 +++++++++++++++++++++-------
 helper/schema/resource_diff_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/helper/schema/resource_diff.go b/helper/schema/resource_diff.go
index 4ed6d9152..05398998c 100644
--- a/helper/schema/resource_diff.go
+++ b/helper/schema/resource_diff.go
@@ -215,8 +215,8 @@ 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 !d.schema[key].Computed {
-		return fmt.Errorf("Clear is allowed on computed attributes only - %s is not one", key)
+	if err := d.checkKey(key, "Clear"); err != nil {
+		return err
 	}
 
 	return d.clear(key)
@@ -256,9 +256,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 {
-	if !d.schema[key].Computed {
-		return fmt.Errorf("SetNew only operates on computed keys - %s is not one", key)
+	if err := d.checkKey(key, "SetNew"); err != nil {
+		return err
 	}
+
 	return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, value, false)
 }
 
@@ -267,9 +268,10 @@ 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 !d.schema[key].Computed {
-		return fmt.Errorf("SetNewComputed only operates on computed keys - %s is not one", key)
+	if err := d.checkKey(key, "SetNewComputed"); err != nil {
+		return err
 	}
+
 	return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, nil, true)
 }
 
@@ -295,7 +297,7 @@ func (d *ResourceDiff) setDiff(key string, old, new interface{}, computed bool)
 // specific ResourceDiff instance.
 func (d *ResourceDiff) ForceNew(key string) error {
 	if !d.HasChange(key) {
-		return fmt.Errorf("ResourceDiff.ForceNew: No changes for %s", key)
+		return fmt.Errorf("ForceNew: No changes for %s", key)
 	}
 
 	old, new := d.GetChange(key)
@@ -444,3 +446,15 @@ func childAddrOf(child, parent string) bool {
 	}
 	return reflect.DeepEqual(ps, cs[:len(ps)])
 }
+
+// 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 {
+		return fmt.Errorf("%s: invalid key: %s", caller, key)
+	}
+	if !s.Computed {
+		return fmt.Errorf("%s only operates on computed keys - %s is not one", caller, key)
+	}
+	return nil
+}
diff --git a/helper/schema/resource_diff_test.go b/helper/schema/resource_diff_test.go
index 634ab21cc..18ba40935 100644
--- a/helper/schema/resource_diff_test.go
+++ b/helper/schema/resource_diff_test.go
@@ -520,6 +520,34 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 			NewValue:      "qux",
 			ExpectedError: true,
 		},
+		resourceDiffTestCase{
+			Name: "bad key, should error",
+			Schema: map[string]*Schema{
+				"foo": &Schema{
+					Type:     TypeString,
+					Required: true,
+				},
+			},
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"foo": "bar",
+				},
+			},
+			Config: testConfig(t, map[string]interface{}{
+				"foo": "baz",
+			}),
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"foo": &terraform.ResourceAttrDiff{
+						Old: "bar",
+						New: "baz",
+					},
+				},
+			},
+			Key:           "bad",
+			NewValue:      "qux",
+			ExpectedError: true,
+		},
 	}
 }