changes after review

Snowflake-Labs · Jan 22, 2024 · 03a73ca · 03a73ca
1 parent 2505c13
commit 03a73ca
Show file tree

Hide file tree

Showing 15 changed files with 343 additions and 182 deletions.
diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md
@@ -92,5 +92,3 @@ provider "snowflake" {
 #### *(behavior change)* authenticator (JWT)
 
 Before the change `authenticator` parameter did not have to be set for private key authentication and was deduced by the provider. The change is a result of the introduced configuration alignment with an underlying [gosnowflake driver](https://github.com/snowflakedb/gosnowflake). The authentication type is required there, and it defaults to user+password one. From this version, set `authenticator` to `JWT` explicitly.
-
-// TODO: Update identifiers 0.82.0 -> 0.83.1 + grant migration (new name ...account_role)
diff --git a/docs/resources/grant_privileges_to_account_role.md b/docs/resources/grant_privileges_to_account_role.md
@@ -35,6 +35,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   on_account = true
 }
 
+## ID: "\"role_name\"|false|false|CREATE DATABASE,CREATE USER|OnAccount"
+
 # all privileges + grant option
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name         = snowflake_role.db_role.name
@@ -43,6 +45,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|false|ALL|OnAccount"
+
 # all privileges + grant option + always apply
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name         = snowflake_role.db_role.name
@@ -52,6 +56,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|true|ALL|OnAccount"
+
 ##################################
 ### on account object privileges
 ##################################
@@ -66,6 +72,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|CREATE SCHEMA,CREATE DATABASE ROLE|OnAccountObject|DATABASE|\"database\""
+
 # all privileges + grant option
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name = snowflake_role.db_role.name
@@ -77,6 +85,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|false|ALL|OnAccountObject|DATABASE|\"database\""
+
 # all privileges + grant option + always apply
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name = snowflake_role.db_role.name
@@ -89,6 +99,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|true|ALL|OnAccountObject|DATABASE|\"database\""
+
 ##################################
 ### schema privileges
 ##################################
@@ -102,6 +114,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|MODIFY,CREATE TABLE|OnSchema|OnSchema|\"database\".\"my_schema\""
+
 # all privileges + grant option
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name = snowflake_role.db_role.name
@@ -112,6 +126,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|false|MODIFY,CREATE TABLE|OnSchema|OnSchema|\"database\".\"my_schema\""
+
 # all schemas in database
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["MODIFY", "CREATE TABLE"]
@@ -121,6 +137,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|MODIFY,CREATE TABLE|OnSchema|OnAllSchemasInDatabase|\"database\""
+
 # future schemas in database
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["MODIFY", "CREATE TABLE"]
@@ -130,6 +148,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|MODIFY,CREATE TABLE|OnSchema|OnFutureSchemasInDatabase|\"database\""
+
 ##################################
 ### schema object privileges
 ##################################
@@ -144,6 +164,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|SELECT,REFERENCES|OnSchemaObject|VIEW|\"database\".\"my_schema\".\"my_view\""
+
 # all privileges + grant option
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name = snowflake_role.db_role.name
@@ -155,6 +177,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|false|ALL|OnSchemaObject|OnObject|VIEW|\"database\".\"my_schema\".\"my_view\""
+
 # all in database
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["SELECT", "INSERT"]
@@ -167,6 +191,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|SELECT,INSERT|OnSchemaObject|OnAll|TABLES|InDatabase|\"database\""
+
 # all in schema
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["SELECT", "INSERT"]
@@ -179,6 +205,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|SELECT,INSERT|OnSchemaObject|OnAll|TABLES|InSchema|\"database\".\"my_schema\""
+
 # future in database
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["SELECT", "INSERT"]
@@ -191,6 +219,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|SELECT,INSERT|OnSchemaObject|OnFuture|TABLES|InDatabase|\"database\""
+
 # future in schema
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["SELECT", "INSERT"]
@@ -202,6 +232,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
     }
   }
 }
