fix: default_role should ignore diffs of the form "thing" -> "\"thing\"" #2836
Conversation
|
Hey @pfnsec. Thanks for the contribution. Before we dig into the details, please make sure that you have checked our contribution guidelines. The issue you mention touches the much wider problem with the identifiers. We will address the identifier problems with the upcoming (should start next week) identifier rework (check more in https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#identifiers-rework). Furthermore, user resource is one of the resources we will redesign as part of https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1. I am afraid that we may change it again as part of the two aforementioned topics, but we may consider accepting it before because it should not break the behavior for other use cases. We just require tests according to our contribution guidelines. If it's okay for you to just wait for the reworks I mentioned, please just create an issue, and I will attach it to the known issues for the user resource here: https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/v1-preparations/ESSENTIAL_GA_OBJECTS.MD. |
|
Fixed in PR: #3013, closing |
Fix known user resource-connected issues: - Change the sensitiveness of name and login_name (References: #2662 #2668) - Handle "null" properly for the nullable bool text attributes in user (References: #2817) - Fix diff suppression for default_x in user resource (References: #2836) - Update the migration guide (References #2938 #2942) - Fix incorrect state after failed to alter (References #2970) - Confirm the problem with the computed disabled attribute (References #1572) - Confirm that the problem with the null-out password was already solved (References #1535) - Add TODO to handle days to expiry in user (References #1155) The next 2 PRs will contain: - adjusting user resource to our rework conventions (also adding additional fields and handling #1155 and #1572) - adjusting user datasource (will handle #2902) User rework will not include handling new types of users (service, legacy service); this will be done a bit later.
We have a few roles in our org that have spaces or other special characters in them. We can't get around this, so we decided default_role should be escaped. In our Terragrunt module, we wrap it in "\"${role_name}\"", and everyone's happy. However, it means that we get a permanent diff of the form "${role_name} -> "\"${role_name}\"".
Before we started wrapping them like this, we got gnarly SQL errors on apply. Perhaps the real issue was that the provider should have been handling sending default_role wrapped in escaped quotes, as I can see occurs for other fields? In any case, this patch only suppresses diffs of the form mentioned, and doesn't alter anything else in that way.
Test Plan
This PR is UNTESTED!. It needs to be tested on my other machine before being considered for merge, but I wanted to check first to make sure I'm not going about this in the wrong way. Thanks!
References