Snowflake-Labs / terraform-provider-snowflake

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

[Feature]: snowflake_unsafe_execute as a permanent feature #2934

Open JESCHO99 opened 2 months ago

JESCHO99 commented 2 months ago

Use Cases or Problem Statement

In our team we were searching for a solution which enables us to review SQL code which should be executed on Snowflake in a way we make sure that we can see at any time which user executed which kind of code and that the code was reviewed by at least one person before it was executed. Because we do not want to take the effort to implement a new custom pipeline which does exactly what we need we were searching for a solution without taking the effort to build and maintain a new pipeline.

Category

category:resource

Object type(s)

resource:unsafe_execute

Proposal

With the snowflake_unsafe_execution ressource we found a solution for our problem. From reading the terraform docs we think that this kind of usage is currently not intended by the team and should originally enable Snowflake Terraform Users to be able to maintan ressources which are already released but not natively supported by the connector until the provider released a new version including those ressources. Thats exactly why we want to suggest that the snowflake_unsafe_execution ressource should be available for the usage we implemented in one of our terraform workspaces.

Here is the code we used for our solution:

terraform { required_providers { snowflake = { source = "Snowflake-Labs/snowflake" version = "~> 0.92.0" } } }

provider "snowflake" { alias = "sysadmin_multistmt" role = "SYSADMIN" params = {"MULTI_STATEMENT_COUNT"="0"} }

resource "snowflake_unsafe_execute" "exec_sql" { provider = "snowflake.sysadmin_multistmt" for_each = fileset(path.module, "exec_stmts/iter/*.sql") execute = templatefile( "${path.module}/${each.key}", { "database" = "EXAMPLE_DATABASE" }) revert = "select 1" }

This could be the input of a .sql-file in the "exec_stmts/iter" directory:

use database ${database}; create schema if not exists TEST_SCHEMA; create table TEST_SCHEMA.TABLES as select * from information_schema.tables;

How much impact is this issue causing?

Medium

Additional Information

Using the feature like this enables us to create feature_branches and merge requests within our standardized processes and make all changes applied to Snowflake visible in a single pipeline. All our files are stored inside a git repository and like all other changes applied to our infrastructure it is only possible to trigger a deployment in a pipeline after a merge request gets an approval from a reviewer assigned to the request.

Would you like to implement a fix?

sfc-gh-asawicki commented 2 months ago

Hey @JESCHO99. Thanks for reaching out to us.

We have already seen a bunch of use cases where the unsafe_execute was leveraged, not only for the purposes it had been intended for. That's why we've decided to keep it for the V1 (we listed it under https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/v1-preparations/ESSENTIAL_GA_OBJECTS.MD). Your use case gives us more confidence that the resource is needed.

What we want to do first, is to make it a bit "safer". We will share any decisions/changes to it as part of our migration guide. I will keep this issue open until we adjust the resource and make it V1 ready.

JESCHO99 commented 2 months ago

Hey @sfc-gh-asawicki, thanks for your fast reply. Our team is really happy to hear that the unsage_execute will be included in V1! Is there an option that the deprication warning included with the usage of the resource right now will be excluded in releases before V1? Having this warning every time we plan or apply something is kind of annoying and you tend to not look at warnings because you think that it is just the deprecation warning for the unsafe_execution, which is not a good behaviour.

Thank you!

sfc-gh-asawicki commented 2 months ago

That's a good pointer. I will add this change to the docs in one of my PRs so that it is gone with the next release.

JESCHO99 commented 2 months ago

Good news for us, thanks for removing this depricated information!

rd-andreas-lay commented 2 months ago

Great to hear. We're currently using it as well to call stored procedures which are provided by our infrastructure team to enable teams to create services like creating tec users, roles etc. in a restricted way.