fix: fixed enable_multiple_grants behaviour for role_grants (#1816)

Previously enable_multiple_grants parameter for the snowflake_role_grants resource did not revoke any permissions given outside of the terraform. My fix actually enables such behavior.
Since everyone who used it previously had resource acting as the enable_multiple_grants were set to true, I suggest we change the default for this parameter to true as well, in order not to break existing configurations. Only the ones who set the parameter explicitly will see the change

docs/resources/role_grants.md

@@ -3,11 +3,12 @@
 page_title: "snowflake_role_grants Resource - terraform-provider-snowflake"
 subcategory: ""
 description: |-
-  
+
 ---
 
 # snowflake_role_grants (Resource)
 
+!> **WARNING:** If parameter `enable_multiple_grants` is set to `false`, the snowflake_role_grants resource creates **exclusive** grants for the selected role. Creating multiple resources for the same role with `enable_multiple_grants = false` will lead to permanent changes in plan.
 
 
 ## Example Usage
@@ -55,7 +56,7 @@ resource "snowflake_role_grants" "grants" {
 
 ### Optional
 
-- `enable_multiple_grants` (Boolean) When this is set to true, multiple grants of the same type can be created. This will cause Terraform to not revoke grants applied to roles and objects outside Terraform.
+- `enable_multiple_grants` (Boolean) When this is set to false, multiple grants of the same type cannot be created. This will cause Terraform to revoke grants applied to roles and objects outside Terraform.
 - `roles` (Set of String) Grants role to this specified role.
 - `users` (Set of String) Grants role to this specified user.
 

pkg/resources/role_grants.go

@@ -49,8 +49,8 @@ func RoleGrants() *schema.Resource {
 			"enable_multiple_grants": {
 				Type:        schema.TypeBool,
 				Optional:    true,
-				Description: "When this is set to true, multiple grants of the same type can be created. This will cause Terraform to not revoke grants applied to roles and objects outside Terraform.",
-				Default:     false,
+				Description: "When this is set to false, multiple grants of the same type cannot be created. This will cause Terraform to revoke grants applied to roles and objects outside Terraform.",
+				Default:     true,
 			},
 		},
 
@@ -147,16 +147,24 @@ func ReadRoleGrants(d *schema.ResourceData, meta interface{}) error {
 	for _, grant := range grants {
 		switch grant.GrantedTo.String {
 		case "ROLE":
-			for _, tfRole := range d.Get("roles").(*schema.Set).List() {
-				if tfRole == grant.GranteeName.String {
-					roles = append(roles, grant.GranteeName.String)
+			if d.Get("enable_multiple_grants").(bool) {
+				for _, tfRole := range d.Get("roles").(*schema.Set).List() {
+					if tfRole == grant.GranteeName.String {
+						roles = append(roles, grant.GranteeName.String)
+					}
 				}
+			} else {
+				roles = append(roles, grant.GranteeName.String)
 			}
 		case "USER":
-			for _, tfUser := range d.Get("users").(*schema.Set).List() {
-				if tfUser == grant.GranteeName.String {
-					users = append(users, grant.GranteeName.String)
+			if d.Get("enable_multiple_grants").(bool) {
+				for _, tfUser := range d.Get("users").(*schema.Set).List() {
+					if tfUser == grant.GranteeName.String {
+						users = append(users, grant.GranteeName.String)
+					}
 				}
+			} else {
+				users = append(users, grant.GranteeName.String)
 			}
 		default:
 			log.Printf("[WARN] Ignoring unknown grant type %s", grant.GrantedTo.String)

pkg/resources/role_grants_test.go

@@ -127,3 +127,62 @@ func TestIgnoreUnknownRoleGrants(t *testing.T) {
 		r.Len(d.Get("roles").(*schema.Set).List(), 2)
 	})
 }
+
+func expectReadUnmanagedGrants(mock sqlmock.Sqlmock) {
+	rows := sqlmock.NewRows([]string{
+		"created_on",
+		"role",
+		"granted_to",
+		"grantee_name",
+		"granted_by",
+	}).
+		AddRow("_", "good_name", "ROLE", "role1", "").
+		AddRow("_", "good_name", "ROLE", "role2", "").
+		AddRow("_", "good_name", "ROLE", "unmanaged_role1", "").
+		AddRow("_", "good_name", "ROLE", "unmanaged_role2", "").
+		AddRow("_", "good_name", "USER", "user1", "").
+		AddRow("_", "good_name", "USER", "user2", "").
+		AddRow("_", "good_name", "USER", "unmanaged_user1", "").
+		AddRow("_", "good_name", "USER", "unmanaged_user2", "")
+	mock.ExpectQuery(`SHOW GRANTS OF ROLE "good_name"`).WillReturnRows(rows)
+}
+
+func TestIgnoreUnmanagedGrants(t *testing.T) {
+	r := require.New(t)
+
+	d := roleGrants(t, "good_name|role1,role2|user1,user2", map[string]interface{}{
+		"role_name":              "good_name",
+		"roles":                  []interface{}{"role1", "role2"},
+		"users":                  []interface{}{"user1", "user2"},
+		"enable_multiple_grants": true,
+	})
+
+	WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
+		// Make sure that extraneous grants are ignored.
+		expectReadUnmanagedGrants(mock)
+		err := resources.ReadRoleGrants(d, db)
+		r.NoError(err)
+		r.Len(d.Get("users").(*schema.Set).List(), 2)
+		r.Len(d.Get("roles").(*schema.Set).List(), 2)
+	})
+}
+
+func TestIncludeUnmanagedGrants(t *testing.T) {
+	r := require.New(t)
+
+	d := roleGrants(t, "good_name|role1,role2|user1,user2", map[string]interface{}{
+		"role_name":              "good_name",
+		"roles":                  []interface{}{"role1", "role2"},
+		"users":                  []interface{}{"user1", "user2"},
+		"enable_multiple_grants": false,
+	})
+
+	WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
+		// Make sure that extraneous grants are ignored.
+		expectReadUnmanagedGrants(mock)
+		err := resources.ReadRoleGrants(d, db)
+		r.NoError(err)
+		r.Len(d.Get("users").(*schema.Set).List(), 4)
+		r.Len(d.Get("roles").(*schema.Set).List(), 4)
+	})
+}