Snowflake-Labs / terraform-provider-snowflake

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

Question: GRANT SELECT ON ALL TABLES #250

Closed funes79 closed 1 year ago

funes79 commented 4 years ago

Hi, based on the docs (and I tested) the tables grant for future tables grants SELECT only for future tables. But when I unset (to false) the future tables then the provider requires a table name.

How can I achieve a

GRANT SELECT ON ALL TABLES IN SCHEMA <schema> TO ROLE <role>

using terraform resource?

Thanks

mhmcdonald commented 4 years ago

I don't believe you can with this terraform provider. It's not currently supported.

tjtaill commented 4 years ago

This is a feature that is greatly missed maybe if schema or table names were * wildcard values we could apply GRANT SELECT ON ALL TABLES IN SCHEMA or GRANT TO ALL SCHEMAS IN DATABASE ?

shaunmaris commented 3 years ago

We would also greatly benefit from this feature

multiprocessingh commented 3 years ago

Bumping this. Unsure where we landed with this. This is a bit of a deal breaker for a lot of folks who use third party loading services like fivetran or segment where it's difficult to administrate the number of tables being created in a database on a table by table basis. An example is the fivetran salesforce integration which has over 100+ tables being ingested.

adamrosenblit-wex commented 3 years ago

My team would also greatly benefit from this feature.

kumarjoshi commented 3 years ago

This will be an extremely useful feature

aicharles commented 3 years ago

ditto

rdouglashg commented 3 years ago

We also would find this useful!

samrogers226 commented 3 years ago

I want this

sfc-gh-bculberson commented 3 years ago

The way terraform lifecycle works, this would only execute once and ONLY grant access to all tables that exist at that time. It wouldn't know when a new table gets added. It's probably much better to create a resource that grants selects on all the tables you want.

adamrosenblit-wex commented 3 years ago

The way terraform lifecycle works, this would only execute once and ONLY grant access to all tables that exist at that time. It wouldn't know when a new table gets added. It's probably much better to create a resource that grants selects on all the tables you want.

This new resource, combined with the snowflake_table_grant with the on_future=true flag set, would cover all the tables without you needing to know when a new one was created. We have a specific use-case at my company for this combination of grants.

sfc-gh-bculberson commented 3 years ago

The way terraform lifecycle works, this would only execute once and ONLY grant access to all tables that exist at that time. It wouldn't know when a new table gets added. It's probably much better to create a resource that grants selects on all the tables you want.

This new resource, combined with the snowflake_table_grant with the on_future=true flag set, would cover all the tables without you needing to know when a new one was created. We have a specific use-case at my company for this combination of grants.

I would fully support this request if it ALWAYS enabled future grants on all tables. If it's enabled manually, most users wouldn't understand this complexity and wouldn't understand why some tables wouldn't have access.

dstuck commented 3 years ago

@sfc-gh-bculberson That's a really good point, and it made me realize all my GRANT <PRIVILEGE> ON ALL use cases are tied to on_future grants. In fact I think the real issue is that on_future grants have those same complexities that you're describing for having a grant on ALL. It feels like the only simple-to-understand way to use future grants is to always run a grant on future followed by a grant all.

I'd love to propose there be a all_future=true option added to grants, but I think the implementation there would be really ugly since future grants are actual snowflake objects that terraform can check the state of while grant all is a one-shot application. Terraform could confirm that all existing objects have the permission granted but I think this wouldn't actually be valid because another role could have added a new table to a schema that the grantee may not be able to grant privileges on but that a future grant would have granted access to, so we're stuck with a dependency on when the grants are run.

Maybe an all_future=true flag would still be worth it because as is I've found on_future too risky for our team to run on its own. Every on_future grant we add needs a local exec running snowsql to do a paired grant on all, and it'd be wonderful to get away from that

badge commented 3 years ago

I've managed to implement this using the existing provider (for our Segment database, @multiprocessingh). I want to grant SELECT on all tables in all schemas in our Segment database for a particular role. We start by defining two data sources for the schemas and tables:

data "snowflake_schemas" "segment" {
  provider = snowflake.sysadmin
  database = "SEGMENT"
}

data "snowflake_tables" "segment" {
  provider = snowflake.sysadmin
  for_each = toset([
    for schema in data.snowflake_schemas.segment.schemas : schema.name
    if schema.name != "INFORMATION_SCHEMA"
  ])
  database = "SEGMENT"
  schema   = each.key
}

