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

[Bug]: Snowflake_Role regression - errors when parentheses are included in the role name #3127

Closed jasonjoneszywave closed 1 week ago

jasonjoneszywave commented 1 month ago

Terraform CLI Version

1.7.0

Terraform Provider Version

0.95.0

Terraform Configuration

Previously we've been able to create roles with names that include parentheses in Snowflake via the Terraform provider without issue. Functionality performs as expected through 0.93.0.

I tried upgrading to 0.95.0 and now get the below error:

Error: unable to parse identifier: "TEST ROLE - (dev)", currently identifiers containing opening and closing parentheses '()' are not supported in the provider

Same behavior is observed whether the snowflake_role or the snowflake_account_role resource is used. I was unable to find any documentation of this behavior change in the change log or the migration guide.

This issue appears to tie back to the below commit related to 0.94.0.

https://github.com/Snowflake-Labs/terraform-provider-snowflake/commit/32c76906c2eb8571e4a995df20f6450ddd6f94b4#diff-763615a19db3cf298ad395f66b38d48a392f0fab8408c5c88596a6cc9f2af898

Category

category:resource

Object type(s)

resource:role

Expected Behavior

Successfully create a role with a name that includes parentheses. Ex. "TEST_ROLE - (dev)"

Actual Behavior

Error: unable to parse identifier: "TEST ROLE - (dev)", currently identifiers containing opening and closing parentheses '()' are not supported in the provider

Steps to Reproduce

  1. Define either a snowflake_role or snowflake_account_role resource with a name that includes parentheses like the examples below.

resource "snowflake_role" "test_role" { name = "TEST ROLE - (dev)" }

resource "snowflake_account_role" "test_account_role" { name = "TEST ACCOUNT ROLE - (dev)" }

  1. Execute a plan and apply.
  2. Error like below will be returned.

Error: unable to parse identifier: "TEST ROLE - (dev)", currently identifiers containing opening and closing parentheses '()' are not supported in the provider

How much impact is this issue causing?

Medium

Logs

No response

Additional Information

I have not found a workaround for this issue as of yet. Our naming convention for SCIM groups includes parentheses to enclose the environment the group pertains to. This naming convention is widely used in our organization and this regression results in a breaking change for us with no way forward for upgrading to versions of the provider above 0.93.0.

Would you like to implement a fix?

sfc-gh-asawicki commented 1 month ago

Hey @jasonjoneszywave. Thanks for reaching out to us.

The change is mentioned here: https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations (the doc was linked e.g. here: https://github.com/Snowflake-Labs/terraform-provider-snowflake/releases/tag/v0.95.0).

However, the document mentions this limitation only for identifiers containing parentheses (like functions) and not for other resource types (like the role mentioned). This may still be a problem, though, in places where we don't know what type of identifier we need to handle (that's why the general limitation makes sense in some cases).

I see that we have an open TODO here: https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/d837341c2d18b6fbb4657ad3a1837190a8ee77d8/pkg/sdk/identifier_parsers.go#L44.

I will consult with the team on this topic in the morning and will let you know if and when we can loosen these restrictions, at least for the role resource.

jasonjoneszywave commented 1 month ago

Hey @sfc-gh-asawicki. Any updates on this after meeting with the team?

sfc-gh-asawicki commented 1 month ago

Hey @jasonjoneszywave.

After initial discussions, we agreed that we have to loosen these restrictions for objects with ids not ending with (...). We want to make sure that it won't break the logic in other places, though. We have it scheduled for early next week, so that it should make the cut in the next week's release. I will keep you posted if anything changes.

jasonjoneszywave commented 1 month ago

Thank you for the update @sfc-gh-asawicki I appreciate it!

Yeah, there's a lot to test for sure. Making sure it works with grants amongst other things will be critical as well.

sfc-gh-asawicki commented 3 weeks ago

Hey @jasonjoneszywave. Just letting you know, that the next release will happen next week or even the week after. Sorry for the inconvenience.

jasonjoneszywave commented 2 weeks ago

Hey @sfc-gh-asawicki . Any updates on when we can expect this release?

sfc-gh-asawicki commented 1 week ago

Hey @jasonjoneszywave. Today :)

jasonjoneszywave commented 1 week ago

Looks like it hasn't made it to the Terraform registry yet, but once it does I'll try it out and report back.

sfc-gh-asawicki commented 1 week ago

Hey @jasonjoneszywave. Unfortunately, we had some technical problems on late Friday evening and the release was unsuccessful. Today, the whole team is absent (because of the national holiday in Poland), and we will prioritize 0.98.0 release tomorrow morning.

sfc-gh-jmichalak commented 1 week ago

Hi @jasonjoneszywave 👋

We've released a new v0.98.0 version (release, migration guide) with a fix. We're sorry for the delay.

jasonjoneszywave commented 1 week ago

Thanks @sfc-gh-asawicki @sfc-gh-jmichalak !

I'm planning to do some testing/validation tomorrow. Will report back on how it goes.

jasonjoneszywave commented 1 week ago

@sfc-gh-asawicki @sfc-gh-jmichalak I did some testing this morning and was able to successfully Terraform granting an account role with () in the name to another account role and also was able to successfully grant an account role with () in the name usage on a warehouse and a database.

Not exhaustive testing by any means, but looking good so far. Will let you know if we hit any issues. Thanks again. This fix helps us a ton!

sfc-gh-asawicki commented 1 week ago

I'm glad to hear that. I will close the issue then. Please let us know if you encounter any more problems.