+
+## ID: "\"role_name\"|false|false|SELECT,INSERT|OnSchemaObject|OnFuture|TABLES|InSchema|\"database\".\"my_schema\""
 ```
 
 <!-- schema generated by tfplugindocs -->
@@ -215,7 +247,7 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
 
 - `all_privileges` (Boolean) Grant all privileges on the account role.
 - `always_apply` (Boolean) If true, the resource will always produce a “plan” and on “apply” it will re-grant defined privileges. It is supposed to be used only in “grant privileges on all X’s in database / schema Y” or “grant all privileges to X” scenarios to make sure that every new object in a given database / schema is granted by the account role and every new privilege is granted to the database role. Important note: this flag is not compliant with the Terraform assumptions of the config being eventually convergent (producing an empty plan).
-- `always_apply_trigger` (String) This field should not be set and its main purpose is to achieve the functionality described by always_apply field. This is value will be flipped to the opposite value on every terraform apply, thus creating a new plan that will re-apply grants.
+- `always_apply_trigger` (String) This is a helper field and should not be set. Its main purpose is to help to achieve the functionality described by the always_apply field.
 - `on_account` (Boolean) If true, the privileges will be granted on the account.
 - `on_account_object` (Block List, Max: 1) Specifies the account object on which privileges will be granted (see [below for nested schema](#nestedblock--on_account_object))
 - `on_schema` (Block List, Max: 1) Specifies the schema on which privileges will be granted. (see [below for nested schema](#nestedblock--on_schema))

diff --git a/docs/resources/grant_privileges_to_database_role.md b/docs/resources/grant_privileges_to_database_role.md
@@ -175,7 +175,7 @@ resource "snowflake_grant_privileges_to_database_role" "example" {
 
 - `all_privileges` (Boolean) Grant all privileges on the database role.
 - `always_apply` (Boolean) If true, the resource will always produce a “plan” and on “apply” it will re-grant defined privileges. It is supposed to be used only in “grant privileges on all X’s in database / schema Y” or “grant all privileges to X” scenarios to make sure that every new object in a given database / schema is granted by the account role and every new privilege is granted to the database role. Important note: this flag is not compliant with the Terraform assumptions of the config being eventually convergent (producing an empty plan).
-- `always_apply_trigger` (String) This field should not be set and its main purpose is to achieve the functionality described by always_apply field. This is value will be flipped to the opposite value on every terraform apply, thus creating a new plan that will re-apply grants.
+- `always_apply_trigger` (String) This is a helper field and should not be set. Its main purpose is to help to achieve the functionality described by the always_apply field.
 - `on_database` (String) The fully qualified name of the database on which privileges will be granted.
 - `on_schema` (Block List, Max: 1) Specifies the schema on which privileges will be granted. (see [below for nested schema](#nestedblock--on_schema))
 - `on_schema_object` (Block List, Max: 1) Specifies the schema object on which privileges will be granted. (see [below for nested schema](#nestedblock--on_schema_object))

diff --git a/examples/resources/snowflake_grant_privileges_to_account_role/resource.tf b/examples/resources/snowflake_grant_privileges_to_account_role/resource.tf
@@ -17,6 +17,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   on_account = true
 }
 
+## ID: "\"role_name\"|false|false|CREATE DATABASE,CREATE USER|OnAccount"
+
 # all privileges + grant option
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name         = snowflake_role.db_role.name
@@ -25,6 +27,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|false|ALL|OnAccount"
+
 # all privileges + grant option + always apply
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name         = snowflake_role.db_role.name
@@ -34,6 +38,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|true|ALL|OnAccount"
+
 ##################################
 ### on account object privileges
 ##################################
@@ -48,6 +54,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|CREATE SCHEMA,CREATE DATABASE ROLE|OnAccountObject|DATABASE|\"database\""
+
 # all privileges + grant option
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name = snowflake_role.db_role.name
@@ -59,6 +67,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|false|ALL|OnAccountObject|DATABASE|\"database\""
+
 # all privileges + grant option + always apply
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name = snowflake_role.db_role.name
@@ -71,6 +81,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|true|ALL|OnAccountObject|DATABASE|\"database\""
+
 ##################################
 ### schema privileges
 ##################################
@@ -84,6 +96,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|MODIFY,CREATE TABLE|OnSchema|OnSchema|\"database\".\"my_schema\""
+
 # all privileges + grant option
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name = snowflake_role.db_role.name
@@ -94,6 +108,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|false|MODIFY,CREATE TABLE|OnSchema|OnSchema|\"database\".\"my_schema\""
+
 # all schemas in database
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["MODIFY", "CREATE TABLE"]
@@ -103,6 +119,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|MODIFY,CREATE TABLE|OnSchema|OnAllSchemasInDatabase|\"database\""
+
 # future schemas in database
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["MODIFY", "CREATE TABLE"]
@@ -112,6 +130,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|MODIFY,CREATE TABLE|OnSchema|OnFutureSchemasInDatabase|\"database\""
+
 ##################################
 ### schema object privileges
 ##################################
@@ -126,6 +146,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|SELECT,REFERENCES|OnSchemaObject|VIEW|\"database\".\"my_schema\".\"my_view\""
+
 # all privileges + grant option
 resource "snowflake_grant_privileges_to_account_role" "example" {
   role_name = snowflake_role.db_role.name
@@ -137,6 +159,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   with_grant_option = true
 }
 
+## ID: "\"role_name\"|true|false|ALL|OnSchemaObject|OnObject|VIEW|\"database\".\"my_schema\".\"my_view\""
+
 # all in database
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["SELECT", "INSERT"]
@@ -149,6 +173,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|SELECT,INSERT|OnSchemaObject|OnAll|TABLES|InDatabase|\"database\""
+
 # all in schema
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["SELECT", "INSERT"]
@@ -161,6 +187,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|SELECT,INSERT|OnSchemaObject|OnAll|TABLES|InSchema|\"database\".\"my_schema\""
+
 # future in database
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["SELECT", "INSERT"]
@@ -173,6 +201,8 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
   }
 }
 
+## ID: "\"role_name\"|false|false|SELECT,INSERT|OnSchemaObject|OnFuture|TABLES|InDatabase|\"database\""
+
 # future in schema
 resource "snowflake_grant_privileges_to_account_role" "example" {
   privileges = ["SELECT", "INSERT"]
@@ -184,3 +214,5 @@ resource "snowflake_grant_privileges_to_account_role" "example" {
     }
   }
 }
+
+## ID: "\"role_name\"|false|false|SELECT,INSERT|OnSchemaObject|OnFuture|TABLES|InSchema|\"database\".\"my_schema\""
diff --git a/pkg/resources/grant_privileges_to_account_role.go b/pkg/resources/grant_privileges_to_account_role.go
@@ -62,7 +62,7 @@ var grantPrivilegesToAccountRoleSchema = map[string]*schema.Schema{
 		Type:        schema.TypeString,
 		Optional:    true,
 		Default:     "",
-		Description: "This field should not be set and its main purpose is to achieve the functionality described by always_apply field. This is value will be flipped to the opposite value on every terraform apply, thus creating a new plan that will re-apply grants.",
+		Description: "This is a helper field and should not be set. Its main purpose is to help to achieve the functionality described by the always_apply field.",
 	},
 	"on_account": {
 		Type:        schema.TypeBool,
@@ -681,6 +681,16 @@ func ReadGrantPrivilegesToAccountRole(ctx context.Context, d *schema.ResourceDat
 	logging.DebugLogger.Printf("[DEBUG] Parsed identifier: %s", id.String())
 
 	if id.AlwaysApply {
+		// The Trigger is a string rather than boolean that would be flipped on every terraform apply
+		// because it's easier to think about and not to worry about edge cases that may occur with 1bit values.
+		// The only place to have the "flip" is Read operation, because there we can set value and produce a plan
+		// that later on will be executed in the Update operation.
+		//
+		// The following example shows that we can end up with the same value as before, which may lead to empty plans:
+		// 1. Create configuration with always_apply = false (let's say trigger will be false by default)
+		// 2. terraform apply: Create (Read will update it to false)
+		// 3. Update config so that always_apply = true
+		// 4. terraform apply: Read (updated trigger to false) -> change is not detected (no plan; no Update)
 		triggerId, err := uuid.GenerateUUID()
 		if err != nil {
 			return diag.Diagnostics{
@@ -932,26 +942,27 @@ func getAccountRoleGrantOn(d *schema.ResourceData) *sdk.AccountRoleGrantOn {
 
 		objectType := onAccountObject["object_type"].(string)
 		objectName := onAccountObject["object_name"].(string)
+		objectIdentifier := sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName)
 
 		switch sdk.ObjectType(objectType) {
 		case sdk.ObjectTypeDatabase:
-			grantOnAccountObject.Database = sdk.Pointer(sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName))
+			grantOnAccountObject.Database = &objectIdentifier
 		case sdk.ObjectTypeFailoverGroup:
-			grantOnAccountObject.FailoverGroup = sdk.Pointer(sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName))
+			grantOnAccountObject.FailoverGroup = &objectIdentifier
 		case sdk.ObjectTypeIntegration:
-			grantOnAccountObject.Integration = sdk.Pointer(sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName))
+			grantOnAccountObject.Integration = &objectIdentifier
 		case sdk.ObjectTypeConnection:
-			grantOnAccountObject.Connection = sdk.Pointer(sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName))
+			grantOnAccountObject.Connection = &objectIdentifier
 		case sdk.ObjectTypeReplicationGroup:
-			grantOnAccountObject.ReplicationGroup = sdk.Pointer(sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName))
+			grantOnAccountObject.ReplicationGroup = &objectIdentifier
 		case sdk.ObjectTypeResourceMonitor:
-			grantOnAccountObject.ResourceMonitor = sdk.Pointer(sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName))
+			grantOnAccountObject.ResourceMonitor = &objectIdentifier
 		case sdk.ObjectTypeUser:
-			grantOnAccountObject.User = sdk.Pointer(sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName))
+			grantOnAccountObject.User = &objectIdentifier
 		case sdk.ObjectTypeWarehouse:
-			grantOnAccountObject.Warehouse = sdk.Pointer(sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName))
+			grantOnAccountObject.Warehouse = &objectIdentifier
 		case sdk.ObjectTypeExternalVolume:
-			grantOnAccountObject.ExternalVolume = sdk.Pointer(sdk.NewAccountObjectIdentifierFromFullyQualifiedName(objectName))
+			grantOnAccountObject.ExternalVolume = &objectIdentifier
 		}
 
 		on.AccountObject = grantOnAccountObject
Original file line number	Diff line number	Diff line change
Expand Up		@@ -92,5 +92,3 @@ provider "snowflake" {
		#### (behavior change) authenticator (JWT)

		Before the change `authenticator` parameter did not have to be set for private key authentication and was deduced by the provider. The change is a result of the introduced configuration alignment with an underlying [gosnowflake driver](https://github.com/snowflakedb/gosnowflake). The authentication type is required there, and it defaults to user+password one. From this version, set `authenticator` to `JWT` explicitly.

		// TODO: Update identifiers 0.82.0 -> 0.83.1 + grant migration (new name ...account_role)