Next, we define a local variable that is a map of all the table, schema pairs in the database:

locals {
  schema_tables = {
    for obj in flatten([
      for schema in data.snowflake_tables.segment : [
        for table in schema.tables : {
          table_name  = table.name
          schema_name = table.schema
        }
      ]
    ]) :
    "${obj.schema_name}~${obj.table_name}" => obj
  }
}

Finally, we use that local variable in a for_each parameter to the snowflake_table_grant resources we need to create:

resource "snowflake_table_grant" "segment_reader" {
  provider      = snowflake.securityadmin
  for_each      = local.schema_tables
  database_name = "SEGMENT"
  schema_name   = each.value.schema_name
  table_name    = each.value.table_name

  privilege = "SELECT"

  roles = [snowflake_role.segment_reader.name]
}

We can separately create the future tables grants, and every time Terraform runs it'll add those explicit grants to any newly-created tables.

badge commented 3 years ago

I created a small module for this; it works with tables, views and stages.

iljau commented 2 years ago

Is it possible to import changes newly added resources utilizing for_each? If new tables are added to database, it looks like it's necessary run terraform import on each table one by one.

ralfsantacruz commented 2 years ago

@iljau I am having a similar issue.

I went ahead and used @badge's implementation for table grants (thank you for this). I understand that this will grant privileges to all existing tables, but not any new tables because it uses the data resource. The data resource pings Snowflake to get all tables that exist in the schema, but does not include tables that are going to be created as part of a terraform apply.

Grants for a new table can be covered by on_future grants, but on the next terraform plan, it will still attempt to create grants for the table, even though the future grants were applied on its creation. Terraform doesn’t seem to pick up on future grants that happen outside of its walls. As far as Terraform is concerned, it doesn’t think that the object has the proper grants yet. This is a minor inconvenience, but it won’t break anything. However, as it stands, our system is doing work twice. Once in Snowflake when future grants are applied, and again in Terraform because it thinks those grants don’t exist yet.

ghost commented 2 years ago

To have an on_existing option that could work in tandem with the on_future argument would so nice. e.g.

resource "snowflake_table_grant" "select" {
  privilege         = "SELECT"
  database_name     = "db"
  schema_name       = "schema"
  roles             = ["role"]
  on_future         = true
  on_existing       = true
  with_grant_option = false
}

Basically anywhere there's an option for on_future, on_existing would be useful.

Maybe just on_existing_and_future?

This would be much nicer than having to grab table names and then assign grants per table using for_each.

swallace-rgare commented 2 years ago

This feature would be very helpful for us as well.

jonmarshall-rgare commented 2 years ago

+1. This would be very helpful

ghost commented 2 years ago

I made a very specific feature request for this: https://github.com/chanzuckerberg/terraform-provider-snowflake/issues/787

a2m1 commented 2 years ago

Similar request https://github.com/chanzuckerberg/terraform-provider-snowflake/issues/284 and one more solution has been already created https://github.com/chanzuckerberg/terraform-provider-snowflake/issues/284#issuecomment-907904158

ghost commented 2 years ago

Would be great if the solution provided in #284 could be added to the provider.

@aidanmelen - curious if you have ever considered merging your snowsql provider into this project?

aidanmelen commented 2 years ago

Would be great if the solution provided in #284 could be added to the provider.

@aidanmelen - curious if you have ever considered merging your snowsql provider into this project?

@astronaut-chris I appreciate the interest. There was a pull request once upon a time, but ultimately it didn't make sense to merge. More context can be found here on the pull request.

What's more:

The Terraform SnowSQL provider allows for the management of the create and delete lifecycles for Snowflake objects with SnowSQL.

Note: This provider is NOT a drop in replacement for the robust resources implemented by terraform-provider-snowflake e.g. if you want to create a virtual warehouse, then use the snowflake_warehouse resource. Use this provider when you require fine grain control of DCL commands or to implement Snowflake objects that are unsupported by the Snowflake provider resources. More usecases for this provider can be found here.

Similiar to the terraform-provider-shell; the provider

this is a backdoor into the Terraform runtime. You can do some pretty dangerous things with this and it is up to you to make sure you don't get in trouble. Since this provider is rather different than most other provider, it is recommended that you at least have some familiarity with the internals of Terraform before attempting to use this provider.

