Merge pull request #18 from getindata/fix/decouple-custom-and-default…

…-modules

feat: Decouple modules to separate database-roles

.pre-commit-config.yaml

@@ -1,11 +1,12 @@
 repos:
   - repo: https://github.com/gruntwork-io/pre-commit
-    # When updating, also check if tflint version in pre-commit workflow can be updated.
     rev: "v0.1.23" # Get the latest from: https://github.com/gruntwork-io/pre-commit/releases
     hooks:
       - id: terraform-validate # It should be the first step as it runs terraform init required by tflint
       - id: terraform-fmt
       - id: tflint
+        args:
+          - "--config=__GIT_ROOT__/.tflint.hcl"
 
   - repo: https://github.com/terraform-docs/terraform-docs
     rev: "v0.18.0" # Get the latest from: https://github.com/terraform-docs/terraform-docs/releases
@@ -14,7 +15,7 @@ repos:
         args: ["."]
 
   - repo: https://github.com/bridgecrewio/checkov.git
-    rev: "3.2.192" # Get the latest from: https://github.com/bridgecrewio/checkov/releases
+    rev: "3.2.216" # Get the latest from: https://github.com/bridgecrewio/checkov/releases
     hooks:
       - id: checkov
         args: [--skip-check, "CKV_TF_1"] # Terraform module sources do not use a git url with a commit hash revision

.tflint.hcl

@@ -0,0 +1,3 @@
+rule "terraform_standard_module_structure" {
+  enabled = false # Fails on context.tf
+}

README.md

@@ -19,7 +19,7 @@ Terraform module for Snowflake stage management.
 * Can create a set of default roles to simplify access management:
     * `READONLY` - granted `USAGE` or `READ` privilages
     * `READWRITE` - granted `WRITE` privileges
-    * `ADMIN` - granted `READ`, `WRITE` privileges (role can be additionally granted with `OWNER` attribute when specified)
+    * `ADMIN` - granted `ALL PRIVILEGES`
 
 ## USAGE
 
@@ -52,7 +52,6 @@ List of code and variable (API) changes:
 - Switched to `snowflake_grant_ownership` resource instead of provider-removed `snowflake_role_ownership_grant`
 - Switched to `snowflake_database_role` module to leverage new `database_roles` mechanism
 - `default_roles` and `custom_roles` are now combined and managed by single module
-- `create_default_roles` variable was renamed to `create_default_database_roles`
 - `roles` variable map received following additions:
   - `all_privileges` - optional, bool
   - `on_all` - optional, bool, defaults to false
