Remove deprecated fields from Snapshot

Author: rileykarson

google/resource_compute_snapshot.go

@@ -21,145 +21,10 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/hashicorp/terraform/helper/customdiff"
 	"github.com/hashicorp/terraform/helper/schema"
 	compute "google.golang.org/api/compute/v1"
 )
 
-func customDiffComputeSnapshotSnapshotEncryptionKeys(diff *schema.ResourceDiff, meta interface{}) error {
-	oldConvenience, newConvenience := diff.GetChange("snapshot_encryption_key_raw")
-	oldNewField, newNewField := diff.GetChange("snapshot_encryption_key.0.raw_key")
-
-	if newConvenience != "" && newNewField != "" {
-		return fmt.Errorf("can't use snapshot_encryption_key_raw and snapshot_encryption_key.0.raw_key at the same time." +
-			"If you're removing snapshot_encryption_key.0.raw_key, set the value to \"\" instead. This is due to limitations in Terraform.")
-	}
-
-	// Either field (convenience or new) has a value
-	// and then has another different value, so we ForceNew.
-	// We need to handle _EVERY_ ForceNew case in this diff
-	if oldConvenience != "" && newConvenience != "" && oldConvenience != newConvenience {
-		return diff.ForceNew("snapshot_encryption_key_raw")
-	}
-
-	if oldNewField != "" && newNewField != "" && oldNewField != newNewField {
-		return diff.ForceNew("snapshot_encryption_key.0.raw_key")
-	}
-
-	// Our resource isn't using either field, then uses one;
-	// ForceNew on whichever one is now using it.
-	if (oldConvenience == "" && oldNewField == "" && newConvenience != "") || (oldConvenience == "" && oldNewField == "" && newNewField != "") {
-		if oldConvenience == "" && newConvenience != "" {
-			return diff.ForceNew("snapshot_encryption_key_raw")
-		} else {
-			return diff.ForceNew("snapshot_encryption_key.0.raw_key")
-		}
-	}
-
-	// convenience no longer used
-	if oldConvenience != "" && newConvenience == "" {
-		if newNewField == "" {
-			// convenience is being nulled, and the new field is empty as well
-			// we've stopped using the field altogether
-			return diff.ForceNew("snapshot_encryption_key_raw")
-		} else if oldConvenience != newNewField {
-			// convenience is being nulled, and the new field has a new value
-			// so we ForceNew on either field
-			return diff.ForceNew("snapshot_encryption_key_raw")
-		} else {
-			// If we reach it here, we're using the same value in the new field as we had in the convenience field
-		}
-	}
-
-	// new no longer used
-	// note that it will remain _set_ because of how Computed fields work
-	// unset fields will have their values kept in state as a non-zero value
-	if oldNewField != "" && newNewField == "" {
-		if newConvenience == "" {
-			// new field is being nulled, and the convenience field is empty as well
-			// we've stopped using the field altogether
-			return diff.ForceNew("snapshot_encryption_key.0.raw_key")
-		} else if oldNewField != newConvenience {
-			// new is being nulled, and the convenience field has a new value
-			// so we ForceNew on either field
-
-			// This stops a really opaque diffs don't match during apply error. Without this, wee see
-			// a diff from the old state -> new state with a ForceNew at plan time (as expected!)
-			// But during apply time the entire nested object is nil in old state unexpectedly.
-			// So we just force the diff to match more by nilling it here, which is unclear why it
-			// works, and probably a worse UX with some real ugly diff, but also makes the tests pass.
-			// Computed nested fields are hard.
-			err := diff.SetNew("snapshot_encryption_key", nil)
-			if err != nil {
-				return err
-			}
-
-			return diff.ForceNew("snapshot_encryption_key.0.raw_key")
-		} else {
-			// If we reach it here, we're using the same value in the convenience field as we had in the new field
-		}
-	}
-
-	return nil
-}
-
-func customDiffComputeSnapshotSourceDiskEncryptionKeys(diff *schema.ResourceDiff, meta interface{}) error {
-	oldConvenience, newConvenience := diff.GetChange("source_disk_encryption_key_raw")
-	oldNewField, newNewField := diff.GetChange("source_disk_encryption_key.0.raw_key")
-
-	// Either field has a value and then has another value
-	// We need to handle _EVERY_ ForceNew case in this diff
-	if oldConvenience != "" && newConvenience != "" && oldConvenience != newConvenience {
-		return diff.ForceNew("source_disk_encryption_key_raw")
-	}
-
-	if oldNewField != "" && newNewField != "" && oldNewField != newNewField {
-		return diff.ForceNew("source_disk_encryption_key.0.raw_key")
-	}
-
-	// Our resource isn't using either field, then uses one;
-	// ForceNew on whichever one is now using it.
-	if (oldConvenience == "" && oldNewField == "" && newConvenience != "") || (oldConvenience == "" && oldNewField == "" && newNewField != "") {
-		if oldConvenience == "" && newConvenience != "" {
-			return diff.ForceNew("source_disk_encryption_key_raw")
-		} else {
-			return diff.ForceNew("source_disk_encryption_key.0.raw_key")
-		}
-	}
-
-	// convenience no longer used
-	if oldConvenience != "" && newConvenience == "" {
-		if newNewField == "" {
-			// convenience is being nulled, and the new field is empty as well
-			// we've stopped using the field altogether
-			return diff.ForceNew("source_disk_encryption_key_raw")
-		} else if oldConvenience != newNewField {
-			// convenience is being nulled, and the new field has a new value
-			// so we ForceNew on either field
-			return diff.ForceNew("source_disk_encryption_key_raw")
-		} else {
-			// If we reach it here, we're using the same value in the new field as we had in the convenience field
-		}
-	}
-
-	// new no longer used
-	if oldNewField != "" && newNewField == "" {
-		if newConvenience == "" {
-			// new field is being nulled, and the convenience field is empty as well
-			// we've stopped using the field altogether
-			return diff.ForceNew("source_disk_encryption_key.0.raw_key")
-		} else if newConvenience != oldNewField {
-			// new is being nulled, and the convenience field has a new value
-			// so we ForceNew on either field
-			return diff.ForceNew("source_disk_encryption_key.0.raw_key")
-		} else {
-			// If we reach it here, we're using the same value in the convenience field as we had in the new field
-		}
-	}
-
-	return nil
-}
-
 func resourceComputeSnapshot() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceComputeSnapshotCreate,
