allow going from no ip_allocation_policy to a blank-equivalent one (#3723)

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

google/resource_container_cluster.go

@@ -3,6 +3,7 @@ package google
 import (
 	"fmt"
 	"log"
+	"reflect"
 	"regexp"
 	"strings"
 	"time"
@@ -53,6 +54,8 @@ func resourceContainerCluster() *schema.Resource {
 		Update: resourceContainerClusterUpdate,
 		Delete: resourceContainerClusterDelete,
 
+		CustomizeDiff: resourceContainerClusterIpAllocationCustomizeDiff,
+
 		Timeouts: &schema.ResourceTimeout{
 			Create: schema.DefaultTimeout(30 * time.Minute),
 			Update: schema.DefaultTimeout(30 * time.Minute),
@@ -619,6 +622,36 @@ func resourceContainerCluster() *schema.Resource {
 	}
 }
 
+func resourceContainerClusterIpAllocationCustomizeDiff(diff *schema.ResourceDiff, meta interface{}) error {
+	// separate func to allow unit testing
+	return resourceContainerClusterIpAllocationCustomizeDiffFunc(diff)
+}
+
+func resourceContainerClusterIpAllocationCustomizeDiffFunc(diff TerraformResourceDiff) error {
+	o, n := diff.GetChange("ip_allocation_policy")
+
+	oList := o.([]interface{})
+	nList := n.([]interface{})
+	if len(oList) > 0 || len(nList) == 0 {
+		// we only care about going from unset to set, so return early if the field was set before
+		// or is unset now
+		return nil
+	}
+
+	// Unset is equivalent to a block where all the values are zero
+	// This might change if use_ip_aliases ends up defaulting to true server-side.
+	// The console says it will eventually, but it's unclear whether that's in the API
+	// too or just client code.
+	polMap := nList[0].(map[string]interface{})
+	for _, v := range polMap {
+		if !isEmptyValue(reflect.ValueOf(v)) {
+			// found a non-empty value, so continue with the diff as it was
+			return nil
+		}
+	}
+	return diff.Clear("ip_allocation_policy")
+}
+
 func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 

google/resource_container_cluster_test.go

@@ -12,6 +12,69 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
+func TestContainerClusterIpAllocationCustomizeDiff(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]struct {
+		BeforePolicy      []interface{}
+		AfterPolicy       []interface{}
+		ExpectDiffCleared bool
+	}{
+		"empty to false value": {
+			BeforePolicy: []interface{}{},
+			AfterPolicy: []interface{}{
+				map[string]interface{}{
+					"use_ip_aliases": false,
+				},
+			},
+			ExpectDiffCleared: true,
+		},
+		"empty to true value": {
+			BeforePolicy: []interface{}{},
+			AfterPolicy: []interface{}{
+				map[string]interface{}{
+					"use_ip_aliases": true,
+				},
+			},
+			ExpectDiffCleared: false,
+		},
+		"empty to empty": {
+			BeforePolicy:      []interface{}{},
+			AfterPolicy:       []interface{}{},
+			ExpectDiffCleared: false,
+		},
+		"non-empty to non-empty": {
+			BeforePolicy: []interface{}{
+				map[string]interface{}{
+					"use_ip_aliases": false,
+				},
+			},
+			AfterPolicy: []interface{}{
+				map[string]interface{}{
+					"use_ip_aliases": false,
+				},
+			},
+		},
+	}
+
+	for tn, tc := range cases {
+		d := &ResourceDiffMock{
+			Before: map[string]interface{}{
+				"ip_allocation_policy": tc.BeforePolicy,
+			},
+			After: map[string]interface{}{
+				"ip_allocation_policy": tc.AfterPolicy,
+			},
+		}
+		if err := resourceContainerClusterIpAllocationCustomizeDiffFunc(d); err != nil {
+			t.Errorf("%s failed, error calculating diff: %s", tn, err)
+		}
+		if _, ok := d.Cleared["ip_allocation_policy"]; ok != tc.ExpectDiffCleared {
+			t.Errorf("%s failed, expected cleared to be %v, was %v", tn, tc.ExpectDiffCleared, ok)
+		}
+	}
+}
+
 func TestAccContainerCluster_basic(t *testing.T) {
 	t.Parallel()
 

google/test_utils.go

@@ -62,6 +62,24 @@ func (d *ResourceDataMock) Id() string {
 	return d.id
 }
 
+type ResourceDiffMock struct {
+	Before  map[string]interface{}
+	After   map[string]interface{}
+	Cleared map[string]struct{}
+}
+
+func (d *ResourceDiffMock) GetChange(key string) (interface{}, interface{}) {
+	return d.Before[key], d.After[key]
+}
+
+func (d *ResourceDiffMock) Clear(key string) error {
+	if d.Cleared == nil {
+		d.Cleared = map[string]struct{}{}
+	}
+	d.Cleared[key] = struct{}{}
+	return nil
+}
+
 func toBool(attribute string) (bool, error) {
 	// Handle the case where an unset value defaults to false
 	if attribute == "" {

google/utils.go

@@ -27,6 +27,11 @@ type TerraformResourceData interface {
 	Id() string
 }
 
+type TerraformResourceDiff interface {
+	GetChange(string) (interface{}, interface{})
+	Clear(string) error
+}
+
 // getRegionFromZone returns the region from a zone for Google cloud.
 func getRegionFromZone(zone string) string {
 	if zone != "" && len(zone) > 2 {