Resource_container_cluster : check node_version and min_master_version on create (#9602) (#16817)

* 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
[upstream:c1b0db390ba4ee0fa37c330a46dc8a5d05df48a1]

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

Author: modular-magician

.changelog/9602.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+container: added check that node_version and min_master_version are the same on create, when running terraform plan
+```

google/services/container/resource_container_cluster.go

@@ -191,6 +191,7 @@ func ResourceContainerCluster() *schema.Resource {
 			containerClusterNetworkPolicyEmptyCustomizeDiff,
 			containerClusterSurgeSettingsCustomizeDiff,
 			containerClusterEnableK8sBetaApisCustomizeDiff,
+			containerClusterNodeVersionCustomizeDiff,
 		),
 
 		Timeouts: &schema.ResourceTimeout{
@@ -5883,3 +5884,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
+}

google/services/container/resource_container_cluster_internal_test.go

@@ -102,3 +102,109 @@ 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)
+		}
+	}
+}

google/services/container/resource_container_cluster_test.go

@@ -3545,6 +3545,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()
 
@@ -3739,6 +3758,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" {