ghost commented 2 years ago

Thank you @aidanmelen!

This might be a silly question, but if I used snowsql_exec to grant permissions on existing roles, would I have to worry about the operator affecting other existing grants or resources?

aidanmelen commented 2 years ago

Thank you @aidanmelen!

This might be a silly question, but if I used snowsql_exec to grant permissions on existing roles, would I have to worry about the operator affecting other existing grants or resources?

Not a silly question at all. The snowsql provider will not revoke existing grants or any snowflake object type that is not explicitly declared. It is merely running the SnowSQL commands you provide, during the terraform lifecycles you configure.

On the other hand, the snowflake provider may revoke grants created by the snowsql provider. For example, I have seen this behavior with the snowflake_user_grant in the past when enable_multiple_grants is false.

DustinMoriarty commented 1 year ago

The snowsql provider sounds like a great workaround for unsupported features. However, it is not a long term substitute for a real solution. This use case is important because for many companies, it is simply not realistic to have the tables and the permissions defined in the same terraform repo. Permissions make sense to centralize into a single repo for an approval process while tables are created and managed by applications, pipelines and other tools outside of terraform. In order to avoid cyclical dependencies in terraform, the snowflake infrastructrure terraform repo needs to have no dependencies on the tables existing prior to running in a new environment. Applications and pipelines often drop and recreate tables, so a central access control repo should not really depend on tables to consistently already exist.

Usually I like terraform resources to stay as close as possible to the underlying API. However, in this case, to get behavior which is consistent with the terraform lifecycle, I think that what is really needed is to be able to run both GRANT ON ALL and GRANT ON FUTURE ALL statements in a single terraform resource. This two statement solution is not as elegant and clean as we would like under the hood. However, I think that in this case it is the only way to get a deterministic relationship between the terraform state and the permissions actually assigned even though the way these permissions are assigned may vary a little based on timing. It will make a terraform interface much more inline with what users are used to for other terraform providers and resources.

If we don't want to change the way this resource works too much, it is OK to make a new one which does similar things a different way and put some warnings in the docs about conflicts. The AWS and GCP providers have overlapping resources all the time, especially for the access control resources. It is better to have a few overlapping resources to chose from than try to make one swiss army knife that does everything and has lots of parameters which can only be combined in certain ways.

I was looking at potentially putting in a pull request for this. However, there are a few abstraction layers in the code which I was not able to fully untangle with my level of Go knowledge.

DustinMoriarty commented 1 year ago

The way terraform lifecycle works, this would only execute once and ONLY grant access to all tables that exist at that time. It wouldn't know when a new table gets added. It's probably much better to create a resource that grants selects on all the tables you want.

This new resource, combined with the snowflake_table_grant with the on_future=true flag set, would cover all the tables without you needing to know when a new one was created. We have a specific use-case at my company for this combination of grants.

I would fully support this request if it ALWAYS enabled future grants on all tables. If it's enabled manually, most users wouldn't understand this complexity and wouldn't understand why some tables wouldn't have access.

The provider is still on a 0.x.x version. In my mind that means it is OK to deprecate some behavior on the next minor version change. Those of us using a provider without it's first major release should be pegging versions anyway.

Alternately, just create a new resource called snowflake_table_grant_all which runs both the ALL and ON FUTURE ALL statements.

rahulnbhandari commented 1 year ago

Future grants is not an option if db is a replica from another sf account . Schema objects and tables associated gets added when we sync the db. We cannot do grants on future tables as schema won’t be there until we sync the db . There currently is no option to just add schemas and not the underlying tables when we set up a replicated db . Future grants only work if on next resync new tables gets added .

aidanmelen commented 1 year ago

If we don't want to change the way this resource works too much, it is OK to make a new one which does similar things a different way and put some warnings in the docs about conflicts. The AWS and GCP providers have overlapping resources all the time, especially for the access control resources. It is better to have a few overlapping resources to chose from than try to make one swiss army knife that does everything and has lots of parameters which can only be combined in certain ways.

This is probably the most straightforward implementation as it is tricky to integrate with the existing grant resources.

aidanmelen commented 1 year ago

Applications and pipelines often drop and recreate tables

There is a good discussion about this problem in Managing Snowflake Architecture with Terraform and dbt, Ritual

