Allow iam member to utilize casing for principle and principleSet (#4595) (#3133)

* Allow iam member to utilize casing for principle and principleSet

* Add integration test

* add testcases and custom diff supress for case sensitive value

* resolve comments

* normalizeCasing in id, add test for binding

* normalize iam import member casing

* add test for iam policy

* probably best to also support deleted behavior for princple and principleSet

* quantify teh set of values where this not the case in the function definition

* remove default go value

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

.changelog/4595.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+iam: fixed issue with principle and principleSet members not retaining their casing 
+```

google-beta/iam.go

@@ -242,6 +242,39 @@ func subtractFromBindings(bindings []*cloudresourcemanager.Binding, toRemove ...
 	return listFromIamBindingMap(currMap)
 }
 
+func iamMemberIsCaseSensitive(member string) bool {
+	return strings.HasPrefix(member, "principalSet:") || strings.HasPrefix(member, "principal:") || strings.HasPrefix(member, "principalHierarchy:")
+}
+
+// normalizeIamMemberCasing returns the case adjusted value of an iamMember
+// this is important as iam will ignore casing unless it is one of the following
+// member types: principalSet, principal, principalHierarchy
+// members are in <type>:<value> format
+// <type> is case sensitive
+// <value> isn't in most cases
+// so lowercase the value unless iamMemberIsCaseSensitive and leave the type alone
+// since Dec '19 members can be prefixed with "deleted:" to indicate the principal
+// has been deleted
+func normalizeIamMemberCasing(member string) string {
+	var pieces []string
+	if strings.HasPrefix(member, "deleted:") {
+		pieces = strings.SplitN(member, ":", 3)
+		if len(pieces) > 2 && !iamMemberIsCaseSensitive(strings.TrimPrefix(member, "deleted:")) {
+			pieces[2] = strings.ToLower(pieces[2])
+		}
+	} else if !iamMemberIsCaseSensitive(member) {
+		pieces = strings.SplitN(member, ":", 2)
+		if len(pieces) > 1 {
+			pieces[1] = strings.ToLower(pieces[1])
+		}
+	}
+
+	if len(pieces) > 0 {
+		member = strings.Join(pieces, ":")
+	}
+	return member
+}
+
 // Construct map of role to set of members from list of bindings.
 func createIamBindingsMap(bindings []*cloudresourcemanager.Binding) map[iamBindingKey]map[string]struct{} {
 	bm := make(map[iamBindingKey]map[string]struct{})
@@ -255,27 +288,7 @@ func createIamBindingsMap(bindings []*cloudresourcemanager.Binding) map[iamBindi
 		}
 		// Get each member (user/principal) for the binding
 		for _, m := range b.Members {
-			// members are in <type>:<value> format
-			// <type> is case sensitive
-			// <value> isn't
-			// so let's lowercase the value and leave the type alone
-			// since Dec '19 members can be prefixed with "deleted:" to indicate the principal
-			// has been deleted
-			var pieces []string
-			if strings.HasPrefix(m, "deleted:") {
-				pieces = strings.SplitN(m, ":", 3)
-				if len(pieces) > 2 {
-					pieces[2] = strings.ToLower(pieces[2])
-				}
-			} else {
-				pieces = strings.SplitN(m, ":", 2)
-				if len(pieces) > 1 {
-					pieces[1] = strings.ToLower(pieces[1])
-				}
-			}
-
-			m = strings.Join(pieces, ":")
-
+			m = normalizeIamMemberCasing(m)
 			// Add the member
 			members[m] = struct{}{}
 		}

google-beta/iam_test.go

@@ -5,6 +5,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"google.golang.org/api/cloudresourcemanager/v1"
 )
 
@@ -658,7 +659,7 @@ func TestIamCreateIamBindingsMap(t *testing.T) {
 			input: []*cloudresourcemanager.Binding{
 				{
 					Role:    "role-1",
-					Members: []string{"deleted:serviceAccount:user-1", "user-2"},
+					Members: []string{"deleted:serviceAccount:useR-1", "user-2"},
 				},
 				{
 					Role:    "role-2",
@@ -676,11 +677,49 @@ func TestIamCreateIamBindingsMap(t *testing.T) {
 					Role:    "role-3",
 					Members: []string{"user-3"},
 				},
+				{
+					Role:    "role-4",
+					Members: []string{"deleted:principal:useR-1"},
+				},
 			},
 			expect: map[iamBindingKey]map[string]struct{}{
 				{"role-1", conditionKey{}}: {"deleted:serviceAccount:user-1": {}, "user-2": {}, "serviceAccount:user-3": {}},
 				{"role-2", conditionKey{}}: {"deleted:user:user-1": {}, "user-2": {}},
 				{"role-3", conditionKey{}}: {"user-3": {}},
+				{"role-4", conditionKey{}}: {"deleted:principal:useR-1": {}},
+			},
+		},
+		{
+			input: []*cloudresourcemanager.Binding{
+				{
+					Role:    "role-1",
+					Members: []string{"principalSet://iam.googleapis.com/projects/1066737951711/locations/global/workloadIdentityPools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdaRole"},
+				},
+				{
+					Role:    "role-2",
+					Members: []string{"principal://iam.googleapis.com/projects/1066737951711/locations/global/workloadIdentityPools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdaRole"},
+				},
+				{
+					Role:    "role-1",
+					Members: []string{"serviceAccount:useR-3"},
+				},
+				{
+					Role:    "role-2",
+					Members: []string{"user-2"},
+				},
+				{
+					Role:    "role-3",
+					Members: []string{"user-3"},
+				},
+				{
+					Role:    "role-3",
+					Members: []string{"principalHierarchy://iam.googleapis.com/projects/1066737951711/locations/global/workloadIdentityPools"},
+				},
+			},
+			expect: map[iamBindingKey]map[string]struct{}{
+				{"role-1", conditionKey{}}: {"principalSet://iam.googleapis.com/projects/1066737951711/locations/global/workloadIdentityPools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdaRole": {}, "serviceAccount:user-3": {}},
+				{"role-2", conditionKey{}}: {"principal://iam.googleapis.com/projects/1066737951711/locations/global/workloadIdentityPools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdaRole": {}, "user-2": {}},
+				{"role-3", conditionKey{}}: {"principalHierarchy://iam.googleapis.com/projects/1066737951711/locations/global/workloadIdentityPools": {}, "user-3": {}},
 			},
 		},
 		{
@@ -748,6 +787,84 @@ func TestIamCreateIamBindingsMap(t *testing.T) {
 	}
 }
 
+func TestIamMember_MemberDiffSuppress(t *testing.T) {
+	type IamMemberTestcase struct {
+		name  string
+		old   string
+		new   string
+		equal bool
+	}
+	var iamMemberTestcases = []IamMemberTestcase{
+		{
+			name:  "control",
+			old:   "somevalue",
+			new:   "somevalue",
+			equal: true,
+		},
+		{
+			name:  "principal same casing",
+			old:   "principal:someValueHere",
+			new:   "principal:someValueHere",
+			equal: true,
+		},
+		{
+			name:  "principal not same casing",
+			old:   "principal:somevalueHere",
+			new:   "principal:someValuehere",
+			equal: false,
+		},
+		{
+			name:  "principalSet same casing",
+			old:   "principalSet:someValueHere",
+			new:   "principalSet:someValueHere",
+			equal: true,
+		},
+		{
+			name:  "principalSet not same casing",
+			old:   "principalSet:somevalueHere",
+			new:   "principalSet:someValuehere",
+			equal: false,
+		},
+		{
+			name:  "principalHierarchy same casing",
+			old:   "principalHierarchy:someValueHere",
+			new:   "principalHierarchy:someValueHere",
+			equal: true,
+		},
+		{
+			name:  "principalHierarchy not same casing",
+			old:   "principalHierarchy:somevalueHere",
+			new:   "principalHierarchy:someValuehere",
+			equal: false,
+		},
+		{
+			name:  "serviceAccount same casing",
+			old:   "serviceAccount:[email protected]",
+			new:   "serviceAccount:[email protected]",
+			equal: true,
+		},
+		{
+			name:  "serviceAccount diff casing",
+			old:   "serviceAccount:[email protected]",
+			new:   "serviceAccount:[email protected]",
+			equal: true,
+		},
+		{
+			name:  "random diff",
+			old:   "serviasfsfljJKLSD",
+			new:   "servicsFDJKLSFJdfjdlkfsf",
+			equal: false,
+		},
+	}
+
+	for _, testcase := range iamMemberTestcases {
+		areEqual := iamMemberCaseDiffSuppress("", testcase.old, testcase.new, &schema.ResourceData{})
+		if areEqual != testcase.equal {
+			t.Errorf("Testcase %s failed: expected equality to be %t but got %t", testcase.name, testcase.equal, areEqual)
+		}
+	}
+}
+
 func TestIamListFromIamBindingMap(t *testing.T) {
 	testCases := []struct {
 		input  map[iamBindingKey]map[string]struct{}

google-beta/resource_dataflow_flex_template_job_test.go

@@ -8,7 +8,7 @@ import (
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
-	compute "google.golang.org/api/compute/v1"
+	"google.golang.org/api/compute/v1"
 )
 
 func TestAccDataflowFlexTemplateJob_basic(t *testing.T) {