Snowflake-Labs / terraform-provider-snowflake

Terraform provider for managing Snowflake accounts
https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest
MIT License
535 stars 414 forks source link

[Bug]: snowflake_user: default_role attribute breaks the apply #2942

Open denzhel opened 2 months ago

denzhel commented 2 months ago

Terraform CLI Version

1.5.5

Terraform Provider Version

0.92.0

Terraform Configuration

resource "snowflake_user" "service_account" {
  depends_on = [snowflake_role.service_account_role, random_password.service_account]
  # parameters
  name                 = "SA-TEST"
  login_name           = "SA-TEST"
  password             = random_password.service_account[each.key].result
  default_role         = "SA-TEST"
  must_change_password = true
}

Category

category:resource

Object type(s)

resource:user

Expected Behavior

Users to be created with a default_role that contains a hyphen as we created up until the upgrade from 0.56.1 to 0.92.0

Actual Behavior

╷
│ Error: 001003 (42000): SQL compilation error:
│ syntax error line 1 at position 179 unexpected '-'.
│ syntax error line 1 at position 179 unexpected '-'.
│ syntax error line 1 at position 184 unexpected '<EOF>'.
│ 
│   with module.snowflake.snowflake_user.service_account["SA-TEST"],
│   on modules/snowflake/main.tf line 376, in resource "snowflake_user" "service_account":
│  376: resource "snowflake_user" "service_account" {
│ 
╵

Query from Snowflake's UI:

CREATE USER "SA-TEST" PASSWORD = '☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺☺' LOGIN_NAME = 'SA-TEST' DISPLAY_NAME = 'SA-TEST' DEFAULT_ROLE = SA-TEST

Steps to Reproduce

  1. Create a new snowflake_user resource and add a default_role attribute that contains a hyphen, e.g "SA-TEST"
  2. Apply
  3. See how it fails
  4. Remove the default_role field and the apply will be successful

How much impact is this issue causing?

High

Logs

No response

Additional Information

No response

Would you like to implement a fix?

sfc-gh-jmichalak commented 2 months ago

Hey @denzhel . Thanks for reaching out to us.

The user resource is currently being redesigned as part of https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1. This happens because DEFAULT_ROLE is not wrapped in '. As a workaround, you can use roles without - or use unsafe_execute. cc @sfc-gh-asawicki

denzhel commented 2 months ago

Hi @sfc-gh-jmichalak ,

Either of those are not an option for us, we have dozens of users that were created with the previous provider we had. We use - through out all of our users.

We're unable to create new users.

When will the redesign happen ?

sfc-gh-jmichalak commented 2 months ago

@sfc-gh-asawicki is currently working on user redesign and this should be fixed in v0.94 in the next few weeks.

sfc-gh-asawicki commented 2 months ago

Hey @denzhel.

First of all, please do not do 40+ versions migrations, it's recommended to do versions-by-version migration iteratively, following our guidelines: https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#migration-guide.

You can use provider aliasing to manage users with the older version of the provider (https://developer.hashicorp.com/terraform/language/providers/configuration#alias-multiple-provider-configurations).

denzhel commented 2 months ago

Hey @denzhel.

First of all, please do not do 40+ versions migrations, it's recommended to do versions-by-version migration iteratively, following our guidelines: https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#migration-guide.

You can use provider aliasing to manage users with the older version of the provider (https://developer.hashicorp.com/terraform/language/providers/configuration#alias-multiple-provider-configurations).

You are right, it was a mistake on our end that we've waited for so long before upgrading the provider. I will take your tip into a consideration and will try to implement the unsafe_execute - will see what are the implications.

Thanks for your honest and quick reply !

denzhel commented 2 months ago

Can you please suggest what to set on the revert command ?


resource "snowflake_unsafe_execute" "default_role" {
  for_each   = try(!var.first_time_setup && var.config.enabled ? local.service_accounts : tomap(false), {})
  depends_on = [snowflake_user.service_account]
  execute    = "ALTER USER ${each.key} SET DEFAULT_ROLE = ${each.value.default_role != "" ? upper(each.value.default_role) : each.key}"
  revert     = "???"
}```
sfc-gh-jmichalak commented 2 months ago

You can either do ALTER USER ${each.key} UNSET DEFAULT_ROLE to unset this field (will be NULL after), or do a no-op with SELECT 1, it's up to you.

denzhel commented 2 months ago

I'm just thinking of a scenario of deleting the user, once it happens the revert command will fail due to the missing user.

sfc-gh-asawicki commented 1 month ago

Fixed in https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/2947 (not released yet).

sfc-gh-asawicki commented 2 weeks ago

Hey @denzhel. We have just released v0.95.0 of the provider. It contains a reworked snowflake_user resource. Please consult the migration guide.