-
Notifications
You must be signed in to change notification settings - Fork 427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding array assignment to snowflake_grant_account_role and snowflake_grant_privileges_to_account_role #2668
Comments
Hey @bob-zarkoob 👋 |
I agree with @bob-zarkoob, we are now forced to use in some cases dozens of more resources in a for_each to create something that used to only take 1. This doesn't seem like a big deal but for those that are on Terraform Cloud (and haven't moved off 😄) every resource is more money. |
for_each works but is flagging usernames as sensitive in the new resource which was not the case previously as a result permissions are being added with index instead which is a workaround. What was the reason for the change ? locals {
role_user_acccess = [
snowflake_user.example_user.name,
snowflake_user.example_user2.name,
snowflake_user.example_user3.name,
]
}
# --- User + Grants
resource "snowflake_grant_account_role" "example_user_role_grant" {
for_each = { for idx, name in local.role_user_acccess : idx => name }
role_name = snowflake_role.exmaple_role.name
user_name = each.value
}
|
Hey @gthomson31 |
Interesting as this problem has only appeared |
@gthomson31 Still not sure what is the issue in your case.
What does it mean permissions are being added with an index (and that it's a workaround) and in what sense the sensitive usernames are causing the issue? I would be grateful if you could provide some examples with a brief description. |
@bob-zarkoob @Bryan-Meier We have it somewhere on our very long list of things to do and it's certainly an important topic, but unfortunately, It probably will be addressed not that soon as we're moving into other things that need to be done to finally achieve the v1.0.0 version. The next topics we'll be looking into are described here. |
@sfc-gh-jcieslak, I see what @gthomson31 was referring to with regard to sensitive values. I could be wrong here but this feels like the error is being generated by Terraform and not the provider. The issue is that in order to replace an old resource that with something like "snowflake_grant_account_role" while using a for_each where users are being iterated over, it seems to violate some terraform rule. Again, I could be wrong on that... This is the error I am getting which is probably the same or similar to what @gthomson31 was reporting: This is the code it's referring to: This was the code that used the deprecated resource which works fine: Without some sort of workaround. I am stuck and not able to move from the deprecated "snowflake_role_grants" to the replacement "snowflake_grant_account_role". |
If there isn't a work around for this, it feels like this is more of a bug than a feature request. |
Yeah @Bryan-Meier exactly that have been trying to replace all our grants with the new method of assign users against a role although previously we used lists of users directly within the role grant but as the new resource doesn't support this then using for_each was the workaround which then brought up the problem of having to use indexing as the username can't be used. Wasn't aware it was possible to do in the past but your comment highlighted it was resource "snowflake_role_grants" "dev_role_grants" {
role_name = snowflake_role.dev_role.name
users = [
snowflake_user.example_user.name,
snowflake_user.example2_user.name,
snowflake_user.example3_user.name
]
} |
@gthomson31 @Bryan-Meier I see that sensitive is put on user Check the example below resource "snowflake_user" "test" {
name = "test_user_test_test"
}
resource "snowflake_role" "test" {
name = "test_role"
}
resource "snowflake_grant_account_role" "test" {
for_each = toset([for u in [snowflake_user.test.name] : nonsensitive(u)])
role_name = snowflake_role.test.name
user_name = each.value
}
|
@sfc-gh-jcieslak, thanks for the response and I'm glad we got down to the bottom of the issue. I had no idea that there was such a function of nonsensitive. That should fix my issue while the resources are being reworked. I appreciate you passing that function my way!! @gthomson31, I am guessing the nonsensitive function helps in your situation also. |
@sfc-gh-jcieslak, from what I can tell your suggestion of using the nonsensitive function doesn't work in that it throws other errors. This seems to be tied to a ton of questions throughout the community. Here is one example: hashicorp/terraform#28222 I have tried applying the nonsensitive function in the for_each and was left with this message:
This is telling me it's nonsensitive but when I remove the nonsensitive function I get:
I have even tried tricking it where I wrap sensitive function with a nonsensitive function which was also unsuccessful: I have tried applying these techniques to the resources within the module I am working with as wells as the within the main caller module which is passing in the variable values (in a nonsensitve form). I'm not sure what else to try. |
I tried one more thing before I gave up and it seemed to work. It's definitely not the preferred method but it does work. Instead of trying to convert the name from a snowflake_user resource (i.e. Again, not what we want to do but it does work. |
Yeah, the function is not perfect and I also got the errors you mentioned, but in the end, I got it working with the example I put in my last comment. We will surely work on a proper solution, but for now, let's treat the nonsensitive function as a workaround. |
Thank you! Will the function be changed before the current granting method is deprecated? I am trying to decide whether to migrate over all of our users grants right now or wait. |
HI! @sfc-gh-jcieslak There's a note here about plans to rework users. Do you have a timeline for roughly when that'll be happening? |
Hey @natashamathur
|
Got it! Your comment further upthread suggested that the users assignment itself will be further changed. Is that the plan? |
It's more about the sensitivity of the fields in the user resource rather than the assignment itself, but yeah, we'll take a closer look at what should be a sensitive value and what shouldn't when going over the user resource when working on the GA feature objects. Highly suggest going over our latest entry in the ROADMAP that describes our priorities in the next months. Especially this part where we linked a lists of objects we'll be refactoring this and the next quarter. |
Thank you!
…On Tue, May 7, 2024 at 8:09 AM Jan Cieślak ***@***.***> wrote:
It's more about the sensitivity of the fields in the user resource rather
than the assignment itself, but yeah, we'll take a closer look at what
should be a sensitive value and what shouldn't when going over GA feature
objects. Highly suggest going over our latest entry in the ROADMAP
<https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md>
that describes our priorities in the next months. Especially this
<https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1>
part where we linked a lists of objects we'll be refactoring this and the
next quarter.
—
Reply to this email directly, view it on GitHub
<#2668 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AIO2SKYQZAT5EGEG6NR4S6LZBC753AVCNFSM6AAAAABFTY4AEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJYGI2TMNJSHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
.com>
--
Natasha Mathur
Director of Data
*Calendly <https://calendly.com/natashamathur/30-minute-meeting>*
Technology Advancing Primary Care
|
@sfc-gh-jcieslak 👋, |
Hey @bob-zarkoob |
Does V1 remove snowflake_role_grants? it seems that removing something that would save costs on users when theres no immediate plan to remedy with snowflake_grant_account_role might not be a great idea? |
Hey @ddemara-indeed. In fact, We do not plan to address this issue before the V1. However, we understand the problem and recently received multiple inquiries about it. We will discuss this topic internally in the context of our V1 roadmap and consider our options. In the meantime, we are open to contributions (check contribution guidelines). |
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.
Terraform CLI and Provider Versions
Provider: 0.87.2
CLI: 1.7.2
Use Cases or Problem Statement
after you added these 2 resources, now we cannot use arrays to create the hierarchy between users and roles, and between roles and roles, now we have created a new resource for each assignment.
Like with snowflake_role_grants we used to have:
and now we to create 3 different resource like this:
resource "snowflake_grant_account_role" "prod_bronze_read_arl_1" {
Proposal
I like to be able to add users and roles in bulk using arrays.
for snowflake_grant_privileges_to_account_role, I like to be able to add more than one object when granting privileges, like instead of doing 3 different resources to grant select privilege to 3 different objects, having the capability of adding multiple privileges to multiple roles.
How much impact is this issue causing?
Medium
Additional Information
No response
The text was updated successfully, but these errors were encountered: