wip

Snowflake-Labs · Mar 22, 2024 · ea19b61 · ea19b61
1 parent 4549720
commit ea19b61
Show file tree

Hide file tree

Showing 4 changed files with 204 additions and 66 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)
+	}
+}