From ea19b61e5dc85815508093aec7b88c83332891e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 22 Mar 2024 14:09:48 +0100 Subject: [PATCH] wip --- .../grant_privileges_to_account_role.go | 63 +++----- ...vileges_to_account_role_acceptance_test.go | 18 +-- .../grant_privileges_to_database_role.go | 41 +++-- ...ileges_to_database_role_acceptance_test.go | 148 ++++++++++++++++++ 4 files changed, 204 insertions(+), 66 deletions(-) diff --git a/pkg/resources/grant_privileges_to_account_role.go b/pkg/resources/grant_privileges_to_account_role.go index ee148bdb2c2..911340fee76 100644 --- a/pkg/resources/grant_privileges_to_account_role.go +++ b/pkg/resources/grant_privileges_to_account_role.go @@ -446,6 +446,10 @@ func UpdateGrantPrivilegesToAccountRole(ctx context.Context, d *schema.ResourceD } logging.DebugLogger.Printf("[DEBUG] Parsed identifier to %s", id.String()) + if d.HasChange("with_grant_option") { + id.WithGrantOption = d.Get("with_grant_option").(bool) + } + // handle all_privileges -> privileges change (revoke all privileges) if d.HasChange("all_privileges") { _, allPrivileges := d.GetChange("all_privileges") @@ -510,39 +514,21 @@ func UpdateGrantPrivilegesToAccountRole(ctx context.Context, d *schema.ResourceD if len(privilegesToAdd) > 0 { logging.DebugLogger.Printf("[DEBUG] Granting privileges: %v", privilegesToAdd) + privilegesToGrant := getAccountRolePrivileges( + false, + privilegesToAdd, + id.Kind == OnAccountAccountRoleGrantKind, + id.Kind == OnAccountObjectAccountRoleGrantKind, + id.Kind == OnSchemaAccountRoleGrantKind, + id.Kind == OnSchemaObjectAccountRoleGrantKind, + ) + if id.WithGrantOption { - err = client.Grants.GrantPrivilegesToAccountRole( - ctx, - getAccountRolePrivileges( - false, - privilegesToAdd, - id.Kind == OnAccountAccountRoleGrantKind, - id.Kind == OnAccountObjectAccountRoleGrantKind, - id.Kind == OnSchemaAccountRoleGrantKind, - id.Kind == OnSchemaObjectAccountRoleGrantKind, - ), - grantOn, - id.RoleName, - &sdk.GrantPrivilegesToAccountRoleOptions{ - WithGrantOption: sdk.Bool(true), - }, - ) + err = client.Grants.GrantPrivilegesToAccountRole(ctx, privilegesToGrant, grantOn, id.RoleName, &sdk.GrantPrivilegesToAccountRoleOptions{ + WithGrantOption: sdk.Bool(true), + }) } else { - err = client.Grants.RevokePrivilegesFromAccountRole( - ctx, - getAccountRolePrivileges( - false, - privilegesToAdd, - id.Kind == OnAccountAccountRoleGrantKind, - id.Kind == OnAccountObjectAccountRoleGrantKind, - id.Kind == OnSchemaAccountRoleGrantKind, - id.Kind == OnSchemaObjectAccountRoleGrantKind, - ), - grantOn, - id.RoleName, - new(sdk.RevokePrivilegesFromAccountRoleOptions), - ) - if err != nil { + if err = client.Grants.RevokePrivilegesFromAccountRole(ctx, privilegesToGrant, grantOn, id.RoleName, new(sdk.RevokePrivilegesFromAccountRoleOptions)); err != nil { return diag.Diagnostics{ diag.Diagnostic{ Severity: diag.Error, @@ -552,20 +538,7 @@ func UpdateGrantPrivilegesToAccountRole(ctx context.Context, d *schema.ResourceD } } - err = client.Grants.GrantPrivilegesToAccountRole( - ctx, - getAccountRolePrivileges( - false, - privilegesToAdd, - id.Kind == OnAccountAccountRoleGrantKind, - id.Kind == OnAccountObjectAccountRoleGrantKind, - id.Kind == OnSchemaAccountRoleGrantKind, - id.Kind == OnSchemaObjectAccountRoleGrantKind, - ), - grantOn, - id.RoleName, - new(sdk.GrantPrivilegesToAccountRoleOptions), - ) + err = client.Grants.GrantPrivilegesToAccountRole(ctx, privilegesToGrant, grantOn, id.RoleName, new(sdk.GrantPrivilegesToAccountRoleOptions)) } if err != nil { return diag.Diagnostics{ 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 3f906cafd26..eca421b9f80 100644 --- a/pkg/resources/grant_privileges_to_account_role_acceptance_test.go +++ b/pkg/resources/grant_privileges_to_account_role_acceptance_test.go @@ -1060,7 +1060,7 @@ func TestAcc_GrantPrivilegesToAccountRole_OnExternalVolume(t *testing.T) { } // proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2459 is fixed -func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerraform_WithoutGrantOptions(t *testing.T) { +func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerraform_WithGrantOptions(t *testing.T) { name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() tableName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) @@ -1073,7 +1073,7 @@ func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerrafo ), "database": config.StringVariable(acc.TestDatabaseName), "schema": config.StringVariable(acc.TestSchemaName), - "with_grant_option": config.BoolVariable(false), + "with_grant_option": config.BoolVariable(true), } resource.Test(t, resource.TestCase{ @@ -1096,11 +1096,11 @@ func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerrafo }, { PreConfig: func() { - revokeAndGrantPrivilegesOnTableToAccount( + revokeAndGrantPrivilegesOnTableToAccountRole( t, name, sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, acc.TestSchemaName, tableName), []sdk.SchemaObjectPrivilege{sdk.SchemaObjectPrivilegeTruncate}, - true, + false, ) }, ConfigPlanChecks: resource.ConfigPlanChecks{ @@ -1116,7 +1116,7 @@ func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerrafo } // proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2459 is fixed -func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerraform_WithGrantOptions(t *testing.T) { +func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerraform_WithoutGrantOptions(t *testing.T) { name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) roleName := sdk.NewAccountObjectIdentifier(name).FullyQualifiedName() tableName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) @@ -1129,7 +1129,7 @@ func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerrafo ), "database": config.StringVariable(acc.TestDatabaseName), "schema": config.StringVariable(acc.TestSchemaName), - "with_grant_option": config.BoolVariable(true), + "with_grant_option": config.BoolVariable(false), } resource.Test(t, resource.TestCase{ @@ -1152,11 +1152,11 @@ func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerrafo }, { PreConfig: func() { - revokeAndGrantPrivilegesOnTableToAccount( + revokeAndGrantPrivilegesOnTableToAccountRole( t, name, sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, acc.TestSchemaName, tableName), []sdk.SchemaObjectPrivilege{sdk.SchemaObjectPrivilegeTruncate}, - false, + true, ) }, ConfigPlanChecks: resource.ConfigPlanChecks{ @@ -1171,7 +1171,7 @@ func TestAcc_GrantPrivilegesToAccountRole_ChangeWithGrantOptionsOutsideOfTerrafo }) } -func revokeAndGrantPrivilegesOnTableToAccount( +func revokeAndGrantPrivilegesOnTableToAccountRole( t *testing.T, accountRoleName string, tableName sdk.SchemaObjectIdentifier, diff --git a/pkg/resources/grant_privileges_to_database_role.go b/pkg/resources/grant_privileges_to_database_role.go index 6608aa10d11..52a7ee7fdd6 100644 --- a/pkg/resources/grant_privileges_to_database_role.go +++ b/pkg/resources/grant_privileges_to_database_role.go @@ -374,6 +374,10 @@ func UpdateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource } } + if d.HasChange("with_grant_option") { + id.WithGrantOption = d.Get("with_grant_option").(bool) + } + // handle all_privileges -> privileges change (revoke all privileges) if d.HasChange("all_privileges") { _, allPrivileges := d.GetChange("all_privileges") @@ -434,19 +438,32 @@ func UpdateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource grantOn := getDatabaseRoleGrantOn(d) if len(privilegesToAdd) > 0 { - err = client.Grants.GrantPrivilegesToDatabaseRole( - ctx, - getDatabaseRolePrivileges( - false, - privilegesToAdd, - id.Kind == OnDatabaseDatabaseRoleGrantKind, - id.Kind == OnSchemaDatabaseRoleGrantKind, - id.Kind == OnSchemaObjectDatabaseRoleGrantKind, - ), - grantOn, - id.DatabaseRoleName, - new(sdk.GrantPrivilegesToDatabaseRoleOptions), + privilegesToGrant := getDatabaseRolePrivileges( + false, + privilegesToAdd, + id.Kind == OnDatabaseDatabaseRoleGrantKind, + id.Kind == OnSchemaDatabaseRoleGrantKind, + id.Kind == OnSchemaObjectDatabaseRoleGrantKind, ) + + if id.WithGrantOption { + err = client.Grants.GrantPrivilegesToDatabaseRole(ctx, privilegesToGrant, grantOn, id.DatabaseRoleName, &sdk.GrantPrivilegesToDatabaseRoleOptions{ + WithGrantOption: sdk.Bool(true), + }) + } else { + if err = client.Grants.RevokePrivilegesFromDatabaseRole(ctx, privilegesToGrant, grantOn, id.DatabaseRoleName, new(sdk.RevokePrivilegesFromDatabaseRoleOptions)); err != nil { + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to revoke privileges to add", + Detail: fmt.Sprintf("Id: %s\nPrivileges to add: %v\nError: %s", d.Id(), privilegesToAdd, err.Error()), + }, + } + } + + err = client.Grants.GrantPrivilegesToDatabaseRole(ctx, privilegesToGrant, grantOn, id.DatabaseRoleName, new(sdk.GrantPrivilegesToDatabaseRoleOptions)) + } + if err != nil { return diag.Diagnostics{ diag.Diagnostic{ 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 f3c94974282..de437083f87 100644 --- a/pkg/resources/grant_privileges_to_database_role_acceptance_test.go +++ b/pkg/resources/grant_privileges_to_database_role_acceptance_test.go @@ -799,6 +799,108 @@ func TestAcc_GrantPrivilegesToDatabaseRole_AlwaysApply(t *testing.T) { }) } +// proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2459 is fixed +func TestAcc_GrantPrivilegesToDatabaseRole_ChangeWithGrantOptionsOutsideOfTerraform_WithGrantOptions(t *testing.T) { + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + configVariables := config.Variables{ + "name": config.StringVariable(name), + "privileges": config.ListVariable( + config.StringVariable(string(sdk.AccountObjectPrivilegeCreateSchema)), + ), + "database": config.StringVariable(acc.TestDatabaseName), + "with_grant_option": config.BoolVariable(true), + } + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: testAccCheckDatabaseRolePrivilegesRevoked, + Steps: []resource.TestStep{ + { + PreConfig: func() { createDatabaseRoleOutsideTerraform(t, name) }, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToDatabaseRole/OnDatabase"), + ConfigVariables: configVariables, + }, + { + PreConfig: func() { + revokeAndGrantPrivilegesOnDatabaseToDatabaseRole( + t, sdk.NewDatabaseObjectIdentifier(acc.TestDatabaseName, name), + acc.TestDatabaseName, + []sdk.AccountObjectPrivilege{sdk.AccountObjectPrivilegeCreateSchema}, + false, + ) + }, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToDatabaseRole/OnDatabase"), + ConfigVariables: configVariables, + }, + }, + }) +} + +// proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2459 is fixed +func TestAcc_GrantPrivilegesToDatabaseRole_ChangeWithGrantOptionsOutsideOfTerraform_WithoutGrantOptions(t *testing.T) { + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + configVariables := config.Variables{ + "name": config.StringVariable(name), + "privileges": config.ListVariable( + config.StringVariable(string(sdk.AccountObjectPrivilegeCreateSchema)), + ), + "database": config.StringVariable(acc.TestDatabaseName), + "with_grant_option": config.BoolVariable(false), + } + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: testAccCheckDatabaseRolePrivilegesRevoked, + Steps: []resource.TestStep{ + { + PreConfig: func() { createDatabaseRoleOutsideTerraform(t, name) }, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToDatabaseRole/OnDatabase"), + ConfigVariables: configVariables, + }, + { + PreConfig: func() { + revokeAndGrantPrivilegesOnDatabaseToDatabaseRole( + t, sdk.NewDatabaseObjectIdentifier(acc.TestDatabaseName, name), + acc.TestDatabaseName, + []sdk.AccountObjectPrivilege{sdk.AccountObjectPrivilegeCreateSchema}, + true, + ) + }, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToDatabaseRole/OnDatabase"), + ConfigVariables: configVariables, + }, + }, + }) +} + func createDatabaseRoleOutsideTerraform(t *testing.T, name string) { t.Helper() client, err := sdk.NewDefaultClient() @@ -862,3 +964,49 @@ func testAccCheckDatabaseRolePrivilegesRevoked(s *terraform.State) error { } return nil } + +func revokeAndGrantPrivilegesOnDatabaseToDatabaseRole( + t *testing.T, + databaseRoleName sdk.DatabaseObjectIdentifier, + databaseName string, + privileges []sdk.AccountObjectPrivilege, + withGrantOption bool, +) { + t.Helper() + client, err := sdk.NewDefaultClient() + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + err = client.Grants.RevokePrivilegesFromDatabaseRole( + ctx, + &sdk.DatabaseRoleGrantPrivileges{ + DatabasePrivileges: privileges, + }, + &sdk.DatabaseRoleGrantOn{ + Database: sdk.Pointer(sdk.NewAccountObjectIdentifier(databaseName)), + }, + databaseRoleName, + new(sdk.RevokePrivilegesFromDatabaseRoleOptions), + ) + if err != nil { + t.Fatal(err) + } + + err = client.Grants.GrantPrivilegesToDatabaseRole( + ctx, + &sdk.DatabaseRoleGrantPrivileges{ + DatabasePrivileges: privileges, + }, + &sdk.DatabaseRoleGrantOn{ + Database: sdk.Pointer(sdk.NewAccountObjectIdentifier(databaseName)), + }, + databaseRoleName, + &sdk.GrantPrivilegesToDatabaseRoleOptions{ + WithGrantOption: sdk.Bool(withGrantOption), + }, + ) + if err != nil { + t.Fatal(err) + } +}