Support for disabling basic auth & client certificate about terraform-google-kubernetes-engine HOT 6 CLOSED

1. For basic auth, I think we should go with enable_basic_auth = false as the default going forward. This will generate a diff for existing module users, but that is acceptable as they can choose to either explicitly encode the desire to enable basic auth or require the new version.

2. Client certificates sound slightly more destructive and we use them internally, so let's start by adding an enable_client_certificate variable with a default of true.

Also, as per the docs it does seem to be possible to enable cluster cert but disable basic auth so we need to support that scenario: https://www.terraform.io/docs/providers/google/r/container_cluster.html#client_certificate_config

from terraform-google-kubernetes-engine.

Yes, I think it's a good idea to add this. Do you think we would want it as a default?

from terraform-google-kubernetes-engine.

Yes I think it makes sense to disable both by default. When creating a cluster via the GCP console, both options have recommendations to disable them. Also, they will be disabled by default in 1.12.

from terraform-google-kubernetes-engine.

So I looked into this a few notes/questions:

Our best practices guide says to name variables in the positive. Defaulting to disabled basic auth (enable_basic_auth = false) will be a breaking change for deployed clusters. Enabling it by default requires a username and password. The default pw for GKE is 'admin', but the password is random.

So:

• Do we want to enable basic auth by default but, change their password using tf resource "random"
• Do we want to name this in the negative? Default: disable_basic_auth = false to prevent breakage?

Disabling client certificates is a destructive change. My assumption is we should name this in the positive and enable it by default. This is a catch-22 since its destructive to change. If we enable client certs by default, its not destructive to current users, but opts new users into a bad practice with the chance of being destructive if they did not notice the variable. If we disable it by default, its destructive to current users, but sets the module up for best practices.

It may also be worth noting, to set the certificate explicitly, its a nested setting that also requires the username and password be set. So its technically not possible to make this non-breaking for current users, because as soon as the certificate setting is set, the username and password has to be as well.

For setting basic auth, which do you prefer?

enable_basic_auth = true
basic_auth_username = "xxx"
basic_auth_password = "xxx"

basic_auth {
  username = "xxx"
  password = "yyy"
}

The later would assume if the map is empty, it is disabled.

@morgante

from terraform-google-kubernetes-engine.

Yep, I got that support in my first pass! Thanks for the input, sounds like a good approach to me.

from terraform-google-kubernetes-engine.

Fixed by #40.

from terraform-google-kubernetes-engine.