@@ -177,11 +42,6 @@ func resourceComputeSnapshot() *schema.Resource {
 			Delete: schema.DefaultTimeout(300 * time.Second),
 		},
 
-		CustomizeDiff: customdiff.All(
-			customDiffComputeSnapshotSnapshotEncryptionKeys,
-			customDiffComputeSnapshotSourceDiskEncryptionKeys,
-		),
-
 		Schema: map[string]*schema.Schema{
 			"name": {
 				Type:     schema.TypeString,
@@ -206,14 +66,15 @@ func resourceComputeSnapshot() *schema.Resource {
 			},
 			"snapshot_encryption_key": {
 				Type:     schema.TypeList,
-				Computed: true,
 				Optional: true,
+				ForceNew: true,
 				MaxItems: 1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"raw_key": {
 							Type:      schema.TypeString,
 							Optional:  true,
+							ForceNew:  true,
 							Sensitive: true,
 						},
 						"sha256": {
@@ -226,12 +87,14 @@ func resourceComputeSnapshot() *schema.Resource {
 			"source_disk_encryption_key": {
 				Type:     schema.TypeList,
 				Optional: true,
+				ForceNew: true,
 				MaxItems: 1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"raw_key": {
 							Type:      schema.TypeString,
 							Optional:  true,
+							ForceNew:  true,
 							Sensitive: true,
 						},
 					},
@@ -278,29 +141,29 @@ func resourceComputeSnapshot() *schema.Resource {
 			},
 
 			"snapshot_encryption_key_raw": {
-				Type:       schema.TypeString,
-				Optional:   true,
-				Sensitive:  true,
-				Deprecated: "Use snapshot_encryption_key.raw_key instead.",
+				Type:      schema.TypeString,
+				Optional:  true,
+				Sensitive: true,
+				Removed:   "Use snapshot_encryption_key.raw_key instead.",
 			},
 
 			"snapshot_encryption_key_sha256": {
-				Type:       schema.TypeString,
-				Computed:   true,
-				Deprecated: "Use snapshot_encryption_key.sha256 instead.",
+				Type:     schema.TypeString,
+				Computed: true,
+				Removed:  "Use snapshot_encryption_key.sha256 instead.",
 			},
 
 			"source_disk_encryption_key_raw": {
-				Type:       schema.TypeString,
-				Optional:   true,
-				Sensitive:  true,
-				Deprecated: "Use source_disk_encryption_key.raw_key instead.",
+				Type:      schema.TypeString,
+				Optional:  true,
+				Sensitive: true,
+				Removed:   "Use source_disk_encryption_key.raw_key instead.",
 			},
 
 			"source_disk_encryption_key_sha256": {
-				Type:       schema.TypeString,
-				Computed:   true,
-				Deprecated: "Use source_disk_encryption_key.sha256 instead.",
+				Type:     schema.TypeString,
+				Computed: true,
+				Removed:  "Use source_disk_encryption_key.sha256 instead.",
 			},
 			"project": {
 				Type:     schema.TypeString,
@@ -713,47 +576,62 @@ func expandComputeSnapshotZone(v interface{}, d *schema.ResourceData, config *Co
 
 func expandComputeSnapshotSnapshotEncryptionKey(v interface{}, d *schema.ResourceData, config *Config) (interface{}, error) {
 	l := v.([]interface{})
-	req := make([]interface{}, 0, 1)
-	if len(l) == 1 && l[0].(map[string]interface{})["raw_key"] != "" {
-		// There is a value
-		outMap := make(map[string]interface{})
-		outMap["rawKey"] = l[0].(map[string]interface{})["raw_key"]
-		req = append(req, outMap)
-	} else {
-		// Check alternative setting?
-		if altV, ok := d.GetOk("snapshot_encryption_key_raw"); ok && altV != "" {
-			outMap := make(map[string]interface{})
-			outMap["rawKey"] = altV
-			req = append(req, outMap)
-		}
+	if len(l) == 0 || l[0] == nil {
+		return nil, nil
+	}
+	raw := l[0]
+	original := raw.(map[string]interface{})
+	transformed := make(map[string]interface{})
+
+	transformedRawKey, err := expandComputeSnapshotSnapshotEncryptionKeyRawKey(original["raw_key"], d, config)
+	if err != nil {
+		return nil, err
+	} else if val := reflect.ValueOf(transformedRawKey); val.IsValid() && !isEmptyValue(val) {
+		transformed["rawKey"] = transformedRawKey
 	}
-	return req, nil
+
+	transformedSha256, err := expandComputeSnapshotSnapshotEncryptionKeySha256(original["sha256"], d, config)
+	if err != nil {
+		return nil, err
+	} else if val := reflect.ValueOf(transformedSha256); val.IsValid() && !isEmptyValue(val) {
+		transformed["sha256"] = transformedSha256
+	}
+
+	return transformed, nil
+}
+
+func expandComputeSnapshotSnapshotEncryptionKeyRawKey(v interface{}, d *schema.ResourceData, config *Config) (interface{}, error) {
+	return v, nil
+}
+
+func expandComputeSnapshotSnapshotEncryptionKeySha256(v interface{}, d *schema.ResourceData, config *Config) (interface{}, error) {
+	return v, nil
 }
 
 func expandComputeSnapshotSourceDiskEncryptionKey(v interface{}, d *schema.ResourceData, config *Config) (interface{}, error) {
 	l := v.([]interface{})
-	req := make([]interface{}, 0, 1)
-	if len(l) == 1 {
-		// There is a value
-		outMap := make(map[string]interface{})
-		outMap["rawKey"] = l[0].(map[string]interface{})["raw_key"]
-		req = append(req, outMap)
-	} else {
-		// Check alternative setting?
-		if altV, ok := d.GetOk("source_disk_encryption_key_raw"); ok && altV != "" {
-			outMap := make(map[string]interface{})
-			outMap["rawKey"] = altV
-			req = append(req, outMap)
-		}
+	if len(l) == 0 || l[0] == nil {
+		return nil, nil
 	}
-	return req, nil
+	raw := l[0]
+	original := raw.(map[string]interface{})
+	transformed := make(map[string]interface{})
+
+	transformedRawKey, err := expandComputeSnapshotSourceDiskEncryptionKeyRawKey(original["raw_key"], d, config)
+	if err != nil {
+		return nil, err
+	} else if val := reflect.ValueOf(transformedRawKey); val.IsValid() && !isEmptyValue(val) {
+		transformed["rawKey"] = transformedRawKey
+	}
+
+	return transformed, nil
+}
+
+func expandComputeSnapshotSourceDiskEncryptionKeyRawKey(v interface{}, d *schema.ResourceData, config *Config) (interface{}, error) {
+	return v, nil
 }
 
 func resourceComputeSnapshotDecoder(d *schema.ResourceData, meta interface{}, res map[string]interface{}) (map[string]interface{}, error) {
 	d.Set("source_disk_link", ConvertSelfLinkToV1(res["sourceDisk"].(string)))
-	if snapshotEncryptionKey := res["snapshotEncryptionKey"]; snapshotEncryptionKey != nil {
-		d.Set("snapshot_encryption_key_sha256", snapshotEncryptionKey.((map[string]interface{}))["sha256"])
-	}
-
 	return res, nil
 }