Add workload_pool to google_container_cluster, deprecate identity_nam…

…espace (#5317) (#10327)

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

.changelog/5317.txt

@@ -0,0 +1,6 @@
+```release-note:deprecation
+container: deprecated `workload_identity_config.0.identity_namespace` and it will be removed in a future major release as it has been deprecated in the API. Use `workload_identity_config.0.workload_pool` instead. Switching your configuration from one value to the other will trigger a diff at plan time, and a spurious update.
+```
+```release-note:enhancement
+container: added `workload_identity_config.0.workload_pool` to `google_container_cluster`
+```

google/resource_container_cluster.go

@@ -933,18 +933,28 @@ func resourceContainerCluster() *schema.Resource {
 				},
 			},
 			"workload_identity_config": {
-				Type:          schema.TypeList,
-				MaxItems:      1,
-				Optional:      true,
+				Type:     schema.TypeList,
+				MaxItems: 1,
+				Optional: true,
+				// Computed is unsafe to remove- this API may return `"workloadIdentityConfig": {},` or omit the key entirely
+				// and both will be valid. Note that we don't handle the case where the API returns nothing & the user has defined
+				// workload_identity_config today.
 				Computed:      true,
 				Description:   `Configuration for the use of Kubernetes Service Accounts in GCP IAM policies.`,
 				ConflictsWith: []string{"enable_autopilot"},
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"identity_namespace": {
 							Type:        schema.TypeString,
-							Required:    true,
+							Optional:    true,
 							Description: `Enables workload identity.`,
+							Deprecated:  "This field will be removed in a future major release as it has been deprecated in the API. Use `workload_pool` instead.",
+						},
+
+						"workload_pool": {
+							Type:        schema.TypeString,
+							Optional:    true,
+							Description: "The workload pool to attach all Kubernetes service accounts to.",
 						},
 					},
 				},
@@ -1610,7 +1620,7 @@ func resourceContainerClusterRead(d *schema.ResourceData, meta interface{}) erro
 		return err
 	}
 