In the past, we managed warehouse, database, and schema grants using the snowflake provider and ran a snowsql script to grant all to the tables outside of terraform. Terraform wants complete control of the resources it creates which is problematic when applications like DBT or Okta want to also managed some part of the snowflake environment. The snowsql provider aims to bring these scripts back into terraform.

lachniej commented 1 year ago

@Relativity74205, I saw that you pushed the grant to all tables feature. This is awesome! Do you know if there is a chance similar logic could be applied to view_grants? https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/resources/view_grant.md

Relativity74205 commented 1 year ago

@lachniej Yes, I plan to do it in the near future. :)

justincandoit commented 1 year ago

@Relativity74205 also for databases would be huge!

GRANT SELECT ON ALL TABLES IN DATABASE <database> TO ROLE <role>
Relativity74205 commented 1 year ago

@justincanney This one should already work (with 0.60)

Relativity74205 commented 1 year ago

@lachniej see https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/1686 :)

lachniej commented 1 year ago

Awesome, thanks @Relativity74205! Can't wait until all of this hits the next release so I can use the on_all parameters for the cdktf snowflake provider!

sfc-gh-swinkler commented 1 year ago

Going to close this issue since it is possible to do this using both snowflake_table_grant and snowflake_grant_privileges_to_role resources. Examples:

New Way

resource "snowflake_grant_privileges_to_role" "grant" {
  privileges = ["SELECT"]
  role_name  = snowflake_role.r.name
  on_schema_object {
    all {
      object_type_plural = "TABLES"
      in_schema          = "\"my_db\".\"my_schema\"" # note this is a fully qualified name!
    }
  }
}

Old Way

resource "snowflake_table_grant" "grant" {
  database_name = "database"
  schema_name   = "schema"

  privilege = "SELECT"
  roles     = [snowflake_role.r.name]
  on_all = true
}
KarthikRajashekaran commented 9 months ago

@sfc-gh-swinkler What the syntax to provide a grant to a list of table_names to the snowflake_share NOT ALL TABLES in the resource snowflake_grant_privileges_to_role

Old-way

resource "snowflake_table_grant" "grant" {
    for_each      = local.select_flat

    database_name = each.value.database_name
    schema_name   = each.value.database_name
    table_name    = each.value.table_name  

    privilege         = "SELECT"
    roles             = ["role1"]
    shares            = [snowflake_share.share.name]

    on_future         = true
    with_grant_option = false
} 
sfc-gh-swinkler commented 9 months ago

@KarthikRajashekaran you can do the same thing as with for_each in your example.

resource "snowflake_grant_privileges_to_role" "grant" {
for_each = local.select_flat  
  privileges = ["SELECT"]
  role_name  = snowflake_role.r.name
  on_schema_object {
    object_type = "TABLE"
    object_name = each.value.table_fully_qualified_name # note this is a fully qualified name!
  }
  }
}

also note that @sfc-gh-jcieslak has been working on some changes to this resource including renaming it. So in the next release that comes out this snowflake_grant_privileges_to_role resource will be deprecated in favor of snowflake_grant_privileges_to_account_role. The syntax is slightly different in this case, but same basic idea.

KarthikRajashekaran commented 9 months ago

@sfc-gh-swinkler Thanks for the code, How about share_name? As to provide a grant of the table to the share_name like in old_way snowflake_table_grant

shares = [snowflake_share.share.name]

sfc-gh-swinkler commented 9 months ago

@KarthikRajashekaran good question. @sfc-gh-jcieslak has a PR that would allow granting privileges from a table to a share. This will be merged soon.

KarthikRajashekaran commented 9 months ago

@sfc-gh-swinkler Please guide me on this, Once the share is created and db/schema/tables grants provided to share .. I am trying to find an example in Terraform that creates the database in the consumer account from the share created in provider account


-- Create database from share
create database <provider_account>_share from share <provider_account>.myshare

-- Grant privileges on the database to other roles (e.g. SYSADMIN) in your account.
grant imported privileges on database <provider_account>_share to SYSADMIN;
sfc-gh-swinkler commented 9 months ago

@KarthikRajashekaran we aim to have better examples in the future, but for now you can take a look at the database resource attribute "from_share" https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/database#from_share. So the process is you first need to create a share, and add the secondary account to that list. It will require two configurations of Terraform provider, one with credentials to primary account and the second with credentials to secondary account.