Change IAM binding to be authoritative

Author: chrisst

google/iam.go

@@ -123,6 +123,25 @@ func iamPolicyReadModifyWrite(updater ResourceIamUpdater, modify iamPolicyModify
 	return nil
 }
 
+// Takes a single binding and will either overwrite the same role in a list or append it to the end
+func overwriteBinding(bindings []*cloudresourcemanager.Binding, overwrite *cloudresourcemanager.Binding) []*cloudresourcemanager.Binding {
+	var found bool
+
+	for i, b := range bindings {
+		if b.Role == overwrite.Role {
+			bindings[i] = overwrite
+			found = true
+			break
+		}
+	}
+
+	if !found {
+		bindings = append(bindings, overwrite)
+	}
+
+	return bindings
+}
+
 // Merge multiple Bindings such that Bindings with the same Role result in
 // a single Binding with combined Members
 func mergeBindings(bindings []*cloudresourcemanager.Binding) []*cloudresourcemanager.Binding {

google/resource_google_project_iam_policy_test.go

@@ -169,6 +169,62 @@ func testAccCheckGoogleProjectIamPolicyExists(projectRes, policyRes, pid string)
 	}
 }
 
+func TestIamOverwriteBinding(t *testing.T) {
+	table := []struct {
+		input    []*cloudresourcemanager.Binding
+		override cloudresourcemanager.Binding
+		expect   []cloudresourcemanager.Binding
+	}{
+		{
+			input: []*cloudresourcemanager.Binding{
+				{
+					Role:    "role-1",
+					Members: []string{"member-1", "member-2"},
+				},
+			},
+			override: cloudresourcemanager.Binding{
+				Role:    "role-1",
+				Members: []string{"new-member"},
+			},
+			expect: []cloudresourcemanager.Binding{
+				{
+					Role:    "role-1",
+					Members: []string{"new-member"},
+				},
+			},
+		},
+		{
+			input: []*cloudresourcemanager.Binding{
+				{
+					Role:    "role-1",
+					Members: []string{"member-1", "member-2"},
+				},
+			},
+			override: cloudresourcemanager.Binding{
+				Role:    "role-2",
+				Members: []string{"member-3"},
+			},
+			expect: []cloudresourcemanager.Binding{
+				{
+					Role:    "role-1",
+					Members: []string{"member-1", "member-2"},
+				},
+				{
+					Role:    "role-2",
+					Members: []string{"member-3"},
+				},
+			},
+		},
+	}
+
+	for _, test := range table {
+		got := overwriteBinding(test.input, &test.override)
+		if !reflect.DeepEqual(derefBindings(got), test.expect) {
+			t.Errorf("OverwriteIamBinding failed.\nGot %+v\nWant %+v", derefBindings(got), test.expect)
+		}
+	}
+}
+
 func TestIamMergeBindings(t *testing.T) {
 	table := []struct {
 		input  []*cloudresourcemanager.Binding
@@ -177,95 +233,61 @@ func TestIamMergeBindings(t *testing.T) {
 		{
 			input: []*cloudresourcemanager.Binding{
 				{
-					Role: "role-1",
-					Members: []string{
-						"member-1",
-						"member-2",
-					},
+					Role:    "role-1",
+					Members: []string{"member-1", "member-2"},
 				},
 				{
-					Role: "role-1",
-					Members: []string{
-						"member-3",
-					},
+					Role:    "role-1",
+					Members: []string{"member-3"},
 				},
 			},
 			expect: []cloudresourcemanager.Binding{
 				{
-					Role: "role-1",
-					Members: []string{
-						"member-1",
-						"member-2",
-						"member-3",
-					},
+					Role:    "role-1",
+					Members: []string{"member-1", "member-2", "member-3"},
 				},
 			},
 		},
 		{
 			input: []*cloudresourcemanager.Binding{
 				{
-					Role: "role-1",
-					Members: []string{
-						"member-3",
-						"member-4",
-					},
+					Role:    "role-1",
+					Members: []string{"member-3", "member-4"},
 				},
 				{
-					Role: "role-1",
-					Members: []string{
-						"member-2",
-						"member-1",
-					},
+					Role:    "role-1",
+					Members: []string{"member-2", "member-1"},
 				},
 				{
-					Role: "role-2",
-					Members: []string{
-						"member-1",
-					},
+					Role:    "role-2",
+					Members: []string{"member-1"},
 				},
 				{
-					Role: "role-1",
-					Members: []string{
-						"member-5",
-					},
+					Role:    "role-1",
+					Members: []string{"member-5"},
 				},
 				{
-					Role: "role-3",
-					Members: []string{
-						"member-1",
-					},
+					Role:    "role-3",
+					Members: []string{"member-1"},
 				},
 				{
-					Role: "role-2",
-					Members: []string{
-						"member-2",
-					},
+					Role:    "role-2",
+					Members: []string{"member-2"},
 				},
 				{Role: "empty-role", Members: []string{}},
 			},
 			expect: []cloudresourcemanager.Binding{
 				{
-					Role: "role-1",
-					Members: []string{
-						"member-1",
-						"member-2",
-						"member-3",
-						"member-4",
-						"member-5",
-					},
+					Role:    "role-1",
+					Members: []string{"member-1", "member-2", "member-3", "member-4", "member-5"},
 				},
 				{
-					Role: "role-2",
-					Members: []string{
-						"member-1",
-						"member-2",
-					},
+					Role:    "role-2",
+					Members: []string{"member-1", "member-2"},
 				},
 				{
-					Role: "role-3",
-					Members: []string{
-						"member-1",
-					},
+					Role:    "role-3",
+					Members: []string{"member-1"},
 				},
 			},
 		},
@@ -416,7 +438,7 @@ data "google_iam_policy" "expanded" {
             "user:[email protected]",
         ]
     }
-    
+
     binding {
         role = "roles/viewer"
         members = [

google/resource_iam_binding.go

@@ -31,9 +31,9 @@ var iamBindingSchema = map[string]*schema.Schema{
 
 func ResourceIamBinding(parentSpecificSchema map[string]*schema.Schema, newUpdaterFunc newResourceIamUpdaterFunc) *schema.Resource {
 	return &schema.Resource{
-		Create: resourceIamBindingCreate(newUpdaterFunc),
+		Create: resourceIamBindingCreateUpdate(newUpdaterFunc),
 		Read:   resourceIamBindingRead(newUpdaterFunc),
-		Update: resourceIamBindingUpdate(newUpdaterFunc),
+		Update: resourceIamBindingCreateUpdate(newUpdaterFunc),
 		Delete: resourceIamBindingDelete(newUpdaterFunc),
 		Schema: mergeSchemas(iamBindingSchema, parentSpecificSchema),
 	}
@@ -47,7 +47,7 @@ func ResourceIamBindingWithImport(parentSpecificSchema map[string]*schema.Schema
 	return r
 }
 
-func resourceIamBindingCreate(newUpdaterFunc newResourceIamUpdaterFunc) schema.CreateFunc {
+func resourceIamBindingCreateUpdate(newUpdaterFunc newResourceIamUpdaterFunc) func(*schema.ResourceData, interface{}) error {
 	return func(d *schema.ResourceData, meta interface{}) error {
 		config := meta.(*Config)
 		updater, err := newUpdaterFunc(d, config)
@@ -57,11 +57,7 @@ func resourceIamBindingCreate(newUpdaterFunc newResourceIamUpdaterFunc) schema.C
 
 		p := getResourceIamBinding(d)
 		err = iamPolicyReadModifyWrite(updater, func(ep *cloudresourcemanager.Policy) error {
-			// Creating a binding does not remove existing members if they are not in the provided members list.
-			// This prevents removing existing permission without the user's knowledge.
-			// Instead, a diff is shown in that case after creation. Subsequent calls to update will remove any
-			// existing members not present in the provided list.
-			ep.Bindings = mergeBindings(append(ep.Bindings, p))
+			ep.Bindings = overwriteBinding(ep.Bindings, p)
 			return nil
 		})
 		if err != nil {
@@ -151,38 +147,6 @@ func iamBindingImport(resourceIdParser resourceIdParserFunc) schema.StateFunc {
 	}
 }
 
-func resourceIamBindingUpdate(newUpdaterFunc newResourceIamUpdaterFunc) schema.UpdateFunc {
-	return func(d *schema.ResourceData, meta interface{}) error {
-		config := meta.(*Config)
-		updater, err := newUpdaterFunc(d, config)
-		if err != nil {
-			return err
-		}
-
-		binding := getResourceIamBinding(d)
-		err = iamPolicyReadModifyWrite(updater, func(p *cloudresourcemanager.Policy) error {
-			var found bool
-			for pos, b := range p.Bindings {
-				if b.Role != binding.Role {
-					continue
-				}
-				found = true
-				p.Bindings[pos] = binding
-				break
-			}
-			if !found {
-				p.Bindings = append(p.Bindings, binding)
-			}
-			return nil
-		})
-		if err != nil {
-			return err
-		}
-
-		return resourceIamBindingRead(newUpdaterFunc)(d, meta)
-	}
-}
-
 func resourceIamBindingDelete(newUpdaterFunc newResourceIamUpdaterFunc) schema.DeleteFunc {
 	return func(d *schema.ResourceData, meta interface{}) error {
 		config := meta.(*Config)

website/docs/d/google_iam_policy.html.markdown

@@ -26,10 +26,10 @@ data "google_iam_policy" "admin" {
     role = "roles/storage.objectViewer"
 
     members = [
-      "user:jane@example.com",
+      "user:alice@gmail.com",
     ]
   }
-    
+
   audit_config {
     service = "cloudkms.googleapis.com"
     audit_log_configs = [
@@ -73,11 +73,11 @@ each accept the following arguments:
   See the [IAM Roles](https://cloud.google.com/compute/docs/access/iam) documentation for a complete list of roles.
   Note that custom roles must be of the format `[projects|organizations]/{parent-name}/roles/{role-name}`.
 
-* `members` (Required) - An array of identites that will be granted the privilege in the `role`.
+* `members` (Required) - An array of identites that will be granted the privilege in the `role`. For more details on format and restrictions see https://cloud.google.com/billing/reference/rest/v1/Policy#Binding
   Each entry can have one of the following values:
   * **allUsers**: A special identifier that represents anyone who is on the internet; with or without a Google account. It **can't** be used with the `google_project` resource.
   * **allAuthenticatedUsers**: A special identifier that represents anyone who is authenticated with a Google account or a service account. It **can't** be used with the `google_project` resource.
-  * **user:{emailid}**: An email address that represents a specific Google account. For example, [email protected] or [email protected].
+  * **user:{emailid}**: An email address that represents a specific Google account. For example, [email protected].
   * **serviceAccount:{emailid}**: An email address that represents a service account. For example, [email protected].
   * **group:{emailid}**: An email address that represents a Google group. For example, [email protected].
   * **domain:{domain}**: A G Suite domain (primary, instead of alias) name that represents all the users of that domain. For example, google.com or example.com.

website/docs/r/google_billing_account_iam_binding.md

@@ -15,6 +15,10 @@ an existing Google Cloud Platform Billing Account.
    `google_billing_account_iam_member` for the __same role__ or they will fight over
    what your policy should be.
 
+~> **Note:** On create, this resource will overwrite members of any existing roles.
+    Use `terraform import` and inspect the `terraform plan` output to ensure
+    your existing members are preserved.
+
 ## Example Usage
 
 ```hcl
@@ -23,7 +27,7 @@ resource "google_billing_account_iam_binding" "binding" {
   role               = "roles/billing.viewer"
 
   members = [
-    "user:jane@example.com",
+    "user:alice@gmail.com",
   ]
 }
 ```
@@ -36,7 +40,7 @@ The following arguments are supported:
 
 * `role` - (Required) The role that should be applied.
 
-* `members` - (Required) A list of users that the role should apply to.
+* `members` - (Required) A list of users that the role should apply to. For more details on format and restrictions see https://cloud.google.com/billing/reference/rest/v1/Policy#Binding
 
 ## Attributes Reference
 

website/docs/r/google_billing_account_iam_member.md

@@ -21,7 +21,7 @@ the IAM policy for an existing Google Cloud Platform Billing Account.
 resource "google_billing_account_iam_member" "binding" {
   billing_account_id = "00AA00-000AAA-00AA0A"
   role               = "roles/billing.viewer"
-  member             = "user:jane@example.com"
+  member             = "user:alice@gmail.com"
 }
 ```
 
@@ -33,8 +33,8 @@ The following arguments are supported:
 
 * `role` - (Required) The role that should be applied.
 
-* `member` - (Required) The user that the role should apply to.
-    
+* `member` - (Required) The user that the role should apply to. For more details on format and restrictions see https://cloud.google.com/billing/reference/rest/v1/Policy#Binding
+
 ## Attributes Reference
 
 In addition to the arguments listed above, the following computed attributes are

website/docs/r/google_folder_iam_binding.html.markdown

@@ -15,6 +15,10 @@ an existing Google Cloud Platform folder.
    `google_folder_iam_policy` or they will fight over what your policy
    should be.
 
+~> **Note:** On create, this resource will overwrite members of any existing roles.
+    Use `terraform import` and inspect the `terraform plan` output to ensure
+    your existing members are preserved.
+
 ## Example Usage
 
 ```hcl
@@ -28,7 +32,7 @@ resource "google_folder_iam_binding" "admin" {
   role    = "roles/editor"
 
   members = [
-    "user:jane@example.com",
+    "user:alice@gmail.com",
   ]
 }
 ```
@@ -41,10 +45,11 @@ The following arguments are supported:
 
 * `members` (Required) - An array of identites that will be granted the privilege in the `role`.
   Each entry can have one of the following values:
-  * **user:{emailid}**: An email address that represents a specific Google account. For example, alice@gmail.com or joe@example.com.
+  * **user:{emailid}**: An email address that is associated with a specific Google account. For example, [email protected].
   * **serviceAccount:{emailid}**: An email address that represents a service account. For example, [email protected].
   * **group:{emailid}**: An email address that represents a Google group. For example, [email protected].
   * **domain:{domain}**: A G Suite domain (primary, instead of alias) name that represents all the users of that domain. For example, google.com or example.com.
+  * For more details on format and restrictions see https://cloud.google.com/billing/reference/rest/v1/Policy#Binding
 
 * `role` - (Required) The role that should be applied. Only one
     `google_folder_iam_binding` can be used per role. Note that custom roles must be of the format