diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 1883c3f32ed..0501f1261a7 100644 --- 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 index b748de7ee3e..e962716f5a4 100644 --- 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\"" ``` @@ -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 index 067ee9caea3..639782db4f0 100644 --- 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 index 83b302df568..677861ae254 100644 --- 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 index 7540bd1c5e8..7011e967535 100644 --- 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 diff --git a/pkg/resources/grant_privileges_to_account_role_acceptance_test.go b/pkg/resources/grant_privileges_to_account_role_acceptance_test.go index d54e806af7a..0a67561f626 100644 --- a/pkg/resources/grant_privileges_to_account_role_acceptance_test.go +++ b/pkg/resources/grant_privileges_to_account_role_acceptance_test.go @@ -4,10 +4,13 @@ import ( "context" "database/sql" "fmt" + "log" "regexp" - "slices" + "strings" "testing" + "github.com/hashicorp/terraform-plugin-testing/helper/acctest" + acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/hashicorp/terraform-plugin-testing/config" @@ -18,7 +21,7 @@ import ( ) func TestAcc_GrantPrivilegesToAccountRole_OnAccount(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() configVariables := config.Variables{ "name": config.StringVariable(roleName), @@ -36,7 +39,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAccount(t *testing.T) { TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -64,7 +67,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAccount(t *testing.T) { } func TestAcc_GrantPrivilegesToAccountRole_OnAccount_PrivilegesReversed(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() configVariables := config.Variables{ "name": config.StringVariable(roleName), @@ -82,7 +85,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAccount_PrivilegesReversed(t *testin TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -110,7 +113,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAccount_PrivilegesReversed(t *testin } func TestAcc_GrantPrivilegesToAccountRole_OnAccountObject(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() databaseName := sdk.NewAccountObjectIdentifier(acc.TestDatabaseName).FullyQualifiedName() configVariables := config.Variables{ @@ -130,7 +133,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAccountObject(t *testing.T) { TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -168,19 +171,23 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAccountObject(t *testing.T) { }) } +// This proves that infinite plan is not produced as in snowflake_grant_privileges_to_role. +// More details can be found in the fix pr https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/2364. func TestAcc_GrantPrivilegesToApplicationRole_OnAccountObject_InfinitePlan(t *testing.T) { + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, PreCheck: func() { acc.TestAccPreCheck(t) }, TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { + PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToAccountRole/OnAccountObject_InfinitePlan"), ConfigVariables: config.Variables{ - "name": config.StringVariable("test_account_role_name"), + "name": config.StringVariable(name), "database": config.StringVariable(acc.TestDatabaseName), }, ConfigPlanChecks: resource.ConfigPlanChecks{ @@ -194,7 +201,7 @@ func TestAcc_GrantPrivilegesToApplicationRole_OnAccountObject_InfinitePlan(t *te } func TestAcc_GrantPrivilegesToAccountRole_OnSchema(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() configVariables := config.Variables{ "name": config.StringVariable(roleName), @@ -216,7 +223,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchema(t *testing.T) { TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -245,15 +252,17 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchema(t *testing.T) { } func TestAcc_GrantPrivilegesToAccountRole_OnSchema_ExactlyOneOf(t *testing.T) { + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, PreCheck: func() { acc.TestAccPreCheck(t) }, TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { + PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToAccountRole/OnSchema_ExactlyOneOf"), PlanOnly: true, ExpectError: regexp.MustCompile("Error: Invalid combination of arguments"), @@ -263,7 +272,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchema_ExactlyOneOf(t *testing.T) { } func TestAcc_GrantPrivilegesToAccountRole_OnAllSchemasInDatabase(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() databaseName := sdk.NewAccountObjectIdentifier(acc.TestDatabaseName).FullyQualifiedName() configVariables := config.Variables{ @@ -283,7 +292,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAllSchemasInDatabase(t *testing.T) { TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -312,7 +321,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnAllSchemasInDatabase(t *testing.T) { } func TestAcc_GrantPrivilegesToAccountRole_OnFutureSchemasInDatabase(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() databaseName := sdk.NewAccountObjectIdentifier(acc.TestDatabaseName).FullyQualifiedName() configVariables := config.Variables{ @@ -332,7 +341,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnFutureSchemasInDatabase(t *testing.T TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -361,7 +370,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnFutureSchemasInDatabase(t *testing.T } func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnObject(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() tblName := "test_database_role_table_name" tableName := sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, acc.TestSchemaName, tblName).FullyQualifiedName() @@ -384,7 +393,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnObject(t *testing.T) TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -414,8 +423,8 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnObject(t *testing.T) } func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnObject_OwnershipPrivilege(t *testing.T) { - name := "test_account_role_name" - tableName := "test_database_role_table_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + tableName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) configVariables := config.Variables{ "name": config.StringVariable(name), "table_name": config.StringVariable(tableName), @@ -433,7 +442,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnObject_OwnershipPrivi TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createDatabaseRoleOutsideTerraform(t, name) }, @@ -446,7 +455,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnObject_OwnershipPrivi } func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnAll_InDatabase(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() databaseName := sdk.NewAccountObjectIdentifier(acc.TestDatabaseName).FullyQualifiedName() configVariables := config.Variables{ @@ -466,7 +475,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnAll_InDatabase(t *tes TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -497,7 +506,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnAll_InDatabase(t *tes } func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnFuture_InDatabase(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() databaseName := sdk.NewAccountObjectIdentifier(acc.TestDatabaseName).FullyQualifiedName() configVariables := config.Variables{ @@ -517,10 +526,10 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnFuture_InDatabase(t * TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { - PreConfig: func() { createDatabaseRoleOutsideTerraform(t, name) }, + PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToAccountRole/OnSchemaObject_OnFuture_InDatabase"), ConfigVariables: configVariables, Check: resource.ComposeTestCheckFunc( @@ -548,7 +557,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnSchemaObject_OnFuture_InDatabase(t * } func TestAcc_GrantPrivilegesToAccountRole_UpdatePrivileges(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() databaseName := sdk.NewAccountObjectIdentifier(acc.TestDatabaseName).FullyQualifiedName() configVariables := func(allPrivileges bool, privileges []sdk.AccountObjectPrivilege) config.Variables { @@ -576,7 +585,7 @@ func TestAcc_GrantPrivilegesToAccountRole_UpdatePrivileges(t *testing.T) { TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -637,7 +646,7 @@ func TestAcc_GrantPrivilegesToAccountRole_UpdatePrivileges(t *testing.T) { } func TestAcc_GrantPrivilegesToAccountRole_UpdatePrivileges_SnowflakeChecked(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name) schemaName := "test_database_role_schema_name" configVariables := func(allPrivileges bool, privileges []string, schemaName string) config.Variables { @@ -667,7 +676,7 @@ func TestAcc_GrantPrivilegesToAccountRole_UpdatePrivileges_SnowflakeChecked(t *t TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -723,7 +732,7 @@ func TestAcc_GrantPrivilegesToAccountRole_UpdatePrivileges_SnowflakeChecked(t *t } func TestAcc_GrantPrivilegesToAccountRole_AlwaysApply(t *testing.T) { - name := "test_account_role_name" + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() databaseName := sdk.NewAccountObjectIdentifier(acc.TestDatabaseName).FullyQualifiedName() configVariables := func(alwaysApply bool) config.Variables { @@ -742,7 +751,7 @@ func TestAcc_GrantPrivilegesToAccountRole_AlwaysApply(t *testing.T) { TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: testAccCheckAccountRolePrivilegesRevoked, + CheckDestroy: testAccCheckAccountRolePrivilegesRevoked(name), Steps: []resource.TestStep{ { PreConfig: func() { createAccountRoleOutsideTerraform(t, name) }, @@ -825,85 +834,61 @@ func createAccountRoleOutsideTerraform(t *testing.T, name string) { } } -func testAccCheckAccountRolePrivilegesRevoked(s *terraform.State) error { - db := acc.TestAccProvider.Meta().(*sql.DB) - client := sdk.NewClientFromDB(db) - for _, rs := range s.RootModule().Resources { - if rs.Type != "snowflake_grant_privileges_to_account_role" { - continue - } - ctx := context.Background() +func testAccCheckAccountRolePrivilegesRevoked(name string) func(*terraform.State) error { + return func(state *terraform.State) error { + db := acc.TestAccProvider.Meta().(*sql.DB) + client := sdk.NewClientFromDB(db) - id := sdk.NewAccountObjectIdentifierFromFullyQualifiedName(rs.Primary.Attributes["role_name"]) - grants, err := client.Grants.Show(ctx, &sdk.ShowGrantOptions{ - To: &sdk.ShowGrantsTo{ - Role: id, - }, - }) - if err != nil { - return err - } - var grantedPrivileges []string - for _, grant := range grants { - grantedPrivileges = append(grantedPrivileges, grant.Privilege) - } - if len(grantedPrivileges) > 0 { - return fmt.Errorf("account role (%s) is still granted, granted privileges %v", id.FullyQualifiedName(), grantedPrivileges) + defer func() { + err := client.Roles.Drop(context.Background(), sdk.NewDropRoleRequest(sdk.NewAccountObjectIdentifier(name))) + if err != nil { + log.Printf("failed to drop account role (%s), err = %s\n", name, err.Error()) + } + }() + + for _, rs := range state.RootModule().Resources { + if rs.Type != "snowflake_grant_privileges_to_account_role" { + continue + } + ctx := context.Background() + + id := sdk.NewAccountObjectIdentifierFromFullyQualifiedName(rs.Primary.Attributes["role_name"]) + grants, err := client.Grants.Show(ctx, &sdk.ShowGrantOptions{ + To: &sdk.ShowGrantsTo{ + Role: id, + }, + }) + if err != nil { + return err + } + var grantedPrivileges []string + for _, grant := range grants { + grantedPrivileges = append(grantedPrivileges, grant.Privilege) + } + if len(grantedPrivileges) > 0 { + return fmt.Errorf("account role (%s) is still granted, granted privileges %v", id.FullyQualifiedName(), grantedPrivileges) + } } + return nil } - return nil } -// queriedAccountRolePrivilegesEqualTo will check if all the privileges specified in the argument are granted in Snowflake. func queriedAccountRolePrivilegesEqualTo(roleName sdk.AccountObjectIdentifier, privileges ...string) func(s *terraform.State) error { - return func(s *terraform.State) error { - db := acc.TestAccProvider.Meta().(*sql.DB) - ctx := context.Background() - client := sdk.NewClientFromDB(db) - grants, err := client.Grants.Show(ctx, &sdk.ShowGrantOptions{ + return queriedPrivilegesEqualTo(func(client *sdk.Client, ctx context.Context) ([]sdk.Grant, error) { + return client.Grants.Show(ctx, &sdk.ShowGrantOptions{ To: &sdk.ShowGrantsTo{ Role: roleName, }, }) - if err != nil { - return err - } - for _, grant := range grants { - if !slices.Contains(privileges, grant.Privilege) { - return fmt.Errorf("grant not expected, grant: %v, not in %v", grants, privileges) - } - } - - return nil - } + }, privileges...) } -// queriedAccountRolePrivilegesContainAtLeast will check if all the privileges specified in the argument are granted in Snowflake. -// Any additional grants will be ignored. func queriedAccountRolePrivilegesContainAtLeast(roleName sdk.AccountObjectIdentifier, privileges ...string) func(s *terraform.State) error { - return func(s *terraform.State) error { - db := acc.TestAccProvider.Meta().(*sql.DB) - ctx := context.Background() - client := sdk.NewClientFromDB(db) - grants, err := client.Grants.Show(ctx, &sdk.ShowGrantOptions{ + return queriedPrivilegesContainAtLeast(func(client *sdk.Client, ctx context.Context) ([]sdk.Grant, error) { + return client.Grants.Show(ctx, &sdk.ShowGrantOptions{ To: &sdk.ShowGrantsTo{ Role: roleName, }, }) - if err != nil { - return err - } - var grantedPrivileges []string - for _, grant := range grants { - grantedPrivileges = append(grantedPrivileges, grant.Privilege) - } - notAllPrivilegesInGrantedPrivileges := slices.ContainsFunc(privileges, func(privilege string) bool { - return !slices.Contains(grantedPrivileges, privilege) - }) - if notAllPrivilegesInGrantedPrivileges { - return fmt.Errorf("not every privilege from the list: %v was found in grant privileges: %v, for database role name: %s", privileges, grantedPrivileges, roleName.FullyQualifiedName()) - } - - return nil - } + }, roleName, privileges...) } diff --git a/pkg/resources/grant_privileges_to_account_role_identifier_test.go b/pkg/resources/grant_privileges_to_account_role_identifier_test.go index fe32bf4f6a0..41905d47055 100644 --- a/pkg/resources/grant_privileges_to_account_role_identifier_test.go +++ b/pkg/resources/grant_privileges_to_account_role_identifier_test.go @@ -64,7 +64,7 @@ func TestParseGrantPrivilegesToAccountRoleId(t *testing.T) { }, }, { - Name: "grant database role on schema with schema name", + Name: "grant account role on schema with schema name", Identifier: `"account-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnSchema|"database-name"."schema-name"`, Expected: GrantPrivilegesToAccountRoleId{ RoleName: sdk.NewAccountObjectIdentifier("account-role"), @@ -78,7 +78,7 @@ func TestParseGrantPrivilegesToAccountRoleId(t *testing.T) { }, }, { - Name: "grant database role on all schemas in database", + Name: "grant account role on all schemas in database", Identifier: `"account-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnAllSchemasInDatabase|"database-name-123"`, Expected: GrantPrivilegesToAccountRoleId{ RoleName: sdk.NewAccountObjectIdentifier("account-role"), @@ -92,7 +92,7 @@ func TestParseGrantPrivilegesToAccountRoleId(t *testing.T) { }, }, { - Name: "grant database role on future schemas in database", + Name: "grant account role on future schemas in database", Identifier: `"account-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnFutureSchemasInDatabase|"database-name-123"`, Expected: GrantPrivilegesToAccountRoleId{ RoleName: sdk.NewAccountObjectIdentifier("account-role"), @@ -227,7 +227,7 @@ func TestParseGrantPrivilegesToAccountRoleId(t *testing.T) { }, }, { - Name: "validation: grant database role not enough parts", + Name: "validation: grant account role not enough parts", Identifier: `"database-name"."role-name"|false|false`, Error: "account role identifier should hold at least 5 parts", }, @@ -273,17 +273,17 @@ func TestParseGrantPrivilegesToAccountRoleId(t *testing.T) { }, { Name: "validation: grant account role empty privileges", - Identifier: `"database-name"."database-role"|false|false||OnAccount`, + Identifier: `"account-role"|false|false||OnAccount`, Error: `invalid Privileges value: , should be either a comma separated list of privileges or "ALL" / "ALL PRIVILEGES" for all privileges`, }, { Name: "validation: grant account role empty with grant option", - Identifier: `"database-name"."database-role"||false|ALL PRIVILEGES|OnAccount`, + Identifier: `"account-role"||false|ALL PRIVILEGES|OnAccount`, Error: `invalid WithGrantOption value: , should be either "true" or "false"`, }, { Name: "validation: grant account role empty always apply", - Identifier: `"database-name"."database-role"|false||ALL PRIVILEGES|OnAccount`, + Identifier: `"account-role"|false||ALL PRIVILEGES|OnAccount`, Error: `invalid AlwaysApply value: , should be either "true" or "false"`, }, { @@ -292,13 +292,14 @@ func TestParseGrantPrivilegesToAccountRoleId(t *testing.T) { Error: "invalid (empty) AccountRoleName value: , should be a fully qualified name of account object ", }, { - Name: "validation: account database role empty type", - Identifier: `"database-name"."database-role"|false|false|ALL PRIVILEGES||"on-database-name"`, + Name: "validation: account role empty type", + Identifier: `"account-role"|false|false|ALL PRIVILEGES||"on-database-name"`, Error: "invalid AccountRoleGrantKind: ", }, } for _, tt := range testCases { + tt := tt t.Run(tt.Name, func(t *testing.T) { id, err := ParseGrantPrivilegesToAccountRoleId(tt.Identifier) if tt.Error == "" { @@ -346,7 +347,7 @@ func TestGrantPrivilegesToAccountRoleIdString(t *testing.T) { Expected: `"account-role"|true|true|ALL|OnAccountObject|DATABASE|"database-name"`, }, { - Name: "grant database role on schema on schema", + Name: "grant account role on schema on schema", Identifier: GrantPrivilegesToAccountRoleId{ RoleName: sdk.NewAccountObjectIdentifier("account-role"), WithGrantOption: false, @@ -360,7 +361,7 @@ func TestGrantPrivilegesToAccountRoleIdString(t *testing.T) { Expected: `"account-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnSchema|"database-name"."schema-name"`, }, { - Name: "grant database role on all schemas in database", + Name: "grant account role on all schemas in database", Identifier: GrantPrivilegesToAccountRoleId{ RoleName: sdk.NewAccountObjectIdentifier("account-role"), WithGrantOption: false, @@ -374,7 +375,7 @@ func TestGrantPrivilegesToAccountRoleIdString(t *testing.T) { Expected: `"account-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnAllSchemasInDatabase|"database-name"`, }, { - Name: "grant database role on future schemas in database", + Name: "grant account role on future schemas in database", Identifier: GrantPrivilegesToAccountRoleId{ RoleName: sdk.NewAccountObjectIdentifier("account-role"), WithGrantOption: false, @@ -388,7 +389,7 @@ func TestGrantPrivilegesToAccountRoleIdString(t *testing.T) { Expected: `"account-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnFutureSchemasInDatabase|"database-name"`, }, { - Name: "grant database role on schema object on object", + Name: "grant account role on schema object on object", Identifier: GrantPrivilegesToAccountRoleId{ RoleName: sdk.NewAccountObjectIdentifier("account-role"), WithGrantOption: false, @@ -405,7 +406,7 @@ func TestGrantPrivilegesToAccountRoleIdString(t *testing.T) { Expected: `"account-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchemaObject|OnObject|TABLE|"database-name"."schema-name"."table-name"`, }, { - Name: "grant database role on schema object on all tables in database", + Name: "grant account role on schema object on all tables in database", Identifier: GrantPrivilegesToAccountRoleId{ RoleName: sdk.NewAccountObjectIdentifier("account-role"), WithGrantOption: false, @@ -423,7 +424,7 @@ func TestGrantPrivilegesToAccountRoleIdString(t *testing.T) { Expected: `"account-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchemaObject|OnAll|TABLES|InDatabase|"database-name"`, }, { - Name: "grant database role on schema object on all tables in schema", + Name: "grant account role on schema object on all tables in schema", Identifier: GrantPrivilegesToAccountRoleId{ RoleName: sdk.NewAccountObjectIdentifier("account-role"), WithGrantOption: false, @@ -443,6 +444,7 @@ func TestGrantPrivilegesToAccountRoleIdString(t *testing.T) { } for _, tt := range testCases { + tt := tt t.Run(tt.Name, func(t *testing.T) { assert.Equal(t, tt.Expected, tt.Identifier.String()) }) diff --git a/pkg/resources/grant_privileges_to_database_role.go b/pkg/resources/grant_privileges_to_database_role.go index cf2d3ac8ffc..9aa096130bb 100644 --- a/pkg/resources/grant_privileges_to_database_role.go +++ b/pkg/resources/grant_privileges_to_database_role.go @@ -62,7 +62,7 @@ var grantPrivilegesToDatabaseRoleSchema = 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_database": { Type: schema.TypeString, @@ -593,6 +593,16 @@ func ReadGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.ResourceDa } 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{ diff --git a/pkg/resources/grant_privileges_to_database_role_acceptance_test.go b/pkg/resources/grant_privileges_to_database_role_acceptance_test.go index aa737e7993a..2a31a1a7c62 100644 --- a/pkg/resources/grant_privileges_to_database_role_acceptance_test.go +++ b/pkg/resources/grant_privileges_to_database_role_acceptance_test.go @@ -4,9 +4,7 @@ import ( "context" "database/sql" "fmt" - "log" "regexp" - "slices" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" @@ -610,7 +608,7 @@ func TestAcc_GrantPrivilegesToDatabaseRole_UpdatePrivileges_SnowflakeChecked(t * sdk.AccountObjectPrivilegeCreateSchema.String(), sdk.AccountObjectPrivilegeModify.String(), }, ""), - Check: queriedPrivilegesEqualTo( + Check: queriedPrivilegesToDatabaseRoleEqualTo( databaseRoleName, sdk.AccountObjectPrivilegeCreateSchema.String(), sdk.AccountObjectPrivilegeModify.String(), @@ -619,7 +617,7 @@ func TestAcc_GrantPrivilegesToDatabaseRole_UpdatePrivileges_SnowflakeChecked(t * { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToDatabaseRole/UpdatePrivileges_SnowflakeChecked/all_privileges"), ConfigVariables: configVariables(true, []string{}, ""), - Check: queriedPrivilegesContainAtLeast( + Check: queriedPrivilegesToDatabaseRoleContainAtLeast( databaseRoleName, sdk.AccountObjectPrivilegeCreateDatabaseRole.String(), sdk.AccountObjectPrivilegeCreateSchema.String(), @@ -634,7 +632,7 @@ func TestAcc_GrantPrivilegesToDatabaseRole_UpdatePrivileges_SnowflakeChecked(t * sdk.AccountObjectPrivilegeModify.String(), sdk.AccountObjectPrivilegeMonitor.String(), }, ""), - Check: queriedPrivilegesEqualTo( + Check: queriedPrivilegesToDatabaseRoleEqualTo( databaseRoleName, sdk.AccountObjectPrivilegeModify.String(), sdk.AccountObjectPrivilegeMonitor.String(), @@ -646,7 +644,7 @@ func TestAcc_GrantPrivilegesToDatabaseRole_UpdatePrivileges_SnowflakeChecked(t * sdk.SchemaPrivilegeCreateTask.String(), sdk.SchemaPrivilegeCreateExternalTable.String(), }, schemaName), - Check: queriedPrivilegesEqualTo( + Check: queriedPrivilegesToDatabaseRoleEqualTo( databaseRoleName, sdk.SchemaPrivilegeCreateTask.String(), sdk.SchemaPrivilegeCreateExternalTable.String(), @@ -760,63 +758,24 @@ func createDatabaseRoleOutsideTerraform(t *testing.T, name string) { } } -// queriedPrivilegesEqualTo will check if all the privileges specified in the argument are granted in Snowflake. -// Any additional grants (other than usage and ownership) will be treated as an error. -func queriedPrivilegesEqualTo(databaseRoleName sdk.DatabaseObjectIdentifier, privileges ...string) func(s *terraform.State) error { - return func(s *terraform.State) error { - db := acc.TestAccProvider.Meta().(*sql.DB) - ctx := context.Background() - client := sdk.NewClientFromDB(db) - grants, err := client.Grants.Show(ctx, &sdk.ShowGrantOptions{ +func queriedPrivilegesToDatabaseRoleEqualTo(databaseRoleName sdk.DatabaseObjectIdentifier, privileges ...string) func(s *terraform.State) error { + return queriedPrivilegesEqualTo(func(client *sdk.Client, ctx context.Context) ([]sdk.Grant, error) { + return client.Grants.Show(ctx, &sdk.ShowGrantOptions{ To: &sdk.ShowGrantsTo{ DatabaseRole: databaseRoleName, }, }) - if err != nil { - return err - } - for _, grant := range grants { - if grant.Privilege == "USAGE" || grant.Privilege == "OWNERSHIP" { - log.Printf("Skipping check for %s privilege as its one of the privileges that are implicitly granted by Snowflake", grant.Privilege) - continue - } - if !slices.Contains(privileges, grant.Privilege) { - return fmt.Errorf("grant not expected, grant: %v, not in %v", grants, privileges) - } - } - - return nil - } + }, privileges...) } -// queriedPrivilegesContainAtLeast will check if all the privileges specified in the argument are granted in Snowflake. -// Any additional grants will be ignored. -func queriedPrivilegesContainAtLeast(databaseRoleName sdk.DatabaseObjectIdentifier, privileges ...string) func(s *terraform.State) error { - return func(s *terraform.State) error { - db := acc.TestAccProvider.Meta().(*sql.DB) - ctx := context.Background() - client := sdk.NewClientFromDB(db) - grants, err := client.Grants.Show(ctx, &sdk.ShowGrantOptions{ +func queriedPrivilegesToDatabaseRoleContainAtLeast(databaseRoleName sdk.DatabaseObjectIdentifier, privileges ...string) func(s *terraform.State) error { + return queriedPrivilegesEqualTo(func(client *sdk.Client, ctx context.Context) ([]sdk.Grant, error) { + return client.Grants.Show(ctx, &sdk.ShowGrantOptions{ To: &sdk.ShowGrantsTo{ DatabaseRole: databaseRoleName, }, }) - if err != nil { - return err - } - var grantedPrivileges []string - for _, grant := range grants { - grantedPrivileges = append(grantedPrivileges, grant.Privilege) - } - notAllPrivilegesInGrantedPrivileges := slices.ContainsFunc(privileges, func(privilege string) bool { - return !slices.Contains(grantedPrivileges, privilege) - }) - if notAllPrivilegesInGrantedPrivileges { - return fmt.Errorf("not every privilege from the list: %v was found in grant privileges: %v, for database role name: %s", privileges, grantedPrivileges, databaseRoleName.FullyQualifiedName()) - } - - return nil - } + }, privileges...) } func testAccCheckDatabaseRolePrivilegesRevoked(s *terraform.State) error { diff --git a/pkg/resources/grant_privileges_to_database_role_identifier_test.go b/pkg/resources/grant_privileges_to_database_role_identifier_test.go index 96253a8de58..ea8cd4404b5 100644 --- a/pkg/resources/grant_privileges_to_database_role_identifier_test.go +++ b/pkg/resources/grant_privileges_to_database_role_identifier_test.go @@ -291,6 +291,7 @@ func TestParseGrantPrivilegesToDatabaseRoleId(t *testing.T) { } for _, tt := range testCases { + tt := tt t.Run(tt.Name, func(t *testing.T) { id, err := ParseGrantPrivilegesToDatabaseRoleId(tt.Identifier) if tt.Error == "" { @@ -422,6 +423,7 @@ func TestGrantPrivilegesToDatabaseRoleIdString(t *testing.T) { } for _, tt := range testCases { + tt := tt t.Run(tt.Name, func(t *testing.T) { assert.Equal(t, tt.Expected, tt.Identifier.String()) }) diff --git a/pkg/resources/helpers_test.go b/pkg/resources/helpers_test.go index 78c07827b66..28b99d138f8 100644 --- a/pkg/resources/helpers_test.go +++ b/pkg/resources/helpers_test.go @@ -1,8 +1,16 @@ package resources_test import ( + "context" + "database/sql" + "fmt" + "slices" "testing" + acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/stretchr/testify/assert" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -470,3 +478,49 @@ func tagGrant(t *testing.T, id string, params map[string]interface{}) *schema.Re d.SetId(id) return d } + +// queriedAccountRolePrivilegesEqualTo will check if all the privileges specified in the argument are granted in Snowflake. +func queriedPrivilegesEqualTo(query func(client *sdk.Client, ctx context.Context) ([]sdk.Grant, error), privileges ...string) func(s *terraform.State) error { + return func(s *terraform.State) error { + db := acc.TestAccProvider.Meta().(*sql.DB) + ctx := context.Background() + client := sdk.NewClientFromDB(db) + grants, err := query(client, ctx) + if err != nil { + return err + } + for _, grant := range grants { + if !slices.Contains(privileges, grant.Privilege) { + return fmt.Errorf("grant not expected, grant: %v, not in %v", grants, privileges) + } + } + + return nil + } +} + +// queriedAccountRolePrivilegesContainAtLeast will check if all the privileges specified in the argument are granted in Snowflake. +// Any additional grants will be ignored. +func queriedPrivilegesContainAtLeast(query func(client *sdk.Client, ctx context.Context) ([]sdk.Grant, error), roleName sdk.ObjectIdentifier, privileges ...string) func(s *terraform.State) error { + return func(s *terraform.State) error { + db := acc.TestAccProvider.Meta().(*sql.DB) + ctx := context.Background() + client := sdk.NewClientFromDB(db) + grants, err := query(client, ctx) + if err != nil { + return err + } + var grantedPrivileges []string + for _, grant := range grants { + grantedPrivileges = append(grantedPrivileges, grant.Privilege) + } + notAllPrivilegesInGrantedPrivileges := slices.ContainsFunc(privileges, func(privilege string) bool { + return !slices.Contains(grantedPrivileges, privilege) + }) + if notAllPrivilegesInGrantedPrivileges { + return fmt.Errorf("not every privilege from the list: %v was found in grant privileges: %v, for role name: %s", privileges, grantedPrivileges, roleName.FullyQualifiedName()) + } + + return nil + } +} diff --git a/pkg/resources/testdata/TestAcc_GrantPrivilegesToAccountRole/OnAccountObject_InfinitePlan/test.tf b/pkg/resources/testdata/TestAcc_GrantPrivilegesToAccountRole/OnAccountObject_InfinitePlan/test.tf index 132795d1700..181a311d0eb 100644 --- a/pkg/resources/testdata/TestAcc_GrantPrivilegesToAccountRole/OnAccountObject_InfinitePlan/test.tf +++ b/pkg/resources/testdata/TestAcc_GrantPrivilegesToAccountRole/OnAccountObject_InfinitePlan/test.tf @@ -1,4 +1,4 @@ -resource "snowflake_grant_privileges_to_role" "test" { +resource "snowflake_grant_privileges_to_account_role" "test" { privileges = ["CREATE SCHEMA"] role_name = "\"${var.name}\"" on_account_object { diff --git a/pkg/sdk/grants.go b/pkg/sdk/grants.go index 11059a7ad1b..7ceecb9b362 100644 --- a/pkg/sdk/grants.go +++ b/pkg/sdk/grants.go @@ -48,6 +48,7 @@ type GrantOnAccountObject struct { User *AccountObjectIdentifier `ddl:"identifier" sql:"USER"` ResourceMonitor *AccountObjectIdentifier `ddl:"identifier" sql:"RESOURCE MONITOR"` Warehouse *AccountObjectIdentifier `ddl:"identifier" sql:"WAREHOUSE"` + ComputePool *AccountObjectIdentifier `ddl:"identifier" sql:"COMPUTE POOL"` Database *AccountObjectIdentifier `ddl:"identifier" sql:"DATABASE"` Integration *AccountObjectIdentifier `ddl:"identifier" sql:"INTEGRATION"` Connection *AccountObjectIdentifier `ddl:"identifier" sql:"CONNECTION"` diff --git a/pkg/sdk/grants_test.go b/pkg/sdk/grants_test.go index 8aecb1cb7a8..d1a129d0f25 100644 --- a/pkg/sdk/grants_test.go +++ b/pkg/sdk/grants_test.go @@ -51,6 +51,65 @@ func TestGrantPrivilegesToAccountRole(t *testing.T) { assertOptsValidAndSQLEquals(t, opts, `GRANT ALL PRIVILEGES ON EXTERNAL VOLUME "ex volume" TO ROLE "role1"`) }) + t.Run("on account object - connection", func(t *testing.T) { + opts := &GrantPrivilegesToAccountRoleOptions{ + privileges: &AccountRoleGrantPrivileges{ + AllPrivileges: Bool(true), + }, + on: &AccountRoleGrantOn{ + AccountObject: &GrantOnAccountObject{ + Connection: Pointer(NewAccountObjectIdentifier("connection")), + }, + }, + accountRole: NewAccountObjectIdentifier("role1"), + } + assertOptsValidAndSQLEquals(t, opts, `GRANT ALL PRIVILEGES ON CONNECTION "connection" TO ROLE "role1"`) + }) + + t.Run("on account object - compute pool", func(t *testing.T) { + opts := &GrantPrivilegesToAccountRoleOptions{ + privileges: &AccountRoleGrantPrivileges{ + AllPrivileges: Bool(true), + }, + on: &AccountRoleGrantOn{ + AccountObject: &GrantOnAccountObject{ + ComputePool: Pointer(NewAccountObjectIdentifier("compute pool")), + }, + }, + accountRole: NewAccountObjectIdentifier("role1"), + } + assertOptsValidAndSQLEquals(t, opts, `GRANT ALL PRIVILEGES ON COMPUTE POOL "compute pool" TO ROLE "role1"`) + }) + + t.Run("on account object - exactly one of validation", func(t *testing.T) { + opts := &GrantPrivilegesToAccountRoleOptions{ + privileges: &AccountRoleGrantPrivileges{ + AllPrivileges: Bool(true), + }, + on: &AccountRoleGrantOn{ + AccountObject: &GrantOnAccountObject{ + Database: Pointer(NewAccountObjectIdentifier("database")), + Connection: Pointer(NewAccountObjectIdentifier("connection")), + }, + }, + accountRole: NewAccountObjectIdentifier("role1"), + } + assertOptsInvalid(t, opts, errExactlyOneOf("GrantOnAccountObject", "User", "ResourceMonitor", "Warehouse", "ComputePool", "Database", "Integration", "Connection", "FailoverGroup", "ReplicationGroup", "ExternalVolume")) + }) + + t.Run("on account object - exactly one of validation - empty options", func(t *testing.T) { + opts := &GrantPrivilegesToAccountRoleOptions{ + privileges: &AccountRoleGrantPrivileges{ + AllPrivileges: Bool(true), + }, + on: &AccountRoleGrantOn{ + AccountObject: &GrantOnAccountObject{}, + }, + accountRole: NewAccountObjectIdentifier("role1"), + } + assertOptsInvalid(t, opts, errExactlyOneOf("GrantOnAccountObject", "User", "ResourceMonitor", "Warehouse", "ComputePool", "Database", "Integration", "Connection", "FailoverGroup", "ReplicationGroup", "ExternalVolume")) + }) + t.Run("on schema", func(t *testing.T) { opts := &GrantPrivilegesToAccountRoleOptions{ privileges: &AccountRoleGrantPrivileges{ @@ -194,6 +253,22 @@ func TestRevokePrivilegesFromAccountRole(t *testing.T) { } assertOptsValidAndSQLEquals(t, opts, `REVOKE CREATE DATABASE ROLE, MODIFY ON DATABASE "db1" FROM ROLE "role1"`) }) + + t.Run("on account object - connection", func(t *testing.T) { + opts := &RevokePrivilegesFromAccountRoleOptions{ + privileges: &AccountRoleGrantPrivileges{ + AccountObjectPrivileges: []AccountObjectPrivilege{AccountObjectPrivilegeCreateDatabaseRole, AccountObjectPrivilegeModify}, + }, + on: &AccountRoleGrantOn{ + AccountObject: &GrantOnAccountObject{ + Connection: Pointer(NewAccountObjectIdentifier("connection")), + }, + }, + accountRole: NewAccountObjectIdentifier("role1"), + } + assertOptsValidAndSQLEquals(t, opts, `REVOKE CREATE DATABASE ROLE, MODIFY ON CONNECTION "connection" FROM ROLE "role1"`) + }) + t.Run("on schema", func(t *testing.T) { opts := &RevokePrivilegesFromAccountRoleOptions{ privileges: &AccountRoleGrantPrivileges{ diff --git a/pkg/sdk/grants_validations.go b/pkg/sdk/grants_validations.go index 2537ea2bf8e..39830804720 100644 --- a/pkg/sdk/grants_validations.go +++ b/pkg/sdk/grants_validations.go @@ -70,8 +70,8 @@ func (v *AccountRoleGrantOn) validate() error { } func (v *GrantOnAccountObject) validate() error { - if !exactlyOneValueSet(v.User, v.ResourceMonitor, v.Warehouse, v.Database, v.Integration, v.Connection, v.FailoverGroup, v.ReplicationGroup, v.ExternalVolume) { - return errExactlyOneOf("GrantOnAccountObject", "User", "ResourceMonitor", "Warehouse", "Database", "Integration", "Connection", "FailoverGroup", "ReplicationGroup", "ExternalVolume") + if !exactlyOneValueSet(v.User, v.ResourceMonitor, v.Warehouse, v.ComputePool, v.Database, v.Integration, v.Connection, v.FailoverGroup, v.ReplicationGroup, v.ExternalVolume) { + return errExactlyOneOf("GrantOnAccountObject", "User", "ResourceMonitor", "Warehouse", "ComputePool", "Database", "Integration", "Connection", "FailoverGroup", "ReplicationGroup", "ExternalVolume") } return nil }