Merge pull request #1034 from emilymye/container_network_policy_1031

Add diff suppress for empty network policy provider in GKE cluster

Author: emilymye

google/resource_container_cluster.go

@@ -303,10 +303,11 @@ func resourceContainerCluster() *schema.Resource {
 							Default:  false,
 						},
 						"provider": {
-							Type:         schema.TypeString,
-							Default:      "PROVIDER_UNSPECIFIED",
-							Optional:     true,
-							ValidateFunc: validation.StringInSlice([]string{"PROVIDER_UNSPECIFIED", "CALICO"}, false),
+							Type:             schema.TypeString,
+							Default:          "PROVIDER_UNSPECIFIED",
+							Optional:         true,
+							ValidateFunc:     validation.StringInSlice([]string{"PROVIDER_UNSPECIFIED", "CALICO"}, false),
+							DiffSuppressFunc: emptyOrDefaultStringSuppress("PROVIDER_UNSPECIFIED"),
 						},
 					},
 				},

google/resource_container_cluster_test.go

@@ -20,13 +20,14 @@ import (
 func TestAccContainerCluster_basic(t *testing.T) {
 	t.Parallel()
 
+	clusterName := fmt.Sprintf("cluster-test-%s", acctest.RandString(10))
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckContainerClusterDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccContainerCluster_basic(fmt.Sprintf("cluster-test-%s", acctest.RandString(10))),
+				Config: testAccContainerCluster_basic(clusterName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckContainerCluster(
 						"google_container_cluster.primary"),
@@ -135,6 +136,20 @@ func TestAccContainerCluster_withNetworkPolicyEnabled(t *testing.T) {
 						"network_policy"),
 				),
 			},
+			{
+				Config: testAccContainerCluster_withNetworkPolicyDisabled(clusterName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckContainerCluster(
+						"google_container_cluster.with_network_policy_enabled"),
+					resource.TestCheckResourceAttr("google_container_cluster.with_network_policy_enabled",
+						"network_policy.0.enabled", "false"),
+				),
+			},
+			{
+				Config:             testAccContainerCluster_withNetworkPolicyDisabled(clusterName),
+				PlanOnly:           true,
+				ExpectNonEmptyPlan: false,
+			},
 		},
 	})
 }
@@ -1085,6 +1100,17 @@ resource "google_container_cluster" "with_network_policy_enabled" {
 }`, clusterName)
 }
 
+func testAccContainerCluster_withNetworkPolicyDisabled(clusterName string) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "with_network_policy_enabled" {
+	name = "%s"
+	zone = "us-central1-a"
+	initial_node_count = 1
+
+	network_policy = {}
+}`, clusterName)
+}
+
 func testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName string, cidrs []string) string {
 
 	cidrBlocks := ""

google/utils.go

@@ -184,6 +184,12 @@ func optionalPrefixSuppress(prefix string) schema.SchemaDiffSuppressFunc {
 	}
 }
 
+func emptyOrDefaultStringSuppress(defaultVal string) schema.SchemaDiffSuppressFunc {
+	return func(k, old, new string, d *schema.ResourceData) bool {
+		return (old == "" && new == defaultVal) || (new == "" && old == defaultVal)
+	}
+}
+
 func ipCidrRangeDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
 	// The range may be a:
 	// A) single IP address (e.g. 10.2.3.4)

google/utils_test.go

@@ -404,3 +404,43 @@ func TestDatasourceSchemaFromResourceSchema(t *testing.T) {
 		})
 	}
 }
+
+func TestEmptyOrDefaultStringSuppress(t *testing.T) {
+	testFunc := emptyOrDefaultStringSuppress("default value")
+
+	cases := map[string]struct {
+		Old, New          string
+		ExpectDiffSupress bool
+	}{
+		"same value, format changed from empty to default": {
+			Old:               "",
+			New:               "default value",
+			ExpectDiffSupress: true,
+		},
+		"same value, format changed from default to empty": {
+			Old:               "default value",
+			New:               "",
+			ExpectDiffSupress: true,
+		},
+		"different value, format changed from empty to non-default": {
+			Old:               "",
+			New:               "not default new",
+			ExpectDiffSupress: false,
+		},
+		"different value, format changed from non-default to empty": {
+			Old:               "not default old",
+			New:               "",
+			ExpectDiffSupress: false,
+		},
+		"different value, format changed from non-default to non-default": {
+			Old:               "not default 1",
+			New:               "not default 2",
+			ExpectDiffSupress: false,
+		},
+	}
+	for tn, tc := range cases {
+		if testFunc("", tc.Old, tc.New, nil) != tc.ExpectDiffSupress {
+			t.Errorf("bad: %s, '%s' => '%s' expect DiffSuppress to return %t", tn, tc.Old, tc.New, tc.ExpectDiffSupress)
+		}
+	}
+}