Fix IAM policy behavior regression for empty members (#2253)

Merged PR #2253.

Author: emilymye

build/terraform

@@ -41,6 +41,23 @@ func TestAccProjectIamPolicy_basic(t *testing.T) {
 	})
 }
 
+// Test that an IAM policy with empty members does not cause a permadiff.
+func TestAccProjectIamPolicy_emptyMembers(t *testing.T) {
+	t.Parallel()
+
+	org := getTestOrgFromEnv(t)
+	pid := "terraform-" + acctest.RandString(10)
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccProjectIamPolicyEmptyMembers(pid, pname, org),
+			},
+		},
+	})
+}
+
 // Test that a non-collapsed IAM policy doesn't perpetually diff
 func TestAccProjectIamPolicy_expanded(t *testing.T) {
 	t.Parallel()
@@ -276,6 +293,27 @@ resource "google_project" "acceptance" {
 }`, pid, name, org)
 }
 
+func testAccProjectIamPolicyEmptyMembers(pid, name, org string) string {
+	return fmt.Sprintf(`
+resource "google_project" "acceptance" {
+    project_id = "%s"
+    name = "%s"
+    org_id = "%s"
+}
+
+resource "google_project_iam_policy" "acceptance" {
+    project = "${google_project.acceptance.id}"
+    policy_data = "${data.google_iam_policy.expanded.policy_data}"
+}
+
+data "google_iam_policy" "expanded" {
+    binding {
+        role = "roles/viewer"
+		members = []
+    }
+}`, pid, name, org)
+}
+
 func testAccProjectAssociatePolicyExpanded(pid, name, org string) string {
 	return fmt.Sprintf(`
 resource "google_project" "acceptance" {

build/terraform-beta

@@ -193,9 +193,10 @@ func createIamBindingsMap(bindings []*cloudresourcemanager.Binding) map[string]m
 	bm := make(map[string]map[string]struct{})
 	// Get each binding
 	for _, b := range bindings {
+		members := make(map[string]struct{})
 		// Initialize members map
-		if _, ok := bm[b.Role]; !ok {
-			bm[b.Role] = make(map[string]struct{})
+		if _, ok := bm[b.Role]; ok {
+			members = bm[b.Role]
 		}
 		// Get each member (user/principal) for the binding
 		for _, m := range b.Members {
@@ -210,7 +211,12 @@ func createIamBindingsMap(bindings []*cloudresourcemanager.Binding) map[string]m
 			m = strings.Join(pieces, ":")
 
 			// Add the member
-			bm[b.Role][m] = struct{}{}
+			members[m] = struct{}{}
+		}
+		if len(members) > 0 {
+			bm[b.Role] = members
+		} else {
+			delete(bm, b.Role)
 		}
 	}
 	return bm

third_party/terraform/tests/resource_google_project_iam_policy_test.go

@@ -17,6 +17,24 @@ func TestIamMergeBindings(t *testing.T) {
 			input:  []*cloudresourcemanager.Binding{},
 			expect: []*cloudresourcemanager.Binding{},
 		},
+		// No members returns no binding
+		{
+			input: []*cloudresourcemanager.Binding{
+				{
+					Role: "role-1",
+				},
+				{
+					Role:    "role-2",
+					Members: []string{"member-2"},
+				},
+			},
+			expect: []*cloudresourcemanager.Binding{
+				{
+					Role:    "role-2",
+					Members: []string{"member-2"},
+				},
+			},
+		},
 		// Nothing to merge - return same list
 		{
 			input: []*cloudresourcemanager.Binding{