@@ -111,7 +110,7 @@ For more information, refer to [variables.tf](variables.tf), list of inputs belo
 | <a name="input_schema"></a> [schema](#input\_schema) | The schema in which to create the stage | `string` | n/a | yes |
 | <a name="input_snowflake_iam_user"></a> [snowflake\_iam\_user](#input\_snowflake\_iam\_user) | Specifies the Snowflake IAM user | `string` | `null` | no |
 | <a name="input_stage"></a> [stage](#input\_stage) | ID element. Usually used to indicate role, e.g. 'prod', 'staging', 'source', 'build', 'test', 'deploy', 'release' | `string` | `null` | no |
-| <a name="input_stage_ownership_grant"></a> [stage\_ownership\_grant](#input\_stage\_ownership\_grant) | To which role the stage ownership should be granted | `string` | `null` | no |
+| <a name="input_stage_ownership_grant"></a> [stage\_ownership\_grant](#input\_stage\_ownership\_grant) | To which account role the stage ownership should be granted | `string` | `null` | no |
 | <a name="input_storage_integration"></a> [storage\_integration](#input\_storage\_integration) | Specifies the name of the storage integration used to delegate authentication responsibility for external cloud storage to a Snowflake identity and access management (IAM) entity | `string` | `null` | no |
 | <a name="input_tags"></a> [tags](#input\_tags) | Additional tags (e.g. `{'BusinessUnit': 'XYZ'}`).<br>Neither the tag keys nor the tag values will be modified by this module. | `map(string)` | `{}` | no |
 | <a name="input_tenant"></a> [tenant](#input\_tenant) | ID element \_(Rarely used, not included by default)\_. A customer identifier, indicating who this instance of a resource is for | `string` | `null` | no |
@@ -122,7 +121,8 @@ For more information, refer to [variables.tf](variables.tf), list of inputs belo
 | Name | Source | Version |
 |------|--------|---------|
 | <a name="module_roles_deep_merge"></a> [roles\_deep\_merge](#module\_roles\_deep\_merge) | Invicton-Labs/deepmerge/null | 0.1.5 |
-| <a name="module_snowflake_database_role"></a> [snowflake\_database\_role](#module\_snowflake\_database\_role) | getindata/database-role/snowflake | 1.1.0 |
+| <a name="module_snowflake_custom_role"></a> [snowflake\_custom\_role](#module\_snowflake\_custom\_role) | getindata/database-role/snowflake | 1.1.1 |
+| <a name="module_snowflake_default_role"></a> [snowflake\_default\_role](#module\_snowflake\_default\_role) | getindata/database-role/snowflake | 1.1.1 |
 | <a name="module_stage_label"></a> [stage\_label](#module\_stage\_label) | cloudposse/label/null | 0.25.0 |
 | <a name="module_this"></a> [this](#module\_this) | cloudposse/label/null | 0.25.0 |
 

examples/complete/README.md

@@ -20,7 +20,6 @@ terraform apply -var-file=fixtures.tfvars
 ```
 
 ## How to destroy
-If `stage_ownership_grant` was used, first you need to hash out this line, run `terraform apply -var-file=fixtures.tfvars` and then:
 ```shell
 terraform destroy -var-file=fixtures.tfvars
 ```
@@ -84,10 +83,10 @@ terraform destroy -var-file=fixtures.tfvars
 
 | Name | Type |
 |------|------|
+| [snowflake_account_role.role_1](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/account_role) | resource |
 | [snowflake_database.this](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/database) | resource |
 | [snowflake_database_role.db_role_1](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/database_role) | resource |
 | [snowflake_database_role.db_role_2](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/database_role) | resource |
 | [snowflake_database_role.db_role_3](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/database_role) | resource |
-| [snowflake_role.role_1](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/role) | resource |
 | [snowflake_schema.this](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/schema) | resource |
 <!-- END_TF_DOCS -->

examples/complete/main.tf

@@ -7,7 +7,7 @@ resource "snowflake_schema" "this" {
   database = snowflake_database.this.name
 }
 
-resource "snowflake_role" "role_1" {
+resource "snowflake_account_role" "role_1" {
   name = "ROLE_1"
 }
 
@@ -58,7 +58,7 @@ module "internal_stage" {
       enabled = false # Disables readwrite default database role creation
     }
     role_1 = { # Database role created by user input
-      granted_to_roles          = [snowflake_role.role_1.name]
+      granted_to_roles          = [snowflake_account_role.role_1.name]
       granted_to_database_roles = ["${snowflake_database.this.name}.${snowflake_database_role.db_role_3.name}"]
       all_privileges            = true
       with_grant_option         = true
@@ -71,8 +71,9 @@ module "internal_stage" {
       with_grant_option         = false
       on_future                 = true
       on_all                    = false
+      enabled                   = false
     }
   }
 
-  stage_ownership_grant = "role_1" # When destroying, please read README.md
+  stage_ownership_grant = snowflake_account_role.role_1.name
 }

locals.tf

@@ -5,33 +5,48 @@ locals {
     lookup(module.stage_label.descriptors, var.descriptor_name, module.stage_label.id), "/${module.stage_label.delimiter}${module.stage_label.delimiter}+/", module.stage_label.delimiter
   ), module.stage_label.delimiter) : null
 
-  schema_object_stage_name = "\"${one(snowflake_stage.this[*].database)}\".\"${one(snowflake_stage.this[*].schema)}\".\"${one(snowflake_stage.this[*].name)}\""
+  create_default_roles     = module.this.enabled && var.create_default_roles
+  schema_object_stage_name = module.this.enabled ? "\"${one(snowflake_stage.this[*].database)}\".\"${one(snowflake_stage.this[*].schema)}\".\"${one(snowflake_stage.this[*].name)}\"" : null
 
   is_internal = var.url == null
 
-  default_roles_definition = var.create_default_roles ? {
+  default_roles_definition = {
     readonly = {
-      enabled      = true
       stage_grants = local.is_internal ? ["READ"] : ["USAGE"]
     }
     readwrite = {
-      enabled      = true
       stage_grants = local.is_internal ? ["READ", "WRITE"] : ["USAGE"]
     }
     admin = {
-      enabled      = true
-      stage_grants = local.is_internal ? ["READ", "WRITE"] : ["USAGE"]
+      stage_grants = local.is_internal ? ["ALL PRIVILEGES"] : ["USAGE"]
     }
-  } : {}
+  }
+
+  roles_definition = module.roles_deep_merge.merged
+
+  default_roles = {
+    for role_name, role in local.roles_definition : role_name => role
+    if contains(keys(local.default_roles_definition), role_name)
+  }
 
-  provided_roles = { for role_name, role in var.roles : role_name => {
-    for k, v in role : k => v
-    if v != null
-  } }
+  custom_roles = {
+    for role_name, role in local.roles_definition : role_name => role
+    if !contains(keys(local.default_roles_definition), role_name)
+  }
+
+  provided_roles = {
+    for role_name, role in var.roles : role_name => {
+      for k, v in role : k => v
+      if v != null
+    }
+  }
 
   roles = {
-    for role_name, role in module.roles_deep_merge.merged : role_name => role
-    if role_name != null && role.enabled
+    for role_name, role in merge(
+      module.snowflake_default_role,
+      module.snowflake_custom_role
+    ) : role_name => role
+    if role.name != null
   }
 }
 

main.tf

@@ -27,12 +27,13 @@ resource "snowflake_stage" "this" {
   url                 = var.url
 }
 
-module "snowflake_database_role" {
-  for_each = local.roles
+module "snowflake_default_role" {
+  for_each = local.default_roles
 
   source  = "getindata/database-role/snowflake"
-  version = "1.1.0"
+  version = "1.1.1"
   context = module.this.context
+  enabled = local.create_default_roles && lookup(each.value, "enabled", true)
 
   database_name = one(snowflake_stage.this[*].database)
   name          = each.key
@@ -59,15 +60,58 @@ module "snowflake_database_role" {
       }
     ]
   }
+
+  depends_on = [
+    snowflake_stage.this
+  ]
 }
 
 resource "snowflake_grant_ownership" "stage_ownership" {
   count = var.stage_ownership_grant != null ? 1 : 0
 
-  database_role_name  = module.snowflake_database_role[var.stage_ownership_grant].fully_qualified_name
+  account_role_name   = var.stage_ownership_grant
   outbound_privileges = "REVOKE"
   on {
     object_type = "STAGE"
     object_name = local.schema_object_stage_name
   }
 }
+
+module "snowflake_custom_role" {
+  for_each = local.custom_roles
+
+  source  = "getindata/database-role/snowflake"
+  version = "1.1.1"
+  context = module.this.context
+  enabled = module.this.enabled && lookup(each.value, "enabled", true)
+
+  database_name = one(snowflake_stage.this[*].database)
+  name          = each.key
+
+  granted_to_roles          = lookup(each.value, "granted_to_roles", [])
+  granted_to_database_roles = lookup(each.value, "granted_to_database_roles", [])
+  granted_database_roles    = lookup(each.value, "granted_database_roles", [])
+
+  attributes = [
+    one(snowflake_stage.this[*].schema),
+    one(snowflake_stage.this[*].name)
+  ]
+
+  schema_objects_grants = {
+    "STAGE" = [
+      {
+        privileges        = lookup(each.value, "stage_grants", null)
+        all_privileges    = lookup(each.value, "all_privileges", null)
+        with_grant_option = lookup(each.value, "with_grant_option", false)
+        on_future         = lookup(each.value, "on_future", false)
+        on_all            = lookup(each.value, "on_all", false)
+        object_name       = (lookup(each.value, "on_future", false) || lookup(each.value, "on_all", false)) ? null : one(snowflake_stage.this[*].name)
+        schema_name       = one(snowflake_stage.this[*].schema)
+      }
+    ]
+  }
+
+  depends_on = [
+    snowflake_stage.this
+  ]
+}

variables.tf

@@ -98,7 +98,7 @@ variable "roles" {
 }
 
 variable "stage_ownership_grant" {
-  description = "To which role the stage ownership should be granted"
+  description = "To which account role the stage ownership should be granted"
   type        = string
   default     = null
 }