Add customize diff for databricks_grant and databricks_grants for…

… case insensitivity & spaces in grants (#3657)

* relax principal checks to be case insensitive

* fix test

* revert acceptance test

* add DiffSuppressFunc

* strings.EqualFold

* normalizePrivilege

* normalize more

* fix tests

* fix int test

* custom hash for set

* fix test

* clean up

catalog/permissions/permissions.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log"
+	"strings"
 	"time"
 
 	"github.com/databricks/databricks-sdk-go"
@@ -119,18 +120,23 @@ var Mappings = SecurableMapping{
 	"volume":             catalog.SecurableType("volume"),
 }
 
+// Unity Catalog accepts privileges with spaces, but will automatically convert them to underscores
+func NormalizePrivilege(privilege string) string {
+	return strings.ToUpper(strings.Replace(privilege, " ", "_", -1))
+}
+
 // Utils for Slice and Set
 func SliceToSet(in []catalog.Privilege) *schema.Set {
 	var out []any
 	for _, v := range in {
-		out = append(out, v.String())
+		out = append(out, NormalizePrivilege(v.String()))
 	}
 	return schema.NewSet(schema.HashString, out)
 }
 
 func SetToSlice(set *schema.Set) (ss []catalog.Privilege) {
 	for _, v := range set.List() {
-		ss = append(ss, catalog.Privilege(v.(string)))
+		ss = append(ss, catalog.Privilege(NormalizePrivilege(v.(string))))
 	}
 	return
 }

catalog/resource_grant.go

@@ -20,15 +20,15 @@ func diffPermissionsForPrincipal(principal string, desired catalog.PermissionsLi
 	// diffs change sets for principal
 	configured := map[string]*schema.Set{}
 	for _, v := range desired.PrivilegeAssignments {
-		if v.Principal == principal {
-			configured[v.Principal] = permissions.SliceToSet(v.Privileges)
+		if strings.EqualFold(v.Principal, principal) {
+			configured[strings.ToLower(v.Principal)] = permissions.SliceToSet(v.Privileges)
 		}
 	}
 	// existing permissions that needs removal for principal
 	remote := map[string]*schema.Set{}
 	for _, v := range existing.PrivilegeAssignments {
-		if v.Principal == principal {
-			remote[v.Principal] = permissions.SliceToSet(v.Privileges)
+		if strings.EqualFold(v.Principal, principal) {
+			remote[strings.ToLower(v.Principal)] = permissions.SliceToSet(v.Privileges)
 		}
 	}
 	// STEP 1: detect overlaps
@@ -87,7 +87,7 @@ func filterPermissionsForPrincipal(in catalog.PermissionsList, principal string)
 	grantsForPrincipal := []permissions.UnityCatalogPrivilegeAssignment{}
 	for _, v := range in.PrivilegeAssignments {
 		privileges := []string{}
-		if v.Principal == principal {
+		if strings.EqualFold(v.Principal, principal) {
 			for _, p := range v.Privileges {
 				privileges = append(privileges, p.String())
 			}
@@ -122,8 +122,13 @@ func parseSecurableId(d *schema.ResourceData) (string, string, string, error) {
 func ResourceGrant() common.Resource {
 	s := common.StructToSchema(permissions.UnityCatalogPrivilegeAssignment{},
 		func(m map[string]*schema.Schema) map[string]*schema.Schema {
+			common.CustomizeSchemaPath(m, "principal").SetForceNew().SetCustomSuppressDiff(common.EqualFoldDiffSuppress)
 
-			m["principal"].ForceNew = true
+			// set custom hash function for privileges
+			common.MustSchemaPath(m, "privileges").Set = func(i any) int {
+				privilege := i.(string)
+				return schema.HashString(permissions.NormalizePrivilege(privilege))
+			}
 
 			allFields := []string{}
 			for field := range permissions.Mappings {

catalog/resource_grant_test.go

@@ -532,6 +532,37 @@ func TestResourceGrantPermissionsList_Diff_ExternallyAddedPrincipal(t *testing.T
 	assert.Len(t, diff, 0)
 }
 
+func TestResourceGrantPermissionsList_Diff_CaseSensitive(t *testing.T) {
+	diff := diffPermissionsForPrincipal(
+		"a",
+		catalog.PermissionsList{ // config
+			PrivilegeAssignments: []catalog.PrivilegeAssignment{
+				{
+					Principal:  "a",
+					Privileges: []catalog.Privilege{"a"},
+				},
+				{
+					Principal:  "c",
+					Privileges: []catalog.Privilege{"a"},
+				},
+			},
+		},
+		catalog.PermissionsList{
+			PrivilegeAssignments: []catalog.PrivilegeAssignment{ // platform
+				{
+					Principal:  "A",
+					Privileges: []catalog.Privilege{"a"},
+				},
+				{
+					Principal:  "b",
+					Privileges: []catalog.Privilege{"a"},
+				},
+			},
+		},
+	)
+	assert.Len(t, diff, 0)
+}
+
 func TestResourceGrantPermissionsList_Diff_ExternallyAddedPriv(t *testing.T) {
 	diff := diffPermissionsForPrincipal(
 		"a",
@@ -555,7 +586,7 @@ func TestResourceGrantPermissionsList_Diff_ExternallyAddedPriv(t *testing.T) {
 	assert.Len(t, diff, 1)
 	assert.Len(t, diff[0].Add, 0)
 	assert.Len(t, diff[0].Remove, 1)
-	assert.Equal(t, catalog.Privilege("b"), diff[0].Remove[0])
+	assert.Equal(t, catalog.Privilege("B"), diff[0].Remove[0])
 }
 
 func TestResourceGrantPermissionsList_Diff_LocalRemoteDiff(t *testing.T) {
@@ -581,8 +612,8 @@ func TestResourceGrantPermissionsList_Diff_LocalRemoteDiff(t *testing.T) {
 	assert.Len(t, diff, 1)
 	assert.Len(t, diff[0].Add, 1)
 	assert.Len(t, diff[0].Remove, 1)
-	assert.Equal(t, catalog.Privilege("a"), diff[0].Add[0])
-	assert.Equal(t, catalog.Privilege("c"), diff[0].Remove[0])
+	assert.Equal(t, catalog.Privilege("A"), diff[0].Add[0])
+	assert.Equal(t, catalog.Privilege("C"), diff[0].Remove[0])
 }
 
 func TestResourceGrantShareGrantCreate(t *testing.T) {

catalog/resource_grants.go

@@ -31,12 +31,12 @@ func diffPermissions(pl catalog.PermissionsList, existing catalog.PermissionsLis
 	// diffs change sets
 	configured := map[string]*schema.Set{}
 	for _, v := range pl.PrivilegeAssignments {
-		configured[v.Principal] = permissions.SliceToSet(v.Privileges)
+		configured[strings.ToLower(v.Principal)] = permissions.SliceToSet(v.Privileges)
 	}
 	// existing permissions that needs removal
 	remote := map[string]*schema.Set{}
 	for _, v := range existing.PrivilegeAssignments {
-		remote[v.Principal] = permissions.SliceToSet(v.Privileges)
+		remote[strings.ToLower(v.Principal)] = permissions.SliceToSet(v.Privileges)
 	}
 	// STEP 1: detect overlaps
 	for principal, confPrivs := range configured {
@@ -128,6 +128,21 @@ func parseId(d *schema.ResourceData) (string, string, error) {
 func ResourceGrants() common.Resource {
 	s := common.StructToSchema(PermissionsList{},
 		func(s map[string]*schema.Schema) map[string]*schema.Schema {
+			// set custom hash function for principal and privileges
+			common.MustSchemaPath(s, "grant", "privileges").Set = func(i any) int {
+				privilege := i.(string)
+				return schema.HashString(permissions.NormalizePrivilege(privilege))
+			}
+			common.MustSchemaPath(s, "grant").Set = func(i any) int {
+				objectStruct := i.(map[string]any)
+				principal := objectStruct["principal"].(string)
+				privileges := objectStruct["privileges"].(*schema.Set)
+				hashString := strings.ToLower(principal)
+				for _, privilege := range privileges.List() {
+					hashString += "|" + permissions.NormalizePrivilege(privilege.(string))
+				}
+				return schema.HashString(hashString)
+			}
 			alof := []string{}
 			for field := range permissions.Mappings {
 				s[field] = &schema.Schema{

catalog/resource_grants_test.go

@@ -389,7 +389,7 @@ func TestPermissionsList_Diff_ExternallyAddedPrincipal(t *testing.T) {
 	assert.Len(t, diff[0].Add, 0)
 	assert.Len(t, diff[0].Remove, 1)
 	assert.Equal(t, "b", diff[0].Principal)
-	assert.Equal(t, catalog.Privilege("a"), diff[0].Remove[0])
+	assert.Equal(t, catalog.Privilege("A"), diff[0].Remove[0])
 	assert.Equal(t, "c", diff[1].Principal)
 }
 
@@ -415,7 +415,29 @@ func TestPermissionsList_Diff_ExternallyAddedPriv(t *testing.T) {
 	assert.Len(t, diff, 1)
 	assert.Len(t, diff[0].Add, 0)
 	assert.Len(t, diff[0].Remove, 1)
-	assert.Equal(t, catalog.Privilege("b"), diff[0].Remove[0])
+	assert.Equal(t, catalog.Privilege("B"), diff[0].Remove[0])
+}
+
+func TestPermissionsList_Diff_CaseSensitivePrincipal(t *testing.T) {
+	diff := diffPermissions(
+		catalog.PermissionsList{ // config
+			PrivilegeAssignments: []catalog.PrivilegeAssignment{
+				{
+					Principal:  "A",
+					Privileges: []catalog.Privilege{"a"},
+				},
+			},
+		},
+		catalog.PermissionsList{
+			PrivilegeAssignments: []catalog.PrivilegeAssignment{ // platform
+				{
+					Principal:  "a",
+					Privileges: []catalog.Privilege{"a"},
+				},
+			},
+		},
+	)
+	assert.Len(t, diff, 0)
 }
 
 func TestPermissionsList_Diff_LocalRemoteDiff(t *testing.T) {
@@ -440,8 +462,8 @@ func TestPermissionsList_Diff_LocalRemoteDiff(t *testing.T) {
 	assert.Len(t, diff, 1)
 	assert.Len(t, diff[0].Add, 1)
 	assert.Len(t, diff[0].Remove, 1)
-	assert.Equal(t, catalog.Privilege("a"), diff[0].Add[0])
-	assert.Equal(t, catalog.Privilege("c"), diff[0].Remove[0])
+	assert.Equal(t, catalog.Privilege("A"), diff[0].Add[0])
+	assert.Equal(t, catalog.Privilege("C"), diff[0].Remove[0])
 }
 
 func TestShareGrantCreate(t *testing.T) {

internal/acceptance/grant_test.go

@@ -103,6 +103,8 @@ func TestUcAccGrant(t *testing.T) {
 		Template: strings.ReplaceAll(grantTemplate, "%s", "{env.TEST_DATA_ENG_GROUP}"),
 	}, step{
 		Template: strings.ReplaceAll(grantTemplate, "%s", "{env.TEST_DATA_SCI_GROUP}"),
+	}, step{
+		Template: strings.ReplaceAll(strings.ReplaceAll(grantTemplate, "ALL_PRIVILEGES", "ALL PRIVILEGES"), `"%s"`, `upper("{env.TEST_DATA_SCI_GROUP}")`),
 	})
 }
 
@@ -131,6 +133,6 @@ func TestUcAccGrantForIdChange(t *testing.T) {
 		Template: grantTemplateForNamePermissionChange("-new", "ALL_PRIVILEGES"),
 	}, step{
 		Template:    grantTemplateForNamePermissionChange("-fail", "abc"),
-		ExpectError: regexp.MustCompile(`cannot create grant: Privilege abc is not applicable to this entity`),
+		ExpectError: regexp.MustCompile(`cannot create grant: Privilege ABC is not applicable to this entity`),
 	})
 }

internal/acceptance/grants_test.go

@@ -109,6 +109,8 @@ func TestUcAccGrants(t *testing.T) {
 		Template: strings.ReplaceAll(grantsTemplate, "%s", "{env.TEST_DATA_ENG_GROUP}"),
 	}, step{
 		Template: strings.ReplaceAll(grantsTemplate, "%s", "{env.TEST_DATA_SCI_GROUP}"),
+	}, step{
+		Template: strings.ReplaceAll(strings.ReplaceAll(grantsTemplate, "ALL_PRIVILEGES", "ALL PRIVILEGES"), `"%s"`, `upper("{env.TEST_DATA_SCI_GROUP}")`),
 	})
 }
 
@@ -139,6 +141,6 @@ func TestUcAccGrantsForIdChange(t *testing.T) {
 		Template: grantsTemplateForNamePermissionChange("-new", "ALL_PRIVILEGES"),
 	}, step{
 		Template:    grantsTemplateForNamePermissionChange("-fail", "abc"),
-		ExpectError: regexp.MustCompile(`Error: cannot create grants: Privilege abc is not applicable to this entity`),
+		ExpectError: regexp.MustCompile(`Error: cannot create grants: Privilege ABC is not applicable to this entity`),
 	})
 }