Remove username and password from master_auth (#5372) (#10441)

* Remove `username` and `password` from `master_auth`

* Add client_certificate_required note

* Update mmv1/third_party/terraform/website/docs/guides/version_4_upgrade.html.markdown

Co-authored-by: Riley Karson <[email protected]>

* Remove bad merge

Co-authored-by: Riley Karson <[email protected]>
Co-authored-by: Sam Levenick <[email protected]>
Signed-off-by: Modular Magician <[email protected]>

Co-authored-by: Riley Karson <[email protected]>
Co-authored-by: Sam Levenick <[email protected]>

Author: 3 people

.changelog/5372.txt

@@ -0,0 +1,6 @@
+```release-note:breaking-change
+container: removed `master_auth.username` and `master_auth.password` from `google_container_cluster`
+```
+```release-note:breaking-change
+container: made `master_auth.client_certificate_config` required
+```

google/resource_container_cluster.go

@@ -554,37 +554,15 @@ func resourceContainerCluster() *schema.Resource {
 				Optional:    true,
 				MaxItems:    1,
 				Computed:    true,
-				Deprecated:  `Basic authentication was removed for GKE cluster versions >= 1.19.`,
-				Description: `The authentication information for accessing the Kubernetes master. Some values in this block are only returned by the API if your service account has permission to get credentials for your GKE cluster. If you see an unexpected diff removing a username/password or unsetting your client cert, ensure you have the container.clusters.getCredentials permission.`,
+				Description: `The authentication information for accessing the Kubernetes master. Some values in this block are only returned by the API if your service account has permission to get credentials for your GKE cluster. If you see an unexpected diff unsetting your client cert, ensure you have the container.clusters.getCredentials permission.`,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
-						"password": {
-							Type:         schema.TypeString,
-							Optional:     true,
-							AtLeastOneOf: []string{"master_auth.0.password", "master_auth.0.username", "master_auth.0.client_certificate_config"},
-							Sensitive:    true,
-							Description:  `The password to use for HTTP basic authentication when accessing the Kubernetes master endpoint.`,
-						},
-
-						"username": {
-							Type:         schema.TypeString,
-							Optional:     true,
-							AtLeastOneOf: []string{"master_auth.0.password", "master_auth.0.username", "master_auth.0.client_certificate_config"},
-							Description:  `The username to use for HTTP basic authentication when accessing the Kubernetes master endpoint. If not present basic auth will be disabled.`,
-						},
-
-						// Ideally, this would be Optional (and not Computed).
-						// In past versions (incl. 2.X series) of the provider
-						// though, being unset was considered identical to set
-						// and the issue_client_certificate value being true.
 						"client_certificate_config": {
-							Type:         schema.TypeList,
-							MaxItems:     1,
-							Optional:     true,
-							Computed:     true,
-							AtLeastOneOf: []string{"master_auth.0.password", "master_auth.0.username", "master_auth.0.client_certificate_config"},
-							ForceNew:     true,
-							Description:  `Whether client certificate authorization is enabled for this cluster.`,
+							Type:        schema.TypeList,
+							MaxItems:    1,
+							Required:    true,
+							ForceNew:    true,
+							Description: `Whether client certificate authorization is enabled for this cluster.`,
 							Elem: &schema.Resource{
 								Schema: map[string]*schema.Schema{
 									"issue_client_certificate": {
@@ -2173,45 +2151,6 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
 		}
 	}
 
-	if d.HasChange("master_auth") {
-		var req *container.SetMasterAuthRequest
-		if ma, ok := d.GetOk("master_auth"); ok {
-			req = &container.SetMasterAuthRequest{
-				Action: "SET_USERNAME",
-				Update: expandMasterAuth(ma),
-			}
-		} else {
-			req = &container.SetMasterAuthRequest{
-				Action: "SET_USERNAME",
-				Update: &container.MasterAuth{
-					Username: "admin",
-				},
-			}
-		}
-
-		updateF := func() error {
-			name := containerClusterFullName(project, location, clusterName)
-			clusterSetMasterAuthCall := config.NewContainerClient(userAgent).Projects.Locations.Clusters.SetMasterAuth(name, req)
-			if config.UserProjectOverride {
-				clusterSetMasterAuthCall.Header().Add("X-Goog-User-Project", project)
-			}
-			op, err := clusterSetMasterAuthCall.Do()
-			if err != nil {
-				return err
-			}
-
-			// Wait until it's updated
-			return containerOperationWait(config, op, project, location, "updating master auth", userAgent, d.Timeout(schema.TimeoutUpdate))
-		}
-
-		// Call update serially.
-		if err := lockedCall(lockKey, updateF); err != nil {
-			return err
-		}
-
-		log.Printf("[INFO] GKE cluster %s: master auth has been updated", d.Id())
-	}
-
 	if d.HasChange("vertical_pod_autoscaling") {
 		if ac, ok := d.GetOk("vertical_pod_autoscaling"); ok {
 			req := &container.UpdateClusterRequest{
@@ -2817,10 +2756,7 @@ func expandMasterAuth(configured interface{}) *container.MasterAuth {
 	}
 
 	masterAuth := l[0].(map[string]interface{})
-	result := &container.MasterAuth{
-		Username: masterAuth["username"].(string),
-		Password: masterAuth["password"].(string),
-	}
+	result := &container.MasterAuth{}
 
 	if v, ok := masterAuth["client_certificate_config"]; ok {
 		if len(v.([]interface{})) > 0 {
@@ -3269,8 +3205,6 @@ func flattenMasterAuth(ma *container.MasterAuth) []map[string]interface{} {
 	}
 	masterAuth := []map[string]interface{}{
 		{
-			"username":               ma.Username,
-			"password":               ma.Password,
 			"client_certificate":     ma.ClientCertificate,
 			"client_key":             ma.ClientKey,
 			"cluster_ca_certificate": ma.ClusterCaCertificate,

google/resource_container_cluster_test.go

@@ -174,6 +174,31 @@ func TestAccContainerCluster_withAddons(t *testing.T) {
 	})
 }
 
+func TestAccContainerCluster_withMasterAuthConfig_NoCert(t *testing.T) {
+	t.Parallel()
+
+	clusterName := fmt.Sprintf("tf-test-cluster-%s", randString(t, 10))
+
+	vcrTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckContainerClusterDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccContainerCluster_withMasterAuthNoCert(clusterName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("google_container_cluster.with_master_auth_no_cert", "master_auth.0.client_certificate", ""),
+				),
+			},
+			{
+				ResourceName:      "google_container_cluster.with_master_auth_no_cert",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAccContainerCluster_withAuthenticatorGroupsConfig(t *testing.T) {
 	t.Parallel()
 	clusterName := fmt.Sprintf("tf-test-cluster-%s", randString(t, 10))
@@ -2298,6 +2323,21 @@ resource "google_container_cluster" "with_version" {
 `, clusterName)
 }
 
+func testAccContainerCluster_withMasterAuthNoCert(clusterName string) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "with_master_auth_no_cert" {
+  name               = "%s"
+  location           = "us-central1-a"
+  initial_node_count = 3
+  master_auth {
+    client_certificate_config {
+      issue_client_certificate = false
+    }
+  }
+}
+`, clusterName)
+}
+
 func testAccContainerCluster_updateVersion(clusterName string) string {
 	return fmt.Sprintf(`
 data "google_container_engine_versions" "central1a" {

website/docs/d/container_cluster.html.markdown

@@ -19,14 +19,6 @@ data "google_container_cluster" "my_cluster" {
   location = "us-east1-a"
 }
 
-output "cluster_username" {
-  value = data.google_container_cluster.my_cluster.master_auth[0].username
-}
-
-output "cluster_password" {
-  value = data.google_container_cluster.my_cluster.master_auth[0].password
-}
-
 output "endpoint" {
   value = data.google_container_cluster.my_cluster.endpoint
 }

website/docs/d/container_engine_versions.html.markdown

@@ -31,11 +31,6 @@ resource "google_container_cluster" "foo" {
   location           = "us-central1-b"
   node_version       = data.google_container_engine_versions.central1b.latest_node_version
   initial_node_count = 1
-
-  master_auth {
-    username = "mr.yoda"
-    password = "adoy.rm"
-  }
 }
 
 output "stable_channel_version" {

website/docs/guides/version_4_upgrade.html.markdown

@@ -49,7 +49,8 @@ description: |-
   - [Resource: `google_container_cluster`](#resource-google_container_cluster)
     - [`enable_shielded_nodes` now defaults to `true`](#enable_shielded_nodes-now-defaults-to-true)
     - [`instance_group_urls` is now removed](#instance_group_urls-is-now-removed)
-    - [`master_auth` is now removed](#master_auth-is-now-removed)
+    - [`master_auth.username` and `master_auth.password` are now removed](#master_authusername-and-master_authpassword-are-now-removed)
+    - [`master_auth.client_certificate_config` is now required](#master_authclient_certificate_config-is-now-required)
     - [`node_config.workload_metadata_config.node_metadata` is now removed](#node_configworkload_metadata_confignode_metadata-is-now-removed)
     - [`workload_identity_config.0.identity_namespace` is now removed](#workload_identity_config0identity_namespace-is-now-removed)
     - [`pod_security_policy_config` is removed from the GA provider](#pod_security_policy_config-is-removed-from-the-ga-provider)
@@ -73,13 +74,13 @@ description: |-
     - [`bigquery-json.googleapis.com` is no longer a valid service name](#bigquery-jsongoogleapiscom-is-no-longer-a-valid-service-name)
   - [Resource: `google_spanner_instance`](#resource-google_spanner_instance)
     - [Exactly one of `num_nodes` or `processing_units` is required](#exactly-one-of-num_nodes-or-processing_units-is-required)
-  - [Resource: `google_sql_database_instance`](#resource-google_sql_database_instance)
+    - [Resource: `google_sql_database_instance`](#resource-google_sql_database_instance)
     - [First-generation fields have been removed](#first-generation-fields-have-been-removed)
     - [Drift detection and defaults enabled on fields](#drift-detection-and-defaults-enabled-on-fields)
   - [Resource: `google_storage_bucket`](#resource-google_storage_bucket)
     - [`bucket_policy_only` field is now removed](#bucket_policy_only-field-is-now-removed)
     - [`location` field is now required.](#location-field-is-now-required)
-  - [Resource: `google_sql_database_instance`](#resource-google_sql_database_instance)
+  - [Resource: `google_sql_database_instance`](#resource-google_sql_database_instance-1)
     - [`database_version` field is now required](#database_version-field-is-now-required)
   - [Resource: `google_pubsub_subscription`](#resource-google_pubsub_subscription)
     - [`path` is now removed](#path-is-now-removed)
@@ -375,11 +376,17 @@ Unless explicitly configured, users may see a diff changing `enable_shielded_nod
 
 `instance_group_urls` has been removed in favor of `node_pool.instance_group_urls`
 
-### `master_auth` is now removed
+### `master_auth.username` and `master_auth.password` are now removed
 
-`master_auth` and its subfields have been removed. 
+`master_auth.username` and `master_auth.password` have been removed. 
 Basic authentication was removed for GKE cluster versions >= 1.19. The cluster cannot be created with basic authentication enabled. Instructions for choosing an alternative authentication method can be found at: cloud.google.com/kubernetes-engine/docs/how-to/api-server-authentication.
 
+### `master_auth.client_certificate_config` is now required
+
+With the removal of `master_auth.username` and `master_auth.password`, `master_auth.client_certificate_config` is now
+the only configurable field in `master_auth`. If you do not wish to configure `master_auth.client_certificate_config`, 
+remove the `master_auth` block from your configuration entirely. You will still be able to reference the outputted fields under `master_auth` without the block defined.
+
 ### `node_config.workload_metadata_config.node_metadata` is now removed
 
 Removed in favor of `node_config.workload_metadata_config.mode`.

website/docs/r/container_cluster.html.markdown

@@ -197,9 +197,9 @@ and requires the `ip_allocation_policy` block to be defined. By default when thi
 * `master_auth` - (Optional) The authentication information for accessing the
 Kubernetes master. Some values in this block are only returned by the API if
 your service account has permission to get credentials for your GKE cluster. If
-you see an unexpected diff removing a username/password or unsetting your client
-cert, ensure you have the `container.clusters.getCredentials` permission.
-Structure is [documented below](#nested_master_auth). This has been deprecated as of GKE 1.19.
+you see an unexpected diff unsetting your client cert, ensure you have the 
+`container.clusters.getCredentials` permission.
+Structure is [documented below](#nested_master_auth).
 
 * `master_authorized_networks_config` - (Optional) The desired
     configuration options for master authorized networks. Omit the
@@ -576,13 +576,7 @@ pick a specific range to use.
 
 <a name="nested_master_auth"></a>The `master_auth` block supports:
 
-* `password` - (Optional) The password to use for HTTP basic authentication when accessing
-    the Kubernetes master endpoint. This has been deprecated as of GKE 1.19.
-
-* `username` - (Optional) The username to use for HTTP basic authentication when accessing
-    the Kubernetes master endpoint. If not present basic auth will be disabled. This has been deprecated as of GKE 1.19.
-
-* `client_certificate_config` - (Optional) Whether client certificate authorization is enabled for this cluster.  For example:
+* `client_certificate_config` - (Required) Whether client certificate authorization is enabled for this cluster.  For example:
 
 ```hcl
 master_auth {