Resource_container_cluster : check node_version and min_master_versio…

…n on create (#9602)

* added verification for min_master_version == node_version when creating a cluster when running terraform plan

* Added unit test, refactored function

* Added more extensive unit tests

* Added more test cases to the unit tests

mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb

@@ -204,6 +204,7 @@ func ResourceContainerCluster() *schema.Resource {
 			containerClusterNetworkPolicyEmptyCustomizeDiff,
 			containerClusterSurgeSettingsCustomizeDiff,
 			containerClusterEnableK8sBetaApisCustomizeDiff,
+			containerClusterNodeVersionCustomizeDiff,
 		),
 
 		Timeouts: &schema.ResourceTimeout{
@@ -6500,3 +6501,32 @@ func containerClusterEnableK8sBetaApisCustomizeDiffFunc(d tpgresource.TerraformR
 
 	return nil
 }
+
+func containerClusterNodeVersionCustomizeDiff(_ context.Context,diff *schema.ResourceDiff, meta interface{}) error {
+	// separate func to allow unit testing
+	return containerClusterNodeVersionCustomizeDiffFunc(diff)
+}
+
+func containerClusterNodeVersionCustomizeDiffFunc(diff tpgresource.TerraformResourceDiff) error {
+	oldValueName, _ := diff.GetChange("name")
+	if oldValueName != "" {
+		return nil
+	}
+
+	_, newValueNode := diff.GetChange("node_version")
+	_, newValueMaster := diff.GetChange("min_master_version")
+
+	if newValueNode == "" || newValueMaster == "" {
+		return nil
+	}
+
+	//ignore -gke.X suffix for now. If it becomes a problem later, we can fix it
+	masterVersion := strings.Split(newValueMaster.(string), "-")[0]
+	nodeVersion := strings.Split(newValueNode.(string), "-")[0]
+
+	if masterVersion != nodeVersion {
+		return fmt.Errorf("Resource argument node_version (value: %s) must either be unset or set to the same value as min_master_version (value: %s) on create." , newValueNode, newValueMaster)
+	}
+
+	return nil
+}

mmv1/third_party/terraform/services/container/resource_container_cluster_internal_test.go.erb

@@ -188,3 +188,111 @@ func TestContainerClusterEnableK8sBetaApisCustomizeDiff(t *testing.T) {
 		}
 	}
 }
+
+func TestContainerCluster_NodeVersionCustomizeDiff(t* testing.T) {
+	t.Parallel()
+
+	cases := map[string]struct{
+		BeforeName string 
+		AfterName string
+		MasterVersion  string
+		NodeVersion  string
+		ExpectError  bool
+	}{
+		"Master version and node version are exactly the same" : {
+			BeforeName : "",
+			AfterName : "test",
+			MasterVersion : "1.10.9-gke.5",
+			NodeVersion : "1.10.9-gke.5",
+			ExpectError : false,
+		},
+		"Master version and node version have the same Kubernetes patch version but not the same gke-N suffix " : {
+			BeforeName : "",
+			AfterName : "test",
+			MasterVersion : "1.10.9-gke.5",
+			NodeVersion : "1.10.9-gke.9",
+			ExpectError : false,
+		},
+		"Master version and node version have different minor versions" :{
+			BeforeName : "",
+			AfterName : "test",
+			MasterVersion : "1.10.9-gke.5",
+			NodeVersion : "1.11.6-gke.11",
+			ExpectError : true,
+		},
+		"Master version and node version have different Kubernetes Patch Versions" :{
+			BeforeName : "",
+			AfterName : "test",
+			MasterVersion : "1.10.9-gke.5",
+			NodeVersion : "1.10.6-gke.11",
+			ExpectError : true,
+		},
+		"Master version is not set, but node version is" : {
+			BeforeName : "",
+			AfterName : "test",
+			MasterVersion : "",
+			NodeVersion : "1.10.6-gke.11",
+			ExpectError : false,
+		},
+		"Node version is not set, but master version is" : {
+			BeforeName : "",
+			AfterName : "test",
+			MasterVersion : "1.10.6-gke.11",
+			NodeVersion : "",
+			ExpectError : false,
+		},
+		"Node version and master version match, both do not have -gke.X suffix" :{
+			BeforeName : "",
+			AfterName : "test",
+			MasterVersion : "1.10.6",
+			NodeVersion : "1.10.6",
+			ExpectError : false,
+
+		},
+		"Node version and master version do not match, both do not have -gke.X suffix" : {
+			BeforeName : "",
+			AfterName : "test",
+			MasterVersion : "1.10.6",
+			NodeVersion : "1.11.6",
+			ExpectError : true,
+
+		},
+		"Node version and master version do not match, node version has -gke.X suffix but master version doesn't" : {
+			BeforeName : "",
+			AfterName : "test",
+			MasterVersion : "1.11.6",
+			NodeVersion : "1.10.6-gke.11",
+			ExpectError : true,
+		},
+		"Diff is executed in non-create scenario, master version and node version do not match" : {
+			BeforeName : "test",
+			AfterName : "test-1",
+			MasterVersion : "1.11.6-gke.11",
+			NodeVersion : "1.10.6-gke.11",
+			ExpectError : false,
+		},
+	}
+
+	for tn,tc := range cases {
+		d := &tpgresource.ResourceDiffMock{
+			Before: map[string]interface{}{
+				"name" : tc.BeforeName,
+				"min_master_version": "",
+				"node_version": "",
+			},
+			After: map[string]interface{}{
+				"name" : tc.AfterName,
+				"min_master_version": tc.MasterVersion,
+				"node_version": tc.NodeVersion,
+			},
+		}
+		err := containerClusterNodeVersionCustomizeDiffFunc(d)
+
+		if tc.ExpectError && err == nil {
+			t.Errorf("%s failed, expected error but was none", tn)
+		}
+		if !tc.ExpectError && err != nil {
+			t.Errorf("%s failed, found unexpected error: %s", tn, err)
+		}
+	}
+}

mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb

@@ -4080,6 +4080,25 @@ func TestAccContainerCluster_withEnableKubernetesBetaAPIsOnExistingCluster(t *te
 	})
 }
 
+func TestAccContainerCluster_withIncompatibleMasterVersionNodeVersion(t *testing.T) {
+	t.Parallel()
+
+	clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10))
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck: func(){acctest.AccTestPreCheck(t)},
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy: testAccCheckContainerClusterDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccContainerCluster_withIncompatibleMasterVersionNodeVersion(clusterName),
+				PlanOnly: true,
+				ExpectError: regexp.MustCompile(`Resource argument node_version`),
+			},
+		},
+	})
+}
+
 func TestAccContainerCluster_withIPv4Error(t *testing.T) {
 	t.Parallel()
 
@@ -4374,6 +4393,20 @@ resource "google_container_cluster" "primary" {
 `, resource_name)
 }
 
+func testAccContainerCluster_withIncompatibleMasterVersionNodeVersion(name string) string {
+	return fmt.Sprintf(`
+	resource "google_container_cluster" "gke_cluster" {
+		name = "%s"
+		location = "us-central1"
+
+		min_master_version = "1.10.9-gke.5"
+		node_version = "1.10.6-gke.11"
+		initial_node_count = 1
+
+	}
+	`, name)
+}
+
 func testAccContainerCluster_SetSecurityPostureToStandard(resource_name, networkName, subnetworkName string) string {
 	return fmt.Sprintf(`
 resource "google_container_cluster" "with_security_posture_config" {