Snowflake-Labs / terraform-provider-snowflake

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

fix: Behaviour change bundle 2024_08 breaks the user resource #3144

Closed Relativity74205 closed 2 weeks ago

Relativity74205 commented 1 month ago

fix of https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/3125

Test Plan

References

Relativity74205 commented 1 month ago

@sfc-gh-asawicki There is one thing: A plan/apply changes the state when run the first time with the fix. The output of the plan can be found further below. The changes come from the changed output of the SHOW USERS command. The changed state works fine also with the unfixed version of the provider.

However, I guess, it might make sense to build in a "converter"/"migrater", which performs the change silently for the user?

  ~ resource "snowflake_user" "name" {
      ~ default_secondary_roles_option                = "ALL" -> "DEFAULT"
        id                                            = "qux"
        name                                          = "qux"
      ~ show_output                                   = [
          - {
              - created_on              = "2024-10-18 05:50:47.372 -0700 PDT"
              - days_to_expiry          = "0.9976851851851852"
              - default_secondary_roles = jsonencode(
                    [
                      - "ALL",
                    ]
                )
              - disabled                = false
              - display_name            = "qux"
              - expires_at_time         = "2024-10-19 06:03:25.477 -0700 PDT"
              - ext_authn_duo           = false
              - has_mfa                 = false
              - has_password            = false
              - has_rsa_public_key      = false
              - last_success_login      = "0001-01-01 00:00:00 +0000 UTC"
              - locked_until_time       = "0001-01-01 00:00:00 +0000 UTC"
              - login_name              = "QUX"
              - mins_to_bypass_mfa      = "0"
              - must_change_password    = false
              - name                    = "qux"
              - owner                   = "ACCOUNTADMIN"
              - snowflake_lock          = false
                # (10 unchanged attributes hidden)
            },
        ] -> (known after apply)
        # (66 unchanged attributes hidden)
    }
sfc-gh-asawicki commented 1 month ago

Hey @Relativity74205. Thanks for the contribution!

Unfortunately, I have the change almost ready locally too. But for future reference, please:

Relativity74205 commented 1 month ago

Hi @sfc-gh-asawicki , I mainly started to look into it, because I wanted to understand why the error occurs in the first place to deepen my understanding of the code. As since I didn't invest much work here (around half an hour), it was well invested time. And I haven't written any tests and state upgrades, since I wasn't sure how to do that, therefore my implicit question above.

Then I will close this PR. Can you please ping me when you create your PR, I would like to have a look on your implementation.

sfc-gh-asawicki commented 1 week ago

Hey @Relativity74205. My PR: https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/3176. After more migration tests we decided not to add any state upgraders after all (you can check the tests). There were more of them than usually because of the overlap with BCR Bundle 2024_07.

Relativity74205 commented 1 week ago

Hey @Relativity74205. My PR: #3176. After more migration tests we decided not to add any state upgraders after all (you can check the tests). There were more of them than usually because of the overlap with BCR Bundle 2024_07.

@sfc-gh-asawicki Thanks for the info, interesting to see. And as posted in the issue thread, we have already upgraded without problems.