-	if err := d.Set("workload_identity_config", flattenWorkloadIdentityConfig(cluster.WorkloadIdentityConfig)); err != nil {
+	if err := d.Set("workload_identity_config", flattenWorkloadIdentityConfig(cluster.WorkloadIdentityConfig, d, config)); err != nil {
 		return err
 	}
 
@@ -2970,13 +2980,19 @@ func expandDefaultSnatStatus(configured interface{}) *containerBeta.DefaultSnatS
 
 func expandWorkloadIdentityConfig(configured interface{}) *containerBeta.WorkloadIdentityConfig {
 	l := configured.([]interface{})
+	v := &containerBeta.WorkloadIdentityConfig{}
+
+	// this API considers unset and set-to-empty equivalent. Note that it will
+	// always return an empty block given that we always send one, but clusters
+	// not created in TF will not always return one (and may return nil)
 	if len(l) == 0 || l[0] == nil {
-		return nil
+		return v
 	}
+
 	config := l[0].(map[string]interface{})
-	return &containerBeta.WorkloadIdentityConfig{
-		IdentityNamespace: config["identity_namespace"].(string),
-	}
+	v.IdentityNamespace = config["identity_namespace"].(string)
+	v.WorkloadPool = config["workload_pool"].(string)
+	return v
 }
 
 func expandPodSecurityPolicyConfig(configured interface{}) *containerBeta.PodSecurityPolicyConfig {
@@ -3199,10 +3215,32 @@ func flattenDefaultSnatStatus(c *containerBeta.DefaultSnatStatus) []map[string]i
 	return result
 }
 
-func flattenWorkloadIdentityConfig(c *containerBeta.WorkloadIdentityConfig) []map[string]interface{} {
+func flattenWorkloadIdentityConfig(c *containerBeta.WorkloadIdentityConfig, d *schema.ResourceData, config *Config) []map[string]interface{} {
 	if c == nil {
 		return nil
 	}
+
+	_, identityNamespaceSet := d.GetOk("workload_identity_config.0.identity_namespace")
+	_, workloadPoolSet := d.GetOk("workload_identity_config.0.workload_pool")
+
+	if identityNamespaceSet && workloadPoolSet {
+		// if both are set, set both
+		return []map[string]interface{}{
+			{
+				"identity_namespace": c.IdentityNamespace,
+				"workload_pool":      c.WorkloadPool,
+			},
+		}
+	} else if workloadPoolSet {
+		// if the new value is set, set it
+		return []map[string]interface{}{
+			{
+				"workload_pool": c.WorkloadPool,
+			},
+		}
+	}
+
+	// otherwise, set the old value (incl. import)
 	return []map[string]interface{}{
 		{
 			"identity_namespace": c.IdentityNamespace,

google/resource_container_cluster_test.go

@@ -146,10 +146,11 @@ func TestAccContainerCluster_withAddons(t *testing.T) {
 				Config: testAccContainerCluster_withAddons(pid, clusterName),
 			},
 			{
-				ResourceName:            "google_container_cluster.primary",
-				ImportState:             true,
-				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"min_master_version"},
+				ResourceName:      "google_container_cluster.primary",
+				ImportState:       true,
+				ImportStateVerify: true,
+				// TODO: clean up this list in `4.0.0`, remove both `workload_identity_config` fields (same for below)
+				ImportStateVerifyIgnore: []string{"min_master_version", "workload_identity_config.0.identity_namespace", "workload_identity_config.0.workload_pool"},
 			},
 			{
 				Config: testAccContainerCluster_updateAddons(pid, clusterName),
@@ -158,7 +159,7 @@ func TestAccContainerCluster_withAddons(t *testing.T) {
 				ResourceName:            "google_container_cluster.primary",
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"min_master_version"},
+				ImportStateVerifyIgnore: []string{"min_master_version", "workload_identity_config.0.identity_namespace", "workload_identity_config.0.workload_pool"},
 			},
 			{
 				Config: testAccContainerCluster_withInternalLoadBalancer(pid, clusterName),
@@ -167,7 +168,7 @@ func TestAccContainerCluster_withAddons(t *testing.T) {
 				ResourceName:            "google_container_cluster.primary",
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"min_master_version"},
+				ImportStateVerifyIgnore: []string{"min_master_version", "workload_identity_config.0.identity_namespace", "workload_identity_config.0.workload_pool"},
 			},
 		},
 	})
@@ -1394,14 +1395,57 @@ func TestAccContainerCluster_withWorkloadIdentityConfig(t *testing.T) {
 			{
 				Config: testAccContainerCluster_withWorkloadIdentityConfigEnabled(pid, clusterName),
 			},
+			{
+				ResourceName:      "google_container_cluster.with_workload_identity_config",
+				ImportState:       true,
+				ImportStateVerify: true,
+				// TODO: clean up this list in `4.0.0`, remove both `workload_identity_config` fields (same for below)
+				ImportStateVerifyIgnore: []string{"remove_default_node_pool", "workload_identity_config.0.identity_namespace", "workload_identity_config.0.workload_pool"},
+			},
+			{
+				Config: testAccContainerCluster_updateWorkloadIdentityConfig(pid, clusterName, false),
+			},
+			{
+				ResourceName:            "google_container_cluster.with_workload_identity_config",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"remove_default_node_pool", "workload_identity_config.0.identity_namespace", "workload_identity_config.0.workload_pool"},
+			},
+			{
+				Config: testAccContainerCluster_updateWorkloadIdentityConfig(pid, clusterName, true),
+			},
+			{
+				ResourceName:            "google_container_cluster.with_workload_identity_config",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"remove_default_node_pool", "workload_identity_config.0.identity_namespace", "workload_identity_config.0.workload_pool"},
+			},
+		},
+	})
+}
+
+func TestAccContainerCluster_withWorkloadIdentityConfigDeprecation(t *testing.T) {
+	t.Parallel()
+
+	clusterName := fmt.Sprintf("tf-test-cluster-%s", randString(t, 10))
+	pid := getTestProjectFromEnv()
+
+	vcrTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckContainerClusterDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccContainerCluster_withWorkloadIdentityConfigDeprecationDisabled(pid, clusterName),
+			},
 			{
 				ResourceName:            "google_container_cluster.with_workload_identity_config",
 				ImportState:             true,
 				ImportStateVerify:       true,
 				ImportStateVerifyIgnore: []string{"remove_default_node_pool"},
 			},
 			{
-				Config: testAccContainerCluster_updateWorkloadIdentityConfig(pid, clusterName, false),
+				Config: testAccContainerCluster_withWorkloadIdentityConfigDeprecationIdentityNamespace(pid, clusterName),
 			},
 			{
 				ResourceName:            "google_container_cluster.with_workload_identity_config",
@@ -1410,7 +1454,14 @@ func TestAccContainerCluster_withWorkloadIdentityConfig(t *testing.T) {
 				ImportStateVerifyIgnore: []string{"remove_default_node_pool"},
 			},
 			{
-				Config: testAccContainerCluster_updateWorkloadIdentityConfig(pid, clusterName, true),
+				Config: testAccContainerCluster_withWorkloadIdentityConfigDeprecationWorkloadPool(pid, clusterName),
+			},
+			// skip ISV here + below as ignoring nested fields is a pain. It will import with `identity_namespace` only.
+			{
+				Config: testAccContainerCluster_withWorkloadIdentityConfigDeprecationBoth(pid, clusterName),
+			},
+			{
+				Config: testAccContainerCluster_withWorkloadIdentityConfigDeprecationDisabled(pid, clusterName),
 			},
 			{
 				ResourceName:            "google_container_cluster.with_workload_identity_config",
@@ -1917,7 +1968,7 @@ resource "google_container_cluster" "primary" {
   min_master_version = "latest"
 
   workload_identity_config {
-    identity_namespace = "${data.google_project.project.project_id}.svc.id.goog"
+    workload_pool = "${data.google_project.project.project_id}.svc.id.goog"
   }
 
   addons_config {
@@ -1952,7 +2003,7 @@ resource "google_container_cluster" "primary" {
   min_master_version = "latest"
 
   workload_identity_config {
-    identity_namespace = "${data.google_project.project.project_id}.svc.id.goog"
+    workload_pool = "${data.google_project.project.project_id}.svc.id.goog"
   }
 
   addons_config {
@@ -1987,7 +2038,7 @@ resource "google_container_cluster" "primary" {
   min_master_version = "latest"
 
   workload_identity_config {
-    identity_namespace = "${data.google_project.project.project_id}.svc.id.goog"
+    workload_pool = "${data.google_project.project.project_id}.svc.id.goog"
   }
 
   addons_config {
@@ -3245,7 +3296,7 @@ resource "google_container_cluster" "with_workload_identity_config" {
   initial_node_count = 1
 
   workload_identity_config {
-    identity_namespace = "${data.google_project.project.project_id}.svc.id.goog"
+    workload_pool = "${data.google_project.project.project_id}.svc.id.goog"
   }
   remove_default_node_pool = true
 
@@ -3258,12 +3309,12 @@ func testAccContainerCluster_updateWorkloadIdentityConfig(projectID string, clus
 	if enable {
 		workloadIdentityConfig = `
 			workload_identity_config {
-			identity_namespace = "${data.google_project.project.project_id}.svc.id.goog"
+		  workload_pool = "${data.google_project.project.project_id}.svc.id.goog"
 		}`
 	} else {
 		workloadIdentityConfig = `
 			workload_identity_config {
-			identity_namespace = ""
+			workload_pool = ""
 		}`
 	}
 	return fmt.Sprintf(`
@@ -3281,6 +3332,80 @@ resource "google_container_cluster" "with_workload_identity_config" {
 `, projectID, clusterName, workloadIdentityConfig)
 }
 
+// TODO: remove until next TODO during `4.0.0`
+func testAccContainerCluster_withWorkloadIdentityConfigDeprecationDisabled(projectID string, clusterName string) string {
+	return fmt.Sprintf(`
+data "google_project" "project" {
+  project_id = "%s"
+}
+
+resource "google_container_cluster" "with_workload_identity_config" {
+  name               = "%s"
+  location           = "us-central1-a"
+  initial_node_count = 1
+
+  workload_identity_config {}
+}
+`, projectID, clusterName)
+}
+
+func testAccContainerCluster_withWorkloadIdentityConfigDeprecationIdentityNamespace(projectID string, clusterName string) string {
+	return fmt.Sprintf(`
+data "google_project" "project" {
+  project_id = "%s"
+}
+
+resource "google_container_cluster" "with_workload_identity_config" {
+  name               = "%s"
+  location           = "us-central1-a"
+  initial_node_count = 1
+
+  workload_identity_config {
+    identity_namespace = "${data.google_project.project.project_id}.svc.id.goog"
+  }
+}
+`, projectID, clusterName)
+}
+
+func testAccContainerCluster_withWorkloadIdentityConfigDeprecationWorkloadPool(projectID string, clusterName string) string {
+	return fmt.Sprintf(`
+data "google_project" "project" {
+  project_id = "%s"
+}
+
+resource "google_container_cluster" "with_workload_identity_config" {
+  name               = "%s"
+  location           = "us-central1-a"
+  initial_node_count = 1
+
+  workload_identity_config {
+    workload_pool = "${data.google_project.project.project_id}.svc.id.goog"
+  }
+}
+`, projectID, clusterName)
+}
+
+func testAccContainerCluster_withWorkloadIdentityConfigDeprecationBoth(projectID string, clusterName string) string {
+	return fmt.Sprintf(`
+data "google_project" "project" {
+  project_id = "%s"
+}
+
+resource "google_container_cluster" "with_workload_identity_config" {
+  name               = "%s"
+  location           = "us-central1-a"
+  initial_node_count = 1
+
+  workload_identity_config {
+    workload_pool = "${data.google_project.project.project_id}.svc.id.goog"
+    identity_namespace = "${data.google_project.project.project_id}.svc.id.goog"
+  }
+}
+`, projectID, clusterName)
+}
+
+// TODO: remove until above TODO during `4.0.0`
+
 func testAccContainerCluster_withInitialCIDR(containerNetName string, clusterName string) string {
 	return fmt.Sprintf(`
 resource "google_compute_network" "container_network" {

google/resource_container_node_pool_test.go

@@ -1268,7 +1268,7 @@ resource "google_container_cluster" "cluster" {
   min_master_version = data.google_container_engine_versions.central1a.latest_master_version
 
   workload_identity_config {
-    identity_namespace = "${data.google_project.project.project_id}.svc.id.goog"
+    workload_pool = "${data.google_project.project.project_id}.svc.id.goog"
   }
 }
 

google/resource_gke_hub_membership_generated_test.go

@@ -100,7 +100,7 @@ resource "google_container_cluster" "primary" {
   location           = "us-central1-a"
   initial_node_count = 1
   workload_identity_config {
-    identity_namespace = "%{project}.svc.id.goog"
+    workload_pool = "%{project}.svc.id.goog"
   }
 }
 

website/docs/guides/version_4_upgrade.html.markdown

@@ -187,6 +187,24 @@ resource definition.
 
 Removed in favor of `node_config.workload_metadata_config.mode`.
 
+### `workload_identity_config.0.identity_namespace` is now removed
+
+Removed in favor of `workload_identity_config.0.workload_pool`. Switching your
+configuration from one value to the other will trigger a diff at plan time, and
+a spurious update.
+
+```diff
+resource "google_container_cluster" "cluster" {
+  name               = "your-cluster"
+  location           = "us-central1-a"
+  initial_node_count = 1
+
+  workload_identity_config {
+-    identity_namespace = "your-project.svc.id.goog"
++   workload_pool = "your-project.svc.id.goog"
+  }
+````
+
 ### `pod_security_policy_config` is removed from the GA provider
 
 This field was incorrectly included in the GA `google` provider in past releases.