Skip to content
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

Open
bob-zarkoob opened this issue Apr 2, 2024 · 25 comments
Labels
category:grants feature-request Used to mark issues with provider's missing functionalities

Comments

@bob-zarkoob
Copy link

bob-zarkoob commented Apr 2, 2024

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:

resource "snowflake_grant_account_role" "prod_bronze_read_arl" {
  role_name = snowflake_role.prod_bronze_read_arl.name

  roles = [
    snowflake_role.data_analyst_frl.name,
    snowflake_role.tableau_developer_frl.name,
    snowflake_role.data_scientist_frl.name
  ]

and now we to create 3 different resource like this:
resource "snowflake_grant_account_role" "prod_bronze_read_arl_1" {

  role_name        = snowflake_role.prod_bronze_read_arl.name
  parent_role_name = snowflake_role.data_analyst_frl.name
}

resource "snowflake_grant_account_role" "prod_bronze_read_arl_2" {
  role_name        = snowflake_role.prod_bronze_read_arl.name
  parent_role_name = snowflake_role.tableau_developer_frl.name
}

resource "snowflake_grant_account_role" "prod_bronze_read_arl_3" {
  role_name = snowflake_role.prod_bronze_read_arl.name
  parent_role_name = snowflake_role.data_scientist_frl.name
}

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

@bob-zarkoob bob-zarkoob added the feature-request Used to mark issues with provider's missing functionalities label Apr 2, 2024
@sfc-gh-jcieslak
Copy link
Collaborator

Hey @bob-zarkoob 👋
We'll consider adding "multiple grants by one resource", but for now, try to use builtin HCL for_each meta-argument to minimize the verbosity.

@Bryan-Meier
Copy link

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.

@gthomson31
Copy link

gthomson31 commented Apr 17, 2024

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
}

@sfc-gh-jcieslak
Copy link
Collaborator

Hey @gthomson31
Not sure what you mean here, for_each doesn't make users' name sensitive. It's already marked as sensitive in the user's schema (it's marked in the documentation).

@gthomson31
Copy link

Interesting as this problem has only appeared snowflake_grant_privileges_to_account_role resource type this issue has not occured on previous grant types

@sfc-gh-jcieslak
Copy link
Collaborator

@gthomson31 Still not sure what is the issue in your case.

as a result permissions are being added with index instead which is a workaround

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.

@sfc-gh-jcieslak
Copy link
Collaborator

@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.

@Bryan-Meier
Copy link

Bryan-Meier commented Apr 22, 2024

@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:
image

This is the code it's referring to:
image

This was the code that used the deprecated resource which works fine:
image

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".

@Bryan-Meier
Copy link

If there isn't a work around for this, it feels like this is more of a bug than a feature request.

@gthomson31
Copy link

gthomson31 commented Apr 22, 2024

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
]
}

@sfc-gh-jcieslak
Copy link
Collaborator

@gthomson31 @Bryan-Meier I see that sensitive is put on user name and not on login_name which is very weird. Anyway, we'll be reworking resources, because ones like user weren't changed for a long time. A workaround for sensitive + for_each could be to wrap "sensitive" values in the nonsensitive function.

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
}

@Bryan-Meier
Copy link

Bryan-Meier commented Apr 23, 2024

@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.

@Bryan-Meier
Copy link

@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:

│ Error: Invalid function argument
│ 
│   on modules/snowflake_role/main.tf line 22, in resource "snowflake_grant_account_role" "grant_role_to_users":
│   22:   for_each = toset([ for u in var.role_users : nonsensitive(u) ])
│ 
│ Invalid value for "value" parameter: the given value is not sensitive, so
│ this call is redundant. 

This is telling me it's nonsensitive but when I remove the nonsensitive function I get:

│ Error: Invalid for_each argument
│ 
│   on modules/snowflake_role/main.tf line 22, in resource "snowflake_grant_account_role" "grant_role_to_users":
│   22:   for_each = toset(var.role_users)
│     ├────────────────
│     │ var.role_users has a sensitive value
│ 
│ Sensitive values, or values derived from sensitive values, cannot be used
│ as for_each arguments. If used, the sensitive value could be exposed as a
│ resource instance key.

I have even tried tricking it where I wrap sensitive function with a nonsensitive function which was also unsuccessful:
for_each = toset([ for u in var.role_users : nonsensitive(sensitive(u.name)) ])

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.

@Bryan-Meier
Copy link

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. for_each = toset([for u in [snowflake_user.test.name] : nonsensitive(u)])), I passed in a list of email addresses for the users. Since it's nothing more than a list of strings it runs without needing the nonsensitive function.

Again, not what we want to do but it does work.

@sfc-gh-jcieslak
Copy link
Collaborator

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.

@natashamathur
Copy link

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.

@natashamathur
Copy link

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?

@sfc-gh-jcieslak
Copy link
Collaborator

Hey @natashamathur

  1. Not sure what function you're referring to, but nonsensitive is the HCL function, it's not provided by us. It's good to migrate sooner than later, because of this.
  2. I can't give the exact timeline, but it should happen this quarter.

@natashamathur
Copy link

Got it! Your comment further upthread suggested that the users assignment itself will be further changed. Is that the plan?

@sfc-gh-jcieslak
Copy link
Collaborator

sfc-gh-jcieslak commented May 7, 2024

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.

@natashamathur
Copy link

natashamathur commented May 7, 2024 via email

@bob-zarkoob
Copy link
Author

@sfc-gh-jcieslak 👋,
do you have any ETA on when this issue can be worked on? The problem is we are paying for Terraform Cloud for each resource and the way this resource works, we ended up with thousands of resources and significantly higher cost for TFC.

@sfc-gh-jcieslak
Copy link
Collaborator

Hey @bob-zarkoob
Unfortunately, right now our focus is on different topics (currently) as we're preparing resources for the V1 while addressing other big issues in the provider (e.g. identifiers). We would like to look at this, but we don't have much time for it right now. We'll most likely make an announcement (in gh discussions or just answer in threads like this one) when we'll be picking this topic.

cc: @sfc-gh-sthyagaraj

@ddemara-indeed
Copy link

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?

@sfc-gh-asawicki
Copy link
Collaborator

Hey @ddemara-indeed.

In fact, snowflake_role_grants has already been removed from the provider's 0.93.0 version. If this is an inconvenience, you can stay on the older version, at least for the time being.

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).

sfc-gh-asawicki added a commit that referenced this issue Aug 28, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:grants feature-request Used to mark issues with provider's missing functionalities
Projects
None yet
Development

No branches or pull requests

7 participants