Make SQL User password optional and generate of omitted

[erademacher]

Previously, password was a required field for SQL users. This caused import problems because we can't read and set the password (nor do we want to).

This change makes it optional, and if it's omitted, we randomly generate a 32-character random password. This password isn't saved to state, though. The user needs to then change it via the web UI. Setting the password field to null after the resource is created will not change the password, but makes Terraform forget it.

[marksoper]

A couple of questions:

1. currently passwords are indeed saved in state right, as is the case for "sensitive" fields?

2. "Setting the password field to null after the resource is created will not change the password, but makes Terraform forget it." The generated password isn't stored in state right, so what is getting forgotten here?

[erademacher]

1. currently passwords are indeed saved in state right, as is the case for "sensitive" fields?

Yes, and non-null passwords in the config will continue to show up in the state. That's important context for your second question.

2. "Setting the password field to null after the resource is created will not change the password, but makes Terraform forget it." The generated password isn't stored in state right, so what is getting forgotten here?

We're not removing the password field from Terraform entirely. The scenario I'm talking about is when you give the password field a value initially, then set it to null in a subsequent update. That'll remove it from Terraform state but won't change it via the API.

[catj-cockroach]

@marksoper This is to allow organizations that want to manage existing users to import them without including their existing passwords into the state. Alternatively, if the password was stored in a separate block, we could have that block be set to null?

For example:

resource "cockroach_sql_user" "crl_engineer" {
  cluster_id = cockroach_cluster.mycluster.id
  name = "catj"
  password_authentication {
      password = "test1234"
  }
}

This would open the door to being able to define SSO users without having to leave the password empty (but would be backwards incompatible with existing configuration):

resource "cockroach_sql_user" "crl_engineer" {
  cluster_id = cockroach_cluster.mycluster.id
  name = "catj"
  oidc_authentication {
      id = "12345678"
  }
}

[marksoper]

Ok, thanks @catj-cockroach and @erademacher. A couple of follow-ups:

1. Why the decision to not store auto-gen passwords in Terraform state? I assume this is mainly in support of importing (bulk) sql users? There's no reason to store the password because those users won't be logging in via TF? We just want to get them into the database easily? 2 @catj-cockroach it looks like we'll be moving toward supporting multiple forms of auth for sql users, which makes sense. What's the timeline? That will be a breaking change right? Can we separate that concern from this PR or should we consider going ahead and making that change sooner rather than later?

[marksoper]

3. If an auto-gen password is used initially, can customer use TF to change that to a user-supplied password by entering a value in the .tf file? Will the initial choice of leaving the password null prevent it from ever being set manually in the future via TF?

[erademacher]

1. Why the decision to not store auto-gen passwords in Terraform state? I assume this is mainly in support of importing (bulk) sql users? There's no reason to store the password because those users won't be logging in via TF? We just want to get them into the database easily?

Correct. CreateSQLUser requires a password, but users won't always want to keep everyone's password in the plain-text .tfstate file. The idea here is that they create users for everyone, but the users themselves manage their own passwords. They can reset the password through the web UI (or the API if they're so inclined).

Also, since the API doesn't return passwords, we need this change to make import work. Password needs to be nullable, or the state can't match the config.

3 If an auto-gen password is used initially, can customer use TF to change that to a user-supplied password by entering a value in the .tf file? Will the initial choice of leaving the password null prevent it from ever being set manually in the future via TF?

Yes, you can still set the password attribute to a non-null value to change it via TF, even if it was initially random and unknown.

Alternatively, if the password was stored in a separate block, we could have that block be set to null?

What's the benefit of that approach as opposed to just making password optional? Seems functionally equivalent to me.

[marksoper]

Looks good to me @erademacher . I don't know if we've incorporated @catj-cockroach 's concerns sufficiently, but this seems like a good change to make that will solve several problems without breakage or painting us into a corner.

[erademacher]

TFTR! I'm not eager to make any breaking changes for the sake of future SSO support, nor am I convinced that breaking changes will really be required. We'll cross that bridge when we get there.